From c8121729274e25d8c04bfc75cf621012ee723e11 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Atte=20Kein=C3=A4nen?= <atte.keinanen@gmail.com>
Date: Tue, 2 May 2017 16:13:12 -0700
Subject: [PATCH] Redirect to /question but preserve card id in dashboard pivot
---
frontend/src/metabase/lib/card.js | 3 ++
frontend/src/metabase/meta/Card.js | 2 +-
frontend/src/metabase/meta/Card.spec.js | 29 ++++++++++---------
.../src/metabase/query_builder/actions.js | 19 ++++++------
.../components/Visualization.jsx | 5 ++--
package.json | 2 +-
6 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/frontend/src/metabase/lib/card.js b/frontend/src/metabase/lib/card.js
index 2b0ac9090b7..d2f93de4e0b 100644
--- a/frontend/src/metabase/lib/card.js
+++ b/frontend/src/metabase/lib/card.js
@@ -78,7 +78,9 @@ export function serializeCardForUrl(card) {
if (dataset_query.query) {
dataset_query.query = Query.cleanQuery(dataset_query.query);
}
+
var cardCopy = {
+ id: card.id,
name: card.name,
description: card.description,
dataset_query: dataset_query,
@@ -86,6 +88,7 @@ export function serializeCardForUrl(card) {
parameters: card.parameters,
visualization_settings: card.visualization_settings
};
+
return utf8_to_b64url(JSON.stringify(cardCopy));
}
diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js
index a0e001808b5..64a736d3a00 100644
--- a/frontend/src/metabase/meta/Card.js
+++ b/frontend/src/metabase/meta/Card.js
@@ -182,5 +182,5 @@ export function questionUrlWithParameters(
console.warn("UNHANDLED PARAMETER", datasetParameter);
}
}
- return Urls.question(card.id, card.dataset_query ? card : undefined, query);
+ return Urls.question(null, card.dataset_query ? card : undefined, query);
}
diff --git a/frontend/src/metabase/meta/Card.spec.js b/frontend/src/metabase/meta/Card.spec.js
index fbef5141fae..3d57d40c659 100644
--- a/frontend/src/metabase/meta/Card.spec.js
+++ b/frontend/src/metabase/meta/Card.spec.js
@@ -57,9 +57,9 @@ describe("metabase/meta/Card", () => {
it("should return question URL with no parameters", () => {
const url = Card.questionUrlWithParameters(card, metadata, []);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
- card: dissoc(card, "id")
+ card: card,
});
});
it("should return question URL with query string parameter", () => {
@@ -71,9 +71,9 @@ describe("metabase/meta/Card", () => {
parameterMappings
);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: { baz: "bar" },
- card: dissoc(card, "id")
+ card: card,
});
});
});
@@ -112,9 +112,9 @@ describe("metabase/meta/Card", () => {
it("should return question URL with no parameters", () => {
const url = Card.questionUrlWithParameters(card, metadata, []);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
- card: dissoc(card, "id")
+ card
});
});
it("should return question URL with string MBQL filter added", () => {
@@ -126,10 +126,10 @@ describe("metabase/meta/Card", () => {
parameterMappings
);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
card: assocIn(
- dissoc(card, "id"),
+ card,
["dataset_query", "query", "filter"],
["AND", ["=", ["field-id", 1], "bar"]]
)
@@ -144,10 +144,10 @@ describe("metabase/meta/Card", () => {
parameterMappings
);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
card: assocIn(
- dissoc(card, "id"),
+ card,
["dataset_query", "query", "filter"],
["AND", ["=", ["field-id", 2], 123]]
)
@@ -161,11 +161,12 @@ describe("metabase/meta/Card", () => {
{ "3": "2017-05" },
parameterMappings
);
+
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
card: assocIn(
- dissoc(card, "id"),
+ card,
["dataset_query", "query", "filter"],
["AND", ["=", ["datetime-field", ["field-id", 3], "month"], "2017-05-01"]]
)
@@ -180,10 +181,10 @@ describe("metabase/meta/Card", () => {
parameterMappings
);
expect(parseUrl(url)).toEqual({
- pathname: "/question/1",
+ pathname: "/question",
query: {},
card: assocIn(
- dissoc(card, "id"),
+ card,
["dataset_query", "query", "filter"],
["AND", ["=", ["datetime-field", ["fk->", 4, 5], "month"], "2017-05-01"]]
)
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index ca6e82c6009..4eb8d420800 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -156,24 +156,25 @@ export const initializeQB = createThunkAction(INITIALIZE_QB, (location, params)
if (params.cardId || serializedCard) {
// existing card being loaded
try {
- if (params.cardId) {
- card = await loadCard(params.cardId);
+ // if we have a serialized card then unpack it and use it
+ card = serializedCard ? deserializeCardFromUrl(serializedCard) : {}
+
+ // load the card either from `cardId` parameter or the serialized card
+ const cardId = params.cardId || card.id
+ if (cardId) {
+ const loadedCard = await loadCard(cardId);
// when we are loading from a card id we want an explicit clone of the card we loaded which is unmodified
- originalCard = Utils.copy(card);
- }
+ originalCard = Utils.copy(loadedCard);
- // if we have a serialized card then unpack it and use it
- if (serializedCard) {
- let deserializedCard = deserializeCardFromUrl(serializedCard);
// the serialized card often differs from the card stored in db so merge the properties to fetched card if needed
- card = card ? _.extend(card, deserializedCard) : deserializedCard;
+ card = {...loadedCard, ...card};
}
MetabaseAnalytics.trackEvent("QueryBuilder", "Query Loaded", card.dataset_query.type);
// if we have deserialized card from the url AND loaded a card by id then the user should be dropped into edit mode
- uiControls.isEditing = (options.edit || (params.cardId && serializedCard)) ? true : false;
+ uiControls.isEditing = !!options.edit
// if this is the users first time loading a saved card on the QB then show them the newb modal
if (params.cardId && currentUser.is_qbnewb) {
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index af1916b4f8a..bbe3ab43ce2 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -222,14 +222,13 @@ export default class Visualization extends Component<*, Props, State> {
}
handleOnChangeCardAndRun = (card: UnsavedCard) => {
- // If the current card has a name, carry that information to the new card for showing lineage
- // The card description is omitted because it isn't relevant for showing the lineage
+ // If the current card is saved, carry that information to the new card for showing lineage
const { series } = this.state
const currentlyInSavedCard = series[0] && series[0].card && series[0].card.id
if (currentlyInSavedCard) {
const savedCard: CardObject = {
...card,
- id: series[0].card.id,
+ id: series[0].card.id
};
this.props.onChangeCardAndRun(savedCard)
diff --git a/package.json b/package.json
index 9dbe719094d..b07cb3f5a32 100644
--- a/package.json
+++ b/package.json
@@ -151,7 +151,7 @@
"test": "yarn run test-jest && yarn run test-karma",
"test-karma": "karma start frontend/test/karma.conf.js --single-run",
"test-karma-watch": "karma start frontend/test/karma.conf.js --auto-watch --reporters nyan",
- "test-jest": "jest",
+ "test-jest": "jest frontend/src/",
"test-jest-watch": "jest --watch",
"test-e2e": "JASMINE_CONFIG_PATH=./frontend/test/e2e/support/jasmine.json jasmine",
"test-e2e-dev": "./frontend/test/e2e-with-persistent-browser.js",
--
GitLab