From 0870fbd5619f631b2c1caf6bfc92b49269efdaee Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Thu, 2 Apr 2020 11:51:31 -0700
Subject: [PATCH] Fix filtering on joined fields (#12244)
* Repro for #12221
* yarn so pretty
* Fix isValidField for joined-field, which fixes filtering on joined fields. Resolves #12221
* s/fit/it/
* Fix column filtering on date fields
* fix lint
Co-authored-by: Damon P. Cortesi <d.lifehacker@gmail.com>
---
frontend/src/metabase/lib/query/field_ref.js | 2 +-
frontend/src/metabase/lib/query/filter.js | 2 +-
.../queries/structured/Filter.unit.spec.js | 35 ++++++++++++++++++-
.../metabase/lib/query/field_ref.unit.spec.js | 18 ++++++++++
.../metabase/support/join_filter.cy.spec.js | 26 ++++++++++++++
5 files changed, 80 insertions(+), 3 deletions(-)
create mode 100644 frontend/test/metabase/lib/query/field_ref.unit.spec.js
create mode 100644 frontend/test/metabase/support/join_filter.cy.spec.js
diff --git a/frontend/src/metabase/lib/query/field_ref.js b/frontend/src/metabase/lib/query/field_ref.js
index 44332da6e42..b3523d3a210 100644
--- a/frontend/src/metabase/lib/query/field_ref.js
+++ b/frontend/src/metabase/lib/query/field_ref.js
@@ -62,7 +62,7 @@ export function isValidField(field) {
(isAggregateField(field) && typeof field[1] === "number") ||
(isJoinedField(field) &&
typeof field[1] === "string" &&
- isValidField(field[0])) ||
+ isValidField(field[2])) ||
isFieldLiteral(field)
);
}
diff --git a/frontend/src/metabase/lib/query/filter.js b/frontend/src/metabase/lib/query/filter.js
index d85ee86c7ad..552c5587970 100644
--- a/frontend/src/metabase/lib/query/filter.js
+++ b/frontend/src/metabase/lib/query/filter.js
@@ -70,7 +70,7 @@ export function canAddFilter(filter: ?FilterClause): boolean {
export function isStandard(filter: FilterClause): boolean {
return (
Array.isArray(filter) &&
- STANDARD_FILTERS.has(filter[0]) &&
+ (STANDARD_FILTERS.has(filter[0]) || filter[0] === null) &&
(filter[1] === undefined || isValidField(filter[1]))
);
}
diff --git a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
index 734d0525fc0..379fa295082 100644
--- a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
@@ -1,6 +1,6 @@
import Filter from "metabase-lib/lib/queries/structured/Filter";
-import { ORDERS } from "__support__/sample_dataset_fixture";
+import { ORDERS, PEOPLE } from "__support__/sample_dataset_fixture";
const query = ORDERS.query();
@@ -72,5 +72,38 @@ describe("Filter", () => {
filter(["segment", 1]).setDimension(["field-id", ORDERS.TOTAL.id]),
).toEqual([null, ["field-id", ORDERS.TOTAL.id]]);
});
+ it("should set joined-field for new filter clause", () => {
+ const q = ORDERS.query().join({
+ alias: "foo",
+ "source-table": PEOPLE.id,
+ });
+ const f = new Filter([], 0, q);
+ expect(
+ f.setDimension(["joined-field", "foo", ["field-id", PEOPLE.EMAIL.id]], {
+ useDefaultOperator: true,
+ }),
+ ).toEqual([
+ "=",
+ ["joined-field", "foo", ["field-id", PEOPLE.EMAIL.id]],
+ undefined,
+ ]);
+ });
});
+
+ const CASES = [
+ ["isStandard", ["=", ["field-id", 1]]],
+ ["isStandard", [null, ["field-id", 1]]], // assume null operator is standard
+ ["isSegment", ["segment", 1]],
+ ["isCustom", ["or", ["=", ["field-id", 1], 42]]],
+ ];
+ for (const method of ["isStandard", "isSegment", "isCustom"]) {
+ describe(method, () => {
+ for (const [method_, mbql] of CASES) {
+ const expected = method_ === method;
+ it(`should return ${expected} for ${JSON.stringify(mbql)}`, () => {
+ expect(filter(mbql)[method]()).toEqual(expected);
+ });
+ }
+ });
+ }
});
diff --git a/frontend/test/metabase/lib/query/field_ref.unit.spec.js b/frontend/test/metabase/lib/query/field_ref.unit.spec.js
new file mode 100644
index 00000000000..d58fd540f46
--- /dev/null
+++ b/frontend/test/metabase/lib/query/field_ref.unit.spec.js
@@ -0,0 +1,18 @@
+import { isValidField } from "metabase/lib/query/field_ref";
+
+describe("field_ref", () => {
+ describe("isValidField", () => {
+ it("should be valid for field id", () => {
+ expect(isValidField(["field-id", 1])).toBe(true);
+ });
+ it("should be valid for fk", () => {
+ expect(isValidField(["fk->", ["field-id", 1], ["field-id", 2]])).toBe(
+ true,
+ );
+ });
+ it("should be valid for joined-field", () => {
+ expect(isValidField(["joined-field", "foo", ["field-id", 1]])).toBe(true);
+ });
+ // TODO: remaininng field types
+ });
+});
diff --git a/frontend/test/metabase/support/join_filter.cy.spec.js b/frontend/test/metabase/support/join_filter.cy.spec.js
new file mode 100644
index 00000000000..bb0ba3eb196
--- /dev/null
+++ b/frontend/test/metabase/support/join_filter.cy.spec.js
@@ -0,0 +1,26 @@
+import { restore, signInAsNormalUser } from "__support__/cypress";
+
+describe("support > join filter (metabase#12221)", () => {
+ before(restore);
+ before(signInAsNormalUser);
+
+ it.skip("can filter by a joined table", () => {
+ cy.visit("/question/new");
+ cy.contains("Custom question").click();
+ cy.contains("Sample Dataset").click();
+ cy.contains("Orders").click();
+ cy.get(".Icon-join_left_outer ").click();
+ cy.contains("People").click();
+ cy.contains("Orders + People");
+ cy.contains("Visualize").click();
+ cy.contains("Showing first 2,000");
+
+ cy.contains("Filter").click();
+ cy.contains("Email").click();
+ cy.contains("People – Email");
+ cy.get('[placeholder="Search by Email"]').type("wolf.");
+ cy.contains("wolf.dina@yahoo.com").click();
+ cy.contains("Add filter").click();
+ cy.contains("Showing 1 row");
+ });
+});
--
GitLab