Fix column scaling ("Multiply by a number") bugs (#43668)

* return empty string so non-empty labels do not get hidden by empty labels * fix function name * apply scaling in transformed dataset * add missing option to OptionsType * fix double scaling * add column scaling to waterfall transformed dataset * add column value scaling to dataset transform * fix stacked bar segment labels not responding to column scaling * fix tooltip values displayed * since scaling as added to transformedDataset, use transformedDataset for formatters * revert changes to display percentage instead of absolute value * fix stacked data labels * add loki variant for custom y-axis range with column scaling * fix stack segmented label formatters compact calculation * scale original dataset and use that instead of modifying transformedDataset * update loki snapshots * fix scatter plot dataset scaling * fix tooltip values being double scaled * add E2E test to validate behavior * add helper for not duplicating the column scaling * port multiple y-axis metrics with column scaling fixes to RowChart * add waterfall and scatterplot loki variants * fix type errors and failing unit tests * fix failing unit tests * update loki snapshots
metabase · Jun 5, 2024 · 6ac4fe9 · 6ac4fe9
1 parent 5bf50a2
commit 6ac4fe9
Show file tree

Hide file tree

Showing 56 changed files with 1,297 additions and 50 deletions.
diff --git a/...i/reference/chrome_laptop_static_viz_ComboChart_Bar_Auto_Formatting_Compact.png b/...i/reference/chrome_laptop_static_viz_ComboChart_Bar_Auto_Formatting_Compact.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ComboChart_Bar_Auto_Formatting_Full.png b/.loki/reference/chrome_laptop_static_viz_ComboChart_Bar_Auto_Formatting_Full.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ComboChart_Bar_Formatting_Full.png b/.loki/reference/chrome_laptop_static_viz_ComboChart_Bar_Formatting_Full.png
diff --git a/...ence/chrome_laptop_static_viz_ComboChart_Bar_Stacked_Total_Formatted_Values.png b/...ence/chrome_laptop_static_viz_ComboChart_Bar_Stacked_Total_Formatted_Values.png
diff --git a/...chrome_laptop_static_viz_ComboChart_Custom_Y_Axis_Range_With_Column_Scaling.png b/...chrome_laptop_static_viz_ComboChart_Custom_Y_Axis_Range_With_Column_Scaling.png
diff --git a/...ence/chrome_laptop_static_viz_ComboChart_Y_Axis_Compact_Without_Data_Labels.png b/...ence/chrome_laptop_static_viz_ComboChart_Y_Axis_Compact_Without_Data_Labels.png
diff --git a/...nce/chrome_laptop_static_viz_ScatterPlot_Auto_Y_Axis_Exclude_Zero_With_Goal.png b/...nce/chrome_laptop_static_viz_ScatterPlot_Auto_Y_Axis_Exclude_Zero_With_Goal.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Bubble_Size.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Bubble_Size.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Custom_Y_Axis_Range.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Custom_Y_Axis_Range.png
diff --git a/...hrome_laptop_static_viz_ScatterPlot_Custom_Y_Axis_Range_With_Column_Scaling.png b/...hrome_laptop_static_viz_ScatterPlot_Custom_Y_Axis_Range_With_Column_Scaling.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Default.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Default.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Goal_Line.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Goal_Line.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Histogram_X_Scale.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Histogram_X_Scale.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Log_X_Scale.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Log_X_Scale.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Log_X_Scale_At_One.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Log_X_Scale_At_One.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Multi_Dimension_Breakout.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Multi_Dimension_Breakout.png
diff --git a/...e/chrome_laptop_static_viz_ScatterPlot_Multi_Dimension_Breakout_Bubble_Size.png b/...e/chrome_laptop_static_viz_ScatterPlot_Multi_Dimension_Breakout_Bubble_Size.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Multi_Metric_Series.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Multi_Metric_Series.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Ordinal_X_Scale.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Ordinal_X_Scale.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Power_X_Scale.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Power_X_Scale.png
diff --git a/...i/reference/chrome_laptop_static_viz_ScatterPlot_Power_X_Scale_Multi_Series.png b/...i/reference/chrome_laptop_static_viz_ScatterPlot_Power_X_Scale_Multi_Series.png
diff --git a/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Timeseries_X_Scale.png b/.loki/reference/chrome_laptop_static_viz_ScatterPlot_Timeseries_X_Scale.png
diff --git a/...me_laptop_static_viz_WaterfallChart_Custom_Y_Axis_Range_With_Column_Scaling.png b/...me_laptop_static_viz_WaterfallChart_Custom_Y_Axis_Range_With_Column_Scaling.png
diff --git a/...rence/chrome_laptop_static_viz_WaterfallChart_Data_Labels_Column_Formatting.png b/...rence/chrome_laptop_static_viz_WaterfallChart_Data_Labels_Column_Formatting.png
diff --git a/.loki/reference/chrome_laptop_viz_LineChart_Embedding_Huge_Font.png b/.loki/reference/chrome_laptop_viz_LineChart_Embedding_Huge_Font.png
diff --git a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
@@ -12,6 +12,8 @@ import {
   chartPathWithFillColor,
   echartsContainer,
   getValueLabels,
+  createQuestion,
+  chartPathsWithFillColors,
 } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID, PEOPLE, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -561,4 +563,64 @@ describe("scenarios > visualizations > bar chart", () => {
         cy.get(".axis.yr").should("not.exist");
       });
   });
+
+  it("should correctly handle bar sizes and tool-tips for multiple y-axis metrics with column scaling  (#43536)", () => {
+    cy.signInAsAdmin();
+
+    const column_settings = { '["name","sum"]': { scale: 0.5 } };
+    const multiMetric = {
+      name: "Should split",
+      type: "query",
+      query: {
+        "source-table": ORDERS_ID,
+        aggregation: [
+          ["sum", ["field", ORDERS.TOTAL]],
+          ["sum", ["field", ORDERS.TOTAL]],
+        ],
+        breakout: [["field", ORDERS.CREATED_AT, { "temporal-unit": "year" }]],
+      },
+      display: "bar",
+      visualization_settings: {
+        column_settings,
+        "graph.show_values": true,
+        "graph.stackable.stack_type": "stacked",
+        series_settings: {
+          sum_2: {
+            axis: "left",
+          },
+          sum: {
+            axis: "left",
+          },
+        },
+      },
+    };
+
+    createQuestion(multiMetric, { visitQuestion: true });
+
+    const [firstMetric, secondMetric] = chartPathsWithFillColors([
+      "#88BF4D",
+      "#98D9D9",
+    ]);
+    firstMetric.then($metricOne => {
+      const { height: heightMetricOne } = $metricOne[0].getBoundingClientRect();
+      secondMetric.then($metricTwo => {
+        const { height: heightMetricTwo } =
+          $metricTwo[0].getBoundingClientRect();
+
+        // since the first metric is scaled to be half of the second metric
+        // the first bar should be half the size of the first bar
+        // within a given tolerance
+        expect(heightMetricOne - heightMetricTwo / 2).to.be.lessThan(0.1);
+      });
+    });
+
+    chartPathWithFillColor("#88BF4D").first().realHover();
+    popover().within(() => {
+      cy.contains("Sum of Total");
+      // half of the unscaled metric
+      cy.contains("21,078.43");
+      // full value of the unscale metric
+      cy.contains("42,156.87");
+    });
+  });
 });
diff --git a/frontend/src/metabase/lib/formatting/types.ts b/frontend/src/metabase/lib/formatting/types.ts
@@ -32,6 +32,7 @@ export interface OptionsType extends TimeOnlyOptions {
   removeDay?: boolean;
   removeYear?: boolean;
   rich?: boolean;
+  scale?: number;
   show_mini_bar?: boolean;
   suffix?: string;
   type?: string;

diff --git a/frontend/src/metabase/static-viz/components/ComboChart/ComboChart.stories.tsx b/frontend/src/metabase/static-viz/components/ComboChart/ComboChart.stories.tsx
@@ -101,6 +101,13 @@ LineCustomYAxisRangeEqualsExtents.args = {
   renderingContext,
 };
 
+export const CustomYAxisRangeWithColumnScaling = Template.bind({});
+CustomYAxisRangeWithColumnScaling.args = {
+  rawSeries: data.customYAxisRangeWithColumnScaling as any,
+  dashcardSettings: {},
+  renderingContext,
+};
+
 export const LineFullyNullDimension37902 = Template.bind({});
 LineFullyNullDimension37902.args = {
   rawSeries: data.lineFullyNullDimension37902 as any,