diff --git a/enterprise/frontend/src/metabase-enterprise/sharing/components/MutableParametersSection.jsx b/enterprise/frontend/src/metabase-enterprise/sharing/components/MutableParametersSection.jsx
index bb37b3fc8ffb2cc616c24c5a4841d86865a9bd54..ceb9e8b40b907c1f861df2aa33156b711f38538e 100644
--- a/enterprise/frontend/src/metabase-enterprise/sharing/components/MutableParametersSection.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/sharing/components/MutableParametersSection.jsx
@@ -10,7 +10,7 @@ import { t } from "ttag";
import CollapseSection from "metabase/components/CollapseSection";
import ParametersList from "metabase/parameters/components/ParametersList";
-import { collateParametersWithValues } from "metabase/meta/Parameter";
+import { getValuePopulatedParameters } from "metabase/meta/Parameter";
import {
getPulseParameters,
getActivePulseParameters,
@@ -31,7 +31,7 @@ function MutableParametersSection({
return map;
}, {});
- const collatedParameters = collateParametersWithValues(
+ const valuePopulatedParameters = getValuePopulatedParameters(
parameters,
pulseParamValuesById,
);
@@ -63,7 +63,7 @@ function MutableParametersSection({
className="align-stretch row-gap-1"
vertical
dashboard={dashboard}
- parameters={collatedParameters}
+ parameters={valuePopulatedParameters}
setParameterValue={setParameterValue}
/>
</CollapseSection>
diff --git a/frontend/src/metabase-lib/lib/Question.js b/frontend/src/metabase-lib/lib/Question.js
index aa7146ee499f30e76b9124f33d7acee6ba9304e1..787ff9c06c1875654c2045032990670f6a0ae793 100644
--- a/frontend/src/metabase-lib/lib/Question.js
+++ b/frontend/src/metabase-lib/lib/Question.js
@@ -35,7 +35,10 @@ import {
findColumnIndexForColumnSetting,
syncTableColumnsToQuery,
} from "metabase/lib/dataset";
-import { getParametersWithExtras, isTransientId } from "metabase/meta/Card";
+import {
+ getValueAndFieldIdPopulatedParametersFromCard,
+ isTransientId,
+} from "metabase/meta/Card";
import {
parameterToMBQLFilter,
normalizeParameterValue,
@@ -969,7 +972,10 @@ export default class Question {
// TODO: Fix incorrect Flow signature
parameters(): ParameterObject[] {
- return getParametersWithExtras(this.card(), this._parameterValues);
+ return getValueAndFieldIdPopulatedParametersFromCard(
+ this.card(),
+ this._parameterValues,
+ );
}
parametersList(): ParameterObject[] {
diff --git a/frontend/src/metabase/dashboard/components/DashCard.jsx b/frontend/src/metabase/dashboard/components/DashCard.jsx
index 776e944400f19b77323c2b76c9c1770e04c3cb49..7edadc1e9b49b8f4e7e6d70508f1d91b7c0de09e 100644
--- a/frontend/src/metabase/dashboard/components/DashCard.jsx
+++ b/frontend/src/metabase/dashboard/components/DashCard.jsx
@@ -29,7 +29,7 @@ import { getClickBehaviorDescription } from "metabase/lib/click-behavior";
import cx from "classnames";
import _ from "underscore";
import { getIn } from "icepick";
-import { getParametersBySlug } from "metabase/meta/Parameter";
+import { getParameterValuesBySlug } from "metabase/meta/Parameter";
import Utils from "metabase/lib/utils";
const DATASET_USUALLY_FAST_THRESHOLD = 15 * 1000;
@@ -159,7 +159,10 @@ export default class DashCard extends Component {
errorIcon = "warning";
}
- const params = getParametersBySlug(dashboard.parameters, parameterValues);
+ const parameterValuesBySlug = getParameterValuesBySlug(
+ dashboard.parameters,
+ parameterValues,
+ );
const hideBackground =
!isEditing &&
@@ -217,7 +220,7 @@ export default class DashCard extends Component {
isDashboard
dispatch={this.props.dispatch}
dashboard={dashboard}
- parameterValuesBySlug={params}
+ parameterValuesBySlug={parameterValuesBySlug}
isEditing={isEditing}
isPreviewing={this.state.isPreviewingCard}
gridSize={
@@ -231,7 +234,7 @@ export default class DashCard extends Component {
className="m1 text-brand-hover text-light"
classNameClose="hover-child"
card={dashcard.card}
- params={params}
+ params={parameterValuesBySlug}
dashcardId={dashcard.id}
token={dashcard.dashboard_id}
icon="download"
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/ParametersWidget/ParametersWidget.jsx b/frontend/src/metabase/dashboard/components/Dashboard/ParametersWidget/ParametersWidget.jsx
index 064d3581ed1ceeacc57fe8972f139dd29fbff938..f20260f16322a068953260c0d45ae49031af2704 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard/ParametersWidget/ParametersWidget.jsx
+++ b/frontend/src/metabase/dashboard/components/Dashboard/ParametersWidget/ParametersWidget.jsx
@@ -1,6 +1,7 @@
import React from "react";
import PropTypes from "prop-types";
+import { getValuePopulatedParameters } from "metabase/meta/Parameter";
import Parameters from "metabase/parameters/components/Parameters/Parameters";
const propTypes = {
@@ -46,10 +47,7 @@ const ParametersWidget = ({
isFullscreen={isFullscreen}
isNightMode={shouldRenderAsNightMode}
hideParameters={hideParameters}
- parameters={parameters.map(p => ({
- ...p,
- value: parameterValues[p.id],
- }))}
+ parameters={getValuePopulatedParameters(parameters, parameterValues)}
query={location.query}
editingParameter={editingParameter}
setEditingParameter={setEditingParameter}
diff --git a/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx b/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
index 3e280c2068f93e8079f9f58a4dc1973c533e011f..86ac8bec089ad327d048a00debeedabec85fbc86 100644
--- a/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
+++ b/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
@@ -8,6 +8,7 @@ import cx from "classnames";
import title from "metabase/hoc/Title";
import withToast from "metabase/hoc/Toast";
import DashboardData from "metabase/dashboard/hoc/DashboardData";
+import { getValuePopulatedParameters } from "metabase/meta/Parameter";
import ActionButton from "metabase/components/ActionButton";
import Button from "metabase/components/Button";
@@ -144,10 +145,10 @@ class AutomaticDashboardApp extends React.Component {
{parameters && parameters.length > 0 && (
<div className="px1 pt1">
<Parameters
- parameters={parameters.map(p => ({
- ...p,
- value: parameterValues && parameterValues[p.id],
- }))}
+ parameters={getValuePopulatedParameters(
+ parameters,
+ parameterValues,
+ )}
query={location.query}
setParameterValue={setParameterValue}
syncQueryString
diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js
index 5a0a0198afda280b9e06be26a5173fca391492d6..9ce2b60270b3ba579d98d451b2d9962aa6c16c32 100644
--- a/frontend/src/metabase/dashboard/dashboard.js
+++ b/frontend/src/metabase/dashboard/dashboard.js
@@ -31,7 +31,10 @@ import {
setParameterDefaultValue as setParamDefaultValue,
} from "metabase/meta/Dashboard";
import { applyParameters, questionUrlWithParameters } from "metabase/meta/Card";
-import { getParametersBySlug } from "metabase/meta/Parameter";
+import {
+ getParameterValuesBySlug,
+ getParameterValuesByIdFromQueryParams,
+} from "metabase/meta/Parameter";
import * as Urls from "metabase/lib/urls";
import type {
@@ -378,7 +381,7 @@ export const saveDashboardAndCards = createThunkAction(
parameter_mappings &&
parameter_mappings.filter(
mapping =>
- // filter out mappings for deleted paramters
+ // filter out mappings for deleted parameters
_.findWhere(dashboard.parameters, {
id: mapping.parameter_id,
}) &&
@@ -593,7 +596,7 @@ export const fetchCardData = createThunkAction(FETCH_CARD_DATA, function(
token: dashcard.dashboard_id,
dashcardId: dashcard.id,
cardId: card.id,
- ...getParametersBySlug(dashboard.parameters, parameterValues),
+ ...getParameterValuesBySlug(dashboard.parameters, parameterValues),
ignore_cache: ignoreCache,
},
queryOptions,
@@ -663,7 +666,6 @@ function expandInlineCard(card) {
export const fetchDashboard = createThunkAction(FETCH_DASHBOARD, function(
dashId,
queryParams,
- enableDefaultParameters = true,
) {
let result;
return async function(dispatch, getState) {
@@ -712,17 +714,6 @@ export const fetchDashboard = createThunkAction(FETCH_DASHBOARD, function(
result = await DashboardApi.get({ dashId: dashId });
}
- const parameterValues = {};
- if (result.parameters) {
- for (const parameter of result.parameters) {
- if (queryParams && queryParams[parameter.slug] != null) {
- parameterValues[parameter.id] = queryParams[parameter.slug];
- } else if (enableDefaultParameters && parameter.default != null) {
- parameterValues[parameter.id] = parameter.default;
- }
- }
- }
-
if (dashboardType === "normal" || dashboardType === "transient") {
dispatch(loadMetadataForDashboard(result.ordered_cards));
}
@@ -744,10 +735,15 @@ export const fetchDashboard = createThunkAction(FETCH_DASHBOARD, function(
dispatch(addFields(result.param_fields));
}
+ const parameterValuesById = getParameterValuesByIdFromQueryParams(
+ result.parameters,
+ queryParams,
+ );
+
return {
...normalize(result, dashboard), // includes `result` and `entities`
dashboardId: dashId,
- parameterValues: parameterValues,
+ parameterValues: parameterValuesById,
};
};
});
@@ -920,9 +916,12 @@ export const setParameterValue = createThunkAction(
},
);
-export const setOrUnsetParameterValues = pairs => (dispatch, getState) => {
+export const setOrUnsetParameterValues = parameterIdValuePairs => (
+ dispatch,
+ getState,
+) => {
const parameterValues = getParameterValues(getState());
- pairs
+ parameterIdValuePairs
.map(([id, value]) =>
setParameterValue(id, value === parameterValues[id] ? null : value),
)
diff --git a/frontend/src/metabase/lib/pulse.js b/frontend/src/metabase/lib/pulse.js
index bcbac0c7d19bb451ff9fa869213d5badb0aefbe2..771302ced34beff4176f88e707e082124f8bde87 100644
--- a/frontend/src/metabase/lib/pulse.js
+++ b/frontend/src/metabase/lib/pulse.js
@@ -136,7 +136,7 @@ export function getActivePulseParameters(pulse, parameters) {
return {
...parameter,
- value: hasParameterValue(pulseParameter)
+ value: hasParameterValue(pulseParameter?.value)
? pulseParameter.value
: parameter.default,
};
diff --git a/frontend/src/metabase/meta/Card.js b/frontend/src/metabase/meta/Card.js
index be6641bcd2f5b0f53961c26e7488b0144b74ad8f..0a51386ff8952d5ab602ae5c4b5fbf73acb7e06e 100644
--- a/frontend/src/metabase/meta/Card.js
+++ b/frontend/src/metabase/meta/Card.js
@@ -3,6 +3,7 @@ import {
getParameterTargetFieldId,
parameterToMBQLFilter,
normalizeParameterValue,
+ getValuePopulatedParameters,
} from "metabase/meta/Parameter";
import * as Query from "metabase/lib/query/query";
@@ -125,7 +126,7 @@ export function getTemplateTagsForParameters(card: ?Card): Array<TemplateTag> {
);
}
-export function getParameters(card: ?Card): Parameter[] {
+export function getParametersFromCard(card: ?Card): Parameter[] {
if (card && card.parameters) {
return card.parameters;
}
@@ -134,15 +135,17 @@ export function getParameters(card: ?Card): Parameter[] {
return getTemplateTagParameters(tags);
}
-export function getParametersWithExtras(
+export function getValueAndFieldIdPopulatedParametersFromCard(
card: Card,
parameterValues?: ParameterValues,
): Parameter[] {
- return getParameters(card).map(parameter => {
- // if we have a parameter value for this parameter, set "value"
- if (parameterValues && parameter.id in parameterValues) {
- parameter = assoc(parameter, "value", parameterValues[parameter.id]);
- }
+ const parameters = getParametersFromCard(card);
+ const valuePopulatedParameters = getValuePopulatedParameters(
+ parameters,
+ parameterValues,
+ );
+
+ return valuePopulatedParameters.map(parameter => {
// if we have a field id for this parameter, set "field_id"
const fieldId = getParameterTargetFieldId(
parameter.target,
@@ -232,7 +235,7 @@ export function questionUrlWithParameters(
card = Utils.copy(card);
- const cardParameters = getParameters(card);
+ const cardParameters = getParametersFromCard(card);
const datasetQuery = applyParameters(
card,
parameters,
diff --git a/frontend/src/metabase/meta/Parameter.js b/frontend/src/metabase/meta/Parameter.js
index 545b0e0ade374021e5beca8698b1b3aad239002b..226c28593f99e951bdaf229a0e323bfddceb3702 100644
--- a/frontend/src/metabase/meta/Parameter.js
+++ b/frontend/src/metabase/meta/Parameter.js
@@ -13,7 +13,6 @@ import type {
ParameterTarget,
ParameterValue,
ParameterValueOrArray,
- ParameterValues,
} from "metabase-types/types/Parameter";
import type { FieldId } from "metabase-types/types/Field";
import type Metadata from "metabase-lib/lib/metadata/Metadata";
@@ -371,19 +370,6 @@ export function getTemplateTagParameters(tags: TemplateTag[]): Parameter[] {
});
}
-export const getParametersBySlug = (
- parameters: Parameter[],
- parameterValues: ParameterValues,
-): { [key: string]: string } => {
- const result = {};
- for (const parameter of parameters) {
- if (parameterValues[parameter.id] != null) {
- result[parameter.slug] = parameterValues[parameter.id];
- }
- }
- return result;
-};
-
/** Returns the field ID that this parameter target points to, or null if it's not a dimension target. */
export function getParameterTargetFieldId(
target: ?ParameterTarget,
@@ -619,21 +605,75 @@ function splitType(parameterOrType) {
return parameterType.split("/");
}
-export function collateParametersWithValues(parameters, parameterValues) {
- if (parameterValues) {
- return parameters.map(p => ({
- ...p,
- value: parameterValues[p.id],
- }));
- } else {
- return parameters;
- }
+export function getValuePopulatedParameters(parameters, parameterValues) {
+ return parameterValues
+ ? parameters.map(parameter => {
+ return parameter.id in parameterValues
+ ? {
+ ...parameter,
+ value: parameterValues[parameter.id],
+ }
+ : parameter;
+ })
+ : parameters;
}
export function hasDefaultParameterValue(parameter) {
return parameter.default != null;
}
-export function hasParameterValue(parameter) {
- return parameter && parameter.value != null;
+export function hasParameterValue(value) {
+ return value != null;
+}
+
+export function getParameterValueFromQueryParams(parameter, queryParams) {
+ queryParams = queryParams || {};
+ const maybeParameterValue = queryParams[parameter.slug];
+ return hasParameterValue(maybeParameterValue)
+ ? maybeParameterValue
+ : parameter.default;
+}
+
+export function getParameterValuePairsFromQueryParams(parameters, queryParams) {
+ return parameters
+ .map(parameter => [
+ parameter,
+ getParameterValueFromQueryParams(parameter, queryParams),
+ ])
+ .filter(([, value]) => hasParameterValue(value));
+}
+
+export function getParameterValuesByIdFromQueryParams(parameters, queryParams) {
+ const idValuePairs = getParameterValuePairsFromQueryParams(
+ parameters,
+ queryParams,
+ ).map(([parameter, value]) => [parameter.id, value]);
+
+ return Object.fromEntries(idValuePairs);
+}
+
+export function getParameterValuesBySlug(parameters, parameterValuesById) {
+ parameterValuesById = parameterValuesById || {};
+ const slugValuePairs = parameters
+ .map(parameter => [
+ parameter.slug,
+ parameter.value || parameterValuesById[parameter.id],
+ ])
+ .filter(([, value]) => hasParameterValue(value));
+
+ return Object.fromEntries(slugValuePairs);
+}
+
+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/Parameters/Parameters.jsx b/frontend/src/metabase/parameters/components/Parameters/Parameters.jsx
index 163f605283b6a76b234d17ba5c84fdfd569f78ee..8dec133915d5d01f35f07d23c0283be82f28b0f7 100644
--- a/frontend/src/metabase/parameters/components/Parameters/Parameters.jsx
+++ b/frontend/src/metabase/parameters/components/Parameters/Parameters.jsx
@@ -5,7 +5,7 @@ import querystring from "querystring";
import ParametersList from "metabase/parameters/components/ParametersList";
import { syncQueryParamsWithURL } from "./syncQueryParamsWithURL";
-import { collateParametersWithValues } from "metabase/meta/Parameter";
+import { getParameterValuesBySlug } from "metabase/meta/Parameter";
import { getMetadata } from "metabase/selectors/metadata";
@connect(state => ({ metadata: getMetadata(state) }))
@@ -25,17 +25,12 @@ export default class Parameters extends Component {
if (this.props.syncQueryString) {
// sync parameters to URL query string
- const queryParams = {};
- for (const parameter of collateParametersWithValues(
+ const parameterValuesBySlug = getParameterValuesBySlug(
parameters,
parameterValues,
- )) {
- if (parameter.value) {
- queryParams[parameter.slug] = parameter.value;
- }
- }
+ );
- let search = querystring.stringify(queryParams);
+ let search = querystring.stringify(parameterValuesBySlug);
search = search ? "?" + search : "";
if (search !== window.location.search) {
diff --git a/frontend/src/metabase/parameters/components/Parameters/syncQueryParamsWithURL.js b/frontend/src/metabase/parameters/components/Parameters/syncQueryParamsWithURL.js
index ad5a3dd33dbaa973331ed18866ba86935e11dd7d..7aaf2668059029d693f0a299aa9cd9d91476c284 100644
--- a/frontend/src/metabase/parameters/components/Parameters/syncQueryParamsWithURL.js
+++ b/frontend/src/metabase/parameters/components/Parameters/syncQueryParamsWithURL.js
@@ -1,4 +1,9 @@
import Dimension from "metabase-lib/lib/Dimension";
+import {
+ getParameterValueFromQueryParams,
+ getParameterValuePairsFromQueryParams,
+ hasParameterValue,
+} from "metabase/meta/Parameter";
export const syncQueryParamsWithURL = props => {
props.commitImmediately
@@ -13,15 +18,18 @@ const syncForInternalQuestion = props => {
return;
}
- for (const parameter of parameters) {
- const queryParam = query && query[parameter.slug];
+ parameters.forEach(parameter => {
+ const parameterValue = getParameterValueFromQueryParams(parameter, query);
- if (queryParam != null || parameter.default != null) {
- const parsedParam = parseQueryParams(queryParam, parameter, metadata);
-
- setParameterValue(parameter.id, parsedParam);
+ if (hasParameterValue(parameterValue)) {
+ const parsedParameterValue = parseQueryParams(
+ parameterValue,
+ parameter,
+ metadata,
+ );
+ setParameterValue(parameter.id, parsedParameterValue);
}
- }
+ });
};
const syncForPublicQuestion = props => {
@@ -31,17 +39,17 @@ const syncForPublicQuestion = props => {
return;
}
- const parameterValues = parameters.reduce((acc, parameter) => {
- const queryParam = query && query[parameter.slug];
-
- if (queryParam != null || parameter.default != null) {
- acc[parameter.id] = parseQueryParams(queryParam, parameter, metadata);
- }
+ const parameterIdValuePairs = getParameterValuePairsFromQueryParams(
+ parameters,
+ query,
+ ).map(([parameter, value]) => [
+ parameter.id,
+ parseQueryParams(value, parameter, metadata),
+ ]);
- return acc;
- }, {});
+ const parameterValuesById = Object.fromEntries(parameterIdValuePairs);
- setMultipleParameterValues(parameterValues);
+ setMultipleParameterValues(parameterValuesById);
};
const parseQueryParams = (queryParam, parameter, metadata) => {
diff --git a/frontend/src/metabase/parameters/components/ParametersList.jsx b/frontend/src/metabase/parameters/components/ParametersList.jsx
index 6975d04068f18820e9bc5077eec103c1561e3297..0f3e00e5eabc59c31653c4d6a5f12e6ecc82dc84 100644
--- a/frontend/src/metabase/parameters/components/ParametersList.jsx
+++ b/frontend/src/metabase/parameters/components/ParametersList.jsx
@@ -9,7 +9,10 @@ import cx from "classnames";
import StaticParameterWidget from "./ParameterWidget";
import Icon from "metabase/components/Icon";
-import { collateParametersWithValues } from "metabase/meta/Parameter";
+import {
+ getValuePopulatedParameters,
+ getVisibleParameters,
+} from "metabase/meta/Parameter";
import type {
ParameterId,
@@ -105,15 +108,16 @@ function ParametersList({
}
};
- const hiddenParameters =
- typeof hideParameters === "string"
- ? new Set(hideParameters.split(","))
- : new Set();
- const collatedParameters = collateParametersWithValues(
+ const valuePopulatedParameters = getValuePopulatedParameters(
parameters,
parameterValues,
);
+ const visibleValuePopulatedParameters = getVisibleParameters(
+ valuePopulatedParameters,
+ hideParameters,
+ );
+
let ParameterWidget;
let ParameterWidgetList;
if (isEditing) {
@@ -137,41 +141,42 @@ function ParametersList({
onSortStart={handleSortStart}
onSortEnd={handleSortEnd}
>
- {collatedParameters
- .filter(p => !hiddenParameters.has(p.slug))
- .map((parameter, index) => (
- <ParameterWidget
- key={parameter.id}
- className={cx({ mb2: vertical })}
- isEditing={isEditing}
- isFullscreen={isFullscreen}
- isNightMode={isNightMode}
- parameter={parameter}
- parameters={collatedParameters}
- dashboard={dashboard}
- editingParameter={editingParameter}
- setEditingParameter={setEditingParameter}
- index={index}
- setName={
- setParameterName && (name => setParameterName(parameter.id, name))
- }
- setValue={
- setParameterValue &&
- (value => setParameterValue(parameter.id, value))
- }
- setDefaultValue={
- setParameterDefaultValue &&
- (value => setParameterDefaultValue(parameter.id, value))
- }
- remove={removeParameter && (() => removeParameter(parameter.id))}
- commitImmediately={commitImmediately}
- dragHandle={
- isEditing && setParameterIndex ? (
- <SortableParameterHandle />
- ) : null
- }
- />
- ))}
+ {visibleValuePopulatedParameters.map((valuePopulatedParameter, index) => (
+ <ParameterWidget
+ key={valuePopulatedParameter.id}
+ className={cx({ mb2: vertical })}
+ isEditing={isEditing}
+ isFullscreen={isFullscreen}
+ isNightMode={isNightMode}
+ parameter={valuePopulatedParameter}
+ parameters={valuePopulatedParameters}
+ dashboard={dashboard}
+ editingParameter={editingParameter}
+ setEditingParameter={setEditingParameter}
+ index={index}
+ setName={
+ setParameterName &&
+ (name => setParameterName(valuePopulatedParameter.id, name))
+ }
+ setValue={
+ setParameterValue &&
+ (value => setParameterValue(valuePopulatedParameter.id, value))
+ }
+ setDefaultValue={
+ setParameterDefaultValue &&
+ (value =>
+ setParameterDefaultValue(valuePopulatedParameter.id, value))
+ }
+ remove={
+ removeParameter &&
+ (() => removeParameter(valuePopulatedParameter.id))
+ }
+ commitImmediately={commitImmediately}
+ dragHandle={
+ isEditing && setParameterIndex ? <SortableParameterHandle /> : null
+ }
+ />
+ ))}
</ParameterWidgetList>
);
}
diff --git a/frontend/src/metabase/public/components/EmbedFrame.jsx b/frontend/src/metabase/public/components/EmbedFrame.jsx
index 3b826c9a506997b1ed408b22a96cc3b9bda24c64..c19d5c42c3420960e0937c30b345bd80ddd41cad 100644
--- a/frontend/src/metabase/public/components/EmbedFrame.jsx
+++ b/frontend/src/metabase/public/components/EmbedFrame.jsx
@@ -5,6 +5,7 @@ import { IFRAMED, initializeIframeResizer } from "metabase/lib/dom";
import { parseHashOptions } from "metabase/lib/browser";
import MetabaseSettings from "metabase/lib/settings";
+import { getValuePopulatedParameters } from "metabase/meta/Parameter";
import TitleAndDescription from "metabase/components/TitleAndDescription";
import Parameters from "metabase/parameters/components/Parameters/Parameters";
@@ -98,10 +99,10 @@ export default class EmbedFrame extends Component {
<div className="flex ml-auto">
<Parameters
dashboard={this.props.dashboard}
- parameters={parameters.map(p => ({
- ...p,
- value: parameterValues && parameterValues[p.id],
- }))}
+ parameters={getValuePopulatedParameters(
+ parameters,
+ parameterValues,
+ )}
query={location.query}
setParameterValue={setParameterValue}
setMultipleParameterValues={setMultipleParameterValues}
diff --git a/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx b/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
index 2b97bd9c6abc2074fc4bd32bd5644889881835fe..40bca0a0d4f19f4e85447ec88622292e9f7d5801 100644
--- a/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
+++ b/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
@@ -1,7 +1,6 @@
import React, { Component } from "react";
import { connect } from "react-redux";
import { titleize } from "inflection";
-
import { t } from "ttag";
import Icon from "metabase/components/Icon";
@@ -132,19 +131,30 @@ export default class EmbedModalContent extends Component {
this.setState({ embeddingParams: resource.embedding_params || {} });
};
- getPreviewParams() {
+ getPreviewParameters(resourceParameters, embeddingParams) {
+ const lockedParameters = resourceParameters.filter(
+ parameter => embeddingParams[parameter.slug] === "locked",
+ );
+
+ return lockedParameters;
+ }
+
+ getPreviewParamsBySlug() {
const { resourceParameters } = this.props;
const { embeddingParams, parameterValues } = this.state;
- const params = {};
- for (const parameter of resourceParameters) {
- if (embeddingParams[parameter.slug] === "locked") {
- params[parameter.slug] =
- parameter.id in parameterValues
- ? parameterValues[parameter.id]
- : null;
- }
- }
- return params;
+
+ const lockedParameters = this.getPreviewParameters(
+ resourceParameters,
+ embeddingParams,
+ );
+
+ const parameterSlugValuePairs = lockedParameters.map(parameter => {
+ const value =
+ parameter.id in parameterValues ? parameterValues[parameter.id] : null;
+ return [parameter.slug, value];
+ });
+
+ return Object.fromEntries(parameterSlugValuePairs);
}
render() {
@@ -164,10 +174,10 @@ export default class EmbedModalContent extends Component {
displayOptions,
} = this.state;
- const params = this.getPreviewParams();
-
- const previewParameters = resourceParameters.filter(
- p => embeddingParams[p.slug] === "locked",
+ const previewParametersBySlug = this.getPreviewParamsBySlug();
+ const previewParameters = this.getPreviewParameters(
+ resourceParameters,
+ embeddingParams,
);
return (
@@ -229,7 +239,7 @@ export default class EmbedModalContent extends Component {
token={getSignedToken(
resourceType,
resource.id,
- params,
+ previewParametersBySlug,
secretKey,
embeddingParams,
)}
@@ -237,14 +247,14 @@ export default class EmbedModalContent extends Component {
siteUrl,
resourceType,
resource.id,
- params,
+ previewParametersBySlug,
displayOptions,
secretKey,
embeddingParams,
)}
siteUrl={siteUrl}
secretKey={secretKey}
- params={params}
+ params={previewParametersBySlug}
displayOptions={displayOptions}
previewParameters={previewParameters}
parameterValues={parameterValues}
diff --git a/frontend/src/metabase/public/containers/PublicQuestion.jsx b/frontend/src/metabase/public/containers/PublicQuestion.jsx
index c6b882e892edbf63b67bae7769f39fad857e8124..c4df0f6d2f37208b4f533effaf416b4c33182a05 100644
--- a/frontend/src/metabase/public/containers/PublicQuestion.jsx
+++ b/frontend/src/metabase/public/containers/PublicQuestion.jsx
@@ -13,10 +13,13 @@ import type { Card } from "metabase-types/types/Card";
import type { Dataset } from "metabase-types/types/Dataset";
import type { ParameterValues } from "metabase-types/types/Parameter";
-import { getParametersBySlug } from "metabase/meta/Parameter";
import {
- getParameters,
- getParametersWithExtras,
+ getParameterValuesBySlug,
+ getParameterValuesByIdFromQueryParams,
+} from "metabase/meta/Parameter";
+import {
+ getParametersFromCard,
+ getValueAndFieldIdPopulatedParametersFromCard,
applyParameters,
} from "metabase/meta/Card";
@@ -112,15 +115,19 @@ export default class PublicQuestion extends Component {
this.props.addFields(card.param_fields);
}
- const parameterValues: ParameterValues = {};
- for (const parameter of getParameters(card)) {
- parameterValues[String(parameter.id)] = query[parameter.slug];
- }
+ const parameters = getParametersFromCard(card);
+ const parameterValuesById = getParameterValuesByIdFromQueryParams(
+ parameters,
+ query,
+ );
- this.setState({ card, parameterValues }, async () => {
- await this.run();
- this.setState({ initialized: true });
- });
+ this.setState(
+ { card, parameterValues: parameterValuesById },
+ async () => {
+ await this.run();
+ this.setState({ initialized: true });
+ },
+ );
} catch (error) {
console.error("error", error);
setErrorPage(error);
@@ -162,7 +169,7 @@ export default class PublicQuestion extends Component {
return;
}
- const parameters = getParameters(card);
+ const parameters = getParametersFromCard(card);
try {
this.setState({ result: null });
@@ -172,7 +179,7 @@ export default class PublicQuestion extends Component {
// embeds apply parameter values server-side
newResult = await maybeUsePivotEndpoint(EmbedApi.cardQuery, card)({
token,
- ...getParametersBySlug(parameters, parameterValues),
+ ...getParameterValuesBySlug(parameters, parameterValues),
});
} else if (uuid) {
// public links currently apply parameters client-side
@@ -207,7 +214,8 @@ export default class PublicQuestion extends Component {
/>
);
- const parameters = card && getParametersWithExtras(card);
+ const parameters =
+ card && getValueAndFieldIdPopulatedParametersFromCard(card);
return (
<EmbedFrame
diff --git a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
index acc734fc9893a537c8621c82513cbc22699c2a7d..8b5683b55b37205f01729ee8396584857cb47e02 100644
--- a/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
+++ b/frontend/src/metabase/query_builder/containers/QuestionEmbedWidget.jsx
@@ -12,7 +12,7 @@ import * as Urls from "metabase/lib/urls";
import MetabaseSettings from "metabase/lib/settings";
import MetabaseAnalytics from "metabase/lib/analytics";
-import { getParameters } from "metabase/meta/Card";
+import { getParametersFromCard } from "metabase/meta/Card";
import {
createPublicLink,
deletePublicLink,
@@ -61,7 +61,7 @@ export default class QuestionEmbedWidget extends Component {
className={className}
resource={card}
resourceType="question"
- resourceParameters={getParameters(card)}
+ resourceParameters={getParametersFromCard(card)}
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 17fcdeb173ea7871fbaff2da19ad82205fbec1a4..5f6f3016da30646444dd03a6aa89337286820fae 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 { getParametersWithExtras } from "metabase/meta/Card";
+import { getValueAndFieldIdPopulatedParametersFromCard } from "metabase/meta/Card";
import { normalizeParameterValue } from "metabase/meta/Parameter";
import Utils from "metabase/lib/utils";
@@ -118,7 +118,8 @@ export const getDatabaseFields = createSelector(
export const getParameters = createSelector(
[getCard, getParameterValues],
- (card, parameterValues) => getParametersWithExtras(card, parameterValues),
+ (card, parameterValues) =>
+ getValueAndFieldIdPopulatedParametersFromCard(card, parameterValues),
);
const getLastRunDatasetQuery = createSelector(
diff --git a/frontend/test/metabase/meta/Parameter.unit.spec.js b/frontend/test/metabase/meta/Parameter.unit.spec.js
index 57a4307305524aa479ff14ca8bb6f8d96ecde162..be2953b72b2a16007ed812306771038d2a8f7d4e 100644
--- a/frontend/test/metabase/meta/Parameter.unit.spec.js
+++ b/frontend/test/metabase/meta/Parameter.unit.spec.js
@@ -4,10 +4,16 @@ import {
stringParameterValueToMBQL,
numberParameterValueToMBQL,
parameterOptionsForField,
- getParametersBySlug,
normalizeParameterValue,
deriveFieldOperatorFromParameter,
getTemplateTagParameters,
+ getValuePopulatedParameters,
+ getParameterValueFromQueryParams,
+ getParameterValuePairsFromQueryParams,
+ getParameterValuesByIdFromQueryParams,
+ getParameterValuesBySlug,
+ buildHiddenParametersSlugSet,
+ getVisibleParameters,
} from "metabase/meta/Parameter";
MetabaseSettings.get = jest.fn();
@@ -212,25 +218,6 @@ describe("metabase/meta/Parameter", () => {
});
});
- describe("getParameterBySlug", () => {
- it("should return an object mapping slug to parameter value", () => {
- const parameters = [
- { id: "foo", slug: "bar" },
- { id: "aaa", slug: "bbb" },
- { id: "cat", slug: "dog" },
- ];
- const parameterValuesById = {
- foo: 123,
- aaa: "ccc",
- something: true,
- };
- expect(getParametersBySlug(parameters, parameterValuesById)).toEqual({
- bar: 123,
- bbb: "ccc",
- });
- });
- });
-
describe("normalizeParameterValue", () => {
it("should return same value for location/category parameters", () => {
expect(normalizeParameterValue("category", "foo")).toEqual("foo");
@@ -446,4 +433,225 @@ describe("metabase/meta/Parameter", () => {
});
});
});
+
+ describe("parameter collection-building utils", () => {
+ // found in queryParams and not defaulted
+ const parameter1 = {
+ id: 1,
+ slug: "foo",
+ };
+ // found in queryParams and defaulted
+ const parameter2 = {
+ id: 2,
+ slug: "bar",
+ default: "parameter2 default value",
+ };
+ // not found in queryParams and defaulted
+ const parameter3 = {
+ id: 3,
+ slug: "baz",
+ default: "parameter3 default value",
+ };
+ // not found in queryParams and not defaulted
+ const parameter4 = {
+ id: 4,
+ slug: "qux",
+ };
+ const parameters = [parameter1, parameter2, parameter3, parameter4];
+ const queryParams = {
+ foo: "parameter1 queryParam value",
+ bar: "parameter2 queryParam value",
+ valueNotFoundInParameters: "nonexistent parameter queryParam value",
+ };
+
+ const parameterValues = getParameterValuesByIdFromQueryParams(
+ parameters,
+ queryParams,
+ );
+
+ describe("getValuePopulatedParameters", () => {
+ it("should return an array of parameter objects with the `value` property set if it exists in the given `parameterValues` id, value map", () => {
+ expect(
+ getValuePopulatedParameters(parameters, {
+ [parameter1.id]: "parameter1 value",
+ [parameter2.id]: "parameter2 value",
+ }),
+ ).toEqual([
+ {
+ ...parameter1,
+ value: "parameter1 value",
+ },
+ {
+ ...parameter2,
+ value: "parameter2 value",
+ },
+ parameter3,
+ parameter4,
+ ]);
+ });
+
+ it("should handle there being an undefined or null parameterValues object", () => {
+ expect(getValuePopulatedParameters(parameters, undefined)).toEqual(
+ parameters,
+ );
+ expect(getValuePopulatedParameters(parameters, null)).toEqual(
+ parameters,
+ );
+ });
+ });
+
+ describe("getParameterValueFromQueryParams", () => {
+ it("should return undefined when given an undefined queryParams arg", () => {
+ expect(getParameterValueFromQueryParams(parameter1)).toBe(undefined);
+ });
+
+ it("should return the parameter's default value when given an undefined queryParams arg", () => {
+ expect(getParameterValueFromQueryParams(parameter2)).toBe(
+ "parameter2 default value",
+ );
+ });
+
+ it("should return the parameter's default value when the parameter value is not found in queryParams", () => {
+ expect(getParameterValueFromQueryParams(parameter3, queryParams)).toBe(
+ "parameter3 default value",
+ );
+ });
+
+ it("should return the parameter value found in the queryParams object", () => {
+ expect(getParameterValueFromQueryParams(parameter1, queryParams)).toBe(
+ "parameter1 queryParam value",
+ );
+ });
+
+ it("should ignore the parameter's default value when the parameter value is found in queryParams", () => {
+ expect(getParameterValueFromQueryParams(parameter2, queryParams)).toBe(
+ "parameter2 queryParam value",
+ );
+ });
+ });
+
+ describe("getParameterValuePairsFromQueryParams", () => {
+ it("should build a list of parameter and parameter value pairs", () => {
+ expect(
+ getParameterValuePairsFromQueryParams(parameters, queryParams),
+ ).toEqual([
+ [parameter1, "parameter1 queryParam value"],
+ [parameter2, "parameter2 queryParam value"],
+ [parameter3, "parameter3 default value"],
+ ]);
+ });
+
+ it("should handle undefined queryParams", () => {
+ expect(getParameterValuePairsFromQueryParams(parameters)).toEqual([
+ [parameter2, "parameter2 default value"],
+ [parameter3, "parameter3 default value"],
+ ]);
+ });
+ });
+
+ describe("getParameterValuesByIdFromQueryParams", () => {
+ it("should generate a map of parameter values found in the queryParams or with default values", () => {
+ expect(
+ getParameterValuesByIdFromQueryParams(parameters, queryParams),
+ ).toEqual({
+ [parameter1.id]: "parameter1 queryParam value",
+ [parameter2.id]: "parameter2 queryParam value",
+ [parameter3.id]: "parameter3 default value",
+ });
+ });
+
+ it("should handle an undefined queryParams", () => {
+ expect(getParameterValuesByIdFromQueryParams(parameters)).toEqual({
+ [parameter2.id]: "parameter2 default value",
+ [parameter3.id]: "parameter3 default value",
+ });
+ });
+ });
+
+ describe("getParameterValuesBySlug", () => {
+ it("should return a map of defined parameter values keyed by the parameter's slug", () => {
+ expect(getParameterValuesBySlug(parameters, parameterValues)).toEqual({
+ [parameter1.slug]: "parameter1 queryParam value",
+ [parameter2.slug]: "parameter2 queryParam value",
+ [parameter3.slug]: "parameter3 default value",
+ });
+ });
+
+ it("should prioritize values found on the parameter object over the parameterValues map", () => {
+ const valuePopulatedParameter1 = {
+ ...parameter1,
+ value: "parameter1 value prop",
+ };
+ const parameters = [valuePopulatedParameter1, parameter2];
+
+ expect(getParameterValuesBySlug(parameters, parameterValues)).toEqual({
+ [parameter1.slug]: "parameter1 value prop",
+ [parameter2.slug]: "parameter2 queryParam value",
+ });
+ });
+
+ it("should handle an undefined parameterValues map", () => {
+ expect(getParameterValuesBySlug(parameters, undefined)).toEqual({});
+ expect(
+ getParameterValuesBySlug([
+ {
+ ...parameter1,
+ value: "parameter1 value prop",
+ },
+ ]),
+ ).toEqual({
+ [parameter1.slug]: "parameter1 value prop",
+ });
+ });
+ });
+ });
+
+ 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",
+ },
+ ]);
+ });
+ });
});