User friendly error message when combining series that cannot be visualized together (#33119)

* Use named import for ErrorView

* Export ErrorView and ErrorViewProps from Visualization

* Define AddSeriesModalErrorView

* Make ErrorView component configurable in Visualization by introducing errorView prop

* Use AddSeriesModalErrorView thanks to new errorView prop

* Fix export type syntax

* Remove the dot for consistency with other error messages

* Rename AddSeriesModalErrorView to ErrorView and extract it to a separate file

* Allow translating the custom error message

* Don't show custom ErrorView when there is only one series (or none)

* Add a test case for a single incomplete chart

* Inline: dashcardData

* Add a reproduction e2e test for #32231

* Add explicit assertion for the error message

* Add a test case for default error message when there is only 1 series

* Remove redundant setup functions

* Wait for series query endpoint

* Rename ErrorView to VisualizationErrorView

* Rename VisualizationErrorView to MultipleSeriesErrorView

* Improve assertions

* Use editDashboard helper

* Use data-testid for AddSeriesModal

* Refactor errorView prop into errorMessageOverride
- Do not export ErrorViewProps and Error out of Visualization

e2e/test/scenarios/dashboard-cards/reproductions/32231-add-incompatible-series.cy.spec.js

+import { editDashboard, restore, visitDashboard } from "e2e/support/helpers";
+
+import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
+
+const { PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
+
+const baseQuestion = {
+  name: "Base question",
+  query: {
+    "source-table": PRODUCTS_ID,
+    aggregation: [["count"]],
+    breakout: [["field", PRODUCTS.CATEGORY, null]],
+  },
+  visualization_settings: {
+    "graph.dimensions": ["CATEGORY"],
+    "graph.metrics": ["count"],
+  },
+  display: "bar",
+};
+
+const incompleteQuestion = {
+  name: "Incomplete question",
+  native: {
+    query: "select 1;",
+  },
+  visualization_settings: {
+    "graph.dimensions": [null],
+    "graph.metrics": ["1"],
+  },
+  display: "bar",
+};
+
+const issue32231Error = "Cannot read properties of undefined (reading 'name')";
+const multipleSeriesError = "Unable to combine these questions";
+const defaultError = "Which fields do you want to use for the X and Y axes?";
+
+describe("issue 32231", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+
+    cy.intercept("GET", "/api/card/*/series?limit=*").as("seriesQuery");
+  });
+
+  it("should show user-friendly error when combining series that cannot be visualized together (metabase#32231)", () => {
+    cy.createNativeQuestion(incompleteQuestion);
+    cy.createQuestionAndDashboard({ questionDetails: baseQuestion }).then(
+      ({ body: { id, card_id, dashboard_id } }) => {
+        cy.request("PUT", `/api/dashboard/${dashboard_id}/cards`, {
+          cards: [
+            {
+              id,
+              card_id,
+              row: 0,
+              col: 0,
+              size_x: 16,
+              size_y: 10,
+            },
+          ],
+        });
+
+        visitDashboard(dashboard_id);
+      },
+    );
+
+    editDashboard();
+    cy.findByTestId("add-series-button").click({ force: true });
+    cy.wait("@seriesQuery");
+
+    cy.findByTestId("add-series-modal").within(() => {
+      cy.get(".LineAreaBarChart").should("exist");
+      cy.findByText(issue32231Error).should("not.exist");
+      cy.findByText(multipleSeriesError).should("not.exist");
+      cy.findByText(defaultError).should("not.exist");
+
+      cy.findByText(incompleteQuestion.name).click();
+
+      cy.get(".LineAreaBarChart").should("not.exist");
+      cy.findByText(issue32231Error).should("not.exist");
+      cy.findByText(multipleSeriesError).should("exist");
+      cy.findByText(defaultError).should("not.exist");
+
+      cy.findByText(incompleteQuestion.name).click();
+
+      cy.get(".LineAreaBarChart").should("exist");
+      cy.findByText(issue32231Error).should("not.exist");
+      cy.findByText(multipleSeriesError).should("not.exist");
+      cy.findByText(defaultError).should("not.exist");
+    });
+  });
+
+  it("should show default visualization error message when the only series is incomplete", () => {
+    cy.createNativeQuestionAndDashboard({
+      questionDetails: incompleteQuestion,
+    }).then(({ body: { id, card_id, dashboard_id } }) => {
+      cy.request("PUT", `/api/dashboard/${dashboard_id}/cards`, {
+        cards: [
+          {
+            id,
+            card_id,
+            row: 0,
+            col: 0,
+            size_x: 16,
+            size_y: 10,
+          },
+        ],
+      });
+
+      visitDashboard(dashboard_id);
+    });
+
+    cy.findByTestId("dashcard").findByText(defaultError).should("exist");
+
+    cy.icon("pencil").click();
+    cy.findByTestId("add-series-button").click({ force: true });
+    cy.wait("@seriesQuery");
+
+    cy.findByTestId("add-series-modal").within(() => {
+      cy.get(".LineAreaBarChart").should("not.exist");
+      cy.findByText(issue32231Error).should("not.exist");
+      cy.findByText(multipleSeriesError).should("not.exist");
+      cy.findByText(defaultError).should("exist");
+    });
+  });
+});

frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.tsx

@@ -144,6 +144,11 @@ export class AddSeriesModal extends Component<Props, State> {
             <Visualization
               canRemoveSeries={CAN_REMOVE_SERIES}
               className="spread"
+              errorMessageOverride={
+                series.length > 1
+                  ? t`Unable to combine these questions`
+                  : undefined
+              }
               rawSeries={series}
               showTitle
               isDashboard

frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.unit.spec.tsx

@@ -32,6 +32,16 @@ const secondCard = createMockCard({
   display: "bar",
 });
 
+const incompleteCard = createMockCard({
+  id: getNextId(),
+  name: "Incomplete card",
+  display: "bar",
+  visualization_settings: {
+    "graph.dimensions": [],
+    "graph.metrics": [],
+  },
+});
+
 const dataset = createMockDataset({
   data: createMockDatasetData({
     rows: [
@@ -56,20 +66,24 @@ const dataset = createMockDataset({
 const dashcard = createMockDashboardOrderedCard({
   id: getNextId(),
   card: baseCard,
-  series: [firstCard, secondCard],
+  series: [firstCard, secondCard, incompleteCard],
 });
 
-const dashcardData = {
-  [dashcard.id]: {
-    [baseCard.id]: dataset,
-    [firstCard.id]: dataset,
-    [secondCard.id]: dataset,
-  },
-};
+const incompleteDashcard = createMockDashboardOrderedCard({
+  id: getNextId(),
+  card: baseCard,
+  series: [incompleteCard],
+});
 
 const defaultProps = {
   dashcard,
-  dashcardData,
+  dashcardData: {
+    [dashcard.id]: {
+      [baseCard.id]: dataset,
+      [firstCard.id]: dataset,
+      [secondCard.id]: dataset,
+    },
+  },
   fetchCardData: jest.fn(),
   setDashCardAttributes: jest.fn(),
   onClose: jest.fn(),
@@ -80,6 +94,21 @@ const setup = (options?: Partial<AddSeriesModalProps>) => {
 };
 
 describe("AddSeriesModal", () => {
+  it("shows chart settings error message for incomplete charts", () => {
+    setup({
+      dashcard: incompleteDashcard,
+      dashcardData: {
+        [incompleteDashcard.id]: {
+          [incompleteCard.id]: dataset,
+        },
+      },
+    });
+
+    expect(
+      screen.getByText("Which fields do you want to use for the X and Y axes?"),
+    ).toBeInTheDocument();
+  });
+
   it("shows the 'x' button in all legend items in visualization legend except for the base series", () => {
     setup();
 

frontend/src/metabase/dashboard/components/DashboardGrid.jsx

@@ -265,7 +265,11 @@ class DashboardGrid extends Component {
     // can't use PopoverWithTrigger due to strange interaction with ReactGridLayout
     const isOpen = this.state.addSeriesModalDashCard != null;
     return (
-      <Modal className="Modal AddSeriesModal" isOpen={isOpen}>
+      <Modal
+        className="Modal AddSeriesModal"
+        data-testid="add-series-modal"
+        isOpen={isOpen}
+      >
         {isOpen && (
           <AddSeriesModal
             dashcard={this.state.addSeriesModalDashCard}

frontend/src/metabase/visualizations/components/Visualization/ErrorView/ErrorView.tsx

@@ -10,7 +10,7 @@ interface ErrorViewProps {
   isSmall: boolean;
 }
 
-function ErrorView({
+export function ErrorView({
   error,
   icon = "warning",
   isDashboard,
@@ -25,6 +25,3 @@ function ErrorView({
     </Root>
   );
 }
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default ErrorView;

frontend/src/metabase/visualizations/components/Visualization/ErrorView/index.ts

-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./ErrorView";
+export { ErrorView } from "./ErrorView";

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

@@ -38,7 +38,7 @@ import { datasetContainsNoResults } from "metabase-lib/queries/utils/dataset";
 import { memoizeClass } from "metabase-lib/utils";
 
 import ChartSettingsErrorButton from "./ChartSettingsErrorButton";
-import ErrorView from "./ErrorView";
+import { ErrorView } from "./ErrorView";
 import LoadingView from "./LoadingView";
 import NoResultsView from "./NoResultsView";
 import {
@@ -49,6 +49,7 @@ import {
 } from "./Visualization.styled";
 
 const defaultProps = {
+  errorMessageOverride: undefined,
   showTitle: false,
   isDashboard: false,
   isEditing: false,
@@ -322,6 +323,7 @@ class Visualization extends PureComponent {
       actionButtons,
       className,
       dashcard,
+      errorMessageOverride,
       showTitle,
       isDashboard,
       width,
@@ -476,7 +478,7 @@ class Visualization extends PureComponent {
             <NoResultsView isSmall={small} />
           ) : error ? (
             <ErrorView
-              error={error}
+              error={errorMessageOverride ?? error}
               icon={errorIcon}
               isSmall={small}
               isDashboard={isDashboard}