diff --git a/e2e/support/helpers/e2e-dashboard-helpers.js b/e2e/support/helpers/e2e-dashboard-helpers.js
index 0d5feaee7fcfd429ffeed67b053a528745587453..e19c04aee97b67fa29a05c66b912685c7594e0d2 100644
--- a/e2e/support/helpers/e2e-dashboard-helpers.js
+++ b/e2e/support/helpers/e2e-dashboard-helpers.js
@@ -138,8 +138,12 @@ export function setFilter(type, subType) {
});
}
+export function getRequiredToggle() {
+ return cy.findByLabelText("Always require a value");
+}
+
export function toggleRequiredParameter() {
- cy.findByLabelText("Always require a value").click();
+ getRequiredToggle().click();
}
export function createEmptyTextBox() {
diff --git a/e2e/support/helpers/e2e-embedding-helpers.js b/e2e/support/helpers/e2e-embedding-helpers.js
index 4dc1a544d88f0be209c59e15e96de2d73aac2de5..3fa187c63d1ece711f3f8ea0fcd8507a5e0e46a9 100644
--- a/e2e/support/helpers/e2e-embedding-helpers.js
+++ b/e2e/support/helpers/e2e-embedding-helpers.js
@@ -1,5 +1,5 @@
import { METABASE_SECRET_KEY } from "e2e/support/cypress_data";
-import { modal } from "e2e/support/helpers/e2e-ui-elements-helpers";
+import { modal, popover } from "e2e/support/helpers/e2e-ui-elements-helpers";
/**
* @typedef {object} QuestionResource
@@ -197,6 +197,44 @@ export function openStaticEmbeddingModal({
});
}
+export function closeStaticEmbeddingModal() {
+ modal().icon("close").click();
+}
+
+/**
+ * Open Static Embedding setup modal
+ * @param {"card" | "dashboard"} apiPath
+ * @param callback
+ */
+export function publishChanges(apiPath, callback) {
+ cy.intercept("PUT", `/api/${apiPath}/*`).as("publishChanges");
+
+ cy.button(/^(Publish|Publish changes)$/).click();
+
+ // TODO this could be simplified when we send one publish request instead of two
+ cy.wait(["@publishChanges", "@publishChanges"]).then(xhrs => {
+ // Unfortunately, the order of requests is not always the same.
+ // Therefore, we must first get the one that has the `embedding_params` and then assert on it.
+ const targetXhr = xhrs.find(({ request }) =>
+ Object.keys(request.body).includes("embedding_params"),
+ );
+ callback?.(targetXhr);
+ });
+}
+
+export function getParametersContainer() {
+ return cy.findByLabelText("Configuring parameters");
+}
+
+export function setEmbeddingParameter(name, value) {
+ getParametersContainer().findByLabelText(name).click();
+ popover().contains(value).click();
+}
+
+export function assertEmbeddingParameter(name, value) {
+ getParametersContainer().findByLabelText(name).should("have.text", value);
+}
+
// @param {("card"|"dashboard")} resourceType - The type of resource we are sharing
export function openNewPublicLinkDropdown(resourceType) {
cy.intercept("POST", `/api/${resourceType}/*/public_link`).as(
diff --git a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
index e9108a0b7f221ab9a764b1426953b94f0747472e..fe2fffc0a5cd3a7b4fe16bd742fb47358126bb5a 100644
--- a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
+++ b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
@@ -16,9 +16,19 @@ import {
setTokenFeatures,
dashboardParametersContainer,
goToTab,
+ editDashboard,
+ toggleRequiredParameter,
+ sidebar,
+ saveDashboard,
+ getRequiredToggle,
+ closeStaticEmbeddingModal,
+ publishChanges,
+ setEmbeddingParameter,
+ assertEmbeddingParameter,
} from "e2e/support/helpers";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
+import { addWidgetStringFilter } from "../native-filters/helpers/e2e-field-filter-helpers";
import {
questionDetails,
questionDetailsWithDefaults,
@@ -103,7 +113,7 @@ describe("scenarios > embedding > dashboard parameters", () => {
});
// publish the embedded dashboard so that we can directly navigate to its url
- publishChanges(({ request }) => {
+ publishChanges("dashboard", ({ request }) => {
assert.deepEqual(request.body.embedding_params, {
id: "locked",
name: "enabled",
@@ -149,7 +159,7 @@ describe("scenarios > embedding > dashboard parameters", () => {
cy.get("@allParameters").findByText("Editable").click();
popover().contains("Disabled").click();
- publishChanges(({ request }) => {
+ publishChanges("dashboard", ({ request }) => {
assert.deepEqual(request.body.embedding_params, {
name: "disabled",
id: "disabled",
@@ -205,6 +215,70 @@ describe("scenarios > embedding > dashboard parameters", () => {
cy.findByText("Not Used Filter").should("not.exist");
});
});
+
+ it("should handle required parameters", () => {
+ visitDashboard("@dashboardId");
+ editDashboard();
+
+ // Make one parameter required
+ getDashboardFilter("Name").click();
+ toggleRequiredParameter();
+ sidebar().findByText("Default value").next().click();
+ addWidgetStringFilter("Ferne Rosenbaum");
+ saveDashboard();
+
+ // Check that parameter visibility is correct
+ openStaticEmbeddingModal({ activeTab: "parameters", acceptTerms: true });
+ assertEmbeddingParameter("Id", "Disabled");
+ assertEmbeddingParameter("Name", "Editable");
+ assertEmbeddingParameter("Source", "Disabled");
+ assertEmbeddingParameter("User", "Disabled");
+ assertEmbeddingParameter("Not Used Filter", "Disabled");
+
+ // We only expect name to be "enabled" because the rest
+ // weren't touched and therefore aren't changed, whereas
+ // "enabled" must be set by default for required params.
+ publishChanges("dashboard", ({ request }) => {
+ assert.deepEqual(request.body.embedding_params, {
+ name: "enabled",
+ });
+ });
+
+ visitIframe();
+
+ // Filter widget must be visible
+ filterWidget().contains("Name");
+ // Its default value must be in the URL
+ cy.location("search").should("contain", "name=Ferne%20Rosenbaum");
+ // And the default should be applied giving us only 1 result
+ cy.get(".ScalarValue").invoke("text").should("eq", "1");
+ });
+
+ it("should (dis)allow setting parameters as required for a published embedding", () => {
+ visitDashboard("@dashboardId");
+
+ // Set an "editable" and "locked" parameters and leave the rest "disabled"
+ openStaticEmbeddingModal({ activeTab: "parameters", acceptTerms: true });
+ setEmbeddingParameter("Name", "Editable");
+ setEmbeddingParameter("Source", "Locked");
+ publishChanges("dashboard", ({ request }) => {
+ assert.deepEqual(request.body.embedding_params, {
+ name: "enabled",
+ source: "locked",
+ });
+ });
+
+ closeStaticEmbeddingModal();
+ editDashboard();
+
+ // Check each parameter's required state
+ assertRequiredEnabledForName({ name: "Name", enabled: true });
+ assertRequiredEnabledForName({ name: "Source", enabled: true });
+ // The rest must be disabled
+ assertRequiredEnabledForName({ name: "Id", enabled: false });
+ assertRequiredEnabledForName({ name: "User", enabled: false });
+ assertRequiredEnabledForName({ name: "Not Used Filter", enabled: false });
+ });
});
context("API", () => {
@@ -450,9 +524,9 @@ describe("scenarios > embedding > dashboard parameters with defaults", () => {
openStaticEmbeddingModal({ activeTab: "parameters" });
// ID param is disabled by default
- setParameter("Name", "Editable");
- setParameter("Source", "Locked");
- publishChanges(({ request }) => {
+ setEmbeddingParameter("Name", "Editable");
+ setEmbeddingParameter("Source", "Locked");
+ publishChanges("dashboard", ({ request }) => {
assert.deepEqual(request.body.embedding_params, {
source: "locked",
name: "enabled",
@@ -578,27 +652,13 @@ function openFilterOptions(name) {
filterWidget().contains(name).click();
}
-function publishChanges(callback) {
- cy.intercept("PUT", "/api/dashboard/*").as("publishChanges");
-
- cy.button(/^(Publish|Publish changes)$/).click();
-
- cy.wait(["@publishChanges", "@publishChanges"]).then(xhrs => {
- // Unfortunately, the order of requests is not always the same.
- // Therefore, we must first get the one that has the `embedding_params` and then assert on it.
- const targetXhr = xhrs.find(({ request }) =>
- Object.keys(request.body).includes("embedding_params"),
- );
- callback && callback(targetXhr);
- });
+function getDashboardFilter(name) {
+ return cy
+ .findByTestId("edit-dashboard-parameters-widget-container")
+ .findByText(name);
}
-function setParameter(name, filter) {
- cy.findByLabelText("Configuring parameters")
- .parent()
- .findByText(name)
- .siblings("a")
- .click();
-
- popover().contains(filter).click();
+function assertRequiredEnabledForName({ name, enabled }) {
+ getDashboardFilter(name).click();
+ getRequiredToggle().should(enabled ? "be.enabled" : "not.be.enabled");
}
diff --git a/e2e/test/scenarios/embedding/embedding-native.cy.spec.js b/e2e/test/scenarios/embedding/embedding-native.cy.spec.js
index eead04bd26c2a7672d0c6db358778580376ef682..5b860f42f70c5fee26cb088cb5f40a290193fe49 100644
--- a/e2e/test/scenarios/embedding/embedding-native.cy.spec.js
+++ b/e2e/test/scenarios/embedding/embedding-native.cy.spec.js
@@ -6,8 +6,13 @@ import {
visitEmbeddedPage,
visitIframe,
openStaticEmbeddingModal,
+ closeStaticEmbeddingModal,
+ publishChanges,
+ setEmbeddingParameter,
+ assertEmbeddingParameter,
} from "e2e/support/helpers";
+import * as SQLFilter from "../native-filters/helpers/e2e-sql-filter-helpers";
import { questionDetails } from "./shared/embedding-native";
import { questionDetailsWithDefaults } from "./shared/embedding-dashboard";
@@ -18,16 +23,25 @@ describe("scenarios > embedding > native questions", () => {
});
context("UI", () => {
- beforeEach(() => {
- cy.createNativeQuestion(questionDetails, {
+ function createAndVisitQuestion({ requiredTagName, defaultValue } = {}) {
+ const details = structuredClone(questionDetails);
+
+ if (requiredTagName) {
+ details.native["template-tags"][requiredTagName].default = defaultValue;
+ details.native["template-tags"][requiredTagName].required = true;
+ }
+
+ cy.createNativeQuestion(details, {
visitQuestion: true,
});
openStaticEmbeddingModal({ activeTab: "parameters" });
- });
+ }
it("should not display disabled parameters", () => {
- publishChanges(({ request }) => {
+ createAndVisitQuestion();
+
+ publishChanges("card", ({ request }) => {
assert.deepEqual(request.body.embedding_params, {});
});
@@ -44,25 +58,15 @@ describe("scenarios > embedding > native questions", () => {
});
it("should display and work with enabled parameters while hiding the locked one", () => {
- setParameter("Order ID", "Editable");
- setParameter("Created At", "Editable");
- setParameter("Total", "Locked");
- setParameter("State", "Editable");
- setParameter("Product ID", "Editable");
-
- // We must enter a value for a locked parameter
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText("Previewing locked parameters")
- .parent()
- .within(() => {
- cy.findByText("Total").click();
- });
+ createAndVisitQuestion();
- // Total is greater than or equal to 0
- cy.findByPlaceholderText("Enter a number").type("0").blur();
- cy.button("Add filter").click();
+ setEmbeddingParameter("Order ID", "Editable");
+ setEmbeddingParameter("Created At", "Editable");
+ setEmbeddingParameter("Total", "Locked");
+ setEmbeddingParameter("State", "Editable");
+ setEmbeddingParameter("Product ID", "Editable");
- publishChanges(({ request }) => {
+ publishChanges("card", ({ request }) => {
const actual = request.body.embedding_params;
const expected = {
@@ -133,6 +137,59 @@ describe("scenarios > embedding > native questions", () => {
"?id=926&created_at=&state=KS&product_id=10",
);
});
+
+ it("should handle required parameters", () => {
+ createAndVisitQuestion({ requiredTagName: "total", defaultValue: [100] });
+
+ assertEmbeddingParameter("Total", "Editable");
+
+ publishChanges("card", ({ request }) => {
+ const actual = request.body.embedding_params;
+
+ // We only expect total to be "enabled" because the rest
+ // weren't touched and therefore aren't changed, whereas
+ // "enabled" must be set by default for required params.
+ const expected = {
+ total: "enabled",
+ };
+
+ assert.deepEqual(actual, expected);
+ });
+
+ visitIframe();
+
+ // Filter widget must be visible
+ filterWidget().contains("Total");
+
+ // And its default value must be in the URL
+ cy.location("search").should("eq", "?total=100");
+ });
+
+ it("should (dis)allow setting parameters as required for a published embedding", () => {
+ createAndVisitQuestion();
+ // Make one parameter editable and one locked
+ setEmbeddingParameter("Order ID", "Editable");
+ setEmbeddingParameter("Total", "Locked");
+
+ publishChanges("card");
+ closeStaticEmbeddingModal();
+
+ cy.findByTestId("native-query-editor-container")
+ .findByText("Open Editor")
+ .click();
+
+ // Open variable editor
+ cy.findByTestId("native-query-editor-sidebar").icon("variable").click();
+
+ // Now check that all disabled parameters can't be required and the rest can
+ assertRequiredEnabledForName({ name: "id", enabled: true });
+ assertRequiredEnabledForName({ name: "total", enabled: true });
+ // disabled parameters
+ assertRequiredEnabledForName({ name: "created_at", enabled: false });
+ assertRequiredEnabledForName({ name: "source", enabled: false });
+ assertRequiredEnabledForName({ name: "state", enabled: false });
+ assertRequiredEnabledForName({ name: "product_id", enabled: false });
+ });
});
context("API", () => {
@@ -270,9 +327,9 @@ describe("scenarios > embedding > native questions with default parameters", ()
openStaticEmbeddingModal({ activeTab: "parameters" });
// Note: ID is disabled
- setParameter("Source", "Locked");
- setParameter("Name", "Editable");
- publishChanges(({ request }) => {
+ setEmbeddingParameter("Source", "Locked");
+ setEmbeddingParameter("Name", "Editable");
+ publishChanges("card", ({ request }) => {
assert.deepEqual(request.body.embedding_params, {
source: "locked",
name: "enabled",
@@ -291,28 +348,10 @@ describe("scenarios > embedding > native questions with default parameters", ()
});
});
-function setParameter(name, filter) {
- cy.findByLabelText("Configuring parameters")
- .parent()
- .within(() => {
- cy.findByText(name).siblings("a").click();
- });
-
- popover().contains(filter).click();
-}
-
-function publishChanges(callback) {
- cy.intercept("PUT", "/api/card/*").as("publishChanges");
-
- cy.button(/^(Publish|Publish changes)$/).click();
-
- cy.wait(["@publishChanges", "@publishChanges"]).then(xhrs => {
- // Unfortunately, the order of requests is not always the same.
- // Therefore, we must first get the one that has the `embedding_params` and then assert on it.
- const targetXhr = xhrs.find(({ request }) =>
- Object.keys(request.body).includes("embedding_params"),
+function assertRequiredEnabledForName({ name, enabled }) {
+ cy.findByTestId(`tag-editor-variable-${name}`).within(() => {
+ SQLFilter.getRequiredInput().should(
+ enabled ? "be.enabled" : "not.be.enabled",
);
-
- callback && callback(targetXhr);
});
}
diff --git a/e2e/test/scenarios/embedding/shared/embedding-native.js b/e2e/test/scenarios/embedding/shared/embedding-native.js
index 34f649edc8c998299c997e16554302aff823ec3e..7ed5ab420b7d32bcb86ffb07d51de2117659683a 100644
--- a/e2e/test/scenarios/embedding/shared/embedding-native.js
+++ b/e2e/test/scenarios/embedding/shared/embedding-native.js
@@ -46,7 +46,6 @@ export const questionDetails = {
dimension: ["field", ORDERS.TOTAL, null],
"widget-type": "number/>=",
default: [0],
- required: true,
},
source: {
id: "44038e73-f909-1bed-0974-2a42ce8979e8",
diff --git a/e2e/test/scenarios/native-filters/helpers/e2e-sql-filter-helpers.js b/e2e/test/scenarios/native-filters/helpers/e2e-sql-filter-helpers.js
index 32a96d465beefd4973ff36ec972f55f317262c36..8c04f91ce22709a5ebcceda798aec434f3eabc30 100644
--- a/e2e/test/scenarios/native-filters/helpers/e2e-sql-filter-helpers.js
+++ b/e2e/test/scenarios/native-filters/helpers/e2e-sql-filter-helpers.js
@@ -58,11 +58,19 @@ export function setDefaultValue(value) {
// UI PATTERNS
+export function getRequiredToggle() {
+ return cy.findByText("Always require a value");
+}
+
+export function getRequiredInput() {
+ return cy.findByLabelText("Always require a value");
+}
+
/**
* Toggle the required SQL filter on or off. It is off by default.
*/
export function toggleRequired() {
- cy.findByText("Always require a value").click();
+ getRequiredToggle().click();
}
// FILTER QUERY
diff --git a/frontend/src/metabase-lib/parameters/utils/cards.ts b/frontend/src/metabase-lib/parameters/utils/cards.ts
index 20b8fbdadf5b320862b6bfa073959b1957e5af25..0ca6663283ad2bdff0067b0b6fcb8a539894fb5c 100644
--- a/frontend/src/metabase-lib/parameters/utils/cards.ts
+++ b/frontend/src/metabase-lib/parameters/utils/cards.ts
@@ -21,7 +21,7 @@ export function getCardUiParameters(
const valuePopulatedParameters: (Parameter[] | ParameterWithTarget[]) & {
value?: any;
- } = getValuePopulatedParameters(parameters, parameterValues);
+ } = getValuePopulatedParameters({ parameters, values: parameterValues });
const question = new Question(card, metadata);
return valuePopulatedParameters.map(parameter => {
diff --git a/frontend/src/metabase-lib/parameters/utils/parameter-values.js b/frontend/src/metabase-lib/parameters/utils/parameter-values.js
index 9895fe410cd184588af8a11ed7caaac554ad5eac..a4aabd4ee097b7095555ad1a19f5a3af20769ae5 100644
--- a/frontend/src/metabase-lib/parameters/utils/parameter-values.js
+++ b/frontend/src/metabase-lib/parameters/utils/parameter-values.js
@@ -9,12 +9,39 @@ import {
export const PULSE_PARAM_EMPTY = null;
export const PULSE_PARAM_USE_DEFAULT = undefined;
-export function getValuePopulatedParameters(parameters, parameterValues) {
+/**
+ * In some cases, we need to use default parameter value in place of an absent one.
+ * Please use this function when dealing with the required parameters.
+ */
+export function getParameterValue({
+ parameter,
+ values = {},
+ defaultRequired = false,
+}) {
+ const value = values?.[parameter.id];
+ const useDefault = defaultRequired && parameter.required;
+ return value ?? (useDefault ? parameter.default : null);
+}
+
+/**
+ * In some cases, we need to use default parameter value in place of an absent one.
+ * Please use this function when dealing with the required parameters.
+ */
+export function getValuePopulatedParameters({
+ parameters,
+ values = {},
+ defaultRequired = false,
+}) {
return parameters.map(parameter => ({
...parameter,
- value: parameterValues?.[parameter.id] ?? null,
+ value: getParameterValue({
+ parameter,
+ values,
+ defaultRequired,
+ }),
}));
}
+
export function getDefaultValuePopulatedParameters(
parameters,
parameterValues,
diff --git a/frontend/src/metabase-lib/parameters/utils/parameter-values.unit.spec.js b/frontend/src/metabase-lib/parameters/utils/parameter-values.unit.spec.js
index d098de792b7aa0724bf84f633a8c88bde96174a3..b1a3f528b4556f2176f4e0bcb78c0d6c3bb74921 100644
--- a/frontend/src/metabase-lib/parameters/utils/parameter-values.unit.spec.js
+++ b/frontend/src/metabase-lib/parameters/utils/parameter-values.unit.spec.js
@@ -73,9 +73,12 @@ describe("parameters/utils/parameter-values", () => {
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, and null if it doesn't exist", () => {
expect(
- getValuePopulatedParameters([parameter1, parameter2], {
- [parameter1.id]: "parameter1 value",
- [parameter2.id]: "parameter2 value",
+ getValuePopulatedParameters({
+ parameters: [parameter1, parameter2],
+ values: {
+ [parameter1.id]: "parameter1 value",
+ [parameter2.id]: "parameter2 value",
+ },
}),
).toEqual([
{
@@ -90,9 +93,9 @@ describe("parameters/utils/parameter-values", () => {
});
it("should return null value if the parameter doesn't exist in the parameterValues arg", () => {
- expect(getValuePopulatedParameters([parameter1], {})).toEqual([
- { ...parameter1, value: null },
- ]);
+ expect(
+ getValuePopulatedParameters({ parameters: [parameter1], values: {} }),
+ ).toEqual([{ ...parameter1, value: null }]);
});
it("should handle there being an undefined or null parameterValues object", () => {
@@ -106,11 +109,14 @@ describe("parameters/utils/parameter-values", () => {
value: null,
},
];
- expect(getValuePopulatedParameters([parameter1, parameter2])).toEqual(
- parametersWithNulls,
- );
expect(
- getValuePopulatedParameters([parameter1, parameter2], null),
+ getValuePopulatedParameters({ parameters: [parameter1, parameter2] }),
+ ).toEqual(parametersWithNulls);
+ expect(
+ getValuePopulatedParameters({
+ parameters: [parameter1, parameter2],
+ values: null,
+ }),
).toEqual(parametersWithNulls);
});
});
diff --git a/frontend/src/metabase-types/api/dashboard.ts b/frontend/src/metabase-types/api/dashboard.ts
index e0a8ae4722516a1f07fb71e46530bc751c8231b5..c20df18063483ca1486383ed894953210f21db84 100644
--- a/frontend/src/metabase-types/api/dashboard.ts
+++ b/frontend/src/metabase-types/api/dashboard.ts
@@ -7,6 +7,7 @@ import type {
ParameterTarget,
} from "metabase-types/api";
+import type { EmbeddingParameters } from "metabase/public/lib/types";
import type { ActionDisplayType, WritebackAction } from "./actions";
import type { SearchModelType } from "./search";
import type { Card, CardId, CardDisplayType } from "./card";
@@ -43,8 +44,8 @@ export interface Dashboard {
auto_apply_filters: boolean;
archived: boolean;
public_uuid: string | null;
- embedding_params?: Record<string, string> | null;
width: "full" | "fixed";
+ embedding_params?: EmbeddingParameters | null;
/* Indicates whether static embedding for this dashboard has been published */
enable_embedding: boolean;
diff --git a/frontend/src/metabase/core/components/Toggle/Toggle.styled.tsx b/frontend/src/metabase/core/components/Toggle/Toggle.styled.tsx
index 5b421087b087fd2ba3bae4e1b2028ce03d1cd74d..2920d18c004a9b2bc1ef59d2eb28b673c66a302f 100644
--- a/frontend/src/metabase/core/components/Toggle/Toggle.styled.tsx
+++ b/frontend/src/metabase/core/components/Toggle/Toggle.styled.tsx
@@ -44,6 +44,12 @@ export const ToggleRoot = styled.input<ToggleRootProps>`
transition: background-color 0.3s;
flex-shrink: 0;
+ &[disabled] {
+ cursor: not-allowed;
+ opacity: 0.5;
+ background-color: ${color("bg-medium")};
+ }
+
&:after {
content: "";
width: ${props => (props.small ? "13px" : "20px")};
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx
index 7dc465146951ca9345ad350d290e1411ecf421e0..454ce2b31746c351fa1890077a8540000f0dc091 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx
+++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx
@@ -38,6 +38,7 @@ import type {
StoreDashcard,
} from "metabase-types/store";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import type Database from "metabase-lib/metadata/Database";
import type { UiParameter } from "metabase-lib/parameters/types";
import type Metadata from "metabase-lib/metadata/Metadata";
@@ -181,6 +182,9 @@ interface DashboardProps {
columnKey: string,
settings?: Record<string, unknown> | null,
) => void;
+ getEmbeddedParameterVisibility: (
+ slug: string,
+ ) => EmbeddingParameterVisibility | null;
}
function DashboardInner(props: DashboardProps) {
@@ -453,10 +457,10 @@ function DashboardInner(props: DashboardProps) {
const parametersWidget = (
<SyncedParametersList
- parameters={getValuePopulatedParameters(
+ parameters={getValuePopulatedParameters({
parameters,
- isAutoApplyFilters ? parameterValues : draftParameterValues,
- )}
+ values: isAutoApplyFilters ? parameterValues : draftParameterValues,
+ })}
editingParameter={editingParameter}
hideParameters={hiddenParameterSlugs}
dashboard={dashboard}
diff --git a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx
index c75886441d572d84e133c0c41504e7d8964f9a64..87b3966e00ed11636074ce291619a531e517beef 100644
--- a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx
+++ b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx
@@ -46,6 +46,7 @@ DashboardSidebars.propTypes = {
closeSidebar: PropTypes.func.isRequired,
setDashboardAttribute: PropTypes.func,
selectedTabId: PropTypes.number,
+ getEmbeddedParameterVisibility: PropTypes.func.isRequired,
};
export function DashboardSidebars({
@@ -75,6 +76,7 @@ export function DashboardSidebars({
closeSidebar,
setDashboardAttribute,
selectedTabId,
+ getEmbeddedParameterVisibility,
}) {
const handleAddCard = useCallback(
cardId => {
@@ -136,6 +138,7 @@ export function DashboardSidebars({
);
return (
<ParameterSidebar
+ getEmbeddedParameterVisibility={getEmbeddedParameterVisibility}
parameter={parameter}
otherParameters={otherParameters}
onChangeName={setParameterName}
diff --git a/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx b/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
index 2cf147204c996584c105c62e3899185997735377..e8f517e488bb2d9f2a5429267cb83f4bf5da2539 100644
--- a/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
+++ b/frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx
@@ -157,10 +157,10 @@ class AutomaticDashboardAppInner extends Component {
<div className="px1 pt1">
<SyncedParametersList
className="mt1"
- parameters={getValuePopulatedParameters(
+ parameters={getValuePopulatedParameters({
parameters,
- parameterValues,
- )}
+ values: parameterValues,
+ })}
setParameterValue={setParameterValue}
/>
</div>
diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx
index f9cad90a6d78f2ea057d5bad60cd8bc8096eb4d8..b5d2d1db0e7a75fddcf347b72d5f2978f32743de 100644
--- a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx
+++ b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx
@@ -44,6 +44,7 @@ import type {
ParameterValueOrArray,
} from "metabase-types/api";
import type { SelectedTabId, State, StoreDashcard } from "metabase-types/store";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import type Database from "metabase-lib/metadata/Database";
import type { UiParameter } from "metabase-lib/parameters/types";
import type Metadata from "metabase-lib/metadata/Metadata";
@@ -76,6 +77,7 @@ import {
getSelectedTabId,
getSidebar,
getSlowCards,
+ getEmbeddedParameterVisibility,
} from "../../selectors";
type OwnProps = {
@@ -116,6 +118,9 @@ type StateProps = {
selectedTabId: SelectedTabId;
isAutoApplyFilters: boolean;
isNavigatingBackToDashboard: boolean;
+ getEmbeddedParameterVisibility: (
+ slug: string,
+ ) => EmbeddingParameterVisibility | null;
};
type DispatchProps = {
@@ -146,6 +151,7 @@ const mapStateToProps = (state: State): StateProps => {
parameters: getParameters(state),
parameterValues: getParameterValues(state),
draftParameterValues: getDraftParameterValues(state),
+
metadata,
loadingStartTime: getLoadingStartTime(state),
clickBehaviorSidebarDashcard: getClickBehaviorSidebarDashcard(state),
@@ -160,6 +166,8 @@ const mapStateToProps = (state: State): StateProps => {
selectedTabId: getSelectedTabId(state),
isAutoApplyFilters: getIsAutoApplyFilters(state),
isNavigatingBackToDashboard: getIsNavigatingBackToDashboard(state),
+ getEmbeddedParameterVisibility: (slug: string) =>
+ getEmbeddedParameterVisibility(state, slug),
};
};
diff --git a/frontend/src/metabase/dashboard/selectors.ts b/frontend/src/metabase/dashboard/selectors.ts
index 186441f07032bc9b5b7bdca608c541dcabe3e936..2bdc7d68556a4807e9519a88f06cc91a8c2a85a3 100644
--- a/frontend/src/metabase/dashboard/selectors.ts
+++ b/frontend/src/metabase/dashboard/selectors.ts
@@ -28,6 +28,7 @@ import type {
EditParameterSidebarState,
State,
} from "metabase-types/store";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import Question from "metabase-lib/Question";
import { isQuestionDashCard } from "./utils";
@@ -395,6 +396,22 @@ export const getDashcardParameterMappingOptions = createCachedSelector(
return props.card.id ?? props.dashcard.id;
});
+// Embeddings might be published without passing embedding_params to the server,
+// in which case it's an empty object. We should treat such situations with
+// caution, assuming that an absent parameter is "disabled".
+export function getEmbeddedParameterVisibility(
+ state: State,
+ slug: string,
+): EmbeddingParameterVisibility | null {
+ const dashboard = getDashboard(state);
+ if (!dashboard?.enable_embedding) {
+ return null;
+ }
+
+ const embeddingParams = dashboard.embedding_params ?? {};
+ return embeddingParams[slug] ?? "disabled";
+}
+
export const getIsHeaderVisible = createSelector(
[getIsEmbedded, getEmbedOptions],
(isEmbedded, embedOptions) => !isEmbedded || !!embedOptions.header,
diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.styled.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.styled.tsx
index 0b01f22675faf1b265df160a94b694404bbd4cf6..a45278663b7de4487b3aca27b1021a603dab64b0 100644
--- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.styled.tsx
+++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.styled.tsx
@@ -29,17 +29,3 @@ export const SettingValueWidget = styled(ParameterValueWidget)`
border-radius: 0.5rem;
background-color: ${color("white")};
`;
-
-export const SettingRequiredContainer = styled.div`
- display: flex;
- gap: 0.5rem;
- margin: 0.5rem 0;
-`;
-
-export const SettingRequiredLabel = styled.label`
- display: block;
- margin-top: 0.35rem;
- font-weight: 700;
- color: ${color("text-medium")};
- cursor: pointer;
-`;
diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx
index 5f629a820f575de5585ece52c1edc40d8f519202..a9f9ede63b1770b20425bedd564744847b4404d6 100644
--- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx
+++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx
@@ -7,17 +7,16 @@ import type {
ValuesSourceConfig,
ValuesSourceType,
} from "metabase-types/api";
-import { TextInput } from "metabase/ui";
-import Toggle from "metabase/core/components/Toggle";
+import { Text, TextInput } from "metabase/ui";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import { canUseCustomSource } from "metabase-lib/parameters/utils/parameter-source";
import { getIsMultiSelect } from "../../utils/dashboards";
import { isSingleOrMultiSelectable } from "../../utils/parameter-type";
import ValuesSourceSettings from "../ValuesSourceSettings";
+import { RequiredParamToggle } from "../RequiredParamToggle";
import {
SettingLabel,
SettingLabelError,
- SettingRequiredContainer,
- SettingRequiredLabel,
SettingSection,
SettingsRoot,
SettingValueWidget,
@@ -33,9 +32,10 @@ export interface ParameterSettingsProps {
onChangeSourceType: (sourceType: ValuesSourceType) => void;
onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void;
onChangeRequired: (value: boolean) => void;
+ embeddedParameterVisibility: EmbeddingParameterVisibility | null;
}
-const ParameterSettings = ({
+export const ParameterSettings = ({
parameter,
isParameterSlugUsed,
onChangeName,
@@ -45,6 +45,7 @@ const ParameterSettings = ({
onChangeSourceType,
onChangeSourceConfig,
onChangeRequired,
+ embeddedParameterVisibility,
}: ParameterSettingsProps): JSX.Element => {
const [tempLabelValue, setTempLabelValue] = useState(parameter.name);
@@ -78,6 +79,8 @@ const ParameterSettings = ({
[onChangeSourceType, onChangeSourceConfig],
);
+ const isEmbeddedDisabled = embeddedParameterVisibility === "disabled";
+
return (
<SettingsRoot>
<SettingSection>
@@ -132,21 +135,33 @@ const ParameterSettings = ({
setValue={onChangeDefaultValue}
/>
- <SettingRequiredContainer>
- <Toggle
- id={`parameter-setting-required_${parameter.id}`}
- value={parameter.required}
- onChange={onChangeRequired}
- />
- <div>
- <SettingRequiredLabel
- htmlFor={`parameter-setting-required_${parameter.id}`}
- >
- {t`Always require a value`}
- </SettingRequiredLabel>
- <p>{t`When enabled, people can change the value or reset it, but can't clear it entirely.`}</p>
- </div>
- </SettingRequiredContainer>
+ <RequiredParamToggle
+ // This forces the toggle to be a new instance when the parameter changes,
+ // so that toggles don't slide, which is confusing.
+ key={`required_param_toggle_${parameter.id}`}
+ uniqueId={parameter.id}
+ disabled={isEmbeddedDisabled}
+ value={parameter.required ?? false}
+ onChange={onChangeRequired}
+ disabledTooltip={
+ <>
+ <Text lh={1.4}>
+ {t`This filter is set to disabled in an embedded dashboard.`}
+ </Text>
+ <Text lh={1.4}>
+ {t`To always require a value, first visit embedding settings,
+ make this filter editable or locked, re-publish the
+ dashboard, then return to this page.`}
+ </Text>
+ <Text size="sm">
+ {t`Note`}:{" "}
+ {t`making it locked, will require updating the
+ embedding code before proceeding, otherwise the embed will
+ break.`}
+ </Text>
+ </>
+ }
+ ></RequiredParamToggle>
</SettingSection>
</SettingsRoot>
);
@@ -170,5 +185,3 @@ function getLabelError({
}
return null;
}
-
-export { ParameterSettings };
diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx
index 671177a3901ef0d04b9c9508b8f5b71d2e3ef6fb..f219c49c735f0472df5515bf9328647dd163a4f0 100644
--- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx
+++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx
@@ -103,6 +103,7 @@ const setup = ({ parameter = createMockUiParameter() }: SetupOpts = {}) => {
renderWithProviders(
<ParameterSettings
+ embeddedParameterVisibility={null}
parameter={parameter}
isParameterSlugUsed={jest.fn()}
onChangeName={onChangeName}
diff --git a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx
index 02cc7622a2dfbb133df8d3b5c961c6cc0320e557..c57af08e5ca3cc2972322ef4b906c81ef1908109 100644
--- a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx
+++ b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx
@@ -10,6 +10,7 @@ import type {
ValuesSourceType,
} from "metabase-types/api";
import { slugify } from "metabase/lib/formatting";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import { canUseLinkedFilters } from "../../utils/linked-filters";
import { ParameterSettings } from "../ParameterSettings";
import ParameterLinkedFilters from "../ParameterLinkedFilters";
@@ -44,6 +45,9 @@ export interface ParameterSidebarProps {
onRemoveParameter: (parameterId: ParameterId) => void;
onShowAddParameterPopover: () => void;
onClose: () => void;
+ getEmbeddedParameterVisibility: (
+ slug: string,
+ ) => EmbeddingParameterVisibility | null;
}
export const ParameterSidebar = ({
@@ -60,6 +64,7 @@ export const ParameterSidebar = ({
onRemoveParameter,
onShowAddParameterPopover,
onClose,
+ getEmbeddedParameterVisibility,
}: ParameterSidebarProps): JSX.Element => {
const parameterId = parameter.id;
const tabs = useMemo(() => getTabs(parameter), [parameter]);
@@ -142,6 +147,9 @@ export const ParameterSidebar = ({
{tab === "settings" ? (
<ParameterSettings
parameter={parameter}
+ embeddedParameterVisibility={getEmbeddedParameterVisibility(
+ parameter.slug,
+ )}
isParameterSlugUsed={isParameterSlugUsed}
onChangeName={handleNameChange}
onChangeDefaultValue={handleDefaultValueChange}
diff --git a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx
index 2383d6b3001efcc798a26e6e6702bde8cc9cc06a..5bc487c12256374fce139ca113435a0612a30312 100644
--- a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx
+++ b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx
@@ -53,6 +53,7 @@ const setup = ({
onShowAddParameterPopover={jest.fn()}
onClose={jest.fn()}
onChangeRequired={jest.fn()}
+ getEmbeddedParameterVisibility={() => null}
/>
</div>
);
diff --git a/frontend/src/metabase/parameters/components/ParameterWidget/ParameterWidget.jsx b/frontend/src/metabase/parameters/components/ParameterWidget/ParameterWidget.jsx
index 1912cf3aa861693762c90f188be7102e520aba59..86cdb3ace2d2dccc9692f14f79c1e09dfbf47cd1 100644
--- a/frontend/src/metabase/parameters/components/ParameterWidget/ParameterWidget.jsx
+++ b/frontend/src/metabase/parameters/components/ParameterWidget/ParameterWidget.jsx
@@ -18,6 +18,8 @@ export class ParameterWidget extends Component {
static propTypes = {
parameter: PropTypes.object,
commitImmediately: PropTypes.bool,
+ setParameterValueToDefault: PropTypes.func,
+ enableParameterRequiredBehavior: PropTypes.bool,
};
static defaultProps = {
diff --git a/frontend/src/metabase/parameters/components/RequiredParamToggle/RequierParamToggle.styled.tsx b/frontend/src/metabase/parameters/components/RequiredParamToggle/RequierParamToggle.styled.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..7505e701294c8bb3910e4fb7220345f78b2e3da4
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/RequiredParamToggle/RequierParamToggle.styled.tsx
@@ -0,0 +1,12 @@
+import styled from "@emotion/styled";
+import { color } from "metabase/lib/colors";
+
+export const SettingRequiredLabel = styled.label`
+ display: flex;
+ align-items: center;
+ gap: 0.5rem;
+ margin-top: 0.35rem;
+ font-weight: 700;
+ color: ${color("text-medium")};
+ cursor: pointer;
+`;
diff --git a/frontend/src/metabase/parameters/components/RequiredParamToggle/RequiredParamToggle.tsx b/frontend/src/metabase/parameters/components/RequiredParamToggle/RequiredParamToggle.tsx
new file mode 100644
index 0000000000000000000000000000000000000000..193f1625cb3fc122f30705b3e8b7f65a5c3d8a91
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/RequiredParamToggle/RequiredParamToggle.tsx
@@ -0,0 +1,46 @@
+import { t } from "ttag";
+import type { ReactNode } from "react";
+import { Icon, HoverCard, Stack, Flex, Text } from "metabase/ui";
+import Toggle from "metabase/core/components/Toggle";
+import { SettingRequiredLabel } from "./RequierParamToggle.styled";
+
+interface RequiredParamToggleProps {
+ disabled?: boolean;
+ uniqueId: string;
+ value: boolean;
+ onChange: (value: boolean) => void;
+ disabledTooltip: ReactNode;
+}
+
+export function RequiredParamToggle(props: RequiredParamToggleProps) {
+ const { disabled, value, onChange, uniqueId, disabledTooltip } = props;
+ const id = `required_param_toggle_${uniqueId}`;
+
+ return (
+ <Flex gap="sm" mt="md">
+ <Toggle disabled={disabled} id={id} value={value} onChange={onChange} />
+ <div>
+ <SettingRequiredLabel htmlFor={id}>
+ {t`Always require a value`}
+ {disabled && (
+ <HoverCard position="top-end" shadow="xs">
+ <HoverCard.Target>
+ <Icon name="info_filled" />
+ </HoverCard.Target>
+ <HoverCard.Dropdown w={320}>
+ <Stack p="md" spacing="sm">
+ {disabledTooltip}
+ </Stack>
+ </HoverCard.Dropdown>
+ </HoverCard>
+ )}
+ </SettingRequiredLabel>
+
+ <Text
+ mt="sm"
+ lh={1.2}
+ >{t`When enabled, people can change the value or reset it, but can't clear it entirely.`}</Text>
+ </div>
+ </Flex>
+ );
+}
diff --git a/frontend/src/metabase/parameters/components/RequiredParamToggle/index.ts b/frontend/src/metabase/parameters/components/RequiredParamToggle/index.ts
new file mode 100644
index 0000000000000000000000000000000000000000..efb3081295e65627d6c25ad924613fc232eddc5e
--- /dev/null
+++ b/frontend/src/metabase/parameters/components/RequiredParamToggle/index.ts
@@ -0,0 +1 @@
+export { RequiredParamToggle } from "./RequiredParamToggle";
diff --git a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
index b959fb85e5352b8986a22368f0c720df32c0dc74..1e3dca6b542888440ede4e494b28081102c7fda2 100644
--- a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
+++ b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
@@ -59,6 +59,8 @@ interface OwnProps {
hiddenParameterSlugs?: string;
setParameterValue?: (parameterId: ParameterId, value: any) => void;
children: ReactNode;
+ enableParameterRequiredBehavior?: boolean;
+ setParameterValueToDefault: (id: ParameterId) => void;
}
interface StateProps {
@@ -100,6 +102,8 @@ function EmbedFrame({
draftParameterValues,
hiddenParameterSlugs,
setParameterValue,
+ setParameterValueToDefault,
+ enableParameterRequiredBehavior,
}: Props) {
const [hasInnerScroll, setInnerScroll] = useState(true);
@@ -158,14 +162,18 @@ function EmbedFrame({
className="mt1"
question={question}
dashboard={dashboard}
- parameters={getValuePopulatedParameters(
+ parameters={getValuePopulatedParameters({
parameters,
- _.isEmpty(draftParameterValues)
+ values: _.isEmpty(draftParameterValues)
? parameterValues
: draftParameterValues,
- )}
+ })}
setParameterValue={setParameterValue}
hideParameters={hideParameters}
+ setParameterValueToDefault={setParameterValueToDefault}
+ enableParameterRequiredBehavior={
+ enableParameterRequiredBehavior
+ }
/>
{dashboard && <FilterApplyButton />}
</ParametersWidgetContainer>
diff --git a/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx b/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx
index 510918373169c35c6edeec4f6843032a3564b6c6..0099c09af50d7c4380120a172a6b2c767eb76bab 100644
--- a/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx
+++ b/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx
@@ -8,6 +8,7 @@ import { ParameterWidget as StaticParameterWidget } from "metabase/parameters/co
import type {
EmbeddingParameters,
EmbeddingParametersValues,
+ EmbeddingParameterVisibility,
EmbedResourceParameter,
EmbedResourceType,
} from "metabase/public/lib/types";
@@ -39,13 +40,18 @@ export const ParametersSettings = ({
}: ParametersSettingsProps): JSX.Element => {
const valuePopulatedLockedParameters = useMemo(
() =>
- getValuePopulatedParameters(
- lockedParameters,
- parameterValues,
- ) as EmbedResourceParameterWithValue[],
+ getValuePopulatedParameters({
+ parameters: lockedParameters,
+ values: parameterValues,
+ defaultRequired: true,
+ }) as EmbedResourceParameterWithValue[],
[lockedParameters, parameterValues],
);
+ const hasRequiredParameters = resourceParameters.some(
+ param => param.required,
+ );
+
return resourceParameters.length > 0 ? (
<>
<StaticEmbedSetupPaneSettingsContentSection
@@ -57,7 +63,14 @@ export const ParametersSettings = ({
{resourceParameters.map(parameter => (
<div key={parameter.id} className="flex align-center">
<Icon name={getIconForParameter(parameter)} className="mr2" />
- <h3>{parameter.name}</h3>
+ <h3>
+ {parameter.name}
+ {parameter.required && (
+ <Text color="error" span>
+ *
+ </Text>
+ )}
+ </h3>
<Select
buttonProps={{
"aria-label": parameter.name,
@@ -67,16 +80,28 @@ export const ParametersSettings = ({
onChange={(e: ChangeEvent<HTMLSelectElement>) =>
onChangeEmbeddingParameters({
...embeddingParams,
- [parameter.slug]: e.target.value,
+ [parameter.slug]: e.target
+ .value as EmbeddingParameterVisibility,
})
}
>
- <Option icon="close" value="disabled">{t`Disabled`}</Option>
+ <Option
+ icon="close"
+ value="disabled"
+ disabled={parameter.required}
+ >{t`Disabled`}</Option>
<Option icon="pencil" value="enabled">{t`Editable`}</Option>
<Option icon="lock" value="locked">{t`Locked`}</Option>
</Select>
</div>
))}
+
+ {hasRequiredParameters && (
+ <Text size="xm">
+ <strong>{t`Note`}: </strong>
+ {t`Parameters marked with a red asterisk are required and can't be disabled.`}
+ </Text>
+ )}
</Stack>
</StaticEmbedSetupPaneSettingsContentSection>
@@ -95,9 +120,16 @@ export const ParametersSettings = ({
className="m0"
parameter={parameter}
parameters={valuePopulatedLockedParameters}
- setValue={(value: string) => {
- onChangeParameterValue(parameter.id, value);
+ setValue={(value: string) =>
+ onChangeParameterValue(parameter.id, value)
+ }
+ setParameterValueToDefault={() => {
+ onChangeParameterValue(
+ parameter.id,
+ parameter.default as any,
+ );
}}
+ enableParameterRequiredBehavior
/>
))}
</Stack>
diff --git a/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx b/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx
index ff14cce6984d68962b3636e500ae19a1b4c87fbd..1db71d6acb402d5be9826c72b9ca27272a54ef81 100644
--- a/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx
+++ b/frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx
@@ -19,6 +19,7 @@ import {
} from "metabase/public/lib/embed";
import { getEmbedServerCodeExampleOptions } from "metabase/public/lib/code";
+import { getParameterValue } from "metabase-lib/parameters/utils/parameter-values";
import { DEFAULT_DISPLAY_OPTIONS } from "./config";
import { ServerEmbedCodePane } from "./ServerEmbedCodePane";
import { EmbedModalContentStatusBar } from "./EmbedModalContentStatusBar";
@@ -290,19 +291,24 @@ function getDefaultEmbeddingParams(
resource: EmbedResource,
resourceParameters: EmbedResourceParameter[],
): EmbeddingParameters {
- return filterValidResourceParameters(
- resourceParameters,
- resource.embedding_params || {},
+ const validSlugs = resourceParameters.map(param => param.slug);
+ // We first pick only dashboard parameters with valid slugs
+ const defaultParams = _.pick(resource.embedding_params || {}, validSlugs);
+ // Then pick valid required dashboard parameters
+ const validRequiredParams = resourceParameters.filter(
+ param => param.slug && param.required,
);
-}
-
-function filterValidResourceParameters(
- resourceParameters: EmbedResourceParameter[],
- embeddingParams: EmbeddingParameters,
-) {
- const validParameters = resourceParameters.map(parameter => parameter.slug);
- return _.pick(embeddingParams, validParameters);
+ // And for each required parameter set its value to "enabled"
+ // (Editable) because this is the default for a required parameter.
+ // This is needed to save embedding_params when a user clicks
+ // "Publish" without changing parameter visibility.
+ return validRequiredParams.reduce((acc, param) => {
+ if (!acc[param.slug] || acc[param.slug] === "disabled") {
+ acc[param.slug] = "enabled";
+ }
+ return acc;
+ }, defaultParams);
}
function getPreviewParamsBySlug({
@@ -322,7 +328,11 @@ function getPreviewParamsBySlug({
return Object.fromEntries(
lockedParameters.map(parameter => [
parameter.slug,
- parameterValues[parameter.id] ?? null,
+ getParameterValue({
+ parameter,
+ values: parameterValues,
+ defaultRequired: true,
+ }),
]),
);
}
diff --git a/frontend/src/metabase/public/containers/PublicDashboard.jsx b/frontend/src/metabase/public/containers/PublicDashboard.jsx
index 30c650b2a47c923ae5d7f2ee5e385cc3a2d5eb61..219f9e80bc9f48ef8eec19e607b870c46810cc0e 100644
--- a/frontend/src/metabase/public/containers/PublicDashboard.jsx
+++ b/frontend/src/metabase/public/containers/PublicDashboard.jsx
@@ -159,6 +159,7 @@ class PublicDashboard extends Component {
draftParameterValues,
isFullscreen,
isNightMode,
+ setParameterValueToDefault,
} = this.props;
const buttons = !isWithinIframe()
@@ -178,6 +179,8 @@ class PublicDashboard extends Component {
actionButtons={
buttons.length > 0 && <div className="flex">{buttons}</div>
}
+ setParameterValueToDefault={setParameterValueToDefault}
+ enableParameterRequiredBehavior
>
<LoadingAndErrorWrapper
className={cx({
diff --git a/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx b/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx
index 1e542ce9873ef62ea62ed495df3b91160818119c..a051dd16bf46edf5361814924fc7c95938b1889b 100644
--- a/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx
+++ b/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx
@@ -119,6 +119,14 @@ class PublicQuestionInner extends Component {
);
};
+ setParameterValueToDefault = parameterId => {
+ const parameters = this.getParameters();
+ const parameter = parameters.find(({ id }) => id === parameterId);
+ if (parameter) {
+ this.setParameterValue(parameterId, parameter.default);
+ }
+ };
+
run = async () => {
const {
setErrorPage,
@@ -166,6 +174,22 @@ class PublicQuestionInner extends Component {
}
};
+ getParameters() {
+ const { metadata } = this.props;
+ const { card, initialized } = this.state;
+
+ if (!initialized || !card) {
+ return [];
+ }
+
+ return getCardUiParameters(
+ card,
+ metadata,
+ {},
+ card.parameters || undefined,
+ );
+ }
+
render() {
const {
params: { uuid, token },
@@ -184,19 +208,17 @@ class PublicQuestionInner extends Component {
/>
);
- const parameters =
- card &&
- getCardUiParameters(card, metadata, {}, card.parameters || undefined);
-
return (
<EmbedFrame
name={card && card.name}
description={card && card.description}
actionButtons={actionButtons}
question={question}
- parameters={initialized ? parameters : []}
+ parameters={this.getParameters()}
parameterValues={parameterValues}
setParameterValue={this.setParameterValue}
+ enableParameterRequiredBehavior
+ setParameterValueToDefault={this.setParameterValueToDefault}
>
<LoadingAndErrorWrapper
className="flex-full"
diff --git a/frontend/src/metabase/public/lib/types.ts b/frontend/src/metabase/public/lib/types.ts
index 86d95f1012ee839b49be1645a723763139193540..7bc26225e5a157fc7e7d51fd29e62428e99bf171 100644
--- a/frontend/src/metabase/public/lib/types.ts
+++ b/frontend/src/metabase/public/lib/types.ts
@@ -13,9 +13,13 @@ export type EmbedResourceParameter = {
name: string;
slug: string;
type: string;
+ required?: boolean;
+ default?: unknown;
};
-export type EmbeddingParameters = Record<string, string>;
+export type EmbeddingParameterVisibility = "disabled" | "enabled" | "locked";
+
+export type EmbeddingParameters = Record<string, EmbeddingParameterVisibility>;
export type EmbeddingParametersValues = Record<string, string>;
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx
index 90952b9504df2a9d4b8a97e01778bd7d4f0869f1..5e86092eeb7fc7c2067eff55469945c5252998d9 100644
--- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx
+++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.styled.tsx
@@ -55,7 +55,8 @@ export const ToggleContainer = styled.div`
`;
export const ToggleLabel = styled.label`
- display: block;
+ display: flex;
+ gap: 0.5rem;
margin-top: 0.35rem;
font-weight: 700;
color: ${color("text-medium")};
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.tsx
index daaeb9738eaa536f3280b668f5afa45fbb491af8..eeebd7fe2c180de25463da0ad7e91f0e377f6549 100644
--- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.tsx
+++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorParam.tsx
@@ -5,11 +5,10 @@ import { connect } from "react-redux";
import { Link } from "react-router";
import Schemas from "metabase/entities/schemas";
-import Toggle from "metabase/core/components/Toggle";
import InputBlurChange from "metabase/components/InputBlurChange";
import type { SelectChangeEvent } from "metabase/core/components/Select";
import Select, { Option } from "metabase/core/components/Select";
-
+import { Text } from "metabase/ui";
import ValuesSourceSettings from "metabase/parameters/components/ValuesSourceSettings";
import { fetchField } from "metabase/redux/metadata";
@@ -30,6 +29,8 @@ import type {
ValuesSourceType,
} from "metabase-types/api";
import type { State } from "metabase-types/store";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
+import { RequiredParamToggle } from "metabase/parameters/components/RequiredParamToggle";
import type Metadata from "metabase-lib/metadata/Metadata";
import type Database from "metabase-lib/metadata/Database";
import type Table from "metabase-lib/metadata/Table";
@@ -49,13 +50,12 @@ import {
InputContainer,
TagContainer,
TagName,
- ToggleContainer,
- ToggleLabel,
} from "./TagEditorParam.styled";
interface Props {
tag: TemplateTag;
parameter: Parameter;
+ embeddedParameterVisibility?: EmbeddingParameterVisibility | null;
database?: Database | null;
databases: Database[];
databaseFields?: Field[];
@@ -202,7 +202,14 @@ class TagEditorParamInner extends Component<Props> {
};
render() {
- const { tag, database, databases, metadata, parameter } = this.props;
+ const {
+ tag,
+ database,
+ databases,
+ metadata,
+ parameter,
+ embeddedParameterVisibility,
+ } = this.props;
let widgetOptions: { name?: string; type: string }[] = [];
let field: Field | null = null;
let table: Table | null | undefined = null;
@@ -224,9 +231,10 @@ class TagEditorParamInner extends Component<Props> {
tag["widget-type"] === "none" || !tag["widget-type"];
const hasNoWidgetLabel = !tag["display-name"];
const hasNoDefaultValue = !tag.default;
+ const isEmbeddedDisabled = embeddedParameterVisibility === "disabled";
return (
- <TagContainer>
+ <TagContainer data-testid={`tag-editor-variable-${tag.name}`}>
<ContainerLabel paddingTop>{t`Variable name`}</ContainerLabel>
<TagName>{tag.name}</TagName>
@@ -386,19 +394,30 @@ class TagEditorParamInner extends Component<Props> {
commitImmediately
/>
- <ToggleContainer>
- <Toggle
- id={`tag-editor-required_${tag.id}`}
- value={tag.required}
- onChange={this.setRequired}
- />
- <div>
- <ToggleLabel htmlFor={`tag-editor-required_${tag.id}`}>
- {t`Always require a value`}
- </ToggleLabel>
- <p>{t`When enabled, people can change the value or reset it, but can't clear it entirely.`}</p>
- </div>
- </ToggleContainer>
+ <RequiredParamToggle
+ uniqueId={tag.id}
+ disabled={isEmbeddedDisabled}
+ value={tag.required ?? false}
+ onChange={this.setRequired}
+ disabledTooltip={
+ <>
+ <Text lh={1.4}>
+ {t`This filter is set to disabled in an embedded question.`}
+ </Text>
+ <Text lh={1.4}>
+ {t`To always require a value, first visit embedding settings,
+ make this filter editable or locked, re-publish the
+ question, then return to this page.`}
+ </Text>
+ <Text size="sm">
+ {t`Note`}:{" "}
+ {t`making it locked, will require updating the
+ embedding code before proceeding, otherwise the embed will
+ break.`}
+ </Text>
+ </>
+ }
+ />
</div>
</TagContainer>
);
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.tsx
index 69038bca9d6c7bc0277d6ff1da3e282da9c86fe5..120b0960afb411d36141ec7b920e551b85cbc9c8 100644
--- a/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.tsx
+++ b/frontend/src/metabase/query_builder/components/template_tags/TagEditorSidebar.tsx
@@ -1,5 +1,4 @@
import { Component } from "react";
-import PropTypes from "prop-types";
import { t } from "ttag";
import cx from "classnames";
import _ from "underscore";
@@ -17,6 +16,7 @@ import type {
TemplateTag,
TemplateTagId,
} from "metabase-types/api";
+import type { EmbeddingParameterVisibility } from "metabase/public/lib/types";
import type NativeQuery from "metabase-lib/queries/NativeQuery";
import type Database from "metabase-lib/metadata/Database";
import type Field from "metabase-lib/metadata/Field";
@@ -25,6 +25,10 @@ import type Question from "metabase-lib/Question";
import { TagEditorParam } from "./TagEditorParam";
import { TagEditorHelp } from "./TagEditorHelp";
+type GetEmbeddedParamVisibility = (
+ slug: string,
+) => EmbeddingParameterVisibility;
+
interface TagEditorSidebarProps {
card: Card;
query: NativeQuery;
@@ -37,6 +41,7 @@ interface TagEditorSidebarProps {
setTemplateTagConfig: (tag: TemplateTag, config: Parameter) => void;
setParameterValue: (tagId: TemplateTagId, value: RowValue) => void;
onClose: () => void;
+ getEmbeddedParameterVisibility: GetEmbeddedParamVisibility;
}
interface TagEditorSidebarState {
@@ -48,17 +53,6 @@ export class TagEditorSidebar extends Component<TagEditorSidebarProps> {
section: "settings",
};
- static propTypes = {
- card: PropTypes.object.isRequired,
- onClose: PropTypes.func.isRequired,
- databaseFields: PropTypes.array,
- sampleDatabaseId: PropTypes.number,
- setDatasetQuery: PropTypes.func.isRequired,
- setTemplateTag: PropTypes.func.isRequired,
- setTemplateTagConfig: PropTypes.func.isRequired,
- setParameterValue: PropTypes.func.isRequired,
- };
-
setSection(section: "settings" | "help") {
this.setState({ section });
MetabaseAnalytics.trackStructEvent(
@@ -80,6 +74,7 @@ export class TagEditorSidebar extends Component<TagEditorSidebarProps> {
setTemplateTagConfig,
setParameterValue,
onClose,
+ getEmbeddedParameterVisibility,
} = this.props;
const tags = query.variableTemplateTags();
const database = question.database();
@@ -121,6 +116,7 @@ export class TagEditorSidebar extends Component<TagEditorSidebarProps> {
setTemplateTag={setTemplateTag}
setTemplateTagConfig={setTemplateTagConfig}
setParameterValue={setParameterValue}
+ getEmbeddedParameterVisibility={getEmbeddedParameterVisibility}
/>
) : (
<TagEditorHelp
@@ -145,6 +141,7 @@ interface SettingsPaneProps {
setTemplateTag: (tag: TemplateTag) => void;
setTemplateTagConfig: (tag: TemplateTag, config: Parameter) => void;
setParameterValue: (tagId: TemplateTagId, value: RowValue) => void;
+ getEmbeddedParameterVisibility: GetEmbeddedParamVisibility;
}
const SettingsPane = ({
@@ -156,14 +153,20 @@ const SettingsPane = ({
setTemplateTag,
setTemplateTagConfig,
setParameterValue,
+ getEmbeddedParameterVisibility,
}: SettingsPaneProps) => (
<div>
{tags.map(tag => (
- <div key={tag.name}>
+ <div key={tag.id}>
<TagEditorParam
tag={tag}
key={tag.name}
parameter={parametersById[tag.id]}
+ embeddedParameterVisibility={
+ parametersById[tag.id]
+ ? getEmbeddedParameterVisibility(parametersById[tag.id].slug)
+ : null
+ }
databaseFields={databaseFields}
database={database}
databases={databases}
diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
index b2105f650e6dda7150415e2e43847547cd63ef68..43a6af5b612bbddb710faa51e5f3ac28f6ad7cc2 100644
--- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
+++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
@@ -89,6 +89,7 @@ import {
isResultsMetadataDirty,
getShouldShowUnsavedChangesWarning,
getRequiredTemplateTags,
+ getEmbeddedParameterVisibility,
} from "../selectors";
import * as actions from "../actions";
import { VISUALIZATION_SLOW_TIMEOUT } from "../constants";
@@ -182,6 +183,8 @@ const mapStateToProps = (state, props) => {
reportTimezone: getSetting(state, "report-timezone-long"),
requiredTemplateTags: getRequiredTemplateTags(state),
+ getEmbeddedParameterVisibility: slug =>
+ getEmbeddedParameterVisibility(state, slug),
};
};
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 0815a3ca27b07afb043e3f157d04df741ec09626..5f7d19f7f0b389cc51a1ee0318b6d91f1a08a942 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -1024,3 +1024,24 @@ export const getRequiredTemplateTags = createSelector(
.map(key => templateTags[key])
: [],
);
+
+export const getEmbeddingParameters = createSelector([getCard], card => {
+ if (!card?.enable_embedding) {
+ return {};
+ }
+
+ return card.embedding_params ?? {};
+});
+
+// Embeddings might be published without passing embedding_params to the server,
+// in which case it's an empty object. We should treat such situations with
+// caution, assuming that an absent parameter is "disabled".
+export function getEmbeddedParameterVisibility(state, slug) {
+ const card = getCard(state);
+ if (!card?.enable_embedding) {
+ return null;
+ }
+
+ const embeddingParams = card.embedding_params ?? {};
+ return embeddingParams[slug] ?? "disabled";
+}