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

@@ -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,

frontend/test/metabase/meta/Dashboard.unit.spec.js

+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", () => {

frontend/test/metabase/meta/Parameter.unit.spec.js

+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", () => {