From 77af6de58da859b5fc6d137e39633f0d9f4653a0 Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Mon, 28 Feb 2022 09:03:50 -0800
Subject: [PATCH] Revert to old template tag validation logic (#20750)
* Handle legacy Field filter parameters that did not specify `:widget-type`
* Fix indentation
* Test fix
* Revert to old template tag validation logic
* Update e2e test
Co-authored-by: Cam Saul <github@camsaul.com>
---
.../metabase-lib/lib/queries/NativeQuery.ts | 16 ++-----
.../lib/queries/NativeQuery.unit.spec.js | 43 -------------------
.../sql-field-filter-none.cy.spec.js | 10 +++--
3 files changed, 11 insertions(+), 58 deletions(-)
diff --git a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
index ad6119c9823..77e8018bc51 100644
--- a/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/lib/queries/NativeQuery.ts
@@ -286,18 +286,10 @@ export default class NativeQuery extends AtomicQuery {
}
allTemplateTagsAreValid() {
- return this.templateTags().every(t => {
- if (["text", "number", "date", "card", "snippet"].includes(t.type)) {
- return true;
- }
-
- const isDimensionType = t.type === "dimension";
- const hasDefinedWidgetType =
- t["widget-type"] && t["widget-type"] !== "none";
- const hasDefinedDimension = t.dimension != null;
-
- return isDimensionType && hasDefinedWidgetType && hasDefinedDimension;
- });
+ return this.templateTags().every(
+ // field filters require a field
+ t => !(t.type === "dimension" && t.dimension == null),
+ );
}
setTemplateTag(name, tag) {
diff --git a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
index 7818d89fbcb..827e1d21db9 100644
--- a/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/queries/NativeQuery.unit.spec.js
@@ -193,49 +193,6 @@ describe("NativeQuery", () => {
expect(q.canRun()).toBe(true);
});
- it("bad type", () => {
- q = q.setDatasetQuery(
- assocIn(
- q.datasetQuery(),
- ["native", "template-tags", "foo", "type"],
- "type-that-does-not-exist",
- ),
- );
- expect(q.canRun()).toBe(false);
-
- q = q.setDatasetQuery(
- assocIn(
- q.datasetQuery(),
- ["native", "template-tags", "foo", "type"],
- "text",
- ),
- );
- expect(q.canRun()).toBe(true);
- });
-
- it("dimension type without a widget-type", () => {
- q = q.setDatasetQuery(
- assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], {
- type: "dimension",
- dimension: ["field", 123, null],
- }),
- );
-
- expect(q.canRun()).toBe(false);
- });
-
- it("dimension type with a widget-type of none", () => {
- q = q.setDatasetQuery(
- assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], {
- type: "dimension",
- "widget-type": "none",
- dimension: ["field", 123, null],
- }),
- );
-
- expect(q.canRun()).toBe(false);
- });
-
it("dimension type without a dimension", () => {
q = q.setDatasetQuery(
assocIn(q.datasetQuery(), ["native", "template-tags", "foo"], {
diff --git a/frontend/test/metabase/scenarios/native-filters/sql-field-filter-none.cy.spec.js b/frontend/test/metabase/scenarios/native-filters/sql-field-filter-none.cy.spec.js
index d481543afe4..f0f65322f09 100644
--- a/frontend/test/metabase/scenarios/native-filters/sql-field-filter-none.cy.spec.js
+++ b/frontend/test/metabase/scenarios/native-filters/sql-field-filter-none.cy.spec.js
@@ -30,9 +30,13 @@ describe("scenarios > filters > sql filters > field filter > None", () => {
filterWidget().should("not.exist");
});
- it("should disallow the running of the query and the saving of the question", () => {
- cy.get(".RunButton").should("be.disabled");
- cy.findByText("Save").should("have.attr", "aria-disabled", "true");
+ it("should be runnable with the None filter being ignored (metabase#20643)", () => {
+ cy.get(".RunButton")
+ .first()
+ .click();
+
+ cy.wait("@dataset");
+ cy.findByText("Hudson Borer");
});
it("should let you change the field filter type to something else and restore the filter widget (metabase#13825)", () => {
--
GitLab