From defd866e61cdd507cc288ae930f13323b669d160 Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Thu, 23 Jun 2022 16:26:03 -0700
Subject: [PATCH] Native query: construct `parameters[]` when necessary
(#23210)
---
frontend/src/metabase-lib/lib/Question.ts | 4 +++
.../query_builder/actions/core/core.js | 13 +++++++
.../metabase/query_builder/actions/native.js | 29 +++------------
.../template_tags/TagEditorSidebar.jsx | 1 +
.../scenarios/native/native.cy.spec.js | 36 +++++++++++++++++++
5 files changed, 59 insertions(+), 24 deletions(-)
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index e5125faa1c3..2e992561b95 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -1355,6 +1355,10 @@ export default class Question extends memoizeClass<QuestionInner>(
dataset_query,
};
+ if (type === "native") {
+ card = assocIn(card, ["parameters"], []);
+ }
+
if (tableId != null) {
card = assocIn(card, ["dataset_query", "query", "source-table"], tableId);
}
diff --git a/frontend/src/metabase/query_builder/actions/core/core.js b/frontend/src/metabase/query_builder/actions/core/core.js
index 704e348cbbb..eb8d157ae1d 100644
--- a/frontend/src/metabase/query_builder/actions/core/core.js
+++ b/frontend/src/metabase/query_builder/actions/core/core.js
@@ -26,6 +26,11 @@ import { fetchAlertsForQuestion } from "metabase/alert/alert";
import Question from "metabase-lib/lib/Question";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
+import {
+ getTemplateTagsForParameters,
+ getTemplateTagParameters,
+} from "metabase/parameters/utils/cards";
+
import { trackNewQuestionSaved } from "../../analytics";
import {
getCard,
@@ -307,6 +312,14 @@ export const updateQuestion = (
);
}
+ const newDatasetQuery = newQuestion.query().datasetQuery();
+ // Sync card's parameters with the template tags;
+ if (newDatasetQuery.type === "native") {
+ const templateTags = getTemplateTagsForParameters(newQuestion.card());
+ const parameters = getTemplateTagParameters(templateTags);
+ newQuestion = newQuestion.setParameters(parameters);
+ }
+
// Replace the current question with a new one
await dispatch.action(UPDATE_QUESTION, { card: newQuestion.card() });
diff --git a/frontend/src/metabase/query_builder/actions/native.js b/frontend/src/metabase/query_builder/actions/native.js
index 7933655c22e..57b3622c6f0 100644
--- a/frontend/src/metabase/query_builder/actions/native.js
+++ b/frontend/src/metabase/query_builder/actions/native.js
@@ -20,7 +20,6 @@ import { SET_UI_CONTROLS } from "./ui";
import {
getTemplateTagsForParameters,
getTemplateTagParameters,
- getTemplateTagParameter,
} from "metabase/parameters/utils/cards";
export const TOGGLE_DATA_REFERENCE = "metabase/qb/TOGGLE_DATA_REFERENCE";
@@ -144,29 +143,11 @@ export const setTemplateTag = createThunkAction(
},
);
- const { parameters } = updatedTagsCard;
- if (parameters && Array.isArray(parameters)) {
- if (parameters.length === 0) {
- // reconstruct from the existing template tags
- const tags = getTemplateTagsForParameters(updatedTagsCard);
- const newParameters = getTemplateTagParameters(tags);
- return assoc(updatedTagsCard, "parameters", newParameters);
- } else {
- // update an existing parameter
- const index = parameters.findIndex(p => p.id === templateTag.id);
- if (index < 0) {
- console.warn(`Can't find parameter with id=${templateTag.id}!`);
- } else {
- parameters[index] = getTemplateTagParameter(templateTag);
- }
- }
- } else {
- const tags = getTemplateTagsForParameters(updatedTagsCard);
- const newParameters = getTemplateTagParameters(tags);
- return assoc(updatedTagsCard, "parameters", newParameters);
- }
-
- return updatedTagsCard;
+ return assoc(
+ updatedTagsCard,
+ "parameters",
+ getTemplateTagParameters(getTemplateTagsForParameters(updatedTagsCard)),
+ );
};
},
);
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
index 2d22ece6958..7ea25645969 100644
--- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
+++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.jsx
@@ -50,6 +50,7 @@ export default class TagEditorSidebar extends React.Component {
setParameterValue,
onClose,
} = this.props;
+
// The tag editor sidebar excludes snippets since they have a separate sidebar.
const tags = query.templateTagsWithoutSnippets();
const database = query.database();
diff --git a/frontend/test/metabase/scenarios/native/native.cy.spec.js b/frontend/test/metabase/scenarios/native/native.cy.spec.js
index 28bc2fe8502..178d34ce886 100644
--- a/frontend/test/metabase/scenarios/native/native.cy.spec.js
+++ b/frontend/test/metabase/scenarios/native/native.cy.spec.js
@@ -216,6 +216,42 @@ describe("scenarios > question > native", () => {
cy.get("@sidebar").contains(/added/i);
});
+ it("should recognize template tags and save them as parameters", () => {
+ openNativeEditor().type(
+ "select * from PRODUCTS where CATEGORY={{cat}} and RATING >= {{stars}}",
+ {
+ parseSpecialCharSequences: false,
+ },
+ );
+ cy.get("input[placeholder*='Cat']").type("Gizmo");
+ cy.get("input[placeholder*='Stars']").type("3");
+
+ cy.get(".NativeQueryEditor .Icon-play").click();
+ cy.wait("@dataset");
+
+ cy.contains("Save").click();
+
+ modal().within(() => {
+ cy.findByLabelText("Name").type("SQL Products");
+ cy.findByText("Save").click();
+
+ // parameters[] should reflect the template tags
+ cy.wait("@card").should(xhr => {
+ const requestBody = xhr.request?.body;
+ expect(requestBody?.parameters?.length).to.equal(2);
+ });
+ });
+ cy.findByText("Not now").click();
+
+ // Now load the question again and parameters[] should still be there
+ cy.intercept("GET", "/api/card/4").as("cardQuestion");
+ cy.visit("/question/4?cat=Gizmo&stars=3");
+ cy.wait("@cardQuestion").should(xhr => {
+ const responseBody = xhr.response?.body;
+ expect(responseBody?.parameters?.length).to.equal(2);
+ });
+ });
+
it("should link correctly from the variables sidebar (metabase#16212)", () => {
cy.createNativeQuestion({
name: "test-question",
--
GitLab