From b303750125c019c3919a2e63396b6399f4c23de0 Mon Sep 17 00:00:00 2001
From: Aleksandr Lesnenko <alxnddr@users.noreply.github.com>
Date: Thu, 17 Nov 2022 11:36:11 +0300
Subject: [PATCH] Fix invisible columns click behavior [ci nocache] (#26547)
---
.../components/RowChart/RowChart.tsx | 2 +-
.../visualizations/shared/types/data.ts | 15 ++-
.../visualizations/shared/utils/data.ts | 104 +++++++++---------
.../shared/utils/data.unit.spec.ts | 51 +++++----
.../visualizations/shared/utils/series.ts | 6 +
.../visualizations/RowChart/RowChart.tsx | 2 +-
.../visualizations/RowChart/utils/events.ts | 50 ++++++++-
package.json | 2 +-
8 files changed, 149 insertions(+), 83 deletions(-)
diff --git a/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx b/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx
index f410cb38cdd..8cb7c73b03f 100644
--- a/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx
+++ b/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx
@@ -64,7 +64,7 @@ const StaticRowChart = ({ data, settings, getColor }: StaticRowChartProps) => {
columnValueFormatter,
);
const groupedData = getGroupedDataset(
- data,
+ data.rows,
chartColumns,
columnValueFormatter,
);
diff --git a/frontend/src/metabase/visualizations/shared/types/data.ts b/frontend/src/metabase/visualizations/shared/types/data.ts
index 9bcdcb2e5c6..7c371e4b0ae 100644
--- a/frontend/src/metabase/visualizations/shared/types/data.ts
+++ b/frontend/src/metabase/visualizations/shared/types/data.ts
@@ -1,4 +1,9 @@
-import { DatasetColumn, DatasetData, RowValue } from "metabase-types/api";
+import {
+ DatasetColumn,
+ DatasetData,
+ RowValue,
+ RowValues,
+} from "metabase-types/api";
export type TwoDimensionalChartData = Pick<DatasetData, "rows" | "cols">;
@@ -17,7 +22,13 @@ export type GroupedDatum = {
dimensionValue: RowValue;
metrics: MetricDatum;
isClickable: boolean;
- breakout?: { [key: BreakoutName]: MetricDatum };
+ breakout?: {
+ [key: BreakoutName]: {
+ metrics: MetricDatum;
+ rawRows: RowValues[];
+ };
+ };
+ rawRows: RowValues[];
};
export type GroupedDataset = GroupedDatum[];
diff --git a/frontend/src/metabase/visualizations/shared/utils/data.ts b/frontend/src/metabase/visualizations/shared/utils/data.ts
index c4c42b06759..3fb4a22e0da 100644
--- a/frontend/src/metabase/visualizations/shared/utils/data.ts
+++ b/frontend/src/metabase/visualizations/shared/utils/data.ts
@@ -4,7 +4,6 @@ import { RowValue, RowValues, SeriesOrderSetting } from "metabase-types/api";
import {
ChartColumns,
ColumnDescriptor,
- getColumnDescriptors,
} from "metabase/visualizations/lib/graph/columns";
import { ColumnFormatter } from "metabase/visualizations/shared/types/format";
import {
@@ -17,7 +16,7 @@ import {
} from "metabase/visualizations/shared/types/data";
import { Series } from "metabase/visualizations/shared/components/RowChart/types";
import { formatNullable } from "metabase/lib/formatting/nullable";
-import { isMetric } from "metabase-lib/types/utils/isa";
+import { getChartMetrics } from "./series";
const getMetricValue = (value: RowValue): MetricValue => {
if (typeof value === "number") {
@@ -27,26 +26,29 @@ const getMetricValue = (value: RowValue): MetricValue => {
return null;
};
+export const sumMetric = (left: RowValue, right: RowValue) => {
+ if (typeof left === "number" && typeof right === "number") {
+ return left + right;
+ } else if (typeof left === "number") {
+ return left;
+ } else if (typeof right === "number") {
+ return right;
+ }
+
+ return null;
+};
+
const sumMetrics = (left: MetricDatum, right: MetricDatum): MetricDatum => {
const keys = new Set([...Object.keys(left), ...Object.keys(right)]);
return Array.from(keys).reduce<MetricDatum>((datum, metricKey) => {
- const leftValue = left[metricKey];
- const rightValue = right[metricKey];
-
- if (typeof leftValue === "number" || typeof rightValue === "number") {
- datum[metricKey] = (leftValue ?? 0) + (rightValue ?? 0);
- } else {
- datum[metricKey] = null;
- }
-
+ datum[metricKey] = sumMetric(left[metricKey], right[metricKey]);
return datum;
}, {});
};
-const groupDataByDimensions = (
+export const getGroupedDataset = (
rows: RowValues[],
chartColumns: ChartColumns,
- allMetrics: ColumnDescriptor[],
columnFormatter: ColumnFormatter,
): GroupedDataset => {
const { dimension } = chartColumns;
@@ -60,12 +62,16 @@ const groupDataByDimensions = (
dimensionValue,
metrics: {},
isClickable: true,
+ rawRows: [],
};
- const rowMetrics = allMetrics.reduce<MetricDatum>((datum, metric) => {
- datum[metric.column.name] = getMetricValue(row[metric.index]);
- return datum;
- }, {});
+ const rowMetrics = getChartMetrics(chartColumns).reduce<MetricDatum>(
+ (datum, metric) => {
+ datum[metric.column.name] = getMetricValue(row[metric.index]);
+ return datum;
+ },
+ {},
+ );
datum.metrics = sumMetrics(rowMetrics, datum.metrics);
@@ -75,47 +81,31 @@ const groupDataByDimensions = (
chartColumns.breakout.column,
);
+ const breakoutRawRows = datum.breakout?.[breakoutName]?.rawRows ?? [];
+ breakoutRawRows.push(row);
+
+ const breakoutMetrics = sumMetrics(
+ rowMetrics,
+ datum.breakout?.[breakoutName]?.metrics ?? {},
+ );
+
datum.breakout = {
...datum.breakout,
- [breakoutName]: sumMetrics(
- rowMetrics,
- datum.breakout?.[breakoutName] ?? {},
- ),
+ [breakoutName]: {
+ metrics: breakoutMetrics,
+ rawRows: breakoutRawRows,
+ },
};
}
+ datum.rawRows.push(row);
+
groupedData.set(dimensionValue, datum);
}
return Array.from(groupedData.values());
};
-export const getGroupedDataset = (
- data: TwoDimensionalChartData,
- chartColumns: ChartColumns,
- columnFormatter: ColumnFormatter,
-): GroupedDataset => {
- // We are grouping all metrics because they are used in chart tooltips
- const allMetricColumns = data.cols
- .filter(
- (column, index) =>
- isMetric(column) && index !== chartColumns.dimension.index,
- )
- .map(column => column.name);
-
- const allMetricDescriptors = getColumnDescriptors(
- allMetricColumns,
- data.cols,
- );
-
- return groupDataByDimensions(
- data.rows,
- chartColumns,
- allMetricDescriptors,
- columnFormatter,
- );
-};
-
export const trimData = (
dataset: GroupedDataset,
valuesCountLimit: number,
@@ -142,19 +132,27 @@ export const trimData = (
Object.keys(currentValue.breakout ?? {}).map(breakoutName => {
groupedValue.breakout ??= {};
-
- groupedValue.breakout[breakoutName] = sumMetrics(
- groupedValue.breakout[breakoutName] ?? {},
- currentValue.breakout?.[breakoutName] ?? {},
- );
+ groupedValue.breakout[breakoutName] = {
+ metrics: sumMetrics(
+ groupedValue.breakout[breakoutName]?.metrics ?? {},
+ currentValue.breakout?.[breakoutName].metrics ?? {},
+ ),
+ rawRows: [
+ ...(groupedValue.breakout[breakoutName]?.rawRows ?? []),
+ ...(currentValue.breakout?.[breakoutName].rawRows ?? []),
+ ],
+ };
});
+ groupedValue.rawRows.push(...currentValue.rawRows);
+
return groupedValue;
},
{
dimensionValue: groupedDatumDimensionValue,
metrics: {},
isClickable: false,
+ rawRows: [],
},
);
@@ -187,7 +185,7 @@ const getBreakoutSeries = (
seriesName: breakoutName,
yAccessor: (datum: GroupedDatum) => formatNullable(datum.dimensionValue),
xAccessor: (datum: GroupedDatum) =>
- datum.breakout?.[breakoutName]?.[metric.column.name] ?? null,
+ datum.breakout?.[breakoutName]?.metrics[metric.column.name] ?? null,
seriesInfo: {
metricColumn: metric.column,
dimensionColumn: dimension.column,
diff --git a/frontend/src/metabase/visualizations/shared/utils/data.unit.spec.ts b/frontend/src/metabase/visualizations/shared/utils/data.unit.spec.ts
index 547bc6cd249..77d2f3336dc 100644
--- a/frontend/src/metabase/visualizations/shared/utils/data.unit.spec.ts
+++ b/frontend/src/metabase/visualizations/shared/utils/data.unit.spec.ts
@@ -1,4 +1,3 @@
-import { DatasetData } from "metabase-types/api";
import { createMockColumn } from "metabase-types/api/mocks";
import {
BreakoutChartColumns,
@@ -20,16 +19,12 @@ const avgMetricColumn = createMockColumn({
name: "avg",
});
-const dataset: DatasetData = {
- cols: [dimensionColumn, breakoutColumn, countMetricColumn, avgMetricColumn],
- rows: [
- [2020, "Doohickey", 400, 90],
- [2020, "Gadget", 450, 100],
- [2021, "Doohickey", 500, 110],
- [2021, "Gadget", 550, 120],
- ],
- rows_truncated: 0,
-};
+const rows = [
+ [2020, "Doohickey", 400, 90],
+ [2020, "Gadget", 450, 100],
+ [2021, "Doohickey", 500, 110],
+ [2021, "Gadget", 550, 120],
+];
const breakoutChartColumns: BreakoutChartColumns = {
dimension: {
@@ -68,7 +63,7 @@ describe("data utils", () => {
describe("chart with multiple metrics", () => {
it("should group dataset by dimension values", () => {
const groupedData = getGroupedDataset(
- dataset,
+ rows,
multipleMetricsChartColumns,
columnFormatter,
);
@@ -81,6 +76,7 @@ describe("data utils", () => {
count: 850,
avg: 190,
},
+ rawRows: [rows[0], rows[1]],
},
{
dimensionValue: 2021,
@@ -89,6 +85,7 @@ describe("data utils", () => {
count: 1050,
avg: 230,
},
+ rawRows: [rows[2], rows[3]],
},
]);
});
@@ -98,7 +95,7 @@ describe("data utils", () => {
describe("chart with a breakout", () => {
it("should group dataset by dimension values and breakout", () => {
const groupedData = getGroupedDataset(
- dataset,
+ rows,
breakoutChartColumns,
columnFormatter,
);
@@ -109,16 +106,20 @@ describe("data utils", () => {
isClickable: true,
metrics: {
count: 850,
- avg: 190,
},
+ rawRows: [rows[0], rows[1]],
breakout: {
Doohickey: {
- count: 400,
- avg: 90,
+ metrics: {
+ count: 400,
+ },
+ rawRows: [rows[0]],
},
Gadget: {
- count: 450,
- avg: 100,
+ metrics: {
+ count: 450,
+ },
+ rawRows: [rows[1]],
},
},
},
@@ -127,16 +128,20 @@ describe("data utils", () => {
isClickable: true,
metrics: {
count: 1050,
- avg: 230,
},
+ rawRows: [rows[2], rows[3]],
breakout: {
Doohickey: {
- count: 500,
- avg: 110,
+ metrics: {
+ count: 500,
+ },
+ rawRows: [rows[2]],
},
Gadget: {
- count: 550,
- avg: 120,
+ metrics: {
+ count: 550,
+ },
+ rawRows: [rows[3]],
},
},
},
diff --git a/frontend/src/metabase/visualizations/shared/utils/series.ts b/frontend/src/metabase/visualizations/shared/utils/series.ts
index b4d42d2dd15..729b2d90a40 100644
--- a/frontend/src/metabase/visualizations/shared/utils/series.ts
+++ b/frontend/src/metabase/visualizations/shared/utils/series.ts
@@ -33,3 +33,9 @@ export const getLabelsMetricColumn = (chartColumns: ChartColumns) => {
? chartColumns.metric
: chartColumns.metrics[0];
};
+
+export const getChartMetrics = (chartColumns: ChartColumns) => {
+ return "breakout" in chartColumns
+ ? [chartColumns.metric]
+ : chartColumns.metrics;
+};
diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
index 50fb845e4d7..f17c71679b4 100644
--- a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
@@ -132,7 +132,7 @@ const RowChartVisualization = ({
);
const groupedData = useMemo(
- () => getGroupedDataset(data, chartColumns, formatColumnValue),
+ () => getGroupedDataset(data.rows, chartColumns, formatColumnValue),
[chartColumns, data, formatColumnValue],
);
const goal = useMemo(() => getChartGoal(settings), [settings]);
diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/utils/events.ts b/frontend/src/metabase/visualizations/visualizations/RowChart/utils/events.ts
index b0eae2f1a77..fca7451ec27 100644
--- a/frontend/src/metabase/visualizations/visualizations/RowChart/utils/events.ts
+++ b/frontend/src/metabase/visualizations/visualizations/RowChart/utils/events.ts
@@ -3,8 +3,13 @@ import {
RowValue,
VisualizationSettings,
} from "metabase-types/api";
+import { isNotNull } from "metabase/core/utils/types";
import { formatNullable } from "metabase/lib/formatting/nullable";
-import { ChartColumns } from "metabase/visualizations/lib/graph/columns";
+import {
+ ChartColumns,
+ ColumnDescriptor,
+ getColumnDescriptors,
+} from "metabase/visualizations/lib/graph/columns";
import {
BarData,
Series,
@@ -14,6 +19,8 @@ import {
MetricDatum,
SeriesInfo,
} from "metabase/visualizations/shared/types/data";
+import { sumMetric } from "metabase/visualizations/shared/utils/data";
+import { isMetric } from "metabase-lib/types/utils/isa";
const getMetricColumnData = (
columns: DatasetColumn[],
@@ -31,6 +38,36 @@ const getMetricColumnData = (
});
};
+const getColumnData = (columns: ColumnDescriptor[], datum: GroupedDatum) => {
+ return columns
+ .map(columnDescriptor => {
+ const { column, index } = columnDescriptor;
+
+ let value = null;
+
+ if (isMetric(column)) {
+ const metricSum = datum.rawRows.reduce<number | null>(
+ (acc, currentRow) => sumMetric(acc, currentRow[index]),
+ null,
+ );
+
+ value = formatNullable(metricSum);
+ } else {
+ const distinctValues = new Set(datum.rawRows.map(row => row[index]));
+ value = distinctValues.size === 1 ? datum.rawRows[0][index] : null;
+ }
+
+ return value != null
+ ? {
+ key: column.display_name,
+ value: formatNullable(value),
+ col: column,
+ }
+ : null;
+ })
+ .filter(isNotNull);
+};
+
const getColumnsData = (
chartColumns: ChartColumns,
series: Series<GroupedDatum, unknown>,
@@ -54,12 +91,21 @@ const getColumnsData = (
col: chartColumns.breakout.column,
});
- metricDatum = datum.breakout[series.seriesKey];
+ metricDatum = datum.breakout[series.seriesKey].metrics;
} else {
metricDatum = datum.metrics;
}
data.push(...getMetricColumnData(datasetColumns, metricDatum));
+
+ const otherColumnsDescriptiors = getColumnDescriptors(
+ datasetColumns
+ .filter(column => !data.some(item => item.col === column))
+ .map(column => column.name),
+ datasetColumns,
+ );
+
+ data.push(...getColumnData(otherColumnsDescriptiors, datum));
return data;
};
diff --git a/package.json b/package.json
index 7832b258645..9a06d6df975 100644
--- a/package.json
+++ b/package.json
@@ -303,7 +303,7 @@
"build-stats": "yarn && webpack --json > stats.json",
"build-shared": "yarn && webpack --config webpack.shared.config.js",
"build-static-viz": "yarn && yarn build:cljs && webpack --config webpack.static-viz.config.js",
- "build-static-viz:watch": "yarn build-static-viz --watch",
+ "build-static-viz:watch": "webpack --config webpack.static-viz.config.js --watch",
"precommit": "lint-staged",
"preinstall": "echo $npm_execpath | grep -q yarn || echo '\\033[0;33mSorry, npm is not supported. Please use Yarn (https://yarnpkg.com/).\\033[0m'",
"prettier": "prettier --write '{enterprise/,}frontend/**/*.{js,jsx,ts,tsx,css}'",
--
GitLab