From 6ca582a00f6fd067c1876fbaf3807efaa73cfb6f Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Wed, 28 Jun 2023 12:54:43 +0300
Subject: [PATCH] Automatically expand the native query editor when editing a
question from a dashboard (#31863)
---
.../dashboard-back-navigation.cy.spec.js | 29 +++++++++++++++++++
frontend/src/metabase-types/store/mocks/qb.ts | 11 +++++++
frontend/src/metabase-types/store/qb.ts | 9 +++++-
.../actions/core/initializeQB.ts | 10 +++++--
.../query_builder/components/QueryModals.tsx | 5 +++-
.../ConvertQueryModal/ConvertQueryModal.tsx | 9 ++++--
.../query_builder/components/view/View.jsx | 4 ++-
.../src/metabase/query_builder/reducers.js | 22 ++++++++++----
.../src/metabase/query_builder/selectors.js | 6 +++-
9 files changed, 91 insertions(+), 14 deletions(-)
diff --git a/e2e/test/scenarios/dashboard/dashboard-back-navigation.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard-back-navigation.cy.spec.js
index 2a410011a96..1bc08955288 100644
--- a/e2e/test/scenarios/dashboard/dashboard-back-navigation.cy.spec.js
+++ b/e2e/test/scenarios/dashboard/dashboard-back-navigation.cy.spec.js
@@ -71,6 +71,22 @@ describe("scenarios > dashboard > dashboard back navigation", () => {
cy.findByLabelText(backButtonLabel).should("not.exist");
});
+ it("should expand the native editor when editing a question from a dashboard", () => {
+ createDashboardWithNativeCard();
+ cy.get("@dashboardId").then(visitDashboard);
+ getDashboardCard().realHover();
+ getDashboardCardMenu().click();
+ popover().findByText("Edit question").click();
+ cy.findByTestId("native-query-editor").should("be.visible");
+
+ queryBuilderHeader().findByLabelText("Back to Test Dashboard").click();
+ getDashboardCard().findByText("Orders SQL").click();
+ cy.findByTestId("native-query-top-bar")
+ .findByText("This question is written in SQL.")
+ .should("be.visible");
+ cy.findByTestId("native-query-editor").should("not.be.visible");
+ });
+
it("should display a back to the dashboard button in table x-ray dashboards", () => {
const cardTitle = "Sales per state";
cy.visit(`/auto/dashboard/table/${ORDERS_ID}`);
@@ -340,6 +356,19 @@ const createDashboardWithCards = () => {
});
};
+const createDashboardWithNativeCard = () => {
+ const questionDetails = {
+ name: "Orders SQL",
+ native: {
+ query: "SELECT * FROM ORDERS",
+ },
+ };
+
+ cy.createNativeQuestionAndDashboard({ questionDetails }).then(
+ ({ body: { dashboard_id } }) => cy.wrap(dashboard_id).as("dashboardId"),
+ );
+};
+
const createDashboardWithSlowCard = () => {
const questionDetails = {
name: "Sleep card",
diff --git a/frontend/src/metabase-types/store/mocks/qb.ts b/frontend/src/metabase-types/store/mocks/qb.ts
index f7c86811717..be2587c1809 100644
--- a/frontend/src/metabase-types/store/mocks/qb.ts
+++ b/frontend/src/metabase-types/store/mocks/qb.ts
@@ -1,4 +1,5 @@
import {
+ QueryBuilderDashboardState,
QueryBuilderState,
QueryBuilderUIControls,
} from "metabase-types/store";
@@ -18,6 +19,7 @@ export const createMockQueryBuilderUIControlsState = (
isShowingTimelineSidebar: false,
initialChartSetting: null,
isShowingRawTable: false,
+ isNativeEditorOpen: false,
queryBuilderMode: "view",
previousQueryBuilderMode: false,
snippetCollectionId: null,
@@ -25,6 +27,14 @@ export const createMockQueryBuilderUIControlsState = (
...opts,
});
+export const createMockQueryBuilderDashboardState = (
+ opts?: Partial<QueryBuilderDashboardState>,
+): QueryBuilderDashboardState => ({
+ dashboardId: null,
+ isEditing: false,
+ ...opts,
+});
+
export const createMockQueryBuilderState = (
opts?: Partial<QueryBuilderState>,
): QueryBuilderState => ({
@@ -34,6 +44,7 @@ export const createMockQueryBuilderState = (
documentTitle: "",
timeoutId: "",
},
+ parentDashboard: createMockQueryBuilderDashboardState(),
queryStatus: "complete",
queryResults: null,
diff --git a/frontend/src/metabase-types/store/qb.ts b/frontend/src/metabase-types/store/qb.ts
index 9d76be31f9b..ed10224db1b 100644
--- a/frontend/src/metabase-types/store/qb.ts
+++ b/frontend/src/metabase-types/store/qb.ts
@@ -1,5 +1,6 @@
import {
Card,
+ DashboardId,
Dataset,
Field,
ParameterValueOrArray,
@@ -25,6 +26,7 @@ export interface QueryBuilderUIControls {
isShowingChartSettingsSidebar: boolean;
isShowingQuestionDetailsSidebar: boolean;
isShowingTimelineSidebar: boolean;
+ isNativeEditorOpen: boolean;
initialChartSetting: null;
isShowingRawTable: boolean;
queryBuilderMode: QueryBuilderMode;
@@ -39,10 +41,15 @@ export interface QueryBuilderLoadingControls {
timeoutId: string;
}
+export interface QueryBuilderDashboardState {
+ dashboardId: DashboardId | null;
+ isEditing: boolean;
+}
+
export interface QueryBuilderState {
uiControls: QueryBuilderUIControls;
-
loadingControls: QueryBuilderLoadingControls;
+ parentDashboard: QueryBuilderDashboardState;
queryStatus: QueryBuilderQueryStatus;
queryResults: Dataset[] | null;
queryStartTime: number | null;
diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
index 21aac978f74..b1ce3313758 100644
--- a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
+++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
@@ -13,6 +13,7 @@ import Snippets from "metabase/entities/snippets";
import Questions from "metabase/entities/questions";
import { loadMetadataForCard } from "metabase/questions/actions";
import { fetchAlertsForQuestion } from "metabase/alert/alert";
+import { getIsEditingInDashboard } from "metabase/query_builder/selectors";
import { Card } from "metabase-types/api";
import {
@@ -28,12 +29,12 @@ import Question from "metabase-lib/Question";
import NativeQuery, {
updateCardTemplateTagNames,
} from "metabase-lib/queries/NativeQuery";
-import StructuredQuery from "metabase-lib/queries/StructuredQuery";
+import StructuredQuery from "metabase-lib/queries/StructuredQuery";
import { getQueryBuilderModeFromLocation } from "../../typed-utils";
import { updateUrl } from "../navigation";
-import { cancelQuery, runQuestionQuery } from "../querying";
+import { cancelQuery, runQuestionQuery } from "../querying";
import { resetQB } from "./core";
import {
propagateDashboardParameters,
@@ -314,6 +315,11 @@ async function handleQBInit(
}
}
+ if (question.isNative()) {
+ const isEditing = getIsEditingInDashboard(getState());
+ uiControls.isNativeEditorOpen = isEditing || !question.isSaved();
+ }
+
if (question.isNative() && !question.query().readOnly()) {
const query = question.query() as NativeQuery;
const newQuery = await updateTemplateTagNames(query, getState, dispatch);
diff --git a/frontend/src/metabase/query_builder/components/QueryModals.tsx b/frontend/src/metabase/query_builder/components/QueryModals.tsx
index 8a525e1b068..f6e32b995a6 100644
--- a/frontend/src/metabase/query_builder/components/QueryModals.tsx
+++ b/frontend/src/metabase/query_builder/components/QueryModals.tsx
@@ -31,7 +31,7 @@ import PreviewQueryModal from "metabase/query_builder/components/view/PreviewQue
import ConvertQueryModal from "metabase/query_builder/components/view/ConvertQueryModal";
import QuestionMoveToast from "metabase/questions/components/QuestionMoveToast";
import { Alert, Card, Collection, User } from "metabase-types/api";
-import { QueryBuilderMode } from "metabase-types/store";
+import { QueryBuilderMode, QueryBuilderUIControls } from "metabase-types/store";
import StructuredQuery from "metabase-lib/queries/StructuredQuery";
import Question from "metabase-lib/Question";
import { UpdateQuestionOpts } from "../actions/core/updateQuestion";
@@ -51,6 +51,7 @@ interface QueryModalsProps {
initialCollectionId: number;
updateQuestion: (question: Question, config?: UpdateQuestionOpts) => void;
setQueryBuilderMode: (mode: QueryBuilderMode) => void;
+ setUIControls: (opts: Partial<QueryBuilderUIControls>) => void;
originalQuestion: Question | null;
card: Card;
onCreate: (question: Question) => void;
@@ -99,6 +100,7 @@ class QueryModals extends Component<QueryModalsProps> {
onOpenModal,
updateQuestion,
setQueryBuilderMode,
+ setUIControls,
} = this.props;
switch (modal) {
@@ -345,6 +347,7 @@ class QueryModals extends Component<QueryModalsProps> {
<Modal fit onClose={onCloseModal}>
<ConvertQueryModal
onUpdateQuestion={updateQuestion}
+ onSetUIControls={setUIControls}
onClose={onCloseModal}
/>
</Modal>
diff --git a/frontend/src/metabase/query_builder/components/view/ConvertQueryModal/ConvertQueryModal.tsx b/frontend/src/metabase/query_builder/components/view/ConvertQueryModal/ConvertQueryModal.tsx
index a575bff09ff..7d969dd54b8 100644
--- a/frontend/src/metabase/query_builder/components/view/ConvertQueryModal/ConvertQueryModal.tsx
+++ b/frontend/src/metabase/query_builder/components/view/ConvertQueryModal/ConvertQueryModal.tsx
@@ -9,7 +9,7 @@ import {
getQuestion,
} from "metabase/query_builder/selectors";
import { NativeQueryForm } from "metabase-types/api";
-import { State } from "metabase-types/store";
+import { QueryBuilderUIControls, State } from "metabase-types/store";
import Question from "metabase-lib/Question";
import NativeQueryModal, { useNativeQuery } from "../NativeQueryModal";
@@ -33,7 +33,8 @@ interface UpdateQuestionOpts {
interface ConvertQueryModalProps {
question: Question;
onLoadQuery: () => Promise<NativeQueryForm>;
- onUpdateQuestion?: (question: Question, opts?: UpdateQuestionOpts) => void;
+ onUpdateQuestion: (question: Question, opts?: UpdateQuestionOpts) => void;
+ onSetUIControls: (changes: Partial<QueryBuilderUIControls>) => void;
onClose?: () => void;
}
@@ -41,6 +42,7 @@ const ConvertQueryModal = ({
question,
onLoadQuery,
onUpdateQuestion,
+ onSetUIControls,
onClose,
}: ConvertQueryModalProps): JSX.Element => {
const engineType = getEngineNativeType(question.database()?.engine);
@@ -55,8 +57,9 @@ const ConvertQueryModal = ({
const newQuestion = question.setDatasetQuery(newDatasetQuery);
onUpdateQuestion?.(newQuestion, { shouldUpdateUrl: true });
+ onSetUIControls({ isNativeEditorOpen: true });
onClose?.();
- }, [question, query, onUpdateQuestion, onClose]);
+ }, [question, query, onUpdateQuestion, onSetUIControls, onClose]);
return (
<NativeQueryModal
diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx
index e4400c2b2b0..852461bd277 100644
--- a/frontend/src/metabase/query_builder/components/view/View.jsx
+++ b/frontend/src/metabase/query_builder/components/view/View.jsx
@@ -296,7 +296,8 @@ class View extends Component {
};
renderNativeQueryEditor = () => {
- const { question, query, card, height, isDirty } = this.props;
+ const { question, query, card, height, isDirty, isNativeEditorOpen } =
+ this.props;
// Normally, when users open native models,
// they open an ad-hoc GUI question using the model as a data source
@@ -315,6 +316,7 @@ class View extends Component {
{...this.props}
viewHeight={height}
isOpen={query.isEmpty() || isDirty}
+ isInitiallyOpen={isNativeEditorOpen}
datasetQuery={card && card.dataset_query}
/>
</NativeQueryEditorContainer>
diff --git a/frontend/src/metabase/query_builder/reducers.js b/frontend/src/metabase/query_builder/reducers.js
index e24f8f837fd..3cd6fabbe29 100644
--- a/frontend/src/metabase/query_builder/reducers.js
+++ b/frontend/src/metabase/query_builder/reducers.js
@@ -82,6 +82,7 @@ const DEFAULT_UI_CONTROLS = {
isShowingChartSettingsSidebar: false,
isShowingQuestionInfoSidebar: false,
isShowingTimelineSidebar: false,
+ isNativeEditorOpen: false,
initialChartSetting: null,
isShowingRawTable: false, // table/viz toggle
queryBuilderMode: false, // "view" | "notebook" | "dataset"
@@ -96,6 +97,11 @@ const DEFAULT_LOADING_CONTROLS = {
timeoutId: "",
};
+const DEFAULT_DASHBOARD_STATE = {
+ dashboardId: null,
+ isEditing: false,
+};
+
const DEFAULT_QUERY_STATUS = "idle";
const UI_CONTROLS_SIDEBAR_DEFAULTS = {
@@ -548,17 +554,23 @@ export const currentState = handleActions(
null,
);
-export const dashboardId = handleActions(
+export const parentDashboard = handleActions(
{
[NAVIGATE_TO_NEW_CARD]: {
- next: (state, { payload: { dashboardId } }) => dashboardId,
+ next: (state, { payload: { dashboardId } }) => ({
+ dashboardId,
+ isEditing: false,
+ }),
},
[EDIT_QUESTION]: {
- next: (state, { payload: { dashboardId } }) => dashboardId,
+ next: (state, { payload: { dashboardId } }) => ({
+ dashboardId,
+ isEditing: true,
+ }),
},
- [CLOSE_QB]: { next: () => null },
+ [CLOSE_QB]: { next: () => DEFAULT_DASHBOARD_STATE },
},
- null,
+ DEFAULT_DASHBOARD_STATE,
);
export const visibleTimelineEventIds = handleActions(
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 8ea93a510c9..ee0330fbde9 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -932,7 +932,11 @@ export const getNativeQueryFn = createSelector(
);
export const getDashboardId = state => {
- return state.qb.dashboardId;
+ return state.qb.parentDashboard.dashboardId;
+};
+
+export const getIsEditingInDashboard = state => {
+ return state.qb.parentDashboard.isEditing;
};
export const getDashboard = state => {
--
GitLab