diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts index fde5d351016b962a432f921ea86ff5de476655ba..665d8d9b07fbf4a383b7ef3db021b227f5e6c15b 100644 --- a/frontend/src/metabase-lib/lib/Question.ts +++ b/frontend/src/metabase-lib/lib/Question.ts @@ -931,17 +931,16 @@ export default class Question { !question.id() || (originalQuestion && question.isDirtyComparedTo(originalQuestion)) ) { - return Urls.question( - null, - question._serializeForUrl({ + return Urls.question(null, { + hash: question._serializeForUrl({ clean, includeDisplayIsLocked, creationType, }), query, - ); + }); } else { - return Urls.question(question.card(), "", query); + return Urls.question(question.card(), { query }); } } diff --git a/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx b/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx index 1b3dcbb25ee6f618ad1254f5f8f6aa6d8b8e437c..2c71e9a3885cca92134fe8cc3ed83af504fb4bb5 100644 --- a/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx +++ b/frontend/src/metabase/admin/datamodel/components/PartialQueryBuilder.jsx @@ -108,7 +108,7 @@ export default class PartialQueryBuilder extends Component { const previewCard = { dataset_query: datasetQuery, }; - const previewUrl = Urls.question(null, previewCard); + const previewUrl = Urls.serializedQuestion(previewCard); return ( <div className="py1"> diff --git a/frontend/src/metabase/lib/card.js b/frontend/src/metabase/lib/card.js index cf6be0d44170806d13f61da6e6ecbd339e3f9cd8..4f6f2667ea4defd9b83d7bb373e20bec29b189ba 100644 --- a/frontend/src/metabase/lib/card.js +++ b/frontend/src/metabase/lib/card.js @@ -63,10 +63,9 @@ export function deserializeCardFromUrl(serialized) { } export function urlForCardState(state, dirty) { - return Urls.question( - state.card, - state.serializedCard && dirty ? state.serializedCard : "", - ); + return Urls.question(state.card, { + hash: state.serializedCard && dirty ? state.serializedCard : "", + }); } export function cleanCopyCard(card) { diff --git a/frontend/src/metabase/lib/urls.js b/frontend/src/metabase/lib/urls.js index edec8d3a42437a721a10a64f4c463876d334c04e..c721c300efe459e2af7f7e920edfdb7711cc631e 100644 --- a/frontend/src/metabase/lib/urls.js +++ b/frontend/src/metabase/lib/urls.js @@ -25,7 +25,7 @@ export const newPulse = () => `/pulse/create`; export const newCollection = collectionId => `collection/${collectionId}/new_collection`; -export function question(card, hash = "", query = "") { +export function question(card, { hash = "", query = "" } = {}) { if (hash && typeof hash === "object") { hash = serializeCardForUrl(hash); } @@ -75,8 +75,8 @@ export function question(card, hash = "", query = "") { return `${path}${query}${hash}`; } -export function serializedQuestion(card) { - return question(null, card); +export function serializedQuestion(card, opts = {}) { + return question(null, { ...opts, hash: card }); } export const extractQueryParams = query => { @@ -163,7 +163,10 @@ export function tableRowsQuery(databaseId, tableId, metricId, segmentId) { query += `&segment=${segmentId}`; } - return question(null, query); + // This will result in a URL like "/question#?db=1&table=1" + // The QB will parse the querystring and use DB and table IDs to create an ad-hoc question + // We should refactor the initializeQB to avoid passing query string to hash as it's pretty confusing + return question(null, { hash: query }); } function slugifyPersonalCollection(collection) { diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js index 5d0c3c2579096291722c9e08ad04a7a81ab0dcb2..21a5ea57dad7495a330f288b32ffc9cb4a373f3f 100644 --- a/frontend/src/metabase/query_builder/actions.js +++ b/frontend/src/metabase/query_builder/actions.js @@ -967,7 +967,7 @@ export const navigateToNewCardInsideQB = createThunkAction( dispatch(setCardAndRun(await loadCard(nextCard.id))); } else { const card = getCardAfterVisualizationClick(nextCard, previousCard); - const url = Urls.question(null, card); + const url = Urls.serializedQuestion(card); if (shouldOpenInBlankWindow(url, { blankOnMetaOrCtrlKey: true })) { open(url); } else { diff --git a/frontend/src/metabase/reference/utils.js b/frontend/src/metabase/reference/utils.js index b52220bf6a581042ffb7a4d28f8b9a78dfc2fec2..69034722fd9cbd9005cdaa94f147dfef823a755e 100644 --- a/frontend/src/metabase/reference/utils.js +++ b/frontend/src/metabase/reference/utils.js @@ -105,7 +105,7 @@ export const getQuestion = ({ }; export const getQuestionUrl = getQuestionArgs => - Urls.question(null, getQuestion(getQuestionArgs)); + Urls.question(null, { hash: getQuestion(getQuestionArgs) }); export const typeToLinkClass = { dashboard: "text-green", diff --git a/frontend/test/metabase/lib/urls.unit.spec.js b/frontend/test/metabase/lib/urls.unit.spec.js index 76955da7dfc27953d30adee17ffcc56ac41867fc..cefead4d0fa3420c5a7a0c4828fd01841cb3983c 100644 --- a/frontend/test/metabase/lib/urls.unit.spec.js +++ b/frontend/test/metabase/lib/urls.unit.spec.js @@ -13,24 +13,28 @@ describe("urls", () => { describe("question", () => { describe("with a query", () => { it("returns the correct url", () => { - expect(question({}, "", { foo: "bar" })).toEqual("/question?foo=bar"); - expect(question({}, "hash", { foo: "bar" })).toEqual( + expect(question({}, { query: { foo: "bar" } })).toEqual( + "/question?foo=bar", + ); + expect(question({}, { hash: "hash", query: { foo: "bar" } })).toEqual( "/question?foo=bar#hash", ); - expect(question(null, "hash", { foo: "bar" })).toEqual( + expect(question(null, { hash: "hash", query: { foo: "bar" } })).toEqual( "/question?foo=bar#hash", ); - expect(question(null, "", { foo: "bar" })).toEqual("/question?foo=bar"); - expect(question(null, "", { foo: "bar+bar" })).toEqual( + expect(question(null, { query: { foo: "bar" } })).toEqual( + "/question?foo=bar", + ); + expect(question(null, { query: { foo: "bar+bar" } })).toEqual( "/question?foo=bar%2Bbar", ); - expect(question(null, "", { foo: ["bar", "baz"] })).toEqual( + expect(question(null, { query: { foo: ["bar", "baz"] } })).toEqual( "/question?foo=bar&foo=baz", ); - expect(question(null, "", { foo: ["bar", "baz+bay"] })).toEqual( + expect(question(null, { query: { foo: ["bar", "baz+bay"] } })).toEqual( "/question?foo=bar&foo=baz%2Bbay", ); - expect(question(null, "", { foo: ["bar", "baz&bay"] })).toEqual( + expect(question(null, { query: { foo: ["bar", "baz&bay"] } })).toEqual( "/question?foo=bar&foo=baz%26bay", ); });