From 5bdde140a7b9165ebbedab07e301876589e0bcd8 Mon Sep 17 00:00:00 2001
From: Dalton <daltojohnso@users.noreply.github.com>
Date: Thu, 16 Jun 2022 08:50:20 -0700
Subject: [PATCH] Refactor card parameter utils (#23063)
Fix unit tests
Update name to match dashboard util fn name
---
frontend/src/metabase-lib/lib/Question.ts | 4 ++--
.../src/metabase/parameters/utils/cards.ts | 21 +++++++++++--------
.../public/containers/PublicQuestion.jsx | 11 +++-------
.../actions/core/parameterUtils.ts | 7 ++-----
.../containers/QuestionEmbedWidget.jsx | 7 ++-----
.../src/metabase/query_builder/selectors.js | 8 ++-----
.../metabase-lib/lib/Question.unit.spec.js | 8 -------
7 files changed, 23 insertions(+), 43 deletions(-)
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index c3ea7619005..a32d4ff42d0 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -33,7 +33,7 @@ import {
} from "metabase/lib/dataset";
import { isTransientId } from "metabase/meta/Card";
import {
- getValueAndFieldIdPopulatedParametersFromCard,
+ getCardUiParameters,
remapParameterValuesToTemplateTags,
} from "metabase/parameters/utils/cards";
import { fieldFilterParameterToMBQLFilter } from "metabase/parameters/utils/mbql";
@@ -1161,7 +1161,7 @@ class QuestionInner {
// TODO: Fix incorrect Flow signature
parameters(): ParameterObject[] {
- return getValueAndFieldIdPopulatedParametersFromCard(
+ return getCardUiParameters(
this.card(),
this.metadata(),
this._parameterValues,
diff --git a/frontend/src/metabase/parameters/utils/cards.ts b/frontend/src/metabase/parameters/utils/cards.ts
index feeadb84f68..c9982bf55d0 100644
--- a/frontend/src/metabase/parameters/utils/cards.ts
+++ b/frontend/src/metabase/parameters/utils/cards.ts
@@ -10,7 +10,7 @@ import {
getValuePopulatedParameters,
hasParameterValue,
} from "metabase/parameters/utils/parameter-values";
-import { ParameterWithTarget } from "metabase/parameters/types";
+import { ParameterWithTarget, UiParameter } from "metabase/parameters/types";
import { Parameter, ParameterTarget } from "metabase-types/types/Parameter";
import { Card } from "metabase-types/types/Card";
import { TemplateTag } from "metabase-types/types/Query";
@@ -89,12 +89,12 @@ export function getParametersFromCard(
return getTemplateTagParameters(tags);
}
-export function getValueAndFieldIdPopulatedParametersFromCard(
+export function getCardUiParameters(
card: Card,
metadata: Metadata,
parameterValues: { [key: string]: any } = {},
parameters = getParametersFromCard(card),
-) {
+): UiParameter[] {
if (!card) {
return [];
}
@@ -109,12 +109,15 @@ export function getValueAndFieldIdPopulatedParametersFromCard(
| ParameterTarget
| undefined = (parameter as ParameterWithTarget).target;
const field = getParameterTargetField(target, metadata, question);
- return {
- ...parameter,
- fields: field == null ? [] : [field],
- field_id: field?.id,
- hasOnlyFieldTargets: field != null,
- };
+ if (field) {
+ return {
+ ...parameter,
+ fields: [field],
+ hasOnlyFieldTargets: true,
+ };
+ }
+
+ return { ...parameter };
});
}
diff --git a/frontend/src/metabase/public/containers/PublicQuestion.jsx b/frontend/src/metabase/public/containers/PublicQuestion.jsx
index 129bdaac12e..4df96df9d53 100644
--- a/frontend/src/metabase/public/containers/PublicQuestion.jsx
+++ b/frontend/src/metabase/public/containers/PublicQuestion.jsx
@@ -16,8 +16,8 @@ import {
} from "metabase/parameters/utils/parameter-values";
import { applyParameters } from "metabase/meta/Card";
import {
- getValueAndFieldIdPopulatedParametersFromCard,
getParametersFromCard,
+ getCardUiParameters,
} from "metabase/parameters/utils/cards";
import {
@@ -88,7 +88,7 @@ class PublicQuestion extends Component {
this.props.addFields(card.param_fields);
}
- const parameters = getValueAndFieldIdPopulatedParametersFromCard(
+ const parameters = getCardUiParameters(
card,
metadata,
{},
@@ -190,12 +190,7 @@ class PublicQuestion extends Component {
const parameters =
card &&
- getValueAndFieldIdPopulatedParametersFromCard(
- card,
- metadata,
- {},
- card.parameters || undefined,
- );
+ getCardUiParameters(card, metadata, {}, card.parameters || undefined);
return (
<EmbedFrame
diff --git a/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts b/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts
index 42ca4ee48b4..f284fe30d8c 100644
--- a/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts
+++ b/frontend/src/metabase/query_builder/actions/core/parameterUtils.ts
@@ -7,7 +7,7 @@ import { DashboardApi } from "metabase/services";
import { setErrorPage } from "metabase/redux/app";
import { getMetadata } from "metabase/selectors/metadata";
-import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
+import { getCardUiParameters } from "metabase/parameters/utils/cards";
import { hasMatchingParameters } from "metabase/parameters/utils/dashboards";
import { getParameterValuesByIdFromQueryParams } from "metabase/parameters/utils/parameter-values";
@@ -100,10 +100,7 @@ export function getParameterValuesForQuestion({
queryParams?: QueryParams;
metadata: Metadata;
}) {
- const parameters = getValueAndFieldIdPopulatedParametersFromCard(
- card,
- metadata,
- );
+ const parameters = getCardUiParameters(card, metadata);
return getParameterValuesByIdFromQueryParams(
parameters,
queryParams,
diff --git a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
index ebfe6eb4546..4bffa93bd15 100644
--- a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
+++ b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
@@ -11,7 +11,7 @@ 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 { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
+import { getCardUiParameters } from "metabase/parameters/utils/cards";
import { getMetadata } from "metabase/selectors/metadata";
import {
@@ -64,10 +64,7 @@ class QuestionEmbedWidget extends Component {
className={className}
resource={card}
resourceType="question"
- resourceParameters={getValueAndFieldIdPopulatedParametersFromCard(
- card,
- metadata,
- )}
+ resourceParameters={getCardUiParameters(card, metadata)}
onCreatePublicLink={() => createPublicLink(card)}
onDisablePublicLink={() => deletePublicLink(card)}
onUpdateEnableEmbedding={enableEmbedding =>
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 7c7b6b7b8be..d9de10bd0fe 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -12,7 +12,7 @@ import {
getVisualizationTransformed,
} from "metabase/visualizations";
import { getComputedSettingsForSeries } from "metabase/visualizations/lib/settings/visualization";
-import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/parameters/utils/cards";
+import { getCardUiParameters } from "metabase/parameters/utils/cards";
import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values";
import { isPK } from "metabase/lib/schema_metadata";
import Utils from "metabase/lib/utils";
@@ -237,11 +237,7 @@ export const getDatabaseFields = createSelector(
export const getParameters = createSelector(
[getCard, getMetadata, getParameterValues],
(card, metadata, parameterValues) =>
- getValueAndFieldIdPopulatedParametersFromCard(
- card,
- metadata,
- parameterValues,
- ),
+ getCardUiParameters(card, metadata, parameterValues),
);
const getLastRunDatasetQuery = createSelector(
diff --git a/frontend/test/metabase-lib/lib/Question.unit.spec.js b/frontend/test/metabase-lib/lib/Question.unit.spec.js
index a6b88009cef..3530aa4d13a 100644
--- a/frontend/test/metabase-lib/lib/Question.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Question.unit.spec.js
@@ -1067,7 +1067,6 @@ describe("Question", () => {
expect(question.parameters()).toEqual([
{
default: undefined,
- field_id: 1,
fields: [
{
id: 1,
@@ -1082,9 +1081,6 @@ describe("Question", () => {
},
{
default: undefined,
- field_id: undefined,
- fields: [],
- hasOnlyFieldTargets: false,
id: "aaa",
name: "Bar",
slug: "bar",
@@ -1122,16 +1118,12 @@ describe("Question", () => {
target: ["dimension", ["field", 1, null]],
value: "abc",
fields: [{ id: 1 }],
- field_id: 1,
hasOnlyFieldTargets: true,
},
{
type: "category",
name: "bar",
id: "bar_id",
- fields: [],
- field_id: undefined,
- hasOnlyFieldTargets: false,
},
]);
});
--
GitLab