From 275e6112d84ec7a65358c8e1fc5572ad0f130693 Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Wed, 27 Oct 2021 09:57:47 -0700
Subject: [PATCH] move parameter ui utils to separate file (#18690)
* move parameter ui utils to separate file
* move utils for building parameters from cards to own utils file (#18691)
---
frontend/src/metabase-lib/lib/Question.js | 6 +-
.../components/ParametersPopover.jsx | 2 +-
frontend/src/metabase/meta/Card.js | 68 ++------------
frontend/src/metabase/meta/Parameter.js | 74 ---------------
.../components/ParameterValueWidget.jsx | 2 +-
.../parameters/components/ParametersList.jsx | 2 +-
.../src/metabase/parameters/utils/cards.js | 89 +++++++++++++++++++
.../parameters/utils/cards.unit.spec.js} | 57 +-----------
frontend/src/metabase/parameters/utils/ui.js | 33 +++++++
.../metabase/parameters/utils/ui.unit.spec.js | 67 ++++++++++++++
.../public/containers/PublicQuestion.jsx | 4 +-
.../src/metabase/query_builder/actions.js | 8 +-
.../containers/QuestionEmbedWidget.jsx | 2 +-
.../src/metabase/query_builder/selectors.js | 2 +-
src/metabase/api/embed.clj | 2 +-
15 files changed, 210 insertions(+), 208 deletions(-)
delete mode 100644 frontend/src/metabase/meta/Parameter.js
create mode 100644 frontend/src/metabase/parameters/utils/cards.js
rename frontend/{test/metabase/meta/Parameter.unit.spec.js => src/metabase/parameters/utils/cards.unit.spec.js} (75%)
create mode 100644 frontend/src/metabase/parameters/utils/ui.js
create mode 100644 frontend/src/metabase/parameters/utils/ui.unit.spec.js
diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js
index 4759faed28a..14bf1ae6848 100644
--- a/frontend/src/metabase-lib/lib/Question.js
+++ b/frontend/src/metabase-lib/lib/Question.js
@@ -34,10 +34,8 @@ import {
findColumnSettingIndexForColumn,
syncTableColumnsToQuery,
} from "metabase/lib/dataset";
-import {
- getValueAndFieldIdPopulatedParametersFromCard,
- isTransientId,
-} from "metabase/meta/Card";
+import { isTransientId } from "metabase/meta/Card";
+import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
import { parameterToMBQLFilter } from "metabase/parameters/utils/mbql";
import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values";
import {
diff --git a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
index d4a0fbe6bcc..50da002e0bb 100644
--- a/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
+++ b/frontend/src/metabase/dashboard/components/ParametersPopover.jsx
@@ -2,7 +2,7 @@ import React, { Component } from "react";
import { t } from "ttag";
import { getDashboardParameterSections } from "metabase/parameters/utils/dashboard-options";
import Icon from "metabase/components/Icon";
-import { getParameterIconName } from "metabase/meta/Parameter";
+import { getParameterIconName } from "metabase/parameters/utils/ui";
import styled from "styled-components";
import type {
diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js
index 49e4a0a41c0..2900230f148 100644
--- a/frontend/src/metabase/meta/Card.js
+++ b/frontend/src/metabase/meta/Card.js
@@ -1,21 +1,16 @@
-import { getTemplateTagParameters } from "metabase/meta/Parameter";
+import _ from "underscore";
+import { assoc, updateIn } from "icepick";
+
+import { getParametersFromCard } from "metabase/parameters/utils/cards";
import { parameterToMBQLFilter } from "metabase/parameters/utils/mbql";
-import { getParameterTargetField } from "metabase/parameters/utils/targets";
-import {
- normalizeParameterValue,
- getValuePopulatedParameters,
-} from "metabase/parameters/utils/parameter-values";
+import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values";
import * as Query from "metabase/lib/query/query";
import * as Q_DEPRECATED from "metabase/lib/query"; // legacy
import Utils from "metabase/lib/utils";
import * as Urls from "metabase/lib/urls";
-import Question from "metabase-lib/lib/Question";
-
-import _ from "underscore";
-import { assoc, updateIn } from "icepick";
-import type { StructuredQuery, TemplateTag } from "metabase-types/types/Query";
+import type { StructuredQuery } from "metabase-types/types/Query";
import type {
Card,
DatasetQuery,
@@ -30,10 +25,6 @@ import type {
import type Metadata from "metabase-lib/lib/metadata/Metadata";
import type Table from "metabase-lib/lib/metadata/Table";
-declare class Object {
- static values<T>(object: { [key: string]: T }): Array<T>;
-}
-
// TODO Atte Keinänen 6/5/17 Should these be moved to corresponding metabase-lib classes?
// Is there any reason behind keeping them in a central place?
@@ -113,53 +104,6 @@ export function getTableMetadata(card: Card, metadata: Metadata): ?Table {
return null;
}
-export function getTemplateTagsForParameters(card: ?Card): Array<TemplateTag> {
- const templateTags =
- card &&
- card.dataset_query &&
- card.dataset_query.type === "native" &&
- card.dataset_query.native["template-tags"]
- ? Object.values(card.dataset_query.native["template-tags"])
- : [];
- return templateTags.filter(
- // this should only return template tags that define a parameter of the card
- tag => tag.type !== "card" && tag.type !== "snippet",
- );
-}
-
-export function getParametersFromCard(card: ?Card): Parameter[] {
- if (card && card.parameters) {
- return card.parameters;
- }
-
- const tags: TemplateTag[] = getTemplateTagsForParameters(card);
- return getTemplateTagParameters(tags);
-}
-
-export function getValueAndFieldIdPopulatedParametersFromCard(
- card: Card,
- metadata,
- parameterValues?: ParameterValues,
-): Parameter[] {
- const parameters = getParametersFromCard(card);
- const valuePopulatedParameters = getValuePopulatedParameters(
- parameters,
- parameterValues,
- );
- const question = new Question(card, metadata);
-
- return valuePopulatedParameters.map(parameter => {
- // if we have a field id for this parameter, set "field_id"
- const field = getParameterTargetField(parameter.target, metadata, question);
- if (field != null) {
- parameter = assoc(parameter, "fields", [field]);
- parameter = assoc(parameter, "field_id", field.id);
- }
- parameter = assoc(parameter, "hasOnlyFieldTargets", field != null);
- return parameter;
- });
-}
-
// NOTE Atte Keinänen 7/5/17: Still used in dashboards and public questions.
// Query builder uses `Question.getResults` which contains similar logic.
export function applyParameters(
diff --git a/frontend/src/metabase/meta/Parameter.js b/frontend/src/metabase/meta/Parameter.js
deleted file mode 100644
index a862dae0c9a..00000000000
--- a/frontend/src/metabase/meta/Parameter.js
+++ /dev/null
@@ -1,74 +0,0 @@
-import MetabaseSettings from "metabase/lib/settings";
-import type { TemplateTag } from "metabase-types/types/Query";
-import type { Parameter } from "metabase-types/types/Parameter";
-import _ from "underscore";
-import { getParameterType } from "metabase/parameters/utils/parameter-type";
-
-const areFieldFilterOperatorsEnabled = () =>
- MetabaseSettings.get("field-filter-operators-enabled?");
-
-// NOTE: this should mirror `template-tag-parameters` in src/metabase/api/embed.clj
-export function getTemplateTagParameters(tags: TemplateTag[]): Parameter[] {
- function getTemplateTagType(tag) {
- const { type } = tag;
- if (type === "date") {
- return "date/single";
- } else if (areFieldFilterOperatorsEnabled() && type === "string") {
- return "string/=";
- } else if (areFieldFilterOperatorsEnabled() && type === "number") {
- return "number/=";
- } else {
- return "category";
- }
- }
-
- return tags
- .filter(
- tag =>
- tag.type != null && (tag["widget-type"] || tag.type !== "dimension"),
- )
- .map(tag => {
- return {
- id: tag.id,
- type: tag["widget-type"] || getTemplateTagType(tag),
- target:
- tag.type === "dimension"
- ? ["dimension", ["template-tag", tag.name]]
- : ["variable", ["template-tag", tag.name]],
- name: tag["display-name"],
- slug: tag.name,
- default: tag.default,
- };
- });
-}
-
-export function getParameterIconName(parameter: ?Parameter) {
- const type = getParameterType(parameter);
- switch (type) {
- case "date":
- return "calendar";
- case "location":
- return "location";
- case "category":
- return "string";
- case "number":
- return "number";
- case "id":
- default:
- return "label";
- }
-}
-
-export function buildHiddenParametersSlugSet(hiddenParameterSlugs) {
- return _.isString(hiddenParameterSlugs)
- ? new Set(hiddenParameterSlugs.split(","))
- : new Set();
-}
-
-export function getVisibleParameters(parameters, hiddenParameterSlugs) {
- const hiddenParametersSlugSet = buildHiddenParametersSlugSet(
- hiddenParameterSlugs,
- );
-
- return parameters.filter(p => !hiddenParametersSlugSet.has(p.slug));
-}
diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
index a20d2210777..27420d5fac0 100644
--- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
+++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx
@@ -23,7 +23,7 @@ import {
makeGetMergedParameterFieldValues,
} from "metabase/selectors/metadata";
-import { getParameterIconName } from "metabase/meta/Parameter";
+import { getParameterIconName } from "metabase/parameters/utils/ui";
import { deriveFieldOperatorFromParameter } from "metabase/parameters/utils/operators";
import { isDashboardParameterWithoutMapping } from "metabase/meta/Dashboard";
diff --git a/frontend/src/metabase/parameters/components/ParametersList.jsx b/frontend/src/metabase/parameters/components/ParametersList.jsx
index a4879c224a0..01e81b76547 100644
--- a/frontend/src/metabase/parameters/components/ParametersList.jsx
+++ b/frontend/src/metabase/parameters/components/ParametersList.jsx
@@ -9,7 +9,7 @@ import {
SortableElement,
SortableHandle,
} from "metabase/components/sortable";
-import { getVisibleParameters } from "metabase/meta/Parameter";
+import { getVisibleParameters } from "metabase/parameters/utils/ui";
import { getValuePopulatedParameters } from "metabase/parameters/utils/parameter-values";
import type {
diff --git a/frontend/src/metabase/parameters/utils/cards.js b/frontend/src/metabase/parameters/utils/cards.js
new file mode 100644
index 00000000000..9bfbbd104de
--- /dev/null
+++ b/frontend/src/metabase/parameters/utils/cards.js
@@ -0,0 +1,89 @@
+import { assoc } from "icepick";
+
+import Question from "metabase-lib/lib/Question";
+
+import { areFieldFilterOperatorsEnabled } from "./feature-flag";
+import { getParameterTargetField } from "metabase/parameters/utils/targets";
+import { getValuePopulatedParameters } from "metabase/parameters/utils/parameter-values";
+
+// NOTE: this should mirror `template-tag-parameters` in src/metabase/api/embed.clj
+export function getTemplateTagParameters(tags) {
+ function getTemplateTagType(tag) {
+ const { type } = tag;
+ if (type === "date") {
+ return "date/single";
+ } else if (areFieldFilterOperatorsEnabled() && type === "string") {
+ return "string/=";
+ } else if (areFieldFilterOperatorsEnabled() && type === "number") {
+ return "number/=";
+ } else {
+ return "category";
+ }
+ }
+
+ return tags
+ .filter(
+ tag =>
+ tag.type != null && (tag["widget-type"] || tag.type !== "dimension"),
+ )
+ .map(tag => {
+ return {
+ id: tag.id,
+ type: tag["widget-type"] || getTemplateTagType(tag),
+ target:
+ tag.type === "dimension"
+ ? ["dimension", ["template-tag", tag.name]]
+ : ["variable", ["template-tag", tag.name]],
+ name: tag["display-name"],
+ slug: tag.name,
+ default: tag.default,
+ };
+ });
+}
+
+export function getTemplateTagsForParameters(card) {
+ const templateTags =
+ card &&
+ card.dataset_query &&
+ card.dataset_query.type === "native" &&
+ card.dataset_query.native["template-tags"]
+ ? Object.values(card.dataset_query.native["template-tags"])
+ : [];
+ return templateTags.filter(
+ // this should only return template tags that define a parameter of the card
+ tag => tag.type !== "card" && tag.type !== "snippet",
+ );
+}
+
+export function getParametersFromCard(card) {
+ if (card && card.parameters) {
+ return card.parameters;
+ }
+
+ const tags = getTemplateTagsForParameters(card);
+ return getTemplateTagParameters(tags);
+}
+
+export function getValueAndFieldIdPopulatedParametersFromCard(
+ card,
+ metadata,
+ parameterValues,
+) {
+ const parameters = getParametersFromCard(card);
+ const valuePopulatedParameters = getValuePopulatedParameters(
+ parameters,
+ parameterValues,
+ );
+ const question = new Question(card, metadata);
+
+ return valuePopulatedParameters.map(parameter => {
+ // if we have a field id for this parameter, set "field_id"
+ const field = getParameterTargetField(parameter.target, metadata, question);
+ if (field != null) {
+ parameter = assoc(parameter, "fields", [field]);
+ parameter = assoc(parameter, "field_id", field.id);
+ }
+ parameter = assoc(parameter, "hasOnlyFieldTargets", field != null);
+ return parameter;
+ });
+}
diff --git a/frontend/test/metabase/meta/Parameter.unit.spec.js b/frontend/src/metabase/parameters/utils/cards.unit.spec.js
similarity index 75%
rename from frontend/test/metabase/meta/Parameter.unit.spec.js
rename to frontend/src/metabase/parameters/utils/cards.unit.spec.js
index b2946e76a70..3da8383cbc8 100644
--- a/frontend/test/metabase/meta/Parameter.unit.spec.js
+++ b/frontend/src/metabase/parameters/utils/cards.unit.spec.js
@@ -1,9 +1,5 @@
import MetabaseSettings from "metabase/lib/settings";
-import {
- getTemplateTagParameters,
- buildHiddenParametersSlugSet,
- getVisibleParameters,
-} from "metabase/meta/Parameter";
+import { getTemplateTagParameters } from "./cards";
MetabaseSettings.get = jest.fn();
@@ -15,7 +11,7 @@ function mockFieldFilterOperatorsFlag(value) {
});
}
-describe("metabase/meta/Parameter", () => {
+describe("parameters/utils/cards", () => {
beforeEach(() => {
jest.clearAllMocks();
MetabaseSettings.get.mockReturnValue(false);
@@ -174,53 +170,4 @@ describe("metabase/meta/Parameter", () => {
});
});
});
-
- describe("buildHiddenParametersSlugSet", () => {
- it("should turn the given string of slugs separated by commas into a set of slug strings", () => {
- expect(buildHiddenParametersSlugSet("a,b,c")).toEqual(
- new Set(["a", "b", "c"]),
- );
- });
-
- it("should return an empty set for any input that is not a string", () => {
- expect(buildHiddenParametersSlugSet(undefined)).toEqual(new Set());
- expect(buildHiddenParametersSlugSet(111111)).toEqual(new Set());
- });
- });
-
- describe("getVisibleParameters", () => {
- const parameters = [
- {
- id: 1,
- slug: "foo",
- },
- {
- id: 2,
- slug: "bar",
- },
- {
- id: 3,
- slug: "baz",
- },
- {
- id: 4,
- slug: "qux",
- },
- ];
-
- const hiddenParameterSlugs = "bar,baz";
-
- it("should return the parameters that are not hidden", () => {
- expect(getVisibleParameters(parameters, hiddenParameterSlugs)).toEqual([
- {
- id: 1,
- slug: "foo",
- },
- {
- id: 4,
- slug: "qux",
- },
- ]);
- });
- });
});
diff --git a/frontend/src/metabase/parameters/utils/ui.js b/frontend/src/metabase/parameters/utils/ui.js
new file mode 100644
index 00000000000..2abcf78ece7
--- /dev/null
+++ b/frontend/src/metabase/parameters/utils/ui.js
@@ -0,0 +1,33 @@
+import _ from "underscore";
+import { getParameterType } from "./parameter-type";
+
+export function getParameterIconName(parameter) {
+ const type = getParameterType(parameter);
+ switch (type) {
+ case "date":
+ return "calendar";
+ case "location":
+ return "location";
+ case "category":
+ return "string";
+ case "number":
+ return "number";
+ case "id":
+ default:
+ return "label";
+ }
+}
+
+export function buildHiddenParametersSlugSet(hiddenParameterSlugs) {
+ return _.isString(hiddenParameterSlugs)
+ ? new Set(hiddenParameterSlugs.split(","))
+ : new Set();
+}
+
+export function getVisibleParameters(parameters, hiddenParameterSlugs) {
+ const hiddenParametersSlugSet = buildHiddenParametersSlugSet(
+ hiddenParameterSlugs,
+ );
+
+ return parameters.filter(p => !hiddenParametersSlugSet.has(p.slug));
+}
diff --git a/frontend/src/metabase/parameters/utils/ui.unit.spec.js b/frontend/src/metabase/parameters/utils/ui.unit.spec.js
new file mode 100644
index 00000000000..8155721214b
--- /dev/null
+++ b/frontend/src/metabase/parameters/utils/ui.unit.spec.js
@@ -0,0 +1,67 @@
+import {
+ getParameterIconName,
+ buildHiddenParametersSlugSet,
+ getVisibleParameters,
+} from "./ui";
+
+describe("parameters/utils/ui", () => {
+ describe("getParameterIconName", () => {
+ it("should return an icon name for the given parameter", () => {
+ expect(getParameterIconName({ type: "category" })).toEqual("string");
+ expect(getParameterIconName({ type: "date/single" })).toEqual("calendar");
+ });
+
+ it("should safely default", () => {
+ expect(getParameterIconName({ type: "???" })).toEqual("label");
+ });
+ });
+
+ describe("buildHiddenParametersSlugSet", () => {
+ it("should turn the given string of slugs separated by commas into a set of slug strings", () => {
+ expect(buildHiddenParametersSlugSet("a,b,c")).toEqual(
+ new Set(["a", "b", "c"]),
+ );
+ });
+
+ it("should return an empty set for any input that is not a string", () => {
+ expect(buildHiddenParametersSlugSet(undefined)).toEqual(new Set());
+ expect(buildHiddenParametersSlugSet(111111)).toEqual(new Set());
+ });
+ });
+
+ describe("getVisibleParameters", () => {
+ const parameters = [
+ {
+ id: 1,
+ slug: "foo",
+ },
+ {
+ id: 2,
+ slug: "bar",
+ },
+ {
+ id: 3,
+ slug: "baz",
+ },
+ {
+ id: 4,
+ slug: "qux",
+ },
+ ];
+
+ const hiddenParameterSlugs = "bar,baz";
+
+ it("should return the parameters that are not hidden", () => {
+ expect(getVisibleParameters(parameters, hiddenParameterSlugs)).toEqual([
+ {
+ id: 1,
+ slug: "foo",
+ },
+ {
+ id: 4,
+ slug: "qux",
+ },
+ ]);
+ });
+ });
+});
diff --git a/frontend/src/metabase/public/containers/PublicQuestion.jsx b/frontend/src/metabase/public/containers/PublicQuestion.jsx
index b1fa133682d..edf5da38333 100644
--- a/frontend/src/metabase/public/containers/PublicQuestion.jsx
+++ b/frontend/src/metabase/public/containers/PublicQuestion.jsx
@@ -17,11 +17,11 @@ import {
getParameterValuesBySlug,
getParameterValuesByIdFromQueryParams,
} from "metabase/parameters/utils/parameter-values";
+import { applyParameters } from "metabase/meta/Card";
import {
getParametersFromCard,
getValueAndFieldIdPopulatedParametersFromCard,
- applyParameters,
-} from "metabase/meta/Card";
+} from "metabase/parameters/utils/cards";
import {
PublicApi,
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index 508b40a4014..32c2474e798 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -29,11 +29,9 @@ import Utils from "metabase/lib/utils";
import { defer } from "metabase/lib/promise";
import Question from "metabase-lib/lib/Question";
import { FieldDimension } from "metabase-lib/lib/Dimension";
-import {
- cardIsEquivalent,
- cardQueryIsEquivalent,
- getValueAndFieldIdPopulatedParametersFromCard,
-} from "metabase/meta/Card";
+import { cardIsEquivalent, cardQueryIsEquivalent } from "metabase/meta/Card";
+import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
+
import { getParameterValuesByIdFromQueryParams } from "metabase/parameters/utils/parameter-values";
import { normalize } from "cljs/metabase.mbql.js";
diff --git a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
index 15f4b3bd381..98f8f6c0a4a 100644
--- a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
+++ b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
@@ -11,8 +11,8 @@ import EmbedModalContent from "metabase/public/components/widgets/EmbedModalCont
import * as Urls from "metabase/lib/urls";
import MetabaseSettings from "metabase/lib/settings";
import * as MetabaseAnalytics from "metabase/lib/analytics";
+import { getParametersFromCard } from "metabase/parameters/utils/cards";
-import { getParametersFromCard } from "metabase/meta/Card";
import {
createPublicLink,
deletePublicLink,
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index c0312d7f374..a12fd8b387b 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -13,7 +13,7 @@ import {
getVisualizationTransformed,
} from "metabase/visualizations";
import { getComputedSettingsForSeries } from "metabase/visualizations/lib/settings/visualization";
-import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/meta/Card";
+import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values";
import Utils from "metabase/lib/utils";
diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj
index fe5a2d99ea6..6b65eaabffe 100644
--- a/src/metabase/api/embed.clj
+++ b/src/metabase/api/embed.clj
@@ -130,7 +130,7 @@
(defn- template-tag-parameters
"Transforms native query's `template-tags` into `parameters`."
[card]
- ;; NOTE: this should mirror `getTemplateTagParameters` in frontend/src/metabase/meta/Parameter.js
+ ;; NOTE: this should mirror `getTemplateTagParameters` in frontend/src/metabase/parameters/utils/cards.js
(for [[_ {tag-type :type, widget-type :widget-type, :as tag}] (get-in card [:dataset_query :native :template-tags])
:when (and tag-type
(or widget-type (not= tag-type :dimension)))]
--
GitLab