From bef09e293de9b1fe77d960236fecf8387d4c70e9 Mon Sep 17 00:00:00 2001 From: Dalton <daltojohnso@users.noreply.github.com> Date: Tue, 19 Oct 2021 08:56:08 -0700 Subject: [PATCH] add more parameters unit tests (#18308) * add more parameters unit tests * add a few meta/Dashboard tests * remove some old, unnecessary comments * fix assertions in tests * fix getParameterTargetField tests * delete nonsensical test --- frontend/src/metabase/meta/Parameter.js | 1 - .../test/metabase/meta/Dashboard.unit.spec.js | 193 +++++++++++- .../test/metabase/meta/Parameter.unit.spec.js | 287 +++++++++++++++++- 3 files changed, 465 insertions(+), 16 deletions(-) diff --git a/frontend/src/metabase/meta/Parameter.js b/frontend/src/metabase/meta/Parameter.js index fcec1ed1b77..965190bbda7 100644 --- a/frontend/src/metabase/meta/Parameter.js +++ b/frontend/src/metabase/meta/Parameter.js @@ -374,7 +374,6 @@ function isDimensionTarget(target) { return target?.[0] === "dimension"; } -/** Returns the field ID that this parameter target points to, or null if it's not a dimension target. */ export function getParameterTargetField( target: ?ParameterTarget, metadata, diff --git a/frontend/test/metabase/meta/Dashboard.unit.spec.js b/frontend/test/metabase/meta/Dashboard.unit.spec.js index 3c9c50ffaaa..19e553471c5 100644 --- a/frontend/test/metabase/meta/Dashboard.unit.spec.js +++ b/frontend/test/metabase/meta/Dashboard.unit.spec.js @@ -1,3 +1,4 @@ +import _ from "underscore"; import { metadata, SAMPLE_DATASET, @@ -5,7 +6,12 @@ import { ORDERS, PRODUCTS, } from "__support__/sample_dataset_fixture"; +import MetabaseSettings from "metabase/lib/settings"; import { + getParameterSections, + createParameter, + setParameterName, + setParameterDefaultValue, getParameterMappingOptions, hasMapping, isDashboardParameterWithoutMapping, @@ -17,27 +23,186 @@ import Field from "metabase-lib/lib/metadata/Field"; function structured(query) { return SAMPLE_DATASET.question(query).card(); - // return { - // dataset_query: { - // database: SAMPLE_DATASET.id, - // type: "query", - // query: query, - // }, - // }; } function native(native) { return SAMPLE_DATASET.nativeQuestion(native).card(); - // return { - // dataset_query: { - // database: SAMPLE_DATASET.id, - // type: "native", - // native: native, - // }, - // }; +} + +MetabaseSettings.get = jest.fn(); + +function mockFieldFilterOperatorsFlag(value) { + MetabaseSettings.get.mockImplementation(flag => { + if (flag === "field-filter-operators-enabled?") { + return value; + } + }); } describe("meta/Dashboard", () => { + describe("getParameterSections", () => { + beforeEach(() => { + mockFieldFilterOperatorsFlag(false); + }); + + describe("when `field-filter-operators-enabled?` is false", () => { + it("should not have a number section", () => { + expect( + _.findWhere(getParameterSections(), { id: "number" }), + ).not.toBeDefined(); + }); + + it("should have location options that map to location/* parameters", () => { + const locationSection = _.findWhere(getParameterSections(), { + id: "location", + }); + expect( + locationSection.options.every(option => + option.type.startsWith("location"), + ), + ).toBe(true); + }); + + it("should have a category section", () => { + expect( + _.findWhere(getParameterSections(), { id: "category" }), + ).toBeDefined(); + + expect( + _.findWhere(getParameterSections(), { id: "string" }), + ).not.toBeDefined(); + }); + }); + + describe("when `field-filter-operators-enabled?` is true", () => { + beforeEach(() => { + mockFieldFilterOperatorsFlag(true); + }); + + it("should have a number section", () => { + expect( + _.findWhere(getParameterSections(), { id: "number" }), + ).toBeDefined(); + }); + + it("should have location options that map to string/* parameters", () => { + const locationSection = _.findWhere(getParameterSections(), { + id: "location", + }); + expect( + locationSection.options.every(option => + option.type.startsWith("string"), + ), + ).toBe(true); + }); + + it("should have a string section", () => { + expect( + _.findWhere(getParameterSections(), { id: "category" }), + ).not.toBeDefined(); + + expect( + _.findWhere(getParameterSections(), { id: "string" }), + ).toBeDefined(); + }); + }); + }); + + describe("createParameter", () => { + it("should create a new parameter using the given parameter option", () => { + expect( + createParameter( + { + name: "foo bar", + type: "category", + sectionId: "abc", + }, + [], + ), + ).toEqual({ + id: expect.any(String), + name: "foo bar", + sectionId: "abc", + slug: "foo_bar", + type: "category", + }); + }); + + it("should prioritize using `combinedName` over `name`", () => { + expect( + createParameter( + { + combinedName: "foo bar baz", + name: "foo bar", + type: "category", + sectionId: "abc", + }, + [], + ), + ).toEqual({ + id: expect.any(String), + name: "foo bar baz", + sectionId: "abc", + slug: "foo_bar_baz", + type: "category", + }); + }); + + it("should prevent a duplicate name", () => { + expect( + createParameter( + { + name: "foo bar", + type: "category", + sectionId: "abc", + }, + [ + createParameter( + { + name: "foo bar", + type: "category", + sectionId: "abc", + }, + [], + ), + ], + ), + ).toEqual({ + id: expect.any(String), + name: "foo bar 1", + sectionId: "abc", + slug: "foo_bar_1", + type: "category", + }); + }); + }); + + describe("setParameterName", () => { + it("should set a name and a slug on parameter", () => { + expect(setParameterName({ abc: 123 }, "foo")).toEqual({ + abc: 123, + name: "foo", + slug: "foo", + }); + }); + + it("should default", () => { + expect(setParameterName({}, "")).toEqual({ + name: "unnamed", + slug: "unnamed", + }); + }); + }); + + describe("setParameterDefaultValue", () => { + it("should set a `default` property on a parameter", () => { + expect(setParameterDefaultValue({ foo: "bar" }, 123)).toEqual({ + foo: "bar", + default: 123, + }); + }); + }); + describe("getParameterMappingOptions", () => { describe("Structured Query", () => { it("should return field-id and fk-> dimensions", () => { diff --git a/frontend/test/metabase/meta/Parameter.unit.spec.js b/frontend/test/metabase/meta/Parameter.unit.spec.js index 30ebbe1a91f..c4c6e54a696 100644 --- a/frontend/test/metabase/meta/Parameter.unit.spec.js +++ b/frontend/test/metabase/meta/Parameter.unit.spec.js @@ -1,5 +1,13 @@ +import _ from "underscore"; + import MetabaseSettings from "metabase/lib/settings"; import { + PARAMETER_OPERATOR_TYPES, + getParameterOptions, + getOperatorDisplayName, + dimensionFilterForParameter, + getTagOperatorFilterForParameter, + getParameterTargetField, dateParameterValueToMBQL, stringParameterValueToMBQL, numberParameterValueToMBQL, @@ -15,7 +23,11 @@ import { buildHiddenParametersSlugSet, getVisibleParameters, } from "metabase/meta/Parameter"; -import { metadata, PRODUCTS } from "__support__/sample_dataset_fixture"; +import { + metadata, + PRODUCTS, + SAMPLE_DATASET, +} from "__support__/sample_dataset_fixture"; MetabaseSettings.get = jest.fn(); @@ -29,9 +41,91 @@ function mockFieldFilterOperatorsFlag(value) { describe("metabase/meta/Parameter", () => { beforeEach(() => { + jest.clearAllMocks(); MetabaseSettings.get.mockReturnValue(false); }); + describe("getParameterOptions", () => { + describe("when `field-filter-operators-enabled?` is false", () => { + beforeEach(() => { + mockFieldFilterOperatorsFlag(false); + }); + + it("should return options without operator subtypes (except for date parameters)", () => { + const options = new Set(_.map(getParameterOptions(), "type")); + const expectedOptionTypes = [ + "id", + "category", + "location/city", + "location/state", + "location/zip_code", + "location/country", + ].concat(_.map(PARAMETER_OPERATOR_TYPES.date, "type")); + + expect(expectedOptionTypes.length).toEqual(options.size); + expect(expectedOptionTypes.every(option => options.has(option))).toBe( + true, + ); + }); + }); + + describe("when `field-filter-operators-enabled?` is true", () => { + beforeEach(() => { + mockFieldFilterOperatorsFlag(true); + }); + + it("should return options with operator subtypes", () => { + const options = new Set(_.map(getParameterOptions(), "type")); + const expectedOptionTypes = ["id"].concat( + _.map(PARAMETER_OPERATOR_TYPES.number, "type"), + _.map(PARAMETER_OPERATOR_TYPES.string, "type"), + _.map(PARAMETER_OPERATOR_TYPES.date, "type"), + ); + + expect(expectedOptionTypes.length).toEqual(options.size); + expect(expectedOptionTypes.every(option => options.has(option))).toBe( + true, + ); + }); + + it("should add a `combinedName` property to options", () => { + const optionsByType = _.indexBy(getParameterOptions(), "type"); + + expect(optionsByType["string/="].combinedName).toEqual("String"); + expect(optionsByType["string/!="].combinedName).toEqual( + "String is not", + ); + expect(optionsByType["number/!="].combinedName).toEqual("Not equal to"); + expect(optionsByType["date/single"].combinedName).toEqual( + "Single Date", + ); + }); + }); + }); + + describe("getOperatorDisplayName", () => { + it("should return an option's name when the operator is a date or a number", () => { + expect(getOperatorDisplayName({ name: "foo" }, "date")).toEqual("foo"); + expect(getOperatorDisplayName({ name: "foo" }, "number")).toEqual("foo"); + }); + + it("should return an option's section name for the string/= option", () => { + expect( + getOperatorDisplayName({ name: "foo", operator: "=" }, "string", "bar"), + ).toEqual("bar"); + }); + + it("should otherwise return a combined sectionName + option name", () => { + expect( + getOperatorDisplayName( + { name: "Foo", operator: "!=" }, + "string", + "Bar", + ), + ).toEqual("Bar foo"); + }); + }); + describe("dateParameterValueToMBQL", () => { it("should parse past30days", () => { expect(dateParameterValueToMBQL("past30days", null)).toEqual([ @@ -334,6 +428,197 @@ describe("metabase/meta/Parameter", () => { availableOptions.every(option => option.type.startsWith("id")), ).toBe(true); }); + + it("should return the specific location/state option for a state field", () => { + const stateField = { + ...field, + isState: () => true, + }; + const availableOptions = parameterOptionsForField(stateField); + expect(availableOptions).toEqual([ + expect.objectContaining({ type: "location/state" }), + ]); + }); + + it("as a result of all location parameters haiving subtypes should return nothing for a generic location field", () => { + const locationField = { ...field, isLocation: () => true }; + const availableOptions = parameterOptionsForField(locationField); + expect(availableOptions).toEqual([]); + }); + }); + + describe("dimensionFilterForParameter", () => { + const field = { + isDate: () => false, + isID: () => false, + isCategory: () => false, + isCity: () => false, + isState: () => false, + isZipCode: () => false, + isCountry: () => false, + isNumber: () => false, + isString: () => false, + isLocation: () => false, + }; + const typelessDimension = { + field: () => field, + }; + + [ + [ + { type: "date/single" }, + { + type: "date", + field: () => ({ ...field, isDate: () => true }), + }, + ], + [ + { type: "id" }, + { + type: "id", + field: () => ({ ...field, isID: () => true }), + }, + ], + [ + { type: "category" }, + { + type: "category", + field: () => ({ ...field, isCategory: () => true }), + }, + ], + [ + { type: "location/city" }, + { + type: "city", + field: () => ({ ...field, isCity: () => true }), + }, + ], + [ + { type: "number/!=" }, + { + type: "number", + field: () => ({ + ...field, + isNumber: () => true, + isCoordinate: () => false, + }), + }, + ], + [ + { type: "string/=" }, + { + type: "category", + field: () => ({ + ...field, + isCategory: () => true, + }), + }, + ], + [ + { type: "string/!=" }, + { + type: "category", + field: () => ({ + ...field, + isCategory: () => true, + }), + }, + ], + [ + { type: "string/starts-with" }, + { + type: "string", + field: () => ({ + ...field, + isString: () => true, + }), + }, + ], + ].forEach(([parameter, dimension]) => { + it(`should return a predicate that evaluates to true for a ${dimension.type} dimension when given a ${parameter.type} parameter`, () => { + const predicate = dimensionFilterForParameter(parameter); + expect(predicate(typelessDimension)).toBe(false); + expect(predicate(dimension)).toBe(true); + }); + }); + + it("should return a predicate that evaluates to false for a coordinate dimension when given a number parameter", () => { + const coordinateDimension = { + field: () => ({ + ...field, + isNumber: () => true, + isCoordinate: () => true, + }), + }; + + const predicate = dimensionFilterForParameter({ type: "number/between" }); + expect(predicate(coordinateDimension)).toBe(false); + }); + + it("should return a predicate that evaluates to false for a location dimension when given a category parameter", () => { + const locationDimension = { + field: () => ({ + ...field, + isLocation: () => true, + }), + }; + + const predicate = dimensionFilterForParameter({ type: "category" }); + expect(predicate(locationDimension)).toBe(false); + }); + }); + + describe("getTagOperatorFilterForParameter", () => { + it("should return a predicate that evaluates to true for a template tag that has the same subtype operator as the given parameter", () => { + const predicate = getTagOperatorFilterForParameter({ + type: "string/starts-with", + }); + const templateTag1 = { + "widget-type": "string/starts-with", + }; + const templateTag2 = { + "widget-type": "foo/starts-with", + }; + const templateTag3 = { + "widget-type": "string/ends-with", + }; + expect(predicate(templateTag1)).toBe(true); + expect(predicate(templateTag2)).toBe(true); + expect(predicate(templateTag3)).toBe(false); + }); + }); + + describe("getParameterTargetField", () => { + it("should return null when the target is not a dimension", () => { + expect(getParameterTargetField(["variable", "foo"], metadata)).toBe(null); + }); + + it("should return the mapped field behind a template tag field filter", () => { + const target = ["dimension", ["template-tag", "foo"]]; + const question = SAMPLE_DATASET.nativeQuestion({ + query: "select * from PRODUCTS where {{foo}}", + "template-tags": { + foo: { + type: "dimension", + dimension: ["field", PRODUCTS.CATEGORY.id, null], + }, + }, + }); + + expect(getParameterTargetField(target, metadata, question)).toBe( + PRODUCTS.CATEGORY, + ); + }); + + it("should return the target field", () => { + const target = ["dimension", ["field", PRODUCTS.CATEGORY.id, null]]; + const question = SAMPLE_DATASET.question({ + "source-table": PRODUCTS.id, + }); + expect(getParameterTargetField(target, metadata, question)).toBe( + PRODUCTS.CATEGORY, + ); + }); }); describe("normalizeParameterValue", () => { -- GitLab