diff --git a/frontend/src/metabase/App.jsx b/frontend/src/metabase/App.jsx index 936508fd7db4480e8bc560062df4008270b2e087..d20a971c33695829d3390675d208c04c10556b68 100644 --- a/frontend/src/metabase/App.jsx +++ b/frontend/src/metabase/App.jsx @@ -25,7 +25,7 @@ const mapStateToProps = (state, props) => ({ const getErrorComponent = ({ status, data, context }) => { if (status === 403) { return <Unauthorized />; - } else if (status === 404) { + } else if (status === 404 || data?.error_code === "not-found") { return <NotFound />; } else if ( data && diff --git a/frontend/src/metabase/lib/urls.js b/frontend/src/metabase/lib/urls.js index 7f6d7149f02a98d346f2a6726739005d060cdbaf..feca8a8bf70a7c72d64a866b7375f80aed61a3f2 100644 --- a/frontend/src/metabase/lib/urls.js +++ b/frontend/src/metabase/lib/urls.js @@ -49,6 +49,8 @@ export function question(card, hash = "", query = "") { } const { card_id, id, name } = card; + const basePath = + card?.dataset || card?.model === "dataset" ? "dataset" : "question"; /** * If the question has been added to the dashboard we're reading the dashCard's properties. @@ -65,10 +67,10 @@ export function question(card, hash = "", query = "") { * Please see: https://github.com/metabase/metabase/pull/15989#pullrequestreview-656646149 */ if (!name) { - return `/question/${questionId}${query}${hash}`; + return `/${basePath}/${questionId}${query}${hash}`; } - const path = appendSlug(`/question/${questionId}`, slugg(name)); + const path = appendSlug(`/${basePath}/${questionId}`, slugg(name)); return `${path}${query}${hash}`; } diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js index 992b014e78dba02e94ab80f3dcc717b2020eb2c6..c5178708653eb8d81fa23a3d7d2d688b10b3f3a0 100644 --- a/frontend/src/metabase/query_builder/actions.js +++ b/frontend/src/metabase/query_builder/actions.js @@ -449,6 +449,18 @@ export const initializeQB = (location, params, queryParams) => { card = null; } + if (!card.dataset && location.pathname.startsWith("/dataset")) { + dispatch( + setErrorPage({ + data: { + error_code: "not-found", + }, + context: "query-builder", + }), + ); + card = null; + } + preserveParameters = true; } catch (error) { console.warn("initializeQb failed because of an error:", error); @@ -1087,7 +1099,7 @@ export const apiCreateQuestion = question => { }; export const API_UPDATE_QUESTION = "metabase/qb/API_UPDATE_QUESTION"; -export const apiUpdateQuestion = question => { +export const apiUpdateQuestion = (question, { rerunQuery = false } = {}) => { return async (dispatch, getState) => { const originalQuestion = getOriginalQuestion(getState()); question = question || getQuestion(getState()); @@ -1135,6 +1147,11 @@ export const apiUpdateQuestion = question => { } dispatch.action(API_UPDATE_QUESTION, updatedQuestion.card()); + + if (rerunQuery) { + await dispatch(loadMetadataForCard(question.card())); + dispatch(runQuestionQuery()); + } }; }; @@ -1482,21 +1499,12 @@ export const turnQuestionIntoDataset = () => async (dispatch, getState) => { // to enable "simple mode" like features dataset = dataset.composeDataset(); } - await dispatch(apiUpdateQuestion(dataset)); - await dispatch(loadMetadataForCard(dataset.card())); - - // When a question is turned into a dataset, - // its visualization is changed to a table - // In order for that transition not to look like something just broke, - // we rerun the query - if (question.display() !== "table") { - dispatch(runQuestionQuery()); - } + await dispatch(apiUpdateQuestion(dataset, { rerunQuery: true })); dispatch( addUndo({ message: t`This is a dataset now.`, - actions: [apiUpdateQuestion(question)], + actions: [apiUpdateQuestion(question, { rerunQuery: true })], }), ); }; @@ -1512,16 +1520,19 @@ export const turnDatasetIntoQuestion = () => async (dispatch, getState) => { const originalQuestion = getOriginalQuestion(getState()); question = question.setDatasetQuery(originalQuestion.datasetQuery()); } - await dispatch(apiUpdateQuestion(question)); + await dispatch(apiUpdateQuestion(question, { rerunQuery: true })); + + const undoAction = apiUpdateQuestion( + dataset.isStructured() ? dataset.composeDataset() : dataset, + { + rerunQuery: true, + }, + ); dispatch( addUndo({ message: t`This is a question now.`, - actions: [ - apiUpdateQuestion( - dataset.isStructured() ? dataset.composeDataset() : dataset, - ), - ], + actions: [undoAction], }), ); }; diff --git a/frontend/src/metabase/routes.jsx b/frontend/src/metabase/routes.jsx index c13b85277f2e715e4af7c536934dad971165f11b..e7a99304e8a33bd74de464a1a93003752c049c4a 100644 --- a/frontend/src/metabase/routes.jsx +++ b/frontend/src/metabase/routes.jsx @@ -240,6 +240,13 @@ export const getRoutes = store => ( <Route path=":slug/notebook" component={QueryBuilder} /> </Route> + <Route path="/dataset"> + <IndexRoute component={QueryBuilder} /> + <Route path="notebook" component={QueryBuilder} /> + <Route path=":slug" component={QueryBuilder} /> + <Route path=":slug/notebook" component={QueryBuilder} /> + </Route> + <Route path="/ready" component={PostSetupApp} /> <Route path="browse" component={BrowseApp}> diff --git a/frontend/test/metabase/lib/urls.unit.spec.js b/frontend/test/metabase/lib/urls.unit.spec.js index 6f5400827614751578942b59f0ea0baadba89d0b..1b489af1a3d9aceaab274282231c8557920b24a7 100644 --- a/frontend/test/metabase/lib/urls.unit.spec.js +++ b/frontend/test/metabase/lib/urls.unit.spec.js @@ -66,6 +66,20 @@ describe("urls", () => { ); }); }); + + describe("dataset", () => { + it("returns /dataset URLS", () => { + expect(question({ id: 1, dataset: true, name: "Foo" })).toEqual( + "/dataset/1-foo", + ); + expect( + question({ id: 1, card_id: 42, dataset: true, name: "Foo" }), + ).toEqual("/dataset/42-foo"); + expect( + question({ id: 1, card_id: 42, model: "dataset", name: "Foo" }), + ).toEqual("/dataset/42-foo"); + }); + }); }); describe("query", () => { diff --git a/frontend/test/metabase/scenarios/question/datasets.cy.spec.js b/frontend/test/metabase/scenarios/question/datasets.cy.spec.js index 34bf71085e8b45cb26da915f6313df13adffc9d8..5643c15db5eceb73bb5f0fa0573744059aa62a6d 100644 --- a/frontend/test/metabase/scenarios/question/datasets.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/datasets.cy.spec.js @@ -83,7 +83,7 @@ describe("scenarios > datasets", () => { it("allows to turn a dataset back into a saved question", () => { cy.request("PUT", "/api/card/1", { dataset: true }); cy.intercept("PUT", "/api/card/1").as("cardUpdate"); - cy.visit("/question/1"); + cy.visit("/dataset/1"); openDetailsSidebar(); cy.findByText("Turn back into a saved question").click(); @@ -97,6 +97,19 @@ describe("scenarios > datasets", () => { assertIsDataset(); }); + it("shows 404 when opening a question with a /dataset URL", () => { + cy.visit("/dataset/1"); + cy.findByText(/We're a little lost/i); + }); + + it("redirects to /dataset URL when opening a dataset with /question URL", () => { + cy.request("PUT", "/api/card/1", { dataset: true }); + cy.visit("/question/1"); + openDetailsSidebar(); + assertIsDataset(); + cy.url().should("include", "/dataset"); + }); + describe("data picker", () => { beforeEach(() => { cy.intercept("GET", "/api/search").as("search"); @@ -508,7 +521,7 @@ function openDetailsSidebar() { cy.findByTestId("saved-question-header-button").click(); } -function getDetailsSidebarActions(iconName) { +function getDetailsSidebarActions() { return cy.findByTestId("question-action-buttons"); }