From 51fc3624aadbf7bb010cbf64ecdc50c1a1a034f2 Mon Sep 17 00:00:00 2001
From: "Mahatthana (Kelvin) Nomsawadi" <mahatthana.n@gmail.com>
Date: Thu, 15 Dec 2022 03:06:55 +0700
Subject: [PATCH] Fix can't change locked parameter names in full-app embedding
(#27179)
* Fix can't change locked parameter names in full-app embedding
* Remove unused param
* Add tests
---
.../widgets/AdvancedSettingsPane.jsx | 5 +-
.../components/widgets/EmbedModalContent.jsx | 23 ++-
.../widgets/EmbedModalContent.unit.spec.tsx | 169 ++++++++++++++++++
3 files changed, 191 insertions(+), 6 deletions(-)
create mode 100644 frontend/src/metabase/public/components/widgets/EmbedModalContent.unit.spec.tsx
diff --git a/frontend/src/metabase/public/components/widgets/AdvancedSettingsPane.jsx b/frontend/src/metabase/public/components/widgets/AdvancedSettingsPane.jsx
index a9fff200042..96d585b15a7 100644
--- a/frontend/src/metabase/public/components/widgets/AdvancedSettingsPane.jsx
+++ b/frontend/src/metabase/public/components/widgets/AdvancedSettingsPane.jsx
@@ -31,8 +31,6 @@ const AdvancedSettingsPane = ({
displayOptions,
onChangeDisplayOptions,
onUnpublish,
- pane,
- onChangePane,
previewParameters,
parameterValues,
onChangeParameterValue,
@@ -73,6 +71,9 @@ const AdvancedSettingsPane = ({
/>
<h3>{parameter.name}</h3>
<Select
+ buttonProps={{
+ "aria-label": parameter.name,
+ }}
className="ml-auto bg-white"
value={embeddingParams[parameter.slug] || "disabled"}
onChange={e =>
diff --git a/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx b/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
index 1759b793774..f19f135103b 100644
--- a/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
+++ b/frontend/src/metabase/public/components/widgets/EmbedModalContent.jsx
@@ -4,6 +4,7 @@ import { connect } from "react-redux";
import { titleize } from "inflection";
import { t } from "ttag";
+import _ from "underscore";
import Icon from "metabase/components/Icon";
import {
@@ -40,7 +41,7 @@ class EmbedModalContent extends Component {
this.state = {
pane: "preview",
embedType: null,
- embeddingParams: props.resource.embedding_params || {},
+ embeddingParams: getDefaultEmbeddingParams(props),
displayOptions,
parameterValues: {},
};
@@ -73,8 +74,7 @@ class EmbedModalContent extends Component {
};
handleDiscard = () => {
- const { resource } = this.props;
- this.setState({ embeddingParams: resource.embedding_params || {} });
+ this.setState({ embeddingParams: getDefaultEmbeddingParams(this.props) });
};
getPreviewParameters(resourceParameters, embeddingParams) {
@@ -235,7 +235,20 @@ class EmbedModalContent extends Component {
}
}
-export default connect(mapStateToProps)(EmbedModalContent);
+function getDefaultEmbeddingParams(props) {
+ const { resource, resourceParameters } = props;
+
+ return filterValidResourceParameters(
+ resource.embedding_params || {},
+ resourceParameters,
+ );
+}
+
+function filterValidResourceParameters(embeddingParams, resourceParameters) {
+ const validParameters = resourceParameters.map(parameter => parameter.slug);
+
+ return _.pick(embeddingParams, validParameters);
+}
export const EmbedTitle = ({ type, onClick }) => (
<a className="flex align-center" onClick={onClick}>
@@ -244,3 +257,5 @@ export const EmbedTitle = ({ type, onClick }) => (
{type}
</a>
);
+
+export default connect(mapStateToProps)(EmbedModalContent);
diff --git a/frontend/src/metabase/public/components/widgets/EmbedModalContent.unit.spec.tsx b/frontend/src/metabase/public/components/widgets/EmbedModalContent.unit.spec.tsx
new file mode 100644
index 00000000000..45d0aedf105
--- /dev/null
+++ b/frontend/src/metabase/public/components/widgets/EmbedModalContent.unit.spec.tsx
@@ -0,0 +1,169 @@
+import React from "react";
+import { screen, waitFor } from "@testing-library/react";
+import _ from "underscore";
+import { renderWithProviders } from "__support__/ui";
+import EmbedModalContent from "./EmbedModalContent";
+
+describe("EmbedModalContent", () => {
+ it("should render", () => {
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={{}}
+ resourceParameters={[]}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ expect(screen.getByText("Sharing")).toBeInTheDocument();
+ expect(screen.getByText("Public link")).toBeInTheDocument();
+ expect(screen.getByText("Public embed")).toBeInTheDocument();
+ expect(screen.getByText("Embed in your application")).toBeInTheDocument();
+ });
+
+ it("should render parameters", () => {
+ const parameters = [
+ { name: "My param", slug: "my_param", type: "category" },
+ ];
+
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={{}}
+ resourceParameters={parameters}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ openEmbedModal();
+ expect(screen.getByText("My param")).toBeInTheDocument();
+ expect(screen.getByLabelText("My param")).toHaveTextContent("Disabled");
+ });
+
+ it("should render unsaved parameters", () => {
+ const parameters = [
+ { name: "My param", slug: "my_param", type: "category" },
+ ];
+
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={{}}
+ resourceParameters={parameters}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ openEmbedModal();
+ expect(screen.getByText("My param")).toBeInTheDocument();
+ expect(screen.getByLabelText("My param")).toHaveTextContent("Disabled");
+ });
+
+ it("should render saved parameters", () => {
+ const resource = {
+ embedding_params: {
+ my_param: "locked",
+ },
+ };
+ const parameters = [
+ { name: "My param", slug: "my_param", type: "category" },
+ ];
+
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={resource}
+ resourceParameters={parameters}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ openEmbedModal();
+ expect(screen.getByText("My param")).toBeInTheDocument();
+ expect(screen.getByLabelText("My param")).toHaveTextContent("Locked");
+ });
+
+ it("should only render valid parameters", () => {
+ const resource = {
+ embedding_params: {
+ old_param: "locked",
+ },
+ };
+ const parameters = [
+ { name: "My param", slug: "my_param", type: "category" },
+ ];
+
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={resource}
+ resourceParameters={parameters}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ openEmbedModal();
+ expect(screen.getByText("My param")).toBeInTheDocument();
+ expect(screen.getByLabelText("My param")).toHaveTextContent("Disabled");
+ });
+
+ it("should update a card with only valid parameters", async () => {
+ const resource = {
+ embedding_params: {
+ old_param: "locked",
+ },
+ };
+ const parameters = [
+ { name: "My param", slug: "my_param", type: "category" },
+ ];
+ const onUpdateEmbeddingParams = jest.fn();
+
+ renderWithConfiguredProviders(
+ <EmbedModalContent
+ resource={resource}
+ resourceParameters={parameters}
+ onUpdateEmbeddingParams={onUpdateEmbeddingParams}
+ onUpdateEnableEmbedding={jest.fn()}
+ getPublicUrl={jest.fn()}
+ />,
+ );
+
+ openEmbedModal();
+ expect(screen.getByText("My param")).toBeInTheDocument();
+ expect(screen.getByLabelText("My param")).toHaveTextContent("Disabled");
+ screen.getByLabelText("My param").click();
+ screen.getByText("Locked").click();
+ screen.getByRole("button", { name: "Publish" }).click();
+ await waitFor(() =>
+ expect(onUpdateEmbeddingParams).toHaveBeenCalledWith({
+ my_param: "locked",
+ }),
+ );
+ });
+});
+
+const storeInitialState = {
+ currentUser: {
+ is_superuser: true,
+ },
+ settings: {
+ values: {
+ "enable-embedding": true,
+ "embedding-secret-key": "my_super_secret_key",
+ },
+ },
+};
+
+function renderWithConfiguredProviders(element: JSX.Element) {
+ renderWithProviders(element, {
+ storeInitialState,
+ reducers: {
+ settings: (state: Record<string, any> = storeInitialState.settings) => {
+ return state;
+ },
+ },
+ });
+}
+
+function openEmbedModal() {
+ screen
+ .getByRole("button", {
+ name: "Set up",
+ })
+ .click();
+}
--
GitLab