From 7ade61330d991d755d9e17a1799453f7b0b59be5 Mon Sep 17 00:00:00 2001
From: Jesse Devaney <22608765+JesseSDevaney@users.noreply.github.com>
Date: Wed, 26 Jun 2024 11:12:24 -0400
Subject: [PATCH] Fix invalid API request fired on chart legend click (#44579)

* remove invalid else conditional

* fix RowChart firing invalid API request on legend item click

* fix clicking on non-breakout legend on dashboards

* add E2E test to validate and enforce the desired behavior
---
 ...alizations-charts-reproductions.cy.spec.js | 63 +++++++++++++++++++
 .../CartesianChart/use-chart-events.ts        | 10 ++-
 .../visualizations/RowChart/RowChart.tsx      |  9 ++-
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js b/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js
index b63e7304f31..d660166686e 100644
--- a/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js
@@ -1085,3 +1085,66 @@ describe("issue 33208", () => {
     cy.findByTestId("scalar-value").should("be.visible");
   });
 });
+
+describe("issue 43077", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+  });
+
+  it("should not fire an invalid API request when clicking a legend item on a cartesian chart with multiple aggregations", () => {
+    const cartesianQuestionDetails = {
+      dataset_query: {
+        type: "query",
+        query: {
+          "source-table": ORDERS_ID,
+          aggregation: [
+            ["sum", ["field", ORDERS.QUANTITY, null]],
+            ["sum", ["field", ORDERS.TOTAL, null]],
+          ],
+          breakout: [
+            ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }],
+          ],
+        },
+        database: 1,
+      },
+      display: "line",
+    };
+    const cardRequestSpy = cy.spy();
+    cy.intercept("/api/card/*", cardRequestSpy);
+
+    visitQuestionAdhoc(cartesianQuestionDetails);
+
+    cy.findAllByTestId("legend-item").first().click();
+
+    cy.wait(100).then(() => expect(cardRequestSpy).not.to.have.been.called);
+  });
+
+  it("should not fire an invalid API request when clicking a legend item on a row chart with multiple aggregations", () => {
+    const rowQuestionDetails = {
+      dataset_query: {
+        type: "query",
+        query: {
+          "source-table": ORDERS_ID,
+          aggregation: [
+            ["sum", ["field", ORDERS.QUANTITY, null]],
+            ["sum", ["field", ORDERS.TOTAL, null]],
+          ],
+          breakout: [
+            ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }],
+          ],
+        },
+        database: 1,
+      },
+      display: "row",
+    };
+    const cardRequestSpy = cy.spy();
+    cy.intercept("/api/card/*", cardRequestSpy);
+
+    visitQuestionAdhoc(rowQuestionDetails);
+
+    cy.findAllByTestId("legend-item").first().click();
+
+    cy.wait(100).then(() => expect(cardRequestSpy).not.to.have.been.called);
+  });
+});
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts
index dc445665c14..0b9f7e27a72 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/use-chart-events.ts
@@ -51,6 +51,7 @@ export const useChartEvents = (
     onDeselectTimelineEvents,
     hovered,
     metadata,
+    isDashboard,
   }: VisualizationProps,
 ) => {
   const isBrushing = useRef<boolean>();
@@ -322,16 +323,12 @@ export const useChartEvents = (
         settings,
       };
 
-      if (
-        !areMultipleCards &&
-        hasBreakout &&
-        visualizationIsClickable(clickData)
-      ) {
+      if (hasBreakout && visualizationIsClickable(clickData)) {
         onVisualizationClick({
           ...clickData,
           element: event.currentTarget,
         });
-      } else {
+      } else if (isDashboard) {
         onOpenQuestion(seriesModel.cardId);
       }
     },
@@ -342,6 +339,7 @@ export const useChartEvents = (
       visualizationIsClickable,
       onVisualizationClick,
       onOpenQuestion,
+      isDashboard,
     ],
   );
 
diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
index 4a430b99c5b..55d103093d3 100644
--- a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
@@ -102,6 +102,7 @@ const RowChartVisualization = ({
   actionButtons,
   isFullscreen,
   isQueryBuilder,
+  isDashboard,
   onRender,
   onHoverChange,
   showTitle,
@@ -216,12 +217,18 @@ const RowChartVisualization = ({
       chartColumns,
     );
 
+    const areMultipleCards = rawMultipleSeries.length > 1;
+    if (areMultipleCards) {
+      openQuestion();
+      return;
+    }
+
     if ("breakout" in chartColumns && visualizationIsClickable(clickData)) {
       onVisualizationClick({
         ...clickData,
         element: event.currentTarget,
       });
-    } else {
+    } else if (isDashboard) {
       openQuestion();
     }
   };
-- 
GitLab