port multiple y-axis metrics with column scaling fixes to RowChart

metabase · Jun 5, 2024 · 4ecef1d · 4ecef1d
1 parent e7b8cb6
commit 4ecef1d
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 27 deletions.
diff --git a/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx b/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx
@@ -76,6 +76,7 @@ const StaticRowChart = ({ data, settings, getColor }: StaticRowChartProps) => {
   const groupedData = getGroupedDataset(
     remappedColumnsData.rows,
     chartColumns,
+    settings,
     columnValueFormatter,
   );
   const labelsFormatter = getLabelsStaticFormatter(chartColumns, settings);

diff --git a/frontend/src/metabase/static-viz/lib/format.ts b/frontend/src/metabase/static-viz/lib/format.ts
@@ -10,6 +10,7 @@ import { formatCoordinate } from "metabase/lib/formatting/geography";
 import { formatNumber } from "metabase/lib/formatting/numbers";
 import { formatTime } from "metabase/lib/formatting/time";
 import type { OptionsType } from "metabase/lib/formatting/types";
+import { getFormattingOptionsWithoutScaling } from "metabase/visualizations/echarts/cartesian/model/util";
 import type { CartesianChartColumns } from "metabase/visualizations/lib/graph/columns";
 import { getStackOffset } from "metabase/visualizations/lib/settings/stacking";
 import type {
@@ -168,16 +169,15 @@ export const getLabelsStaticFormatter = (
 ): ValueFormatter => {
   const column = getLabelsMetricColumn(chartColumns).column;
   const columnSettings = settings.column_settings?.[getColumnKey(column)];
+  const options = getFormattingOptionsWithoutScaling({
+    column,
+    ...columnSettings,
+    jsx: false,
+    compact: settings["graph.label_value_formatting"] === "compact",
+  });
 
   const labelsFormatter = (value: any) =>
-    String(
-      formatStaticValue(value, {
-        column,
-        ...columnSettings,
-        jsx: false,
-        compact: settings["graph.label_value_formatting"] === "compact",
-      }),
-    );
+    String(formatStaticValue(value, options));
 
   return labelsFormatter;
 };

diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/dataset.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/dataset.ts
@@ -45,7 +45,7 @@ import type {
 import { tryGetDate } from "../utils/timeseries";
 
 import { isCategoryAxis, isNumericAxis, isTimeSeriesAxis } from "./guards";
-import { getBarSeriesDataLabelKey } from "./util";
+import { getBarSeriesDataLabelKey, getColumnScaling } from "./util";
 
 /**
  * Sums two metric column values.
@@ -636,9 +636,7 @@ export function scaleDataset(
   const transformFn = (datum: Datum) => {
     const transformedRecord = { ...datum };
     for (const seriesModel of seriesModels) {
-      const { scale: seriesScale } =
-        settings.column?.(seriesModel.column) ?? {};
-      const scale = Number.isFinite(seriesScale) ? (seriesScale as number) : 1;
+      const scale = getColumnScaling(seriesModel.column, settings);
 
       const key = seriesModel.dataKey;
       if (key in datum) {

diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/util.ts
@@ -1,4 +1,9 @@
 import type { OptionsType } from "metabase/lib/formatting/types";
+import type {
+  ComputedVisualizationSettings,
+  RemappingHydratedDatasetColumn,
+} from "metabase/visualizations/types";
+import { getColumnKey } from "metabase-lib/v1/queries/utils/get-column-key";
 
 import {
   NEGATIVE_BAR_DATA_LABEL_KEY_SUFFIX,
@@ -17,3 +22,14 @@ export function getBarSeriesDataLabelKey(dataKey: DataKey, sign: "+" | "-") {
 export function getFormattingOptionsWithoutScaling(options: OptionsType) {
   return { ...options, scale: undefined };
 }
+
+export function getColumnScaling(
+  column: RemappingHydratedDatasetColumn,
+  settings: ComputedVisualizationSettings,
+) {
+  const columnSettings =
+    settings.column?.(column) ??
+    settings.column_settings?.[getColumnKey(column)];
+  const { scale: columnScale } = columnSettings;
+  return Number.isFinite(columnScale) ? (columnScale as number) : 1;
+}
diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/waterfall/model/dataset.ts b/frontend/src/metabase/visualizations/echarts/cartesian/waterfall/model/dataset.ts
@@ -26,6 +26,7 @@ import type { ComputedVisualizationSettings } from "metabase/visualizations/type
 import type { RowValue } from "metabase-types/api";
 
 import { isNumericAxis, isTimeSeriesAxis } from "../../model/guards";
+import { getColumnScaling } from "../../model/util";
 
 export const getWaterfallDataset = (
   dataset: ChartDataset,
@@ -36,8 +37,7 @@ export const getWaterfallDataset = (
 ): ChartDataset => {
   let transformedDataset: ChartDataset = [];
 
-  const { scale: seriesScale } = settings.column?.(seriesModel.column) ?? {};
-  const scale = Number.isFinite(seriesScale) ? (seriesScale as number) : 1;
+  const scale = getColumnScaling(seriesModel.column, settings);
 
   const key = seriesModel.dataKey;
   dataset.forEach((datum, index) => {

diff --git a/frontend/src/metabase/visualizations/shared/utils/data.ts b/frontend/src/metabase/visualizations/shared/utils/data.ts
@@ -2,6 +2,7 @@ import { t } from "ttag";
 
 import { formatNullable } from "metabase/lib/formatting/nullable";
 import { sumMetric } from "metabase/visualizations/echarts/cartesian/model/dataset";
+import { getColumnScaling } from "metabase/visualizations/echarts/cartesian/model/util";
 import type {
   CartesianChartColumns,
   ColumnDescriptor,
@@ -15,6 +16,10 @@ import type {
   SeriesInfo,
 } from "metabase/visualizations/shared/types/data";
 import type { ColumnFormatter } from "metabase/visualizations/shared/types/format";
+import type {
+  ComputedVisualizationSettings,
+  RemappingHydratedDatasetColumn,
+} from "metabase/visualizations/types";
 import type {
   RowValue,
   RowValues,
@@ -24,9 +29,15 @@ import type {
 
 import { getChartMetrics } from "./series";
 
-const getMetricValue = (value: RowValue): MetricValue => {
+const getMetricValue = (
+  value: RowValue,
+  metric: RemappingHydratedDatasetColumn,
+  settings: ComputedVisualizationSettings,
+): MetricValue => {
+  const scale = getColumnScaling(metric, settings);
+
   if (typeof value === "number") {
-    return value;
+    return scale * value;
   }
 
   return null;
@@ -43,6 +54,7 @@ const sumMetrics = (left: MetricDatum, right: MetricDatum): MetricDatum => {
 export const getGroupedDataset = (
   rows: RowValues[],
   chartColumns: CartesianChartColumns,
+  settings: ComputedVisualizationSettings,
   columnFormatter: ColumnFormatter,
 ): GroupedDataset => {
   const { dimension } = chartColumns;
@@ -61,7 +73,11 @@ export const getGroupedDataset = (
 
     const rowMetrics = getChartMetrics(chartColumns).reduce<MetricDatum>(
       (datum, metric) => {
-        datum[metric.column.name] = getMetricValue(row[metric.index]);
+        datum[metric.column.name] = getMetricValue(
+          row[metric.index],
+          metric.column,
+          settings,
+        );
         return datum;
       },
       {},

diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
@@ -130,8 +130,9 @@ const RowChartVisualization = ({
   );
 
   const groupedData = useMemo(
-    () => getGroupedDataset(data.rows, chartColumns, formatColumnValue),
-    [chartColumns, data, formatColumnValue],
+    () =>
+      getGroupedDataset(data.rows, chartColumns, settings, formatColumnValue),
+    [chartColumns, data, settings, formatColumnValue],
   );
   const goal = useMemo(() => getChartGoal(settings), [settings]);
   const stackOffset = getStackOffset(settings);
@@ -190,6 +191,9 @@ const RowChartVisualization = ({
 
     onHoverChange?.({
       ...hoverData,
+      // since we already scaled the dataset, we do not want the tool-tip
+      // formatter to apply scaling a second time
+      isAlreadyScaled: true,
       event: event.nativeEvent,
       element: event.currentTarget,
     });

diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/utils/format.ts b/frontend/src/metabase/visualizations/visualizations/RowChart/utils/format.ts
@@ -3,6 +3,7 @@ import type { NumberLike, StringLike } from "@visx/scale";
 import { NULL_DISPLAY_VALUE } from "metabase/lib/constants";
 import { formatValue } from "metabase/lib/formatting";
 import { isEmpty } from "metabase/lib/validate";
+import { getFormattingOptionsWithoutScaling } from "metabase/visualizations/echarts/cartesian/model/util";
 import type { CartesianChartColumns } from "metabase/visualizations/lib/graph/columns";
 import { getStackOffset } from "metabase/visualizations/lib/settings/stacking";
 import type {
@@ -70,15 +71,13 @@ export const getLabelsFormatter = (
   settings: VisualizationSettings,
 ): ValueFormatter => {
   const column = getLabelsMetricColumn(chartColumns).column;
+  const options = getFormattingOptionsWithoutScaling({
+    ...settings.column(column),
+    jsx: false,
+    compact: settings["graph.label_value_formatting"] === "compact",
+  });
 
-  const labelsFormatter = (value: any) =>
-    String(
-      formatValue(value, {
-        ...settings.column(column),
-        jsx: false,
-        compact: settings["graph.label_value_formatting"] === "compact",
-      }),
-    );
+  const labelsFormatter = (value: any) => String(formatValue(value, options));
 
   return labelsFormatter;
 };