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

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 =>

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);

frontend/src/metabase/public/components/widgets/EmbedModalContent.unit.spec.tsx

+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();
+}