fix(sdk): support hiding columns in InteractiveQuestion (#49013) (#49158)

* investigate hide columns

* add update viz settings in the sdk

* cleanup implementation

* add storybook for interactive question

* use the question callback to update viz settings

* remove onUpdateVisualizationSettings

* revert change to onUpdateVisualizationSettings

* add click action behaviour

* fix misspelling

* add onUpdateQuestion mock function

* add unit test for click action behaviour

* add e2e test for hiding question column header

Co-authored-by: Phoomparin Mano <poom@metabase.com>

e2e/test/scenarios/embedding-sdk/interactive-question.cy.spec.ts

@@ -5,6 +5,8 @@ import {
   popover,
   restore,
   setTokenFeatures,
+  tableHeaderClick,
+  tableInteractive,
   visitFullAppEmbeddingUrl,
 } from "e2e/support/helpers";
 import {
@@ -100,6 +102,22 @@ describeSDK("scenarios > embedding-sdk > interactive-question", () => {
     cy.icon("warning").should("not.exist");
   });
 
+  it("should be able to hide columns from a table", () => {
+    cy.wait("@cardQuery").then(({ response }) => {
+      expect(response?.statusCode).to.equal(202);
+    });
+
+    tableInteractive().findByText("Max of Quantity").should("be.visible");
+
+    tableHeaderClick("Max of Quantity");
+
+    popover()
+      .findByTestId("click-actions-sort-control-formatting-hide")
+      .click();
+
+    tableInteractive().findByText("Max of Quantity").should("not.exist");
+  });
+
   it("can save a question", () => {
     cy.intercept("POST", "/api/card").as("createCard");
 

enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestion/components/Visualization.tsx

@@ -7,6 +7,7 @@ import {
 } from "embedding-sdk/components/private/PublicComponentWrapper";
 import CS from "metabase/css/core/index.css";
 import QueryVisualization from "metabase/query_builder/components/QueryVisualization";
+import type Question from "metabase-lib/v1/Question";
 
 import { useInteractiveQuestionContext } from "../context";
 
@@ -19,6 +20,7 @@ export const QuestionVisualization = () => {
     isQueryRunning,
     navigateToNewCard,
     onNavigateBack,
+    updateQuestion,
   } = useInteractiveQuestionContext();
 
   if (isQuestionLoading) {
@@ -46,6 +48,9 @@ export const QuestionVisualization = () => {
       mode={mode}
       navigateToNewCardInsideQB={navigateToNewCard}
       onNavigateBack={onNavigateBack}
+      onUpdateQuestion={(question: Question) =>
+        updateQuestion(question, { run: false })
+      }
     />
   );
 };

frontend/src/metabase/query_builder/components/VisualizationResult.jsx

@@ -139,6 +139,7 @@ export default class VisualizationResult extends Component {
             onSelectTimelineEvents={this.props.selectTimelineEvents}
             onDeselectTimelineEvents={this.props.deselectTimelineEvents}
             onOpenChartSettings={this.props.onOpenChartSettings}
+            onUpdateQuestion={this.props.onUpdateQuestion}
             onUpdateWarnings={this.props.onUpdateWarnings}
             onUpdateVisualizationSettings={
               this.props.onUpdateVisualizationSettings

frontend/src/metabase/query_builder/components/view/View/ViewMainContainer/ViewMainContainer.jsx

@@ -54,6 +54,7 @@ export const ViewMainContainer = props => {
           noHeader
           className={CS.spread}
           mode={queryMode}
+          onUpdateQuestion={updateQuestion}
         />
       </StyledDebouncedFrame>
       <TimeseriesChrome

frontend/src/metabase/visualizations/click-actions/actions/HideColumnAction/HideColumnAction.tsx

 import { t } from "ttag";
 
-import { onUpdateVisualizationSettings } from "metabase/query_builder/actions";
 import type { LegacyDrill } from "metabase/visualizations/types";
 import * as Lib from "metabase-lib";
 import { findColumnSettingIndexesForColumns } from "metabase-lib/v1/queries/utils/dataset";
@@ -33,7 +32,7 @@ export const HideColumnAction: LegacyDrill = ({
       icon: "eye_crossed_out",
       tooltip: t`Hide column`,
       default: true,
-      action: () => {
+      question: () => {
         const columnSettings = settings?.["table.columns"] ?? [];
         const [columnSettingIndex] = findColumnSettingIndexesForColumns(
           [column],
@@ -48,10 +47,9 @@ export const HideColumnAction: LegacyDrill = ({
           };
         }
 
-        return onUpdateVisualizationSettings({
-          "table.columns": columnSettingsCopy,
-        });
+        return question.updateSettings({ "table.columns": columnSettingsCopy });
       },
+      questionChangeBehavior: "updateQuestion",
     },
   ];
 };

frontend/src/metabase/visualizations/components/ClickActions/ClickActionControl.tsx

@@ -86,7 +86,11 @@ export const ClickActionControl = ({
     case "sort":
       return (
         <Tooltip tooltip={action.tooltip}>
-          <SortControl onlyIcon onClick={handleClick}>
+          <SortControl
+            onlyIcon
+            onClick={handleClick}
+            data-testid={`click-actions-sort-control-${action.name}`}
+          >
             {typeof action.icon === "string" && (
               <Icon size={14} name={action.icon as unknown as IconName} />
             )}

frontend/src/metabase/visualizations/components/ClickActions/ClickActionsPopover.tsx

@@ -11,6 +11,7 @@ import type {
   RegularClickAction,
 } from "metabase/visualizations/types";
 import { isPopoverClickAction } from "metabase/visualizations/types";
+import type Question from "metabase-lib/v1/Question";
 import type { Series } from "metabase-types/api";
 import type { Dispatch } from "metabase-types/store";
 
@@ -24,6 +25,7 @@ interface ChartClickActionsProps {
   dispatch: Dispatch;
   onChangeCardAndRun: OnChangeCardAndRun;
   onUpdateVisualizationSettings: () => void;
+  onUpdateQuestion?: (question: Question) => void;
   onClose?: () => void;
 }
 
@@ -49,13 +51,14 @@ export class ClickActionsPopover extends Component<
   };
 
   handleClickAction = (action: RegularClickAction) => {
-    const { dispatch, onChangeCardAndRun } = this.props;
+    const { dispatch, onChangeCardAndRun, onUpdateQuestion } = this.props;
     if (isPopoverClickAction(action)) {
       this.setState({ popoverAction: action });
     } else {
       const didPerform = performAction(action, {
         dispatch,
         onChangeCardAndRun,
+        onUpdateQuestion,
       });
       if (didPerform) {
         this.close();

frontend/src/metabase/visualizations/components/Visualization/Visualization.jsx

@@ -56,6 +56,7 @@ const defaultProps = {
   isSettings: false,
   isQueryBuilder: false,
   isEmbeddingSdk: false,
+  onUpdateQuestion: () => {},
   onUpdateVisualizationSettings: () => {},
   // prefer passing in a function that doesn't cause the application to reload
   onChangeLocation: location => {
@@ -540,6 +541,7 @@ class Visualization extends PureComponent {
               clicked={clicked}
               clickActions={regularClickActions}
               onChangeCardAndRun={this.handleOnChangeCardAndRun}
+              onUpdateQuestion={this.props.onUpdateQuestion}
               onClose={this.hideActions}
               series={series}
               onUpdateVisualizationSettings={onUpdateVisualizationSettings}

frontend/src/metabase/visualizations/lib/action.js

@@ -4,7 +4,10 @@ import _ from "underscore";
 import { setParameterValuesFromQueryParams } from "metabase/dashboard/actions";
 import { open } from "metabase/lib/dom";
 
-export function performAction(action, { dispatch, onChangeCardAndRun }) {
+export function performAction(
+  action,
+  { dispatch, onChangeCardAndRun, onUpdateQuestion },
+) {
   let didPerform = false;
   if (action.action) {
     const reduxAction = action.action();
@@ -28,14 +31,22 @@ export function performAction(action, { dispatch, onChangeCardAndRun }) {
     }
   }
   if (action.question) {
+    const { questionChangeBehavior = "changeCardAndRun" } = action;
+
     const question = action.question();
     const extra = action?.extra?.() ?? {};
+
     if (question) {
-      onChangeCardAndRun({
-        nextCard: question.card(),
-        ...extra,
-        objectId: extra.objectId,
-      });
+      if (questionChangeBehavior === "changeCardAndRun") {
+        onChangeCardAndRun({
+          nextCard: question.card(),
+          ...extra,
+          objectId: extra.objectId,
+        });
+      } else if (questionChangeBehavior === "updateQuestion") {
+        onUpdateQuestion(question);
+      }
+
       didPerform = true;
     }
   }

frontend/src/metabase/visualizations/lib/action.unit.spec.ts

 import MetabaseSettings from "metabase/lib/settings";
-import type { UrlClickAction } from "metabase/visualizations/types";
+import type {
+  QuestionChangeClickAction,
+  UrlClickAction,
+} from "metabase/visualizations/types";
+import Question from "metabase-lib/v1/Question";
 
 import { performAction } from "./action";
 
@@ -16,6 +20,7 @@ describe("performAction", () => {
     const extraProps = {
       dispatch: jest.fn(),
       onChangeCardAndRun: jest.fn(),
+      onUpdateQuestion: jest.fn(),
     };
 
     expect(performAction(action, extraProps)).toBe(true);
@@ -79,6 +84,7 @@ describe("performAction", () => {
         const extraProps = {
           dispatch: jest.fn(),
           onChangeCardAndRun: jest.fn(),
+          onUpdateQuestion: jest.fn(),
         };
 
         expect(performAction(action, extraProps)).toBe(true);
@@ -117,6 +123,7 @@ describe("performAction", () => {
         const extraProps = {
           dispatch: jest.fn(),
           onChangeCardAndRun: jest.fn(),
+          onUpdateQuestion: jest.fn(),
         };
 
         expect(performAction(action, extraProps)).toBe(true);
@@ -156,6 +163,7 @@ describe("performAction", () => {
       const extraProps = {
         dispatch: jest.fn(),
         onChangeCardAndRun: jest.fn(),
+        onUpdateQuestion: jest.fn(),
       };
 
       expect(performAction(action, extraProps)).toBe(true);
@@ -194,6 +202,7 @@ describe("performAction", () => {
       const extraProps = {
         dispatch: jest.fn(),
         onChangeCardAndRun: jest.fn(),
+        onUpdateQuestion: jest.fn(),
       };
 
       expect(performAction(action, extraProps)).toBe(true);
@@ -218,4 +227,38 @@ describe("performAction", () => {
       });
     });
   });
+
+  it("performs question change actions according to question change behavior", () => {
+    const mockQuestion = Question.create();
+
+    const defaultQuestionAction: QuestionChangeClickAction = {
+      name: "bar",
+      question: () => mockQuestion,
+      buttonType: "horizontal",
+      section: "auto",
+    };
+
+    const updateQuestionAction: QuestionChangeClickAction = {
+      ...defaultQuestionAction,
+      questionChangeBehavior: "updateQuestion",
+    };
+
+    const extraProps = {
+      dispatch: jest.fn(),
+      onChangeCardAndRun: jest.fn(),
+      onUpdateQuestion: jest.fn(),
+    };
+
+    expect(performAction(defaultQuestionAction, extraProps)).toBe(true);
+
+    expect(extraProps.onChangeCardAndRun).toHaveBeenCalledTimes(1);
+    expect(extraProps.onChangeCardAndRun).toHaveBeenCalledWith({
+      nextCard: mockQuestion.card(),
+    });
+
+    expect(performAction(updateQuestionAction, extraProps)).toBe(true);
+
+    expect(extraProps.onUpdateQuestion).toHaveBeenCalledTimes(1);
+    expect(extraProps.onUpdateQuestion).toHaveBeenCalledWith(mockQuestion);
+  });
 });

frontend/src/metabase/visualizations/types/click-actions.ts

@@ -66,8 +66,19 @@ type ReduxClickActionBase = {
 
 export type ReduxClickAction = ClickActionBase & ReduxClickActionBase;
 
+/**
+ * What should happen when a "question change" click action is performed?
+ *
+ * - `changeCardAndRun`: the card is changed and the query is run. this is the default behavior.
+ * - `updateQuestion`: the question is updated (without running the query)
+ */
+export type QuestionChangeClickActionBehavior =
+  | "changeCardAndRun"
+  | "updateQuestion";
+
 export type QuestionChangeClickActionBase = {
   question: () => Question;
+  questionChangeBehavior?: QuestionChangeClickActionBehavior;
 };
 
 export type QuestionChangeClickAction = ClickActionBase &