diff --git a/frontend/src/metabase-types/api/card.ts b/frontend/src/metabase-types/api/card.ts index 14a749e28d0176176c02f23cffe6eeec5f0997d7..b19af9e68230e45da421fe42af5d41f500e65571 100644 --- a/frontend/src/metabase-types/api/card.ts +++ b/frontend/src/metabase-types/api/card.ts @@ -70,6 +70,9 @@ export type VisualizationSettings = { "graph.series_order"?: SeriesOrderSetting[]; + // Funnel settings + "funnel.rows"?: SeriesOrderSetting[]; + [key: string]: any; }; diff --git a/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx b/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx index 57dc8c889d33ea7cb0a49bb1c05d5faa10c2b3b2..617bdc41d70bd8a694345d3361b4cb82c70fd2bb 100644 --- a/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx +++ b/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx @@ -12,6 +12,7 @@ import { calculateFunnelSteps, calculateStepOpacity, getFormattedStep, + reorderData, } from "metabase/static-viz/components/FunnelChart/utils/funnel"; import { calculateMargin } from "./utils/margin"; @@ -43,8 +44,10 @@ type FunnelProps = { const Funnel = ({ data, settings }: FunnelProps) => { const palette = { ...layout.colors, ...settings.colors }; + const reorderedData = reorderData(data, settings); + const margin = calculateMargin( - data[0], + reorderedData[0], layout.stepFontSize, layout.percentFontSize, layout.measureFontSize, @@ -59,7 +62,7 @@ const Funnel = ({ data, settings }: FunnelProps) => { const stepWidth = (layout.width - margin.left) / (data.length - 1); const maxStepTextWidth = stepWidth - layout.stepTextOffset * 2; - const steps = calculateFunnelSteps(data, stepWidth, funnelHeight); + const steps = calculateFunnelSteps(reorderedData, stepWidth, funnelHeight); const firstMeasureTop = margin.top + steps[0].top + steps[0].height / 2; const stepLabelTop = firstMeasureTop + measureTextHeight(layout.nameFontSize); diff --git a/frontend/src/metabase/static-viz/components/FunnelChart/types.ts b/frontend/src/metabase/static-viz/components/FunnelChart/types.ts index 5c6e104acee2fa73826c3f5bbc8a4a5dbdf55666..a9d93dab7005b84659120e2bb2f0f235dad1ed56 100644 --- a/frontend/src/metabase/static-viz/components/FunnelChart/types.ts +++ b/frontend/src/metabase/static-viz/components/FunnelChart/types.ts @@ -1,3 +1,4 @@ +import { VisualizationSettings } from "metabase-types/api"; import { NumberFormatOptions } from "metabase/static-viz/lib/numbers"; export type Step = string | number; @@ -18,6 +19,7 @@ export type FunnelSettings = { brand: string; border: string; }; + visualization_settings: VisualizationSettings; }; export type FunnelStep = { diff --git a/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.ts b/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.ts index 5aebe7a0c9a8d1dcf9a0ac3eae10f2d6f79f2505..b9d01a0be171464a1aa6fdf355d6e6ab63b822dc 100644 --- a/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.ts +++ b/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.ts @@ -1,4 +1,5 @@ import { PolygonProps } from "@visx/shape/lib/shapes/Polygon"; +import { isNotNull } from "metabase/core/utils/types"; import { formatNumber, formatPercent } from "metabase/static-viz/lib/numbers"; import { truncateText } from "metabase/static-viz/lib/text"; import { FunnelDatum, FunnelSettings, FunnelStep } from "../types"; @@ -86,3 +87,24 @@ export const getFormattedStep = ( stepName, }; }; + +export const reorderData = ( + data: FunnelDatum[], + settings: FunnelSettings, +): FunnelDatum[] => { + const funnelOrder = settings.visualization_settings["funnel.rows"]; + if (funnelOrder == null) { + return data; + } + + const keys = data.map(datum => String(datum[0])); + + return funnelOrder + .map(orderedItem => { + if (orderedItem.enabled) { + const dataIndex = keys.findIndex(key => key === orderedItem.key); + return data[dataIndex]; + } + }) + .filter(isNotNull); +}; diff --git a/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.unit.spec.ts b/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.unit.spec.ts index c73c55cd5ec69fb0fe1736872ffee49f1f5b71c4..717b335f652c1d60a2ad13a12545314430b23bcf 100644 --- a/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.unit.spec.ts +++ b/frontend/src/metabase/static-viz/components/FunnelChart/utils/funnel.unit.spec.ts @@ -1,4 +1,10 @@ -import { calculateFunnelPolygonPoints, calculateFunnelSteps } from "./funnel"; +import { merge } from "icepick"; +import { FunnelDatum, FunnelSettings } from "../types"; +import { + calculateFunnelPolygonPoints, + calculateFunnelSteps, + reorderData, +} from "./funnel"; describe("calculateFunnelSteps", () => { it("calculates funnel steps from data", () => { @@ -69,3 +75,71 @@ describe("calculateFunnelPolygonPoints", () => { ]); }); }); + +describe("reorderData", () => { + const data: FunnelDatum[] = [ + ["funnel step 1", 100], + ["funnel step 2", 50], + ["funnel step 3", 0], + ]; + + const settings: FunnelSettings = { + colors: { + border: "#ffffff", + brand: "#ffffff", + textMedium: "#ffffff", + }, + step: { + name: "Step", + format: {}, + }, + measure: { + format: {}, + }, + visualization_settings: {}, + }; + + it("should not reorder data when `funnel.rows` isn't set", () => { + const reorderedData = reorderData(data, settings); + expect(reorderedData).toEqual(data); + }); + + it("reorders data given `funnel.rows` is set", () => { + const reorderedData = reorderData( + data, + merge(settings, { + visualization_settings: { + "funnel.rows": [ + { enabled: true, key: "funnel step 1", name: "funnel step 1" }, + { enabled: true, key: "funnel step 3", name: "funnel step 3" }, + { enabled: true, key: "funnel step 2", name: "funnel step 2" }, + ], + }, + }), + ); + expect(reorderedData).toEqual([ + ["funnel step 1", 100], + ["funnel step 3", 0], + ["funnel step 2", 50], + ]); + }); + + it("reorders data for only enabled row in `funnel.rows`", () => { + const reorderedData = reorderData( + data, + merge(settings, { + visualization_settings: { + "funnel.rows": [ + { enabled: true, key: "funnel step 1", name: "funnel step 1" }, + { enabled: true, key: "funnel step 3", name: "funnel step 3" }, + { enabled: false, key: "funnel step 2", name: "funnel step 2" }, + ], + }, + }), + ); + expect(reorderedData).toEqual([ + ["funnel step 1", 100], + ["funnel step 3", 0], + ]); + }); +}); diff --git a/frontend/src/metabase/static-viz/components/LineAreaBarChart/LineAreaBarChart.tsx b/frontend/src/metabase/static-viz/components/LineAreaBarChart/LineAreaBarChart.tsx index 99d997a0d274640b25eebce11b79788985533bd2..ac22b0a4727526dcbf1d7c9ccceaade34f103484 100644 --- a/frontend/src/metabase/static-viz/components/LineAreaBarChart/LineAreaBarChart.tsx +++ b/frontend/src/metabase/static-viz/components/LineAreaBarChart/LineAreaBarChart.tsx @@ -1,4 +1,5 @@ import React from "react"; +import _ from "underscore"; import { ColorGetter } from "metabase/static-viz/lib/colors"; import { XYChart } from "../XYChart"; import { CardSeries, ChartSettings, ChartStyle } from "../XYChart/types"; @@ -12,6 +13,7 @@ import { getSeriesWithColors, getSeriesWithLegends, removeNoneSeriesFields, + reorderSeries, } from "./utils/series"; interface LineAreaBarChartProps { @@ -56,13 +58,13 @@ const LineAreaBarChart = ({ goalColor: getColor("text-medium"), }; - const seriesWithColors = getSeriesWithColors( - multipleSeries, - settings, - instanceColors, - ); - const seriesWithLegends = getSeriesWithLegends(seriesWithColors, settings); - const series = removeNoneSeriesFields(seriesWithLegends); + const series = pipe( + _.partial(getSeriesWithColors, settings, instanceColors), + _.partial(getSeriesWithLegends, settings), + _.partial(reorderSeries, settings), + _.flatten, + removeNoneSeriesFields, + )(multipleSeries); const minTickSize = chartStyle.axes.ticks.fontSize * 1.5; const xValuesCount = getXValuesCount(series); @@ -85,4 +87,8 @@ const LineAreaBarChart = ({ ); }; +function pipe(...functions: ((arg: any) => any)[]) { + return _.compose(...functions.reverse()); +} + export default LineAreaBarChart; diff --git a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.tsx b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.tsx index 1fd5bdbe9ec7479655858ace38c2302e44599399..9c77b8bc3d124f1e53d66b4c15cf2d36672d4422 100644 --- a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.tsx +++ b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.tsx @@ -5,6 +5,7 @@ import { getColorsForValues } from "metabase/lib/colors/charts"; import { formatStaticValue } from "metabase/static-viz/lib/format"; import { ColorPalette } from "metabase/lib/colors/types"; import { + CardSeries, ChartSettings, Series, SeriesWithOneOrLessDimensions, @@ -12,15 +13,15 @@ import { } from "../../XYChart/types"; export function getSeriesWithColors( - multipleSeries: (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][], settings: ChartSettings, palette: ColorPalette, -): (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][] { - const isMultipleSeries = multipleSeries.length > 1; - const keys = getSeriesKeys(multipleSeries, isMultipleSeries); + multipleCardSeries: CardSeries[], +): CardSeries[] { + const keys = getSeriesKeys(multipleCardSeries); - const seriesColors = settings.series_settings - ? _.mapObject(settings.series_settings, value => { + const seriesSettings = settings.visualization_settings.series_settings; + const seriesColors = seriesSettings + ? _.mapObject(seriesSettings, value => { return value.color; }) : undefined; @@ -31,7 +32,7 @@ export function getSeriesWithColors( ); let index = -1; - return multipleSeries.map(questionSeries => + return multipleCardSeries.map(questionSeries => questionSeries.map(series => { index++; @@ -43,21 +44,22 @@ export function getSeriesWithColors( } export function getSeriesWithLegends( - multipleSeries: (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][], settings: ChartSettings, -): (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][] { - const keys = getSeriesKeys(multipleSeries, multipleSeries.length > 1); - const isMultipleSeries = multipleSeries.length > 1; - - const seriesTitles = settings.series_settings - ? _.mapObject(settings.series_settings, value => { + multipleCardSeries: CardSeries[], +): CardSeries[] { + const keys = getSeriesKeys(multipleCardSeries); + const isMultipleSeries = multipleCardSeries.length > 1; + + const seriesSettings = settings.visualization_settings.series_settings; + const seriesTitles = seriesSettings + ? _.mapObject(seriesSettings, value => { return value.title; }) : undefined; let index = -1; - const legends = multipleSeries - .flatMap((questionSeries, seriesIndex) => { + const legends = multipleCardSeries + .flatMap(questionSeries => { return questionSeries.map(series => { index++; @@ -74,10 +76,6 @@ export function getSeriesWithLegends( if (!hasTwoDimensions(series)) { // One or zero dimensions - if (seriesIndex === 0 && series.name) { - return series.name; - } - const hasOneMetric = questionSeries.length === 1; if (hasOneMetric) { return series.cardName; @@ -108,7 +106,7 @@ export function getSeriesWithLegends( .filter(isNotNull); index = -1; - return multipleSeries.map(questionSeries => + return multipleCardSeries.map(questionSeries => questionSeries.map(series => { index++; @@ -119,10 +117,35 @@ export function getSeriesWithLegends( ); } -function getSeriesKeys( - multipleSeries: (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][], - isMultipleSeries: boolean, +export function reorderSeries( + settings: ChartSettings, + multipleCardSeries: CardSeries[], ) { + const seriesOrder = settings.visualization_settings["graph.series_order"]; + // We don't sort series when there's is multiple questions on a dashcard + if (multipleCardSeries.length > 1 || seriesOrder == null) { + return multipleCardSeries; + } + + const keys = getSeriesKeys(multipleCardSeries); + + // visualization settings only applies to a dashcard's first question's series. + const firstCardSeries = multipleCardSeries[0]; + return [ + seriesOrder + ?.map(orderedItem => { + if (orderedItem.enabled) { + const seriesIndex = keys.findIndex(key => key === orderedItem.key); + return firstCardSeries[seriesIndex]; + } + }) + .filter(isNotNull), + ]; +} + +function getSeriesKeys(multipleSeries: CardSeries[]) { + const hasMultipleCards = multipleSeries.length > 1; + return multipleSeries .flatMap((questionSeries, seriesIndex) => { return questionSeries.map(series => { @@ -138,7 +161,7 @@ function getSeriesKeys( } const hasOneMetric = questionSeries.length === 1; - if (!isMultipleSeries || hasOneMetric) { + if (!hasMultipleCards || hasOneMetric) { return series.cardName; } @@ -150,7 +173,7 @@ function getSeriesKeys( column: series.column, }); - if (!isMultipleSeries) { + if (!hasMultipleCards) { return columnKey; } @@ -168,14 +191,10 @@ function hasTwoDimensions( return "breakoutValue" in series; } -export function removeNoneSeriesFields( - series: (SeriesWithOneOrLessDimensions | SeriesWithTwoDimensions)[][], -): Series[] { - return series - .flat() - .map( - series => _.omit(series, "cardName", "column", "breakoutValue") as Series, - ); +export function removeNoneSeriesFields(series: CardSeries): Series[] { + return series.map( + series => _.omit(series, "cardName", "column", "breakoutValue") as Series, + ); } function removeEmptyValues( diff --git a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.unit.spec.ts b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.unit.spec.ts index 4e3321e0a393d95631c881106f2914f1597bc6d6..094a2d3afdbd35eea8c7fbbde111d50ebe09d9cd 100644 --- a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.unit.spec.ts +++ b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/series.unit.spec.ts @@ -1,11 +1,15 @@ -import { merge, setIn } from "icepick"; +import { merge } from "icepick"; import { colors } from "metabase/lib/colors"; import type { ChartSettings, SeriesWithOneOrLessDimensions, SeriesWithTwoDimensions, } from "../../XYChart/types"; -import { getSeriesWithColors, getSeriesWithLegends } from "./series"; +import { + getSeriesWithColors, + getSeriesWithLegends, + reorderSeries, +} from "./series"; const settings: ChartSettings = { x: { @@ -18,20 +22,20 @@ const settings: ChartSettings = { left: "Count", bottom: "Date", }, + visualization_settings: {}, }; describe("getSeriesWithColors", () => { it("should return an empty series given an empty series", () => { - const seriesWithColors = getSeriesWithColors([], settings, getPalette({})); + const seriesWithColors = getSeriesWithColors(settings, getPalette({}), []); expect(seriesWithColors).toEqual([]); }); - describe("Series without ones or less dimensions", () => { - const multipleSeries: SeriesWithOneOrLessDimensions[][] = [ + describe("Series without one or less dimensions", () => { + const singleCardSeries: SeriesWithOneOrLessDimensions[][] = [ [ { - name: "Count", cardName: "Bar chart", yAxisPosition: "left", type: "bar", @@ -50,10 +54,9 @@ describe("getSeriesWithColors", () => { ], ]; - const multipleSeriesDashcard: SeriesWithOneOrLessDimensions[][] = [ + const multipleCardSeries: SeriesWithOneOrLessDimensions[][] = [ [ { - name: "Count", cardName: "Bar chart", yAxisPosition: "left", type: "bar", @@ -72,7 +75,6 @@ describe("getSeriesWithColors", () => { ], [ { - name: "Count", cardName: "Area chart", yAxisPosition: "left", type: "area", @@ -93,16 +95,15 @@ describe("getSeriesWithColors", () => { it("should assign colors given series", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, settings, getPalette({}), + singleCardSeries, ); const expectedSeries = [ [ { color: "#509EE3", // brand color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -117,16 +118,15 @@ describe("getSeriesWithColors", () => { it("should assign colors from whitelabel colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, settings, getPalette({ brand: "#123456", summarize: "#ffffff" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#123456", // whitelabel color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -139,24 +139,25 @@ describe("getSeriesWithColors", () => { expect(seriesWithColors).toEqual(expectedSeries); }); - it("it should assign colors from column colors", () => { + it("should assign colors from column colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { - series_settings: { - count: { - color: "#987654", + visualization_settings: { + series_settings: { + count: { + color: "#987654", + }, }, }, }), getPalette({ brand: "#123456", summarize: "#ffffff" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#987654", // column color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -169,18 +170,17 @@ describe("getSeriesWithColors", () => { expect(seriesWithColors).toEqual(expectedSeries); }); - it("it should assign colors on multiple series dashcard", () => { + it("should assign colors on multiple series dashcard", () => { const seriesWithColors = getSeriesWithColors( - multipleSeriesDashcard, settings, getPalette({}), + multipleCardSeries, ); const expectedSeries = [ [ { color: "#509EE3", // brand color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -191,7 +191,6 @@ describe("getSeriesWithColors", () => { [ { color: "#EF8C8C", - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -206,10 +205,9 @@ describe("getSeriesWithColors", () => { }); describe("Series with preferred colors", () => { - const multipleSeries: SeriesWithOneOrLessDimensions[][] = [ + const singleCardSeries: SeriesWithOneOrLessDimensions[][] = [ [ { - name: "Sum of Total", cardName: "Bar chart", yAxisPosition: "left", type: "bar", @@ -225,16 +223,15 @@ describe("getSeriesWithColors", () => { it("should assign colors from preferred color", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" } }), getPalette({ brand: "#123456", summarize: "#ffffff" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#88BF4D", // accent1 color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -249,18 +246,17 @@ describe("getSeriesWithColors", () => { it("should assign colors from whitelabel colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" }, }), getPalette({ accent1: "#123456", summarize: "#ffffff" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#123456", // whitelabel color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -275,23 +271,24 @@ describe("getSeriesWithColors", () => { it("should assign colors from column colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" }, - series_settings: { - sum: { - color: "#987654", + visualization_settings: { + series_settings: { + sum: { + color: "#987654", + }, }, }, }), getPalette({ brand: "#123456", summarize: "#ffffff" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#987654", // column color - name: expect.anything(), cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -306,10 +303,9 @@ describe("getSeriesWithColors", () => { }); describe("Series with 2 dimension", () => { - const multipleSeries: SeriesWithTwoDimensions[][] = [ + const singleCardSeries: SeriesWithTwoDimensions[][] = [ [ { - name: null, cardName: "Area chart", type: "area", data: [ @@ -329,7 +325,6 @@ describe("getSeriesWithColors", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Area chart", type: "area", data: [ @@ -351,10 +346,9 @@ describe("getSeriesWithColors", () => { ], ]; - const multipleSeriesDashcard: SeriesWithTwoDimensions[][] = [ + const multipleCardSeries: SeriesWithTwoDimensions[][] = [ [ { - name: null, cardName: "Area chart", type: "area", data: [ @@ -374,7 +368,6 @@ describe("getSeriesWithColors", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Area chart", type: "area", data: [ @@ -397,7 +390,6 @@ describe("getSeriesWithColors", () => { [ { - name: null, cardName: "Bar chart", type: "bar", data: [ @@ -417,7 +409,6 @@ describe("getSeriesWithColors", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Bar chart", type: "bar", data: [ @@ -441,18 +432,17 @@ describe("getSeriesWithColors", () => { it("should assign colors given series", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" }, }), getPalette({}), + singleCardSeries, ); const expectedSeries = [ [ { color: "#EF8C8C", // accent3 color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -462,7 +452,6 @@ describe("getSeriesWithColors", () => { }, { color: "#F9D45C", // accent4 color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -478,18 +467,17 @@ describe("getSeriesWithColors", () => { it("should assign colors from whitelabel colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" }, }), getPalette({ accent3: "#123456" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#123456", // whitelabel color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -499,7 +487,6 @@ describe("getSeriesWithColors", () => { }, { color: "#F9D45C", // accent4 color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -515,23 +502,24 @@ describe("getSeriesWithColors", () => { it("should assign colors from column colors", () => { const seriesWithColors = getSeriesWithColors( - multipleSeries, merge(settings, { x: { type: "timeseries" }, - series_settings: { - 2017: { - color: "#987654", + visualization_settings: { + series_settings: { + 2017: { + color: "#987654", + }, }, }, }), getPalette({ accent3: "#123456" }), + singleCardSeries, ); const expectedSeries = [ [ { color: "#123456", // whitelabel color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -541,7 +529,6 @@ describe("getSeriesWithColors", () => { }, { color: "#987654", // column color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -555,18 +542,17 @@ describe("getSeriesWithColors", () => { expect(seriesWithColors).toEqual(expectedSeries); }); - it("it should assign colors on multiple series dashcard", () => { + it("should assign colors on multiple series dashcard", () => { const seriesWithColors = getSeriesWithColors( - multipleSeriesDashcard, settings, getPalette({}), + multipleCardSeries, ); const expectedSeries = [ [ { color: "#F9D45C", // brand color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -576,7 +562,6 @@ describe("getSeriesWithColors", () => { }, { color: "#F2A86F", // column color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -588,7 +573,6 @@ describe("getSeriesWithColors", () => { [ { color: "#98D9D9", - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -598,7 +582,6 @@ describe("getSeriesWithColors", () => { }, { color: "#7172AD", // column color - name: null, cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -623,16 +606,15 @@ function getPalette(instanceColors: Record<string, string>) { describe("getSeriesWithLegends", () => { it("should return an empty series given an empty series", () => { - const seriesWithLegends = getSeriesWithLegends([], settings); + const seriesWithLegends = getSeriesWithLegends(settings, []); expect(seriesWithLegends).toEqual([]); }); describe("Series without ones or less dimensions", () => { - const multipleSeries: SeriesWithOneOrLessDimensions[][] = [ + const singleCardSeries: SeriesWithOneOrLessDimensions[][] = [ [ { - name: "Count", cardName: "Bar chart", yAxisPosition: "left", type: "bar", @@ -651,10 +633,9 @@ describe("getSeriesWithLegends", () => { ], ]; - const multipleSeriesDashcard: SeriesWithOneOrLessDimensions[][] = [ + const multipleCardSeries: SeriesWithOneOrLessDimensions[][] = [ [ { - name: "Count", cardName: "Bar chart", yAxisPosition: "left", type: "bar", @@ -673,7 +654,6 @@ describe("getSeriesWithLegends", () => { ], [ { - name: "Count", cardName: "Area chart", yAxisPosition: "left", type: "area", @@ -693,12 +673,15 @@ describe("getSeriesWithLegends", () => { ]; it("should assign legends given series", () => { - const seriesWithLegends = getSeriesWithLegends(multipleSeries, settings); + const seriesWithLegends = getSeriesWithLegends( + settings, + singleCardSeries, + ); const expectedSeries = [ [ { - name: "Count", + name: "Bar chart", cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -711,12 +694,19 @@ describe("getSeriesWithLegends", () => { expect(seriesWithLegends).toEqual(expectedSeries); }); - it("it should assign legends from column custom name", () => { + it("should assign legends from column custom name", () => { const seriesWithLegends = getSeriesWithLegends( - // This might not be apparent, but series' `name` would be set to - // custom metric name for series with one or less dimensions. - setIn(multipleSeries, [0, 0, "name"], "Custom count"), - settings, + merge(settings, { + x: { type: "timeseries" }, + visualization_settings: { + series_settings: { + count: { + title: "Custom count", + }, + }, + }, + }), + singleCardSeries, ); const expectedSeries = [ @@ -735,16 +725,16 @@ describe("getSeriesWithLegends", () => { expect(seriesWithLegends).toEqual(expectedSeries); }); - it("it should assign legends on multiple series dashcard", () => { + it("should assign legends on multiple series dashcard", () => { const seriesWithLegends = getSeriesWithLegends( - multipleSeriesDashcard, settings, + multipleCardSeries, ); const expectedSeries = [ [ { - name: "Count", + name: "Bar chart", cardName: expect.anything(), yAxisPosition: expect.anything(), type: expect.anything(), @@ -769,10 +759,9 @@ describe("getSeriesWithLegends", () => { }); describe("Series with 2 dimension", () => { - const multipleSeries: SeriesWithTwoDimensions[][] = [ + const singleCardSeries: SeriesWithTwoDimensions[][] = [ [ { - name: null, cardName: "Area chart", type: "area", data: [ @@ -792,7 +781,6 @@ describe("getSeriesWithLegends", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Area chart", type: "area", data: [ @@ -814,10 +802,9 @@ describe("getSeriesWithLegends", () => { ], ]; - const multipleSeriesDashcard: SeriesWithTwoDimensions[][] = [ + const multipleCardSeries: SeriesWithTwoDimensions[][] = [ [ { - name: null, cardName: "Area chart", type: "area", data: [ @@ -837,7 +824,6 @@ describe("getSeriesWithLegends", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Area chart", type: "area", data: [ @@ -860,7 +846,6 @@ describe("getSeriesWithLegends", () => { [ { - name: null, cardName: "Bar chart", type: "bar", data: [ @@ -880,7 +865,6 @@ describe("getSeriesWithLegends", () => { breakoutValue: "2016-01-01T00:00:00Z", }, { - name: null, cardName: "Bar chart", type: "bar", data: [ @@ -904,10 +888,10 @@ describe("getSeriesWithLegends", () => { it("should assign legends given series", () => { const seriesWithLegends = getSeriesWithLegends( - multipleSeries, merge(settings, { x: { type: "timeseries" }, }), + singleCardSeries, ); const expectedSeries = [ @@ -938,15 +922,17 @@ describe("getSeriesWithLegends", () => { it("should assign legends from column custom name", () => { const seriesWithLegends = getSeriesWithLegends( - multipleSeries, merge(settings, { x: { type: "timeseries" }, - series_settings: { - 2017: { - title: "custom 2017", + visualization_settings: { + series_settings: { + 2017: { + title: "custom 2017", + }, }, }, }), + singleCardSeries, ); const expectedSeries = [ @@ -975,10 +961,10 @@ describe("getSeriesWithLegends", () => { expect(seriesWithLegends).toEqual(expectedSeries); }); - it("it should assign legends on multiple series dashcard", () => { + it("should assign legends on multiple series dashcard", () => { const seriesWithLegends = getSeriesWithLegends( - multipleSeriesDashcard, settings, + multipleCardSeries, ); const expectedSeries = [ @@ -1028,3 +1014,228 @@ describe("getSeriesWithLegends", () => { }); }); }); + +describe("reorderSeries", () => { + it("should return an empty series given an empty series", () => { + const reorderedSeries = reorderSeries(settings, []); + + expect(reorderedSeries).toEqual([]); + }); + + describe("Series with 2 dimension", () => { + const singleCardSeries: SeriesWithTwoDimensions[][] = [ + [ + { + cardName: "Area chart", + type: "area", + data: [ + ["Doohickey", 177], + ["Gadget", 199], + ["Gizmo", 158], + ["Widget", 210], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2016-01-01T00:00:00Z", + }, + { + cardName: "Area chart", + type: "area", + data: [ + ["Doohickey", 1206], + ["Gadget", 1505], + ["Gizmo", 1592], + ["Widget", 1531], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2017-01-01T00:00:00Z", + }, + ], + ]; + + const multipleCardSeries: SeriesWithTwoDimensions[][] = [ + [ + { + cardName: "Area chart", + type: "area", + data: [ + ["Doohickey", 177], + ["Gadget", 199], + ["Gizmo", 158], + ["Widget", 210], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2016-01-01T00:00:00Z", + }, + { + cardName: "Area chart", + type: "area", + data: [ + ["Doohickey", 1206], + ["Gadget", 1505], + ["Gizmo", 1592], + ["Widget", 1531], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2017-01-01T00:00:00Z", + }, + ], + [ + { + cardName: "Bar chart", + type: "bar", + data: [ + ["Doohickey", 177], + ["Gadget", 199], + ["Gizmo", 158], + ["Widget", 210], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2016-01-01T00:00:00Z", + }, + { + cardName: "Bar chart", + type: "bar", + data: [ + ["Doohickey", 1206], + ["Gadget", 1505], + ["Gizmo", 1592], + ["Widget", 1531], + ], + yAxisPosition: "left", + column: { + semantic_type: "type/CreationTimestamp", + unit: "year", + name: "CREATED_AT", + source: "breakout", + display_name: "Created At", + }, + breakoutValue: "2017-01-01T00:00:00Z", + }, + ], + ]; + + it("should return the same series given no `graph.series_order` set", () => { + const reorderedSeries = reorderSeries(settings, singleCardSeries); + + expect(reorderedSeries).toEqual(singleCardSeries); + }); + + it("should sort the series following `graph.series_order`", () => { + const reorderedSeries = reorderSeries( + merge(settings, { + visualization_settings: { + "graph.series_order": [ + { + enabled: true, + key: "2017", + name: "2017", + }, + { + enabled: true, + key: "2016", + name: "2016", + }, + ], + }, + }), + singleCardSeries, + ); + + const expectedSeries = [ + [ + expect.objectContaining({ breakoutValue: "2017-01-01T00:00:00Z" }), + expect.objectContaining({ breakoutValue: "2016-01-01T00:00:00Z" }), + ], + ]; + + expect(reorderedSeries).toEqual(expectedSeries); + }); + + it("should sort the series following `graph.series_order` on only enabled series", () => { + const reorderedSeries = reorderSeries( + merge(settings, { + visualization_settings: { + "graph.series_order": [ + { + enabled: true, + key: "2017", + name: "2017", + }, + { + enabled: false, + key: "2016", + name: "2016", + }, + ], + }, + }), + singleCardSeries, + ); + + const expectedSeries = [ + [expect.objectContaining({ breakoutValue: "2017-01-01T00:00:00Z" })], + ]; + + expect(reorderedSeries).toEqual(expectedSeries); + }); + + it("should not reorder when there are multiple cards", () => { + const reorderedSeries = reorderSeries( + merge(settings, { + visualization_settings: { + "graph.series_order": [ + { + enabled: true, + key: "2017", + name: "2017", + }, + { + enabled: false, + key: "2016", + name: "2016", + }, + ], + }, + }), + multipleCardSeries, + ); + + expect(reorderedSeries).toEqual(multipleCardSeries); + }); + }); +}); diff --git a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/settings.unit.spec.ts b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/settings.unit.spec.ts index a71c3cc269fd97132250e4eb99b0f9b3846142a4..59ae97ec1a4bfdccea34bf07bb0e3c5b2ffbbdd3 100644 --- a/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/settings.unit.spec.ts +++ b/frontend/src/metabase/static-viz/components/LineAreaBarChart/utils/settings.unit.spec.ts @@ -1,5 +1,5 @@ import _ from "underscore"; -import { ChartSettings, Series } from "../../XYChart/types"; +import { ChartSettings } from "../../XYChart/types"; import { adjustSettings } from "./settings"; const settings: ChartSettings = { @@ -13,6 +13,7 @@ const settings: ChartSettings = { left: "Count", bottom: "Date", }, + visualization_settings: {}, }; const chartSize = { @@ -22,16 +23,6 @@ const chartSize = { const minTickSize = 12; -const getSeries = (length: number): Series[] => [ - { - name: "series", - color: "black", - yAxisPosition: "left", - type: "line", - data: _.range(length).map(index => [`X:${index}`, index]), - }, -]; - describe("adjustSettings", () => { describe("ordinal x-axis", () => { it("returns unchanged settings when the number X-ticks is less or equal than 10", () => { diff --git a/frontend/src/metabase/static-viz/components/XYChart/XYChart.tsx b/frontend/src/metabase/static-viz/components/XYChart/XYChart.tsx index 47cd86961c35bd4a3831bdc877c9f806dc8bb49a..64dcd42da92b120d28466f745728f625f5dd7b4a 100644 --- a/frontend/src/metabase/static-viz/components/XYChart/XYChart.tsx +++ b/frontend/src/metabase/static-viz/components/XYChart/XYChart.tsx @@ -79,6 +79,8 @@ export const XYChart = ({ const yLabelOffsetRight = LABEL_PADDING; const xTickVerticalMargins = style.axes.labels.fontSize * 2; + const showValues = settings.visualization_settings["graph.show_values"]; + const margin = calculateMargin( yTickWidths.left, yTickWidths.right, @@ -86,7 +88,7 @@ export const XYChart = ({ xTicksDimensions.width, settings.labels, style.axes.ticks.fontSize, - !!settings.goal || !!settings.show_values, + !!settings.goal || showValues, ); const { xMin, xMax, yMin, innerHeight, innerWidth } = calculateBounds( @@ -303,7 +305,7 @@ export const XYChart = ({ /> )} - {settings.show_values && ( + {showValues && ( <Values series={series} formatter={(value: number, compact: boolean): string => @@ -353,7 +355,10 @@ function getValuesLeftOffset( const maxSeriesLength = Math.max( ...multipleSeries.map(series => series.data.length), ); - if (settings.show_values && maxSeriesLength > MAX_SERIES_LENGTH) { + if ( + settings.visualization_settings["graph.show_values"] && + maxSeriesLength > MAX_SERIES_LENGTH + ) { return valueCharSize * (APPROXIMATE_MAX_VALUE_CHAR_LENGTH / 2); } diff --git a/frontend/src/metabase/static-viz/components/XYChart/types.ts b/frontend/src/metabase/static-viz/components/XYChart/types.ts index 56f9e97769c054c9e3542296932567b7cac73317..e9e268d0269c8835549c55a69f7162fba76855e4 100644 --- a/frontend/src/metabase/static-viz/components/XYChart/types.ts +++ b/frontend/src/metabase/static-viz/components/XYChart/types.ts @@ -1,5 +1,5 @@ import type { ScaleBand, ScaleLinear, ScaleTime } from "d3-scale"; -import { DatasetColumn } from "metabase-types/api"; +import { DatasetColumn, VisualizationSettings } from "metabase-types/api"; import type { DateFormatOptions } from "metabase/static-viz/lib/dates"; import type { NumberFormatOptions } from "metabase/static-viz/lib/numbers"; import { ContinuousScaleType } from "metabase/visualizations/shared/types/scale"; @@ -17,7 +17,6 @@ export type YAxisPosition = "left" | "right"; export type VisualizationType = "line" | "area" | "bar" | "waterfall"; interface BaseSeries { - name: string | null; data: SeriesData; type: VisualizationType; yAxisPosition: YAxisPosition; @@ -69,13 +68,12 @@ export type ChartSettings = { value: number; label: string; }; - show_values?: boolean; labels: { left?: string; bottom?: string; right?: string; }; - series_settings?: Record<string, { color?: string; title?: string }>; + visualization_settings: VisualizationSettings; }; export interface Dimensions { diff --git a/frontend/src/metabase/visualizations/lib/settings.js b/frontend/src/metabase/visualizations/lib/settings.js index f52dc8900e6fe4a3d1791568581cd4480f1c3726..b8fbbc0e6f0b93e429f5a9ded8ea50eccff2c6c9 100644 --- a/frontend/src/metabase/visualizations/lib/settings.js +++ b/frontend/src/metabase/visualizations/lib/settings.js @@ -128,6 +128,9 @@ function getSettingWidget( for (const settingId of settingDef.writeDependencies || []) { newSettings[settingId] = computedSettings[settingId]; } + for (const settingId of settingDef.eraseDependencies || []) { + newSettings[settingId] = null; + } onChangeSettings(newSettings); }; if (settingDef.useRawSeries && object._raw) { diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js index 01504bb99859ad9ff083ddd9bedd446c78bec7fe..3c61976d8343deaf6a916e871eed418350254e20 100644 --- a/frontend/src/metabase/visualizations/lib/settings/graph.js +++ b/frontend/src/metabase/visualizations/lib/settings/graph.js @@ -153,6 +153,7 @@ export const GRAPH_DATA_SETTINGS = { }, readDependencies: ["graph._dimension_filter", "graph._metric_filter"], writeDependencies: ["graph.metrics"], + eraseDependencies: ["graph.series_order_dimension", "graph.series_order"], dashboard: false, useRawSeries: true, }, diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj index 53edb33fbca13e31e567f21690a5a0b72e256442..94fd20dfc2dbfac32157f4ceb8c20fa329d8edfa 100644 --- a/src/metabase/pulse/render/body.clj +++ b/src/metabase/pulse/render/body.clj @@ -289,10 +289,11 @@ - there are some date overrides done from lib/formatting.js - chop off and underscore the nasty keys in our map - backfill currency to the default of USD if not present" - [x-col y-col {::mb.viz/keys [column-settings] :as _viz-settings}] + [x-col y-col {::mb.viz/keys [column-settings] :as viz-settings}] (let [x-col-settings (settings-from-column x-col column-settings) y-col-settings (settings-from-column y-col column-settings)] - (cond-> {:colors (public-settings/application-colors)} + (cond-> {:colors (public-settings/application-colors) + :visualization_settings (or viz-settings {})} x-col-settings (assoc :x x-col-settings) y-col-settings @@ -325,19 +326,17 @@ "timeseries" "ordinal")] (merge - {:colors (public-settings/application-colors) - :stacking (if (:stackable.stack_type viz-settings) "stack" "none") - :show_values (boolean (:graph.show_values viz-settings)) - :x {:type (or (:graph.x_axis.scale viz-settings) default-x-type) - :format x-format} - :y {:type (or (:graph.y_axis.scale viz-settings) "linear") - :format y-format} - :labels labels} + {:colors (public-settings/application-colors) + :stacking (if (:stackable.stack_type viz-settings) "stack" "none") + :x {:type (or (:graph.x_axis.scale viz-settings) default-x-type) + :format x-format} + :y {:type (or (:graph.y_axis.scale viz-settings) "linear") + :format y-format} + :labels labels + :visualization_settings (or viz-settings {})} (when (:graph.show_goal viz-settings) {:goal {:value (:graph.goal_value viz-settings) - :label (or (:graph.goal_label viz-settings) (tru "Goal"))}}) - (when (:series_settings viz-settings) - {:series_settings (:series_settings viz-settings)})))) + :label (or (:graph.goal_label viz-settings) (tru "Goal"))}})))) (defn- set-default-stacked "Default stack type is stacked for area chart with more than one metric. @@ -610,8 +609,7 @@ (defn- multiple-scalar-series [joined-rows _x-cols _y-cols _viz-settings] [(for [[row-val] (map vector joined-rows)] - {:name nil - :cardName (first row-val) + {:cardName (first row-val) :type :bar :data [row-val] :yAxisPosition "left" @@ -645,16 +643,13 @@ [chart-type joined-rows _x-cols y-cols {:keys [viz-settings] :as data} card-name] (for [[idx y-col] (map-indexed vector y-cols)] (let [y-col-key (keyword (:name y-col)) - metric-name (or (series-setting viz-settings y-col-key :name) - (series-setting viz-settings y-col-key :title)) card-type (or (series-setting viz-settings y-col-key :display) chart-type (nth default-combo-chart-types idx)) selected-rows (mapv #(vector (ffirst %) (nth (second %) idx)) joined-rows) y-axis-pos (or (series-setting viz-settings y-col-key :axis) (nth (default-y-pos data axis-group-threshold) idx))] - {:name metric-name - :cardName card-name + {:cardName card-name :type card-type :data selected-rows :yAxisPosition y-axis-pos @@ -672,15 +667,12 @@ (for [[idx group-key] (map-indexed vector groups)] (let [row-group (get grouped-rows group-key) selected-row-group (mapv #(vector (ffirst %) (first (second %))) row-group) - column-name (or (series-setting viz-settings group-key :name) - (series-setting viz-settings group-key :title)) card-type (or (series-setting viz-settings group-key :display) chart-type (nth default-combo-chart-types idx)) y-axis-pos (or (series-setting viz-settings group-key :axis) (nth (default-y-pos data axis-group-threshold) idx))] - {:name column-name - :cardName card-name + {:cardName card-name :type card-type :data selected-row-group :yAxisPosition y-axis-pos diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj index edb029f9a4dc3e6e056114d45b4c643af843888e..cc3a4a9d035bfc3cae405d888d68109b86b2e8e3 100644 --- a/test/metabase/pulse/render/body_test.clj +++ b/test/metabase/pulse/render/body_test.clj @@ -410,25 +410,30 @@ (deftest render-bar-graph-test (testing "Render a bar graph with non-nil values for the x and y axis" (is (has-inline-image? - (render-bar-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) + (render-bar-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the y-axis" (is (has-inline-image? - (render-bar-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]]})))) + (render-bar-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the y-axis" (is (has-inline-image? - (render-bar-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]]})))) + (render-bar-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for both x and y on different rows" (is (has-inline-image? - (render-bar-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]]})))) + (render-bar-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]] + :viz-settings {}})))) (testing "Check multiseries in one card but without explicit combo" (is (has-inline-image? - (render-multiseries-bar-graph - {:cols default-multi-columns - :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) + (render-multiseries-bar-graph + {:cols default-multi-columns + :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]] + :viz-settings {}}))))) (defn- render-area-graph [results] (body/render :area :inline pacific-tz render.tu/test-card nil results)) @@ -442,59 +447,35 @@ (deftest render-area-graph-test (testing "Render an area graph with non-nil values for the x and y axis" (is (has-inline-image? - (render-area-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) + (render-area-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]] + :viz-settings {}})))) (testing "Render a stacked area graph" (is (has-inline-image? - (render-stack-area-graph {:cols default-multi-columns - :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]})))) + (render-stack-area-graph {:cols default-multi-columns + :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the y-axis" (is (has-inline-image? - (render-area-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]]})))) + (render-area-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the y-axis" (is (has-inline-image? - (render-area-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]]})))) + (render-area-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for both x and y on different rows" (is (has-inline-image? - (render-area-graph {:cols default-columns - :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]]})))) + (render-area-graph {:cols default-columns + :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]] + :viz-settings {}})))) (testing "Check multiseries in one card but without explicit combo" (is (has-inline-image? - (render-multiseries-area-graph - {:cols default-multi-columns - :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) - -(deftest series-with-custom-names-test - (testing "Check if single x-axis combo series uses custom series names (#21503)" - (is (= #{"Bought" "Sold"} - (set (map :name - (#'body/single-x-axis-combo-series - :bar - [[[10.0] [1 -1]] [[5.0] [10 -10]] [[1.25] [20 -20]]] - [{:name "Price", :display_name "Price", :base_type :type/Number}] - [{:name "NumPurchased", :display_name "NumPurchased", :base_type :type/Number} - {:name "NumSold", :display_name "NumSold", :base_type :type/Number}] - {:viz-settings - {:series_settings {:NumPurchased {:color "#a7cf7b" :title "Bought"} - :NumSold {:color "#a7cf7b" :title "Sold"}}}} - "Card name")))))) - (testing "Check if double x-axis combo series uses custom series names (#21503)" - (is (= #{"Bobby" "Dobby" "Robby" "Mobby"} - (set (map :name - (#'body/double-x-axis-combo-series - nil - [[[10.0 "Bob"] [123]] [[5.0 "Dobbs"] [12]] [[2.5 "Robbs"] [1337]] [[1.25 "Mobbs"] [-22]]] - [{:base_type :type/BigInteger, :display_name "Price", :name "Price", :semantic_type nil} - {:base_type :type/BigInteger, :display_name "NumPurchased", :name "NumPurchased", :semantic_type nil}] - [{:base_type :type/BigInteger, :display_name "NumKazoos", :name "NumKazoos", :semantic_type nil}] - {:viz-settings - {:series_settings {:Bob {:color "#c5a9cf" :title "Bobby"} - :Dobbs {:color "#a7cf7b" :title "Dobby"} - :Robbs {:color "#34517d" :title "Robby"} - :Mobbs {:color "#e0be40" :title "Mobby"}}}} - "Card name"))))))) + (render-multiseries-area-graph + {:cols default-multi-columns + :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]] + :viz-settings {}}))))) (defn- render-waterfall [results] (body/render :waterfall :inline pacific-tz render.tu/test-card nil results)) @@ -503,23 +484,28 @@ (testing "Render a waterfall graph with non-nil values for the x and y axis" (is (has-inline-image? (render-waterfall {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]] + :viz-settings {}})))) (testing "Render a waterfall graph with bigdec, bigint values for the x and y axis" (is (has-inline-image? - (render-waterfall {:cols default-columns - :rows [[10.0M 1M] [5.0 10N] [2.50 20N] [1.25M 30]]})))) + (render-waterfall {:cols default-columns + :rows [[10.0M 1M] [5.0 10N] [2.50 20N] [1.25M 30]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the y-axis" (is (has-inline-image? - (render-waterfall {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]]})))) + (render-waterfall {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 nil]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for the x-axis" (is (has-inline-image? - (render-waterfall {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]]})))) + (render-waterfall {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [nil 30]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for both x and y on different rows" (is (has-inline-image? - (render-waterfall {:cols default-columns - :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]]}))))) + (render-waterfall {:cols default-columns + :rows [[10.0 1] [5.0 10] [nil 20] [1.25 nil]] + :viz-settings {}}))))) (defn- render-combo [results] (body/render :combo :inline pacific-tz render.tu/test-combo-card nil results)) @@ -530,16 +516,19 @@ (deftest render-combo-test (testing "Render a combo graph with non-nil values for the x and y axis" (is (has-inline-image? - (render-combo {:cols default-multi-columns - :rows [[10.0 1 123 111] [5.0 10 12 111] [2.50 20 1337 12312] [1.25 30 -22 123124]]})))) + (render-combo {:cols default-multi-columns + :rows [[10.0 1 123 111] [5.0 10 12 111] [2.50 20 1337 12312] [1.25 30 -22 123124]] + :viz-settings {}})))) (testing "Render a combo graph with multiple x axes" (is (has-inline-image? - (render-combo-multi-x {:cols default-multi-columns - :rows [[10.0 "Bob" 123 123124] [5.0 "Dobbs" 12 23423] [2.50 "Robbs" 1337 234234] [1.25 "Mobbs" -22 1234123]]})))) + (render-combo-multi-x {:cols default-multi-columns + :rows [[10.0 "Bob" 123 123124] [5.0 "Dobbs" 12 23423] [2.50 "Robbs" 1337 234234] [1.25 "Mobbs" -22 1234123]] + :viz-settings {}})))) (testing "Check to make sure we allow nil values for any axis" (is (has-inline-image? - (render-combo {:cols default-multi-columns - :rows [[nil 1 1 23453] [10.0 1 nil nil] [5.0 10 22 1337] [2.50 nil 22 1231] [1.25 nil nil 1231232]]}))))) + (render-combo {:cols default-multi-columns + :rows [[nil 1 1 23453] [10.0 1 nil nil] [5.0 10 22 1337] [2.50 nil 22 1231] [1.25 nil nil 1231232]] + :viz-settings {}}))))) (defn- render-funnel [results] (body/render :funnel :inline pacific-tz render.tu/test-card nil results)) @@ -548,18 +537,21 @@ (testing "Test that we can render a funnel with all valid values" (is (has-inline-image? (render-funnel - {:cols default-columns - :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) + {:cols default-columns + :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]] + :viz-settings {}})))) (testing "Test that we can render a funnel with extraneous columns and also weird strings stuck in places" (is (has-inline-image? - (render-funnel - {:cols default-multi-columns - :rows [[10.0 1 2 2] [5.0 10 "11.1" 1] ["2.50" 20 1337 0] [1.25 30 -2 "-2"]]})))) + (render-funnel + {:cols default-multi-columns + :rows [[10.0 1 2 2] [5.0 10 "11.1" 1] ["2.50" 20 1337 0] [1.25 30 -2 "-2"]] + :viz-settings {}})))) (testing "Test that we can have some nil values stuck everywhere" (is (has-inline-image? (render-funnel - {:cols default-columns - :rows [[nil 1] [11.0 nil] [nil nil] [2.50 20] [1.25 30]]}))))) + {:cols default-columns + :rows [[nil 1] [11.0 nil] [nil nil] [2.50 20] [1.25 30]] + :viz-settings {}}))))) (deftest render-categorical-donut-test (let [columns [{:name "category", diff --git a/test/metabase/pulse/render/js_svg_test.clj b/test/metabase/pulse/render/js_svg_test.clj index 573a9a67f0ef0da76b4c1276c8e7addf320006ac..6ceb004bfd1929934daa7f96397aa148fe5891a9 100644 --- a/test/metabase/pulse/render/js_svg_test.clj +++ b/test/metabase/pulse/render/js_svg_test.clj @@ -112,21 +112,21 @@ (let [rows [[#t "2020" 2] [#t "2021" 3]] series-seqs [[{:type :line - :color "#999AC4" :data rows :yAxisPosition "left" :column {:name "count"}}]] - settings {:colors {:brand "#5E81AC" - :filter "#A3BE8C" - :summarize "#B48EAD"}, - :x {:type "timeseries" - :format {:date_style "YYYY/MM/DD"}} - :y {:type "linear" - :format {:prefix "prefix" - :decimals 2}} - :labels {:bottom "" - :left "" - :right ""}} + settings {:colors {:brand "#5E81AC" + :filter "#A3BE8C" + :summarize "#B48EAD"}, + :x {:type "timeseries" + :format {:date_style "YYYY/MM/DD"}} + :y {:type "linear" + :format {:prefix "prefix" + :decimals 2}} + :labels {:bottom "" + :left "" + :right ""} + :visualization_settings {}} svg-string (combo-chart-string series-seqs settings)] (testing "It returns bytes" (let [svg-bytes (js-svg/combo-chart series-seqs settings)] @@ -153,21 +153,21 @@ (let [rows [[#t "2020" 2] [#t "2021" 3]] series-seqs [[{:type :bar - :color "#999AC4" :data rows :yAxisPosition "left" :column {:name "count"}}]] - settings {:colors {:brand "#5E81AC" - :filter "#A3BE8C" - :summarize "#B48EAD"}, - :x {:type "timeseries" - :format {:date_style "YYYY/MM/DD"}} - :y {:type "linear" - :format {:prefix "prefix" - :decimals 4}} - :labels {:bottom "" - :left "" - :right ""}} + settings {:colors {:brand "#5E81AC" + :filter "#A3BE8C" + :summarize "#B48EAD"}, + :x {:type "timeseries" + :format {:date_style "YYYY/MM/DD"}} + :y {:type "linear" + :format {:prefix "prefix" + :decimals 4}} + :labels {:bottom "" + :left "" + :right ""} + :visualization_settings {}} svg-string (combo-chart-string series-seqs settings)] (testing "It returns bytes" (let [svg-bytes (js-svg/combo-chart series-seqs settings)] @@ -194,20 +194,20 @@ (let [rows [["bob" 2] ["dobbs" 3]] series-seqs [[{:type :area - :color "#999AC4" :data rows :yAxisPosition "left" :column {:name "count"}}]] - settings {:colors {:brand "#5E81AC" - :filter "#A3BE8C" - :summarize "#B48EAD"}, - :x {:type "ordinal"} - :y {:type "linear" - :format {:prefix "prefix" - :decimals 4}} - :labels {:bottom "" - :left "" - :right ""}} + settings {:colors {:brand "#5E81AC" + :filter "#A3BE8C" + :summarize "#B48EAD"}, + :x {:type "ordinal"} + :y {:type "linear" + :format {:prefix "prefix" + :decimals 4}} + :labels {:bottom "" + :left "" + :right ""} + :visualization_settings {}} svg-string (combo-chart-string series-seqs settings)] (testing "It returns bytes" (let [svg-bytes (js-svg/combo-chart series-seqs settings)] @@ -234,11 +234,12 @@ :data [["A" 1] ["B" 20] ["C" -4] ["D" 100]] :yAxisPosition "left" :column {:name "count"}}]] - settings {:x {:type "ordinal"} - :y {:type "linear"} - :labels {:bottom "" - :left "" - :right ""}} + settings {:x {:type "ordinal"} + :y {:type "linear"} + :labels {:bottom "" + :left "" + :right ""} + :visualization_settings {}} non-goal-hiccup (combo-chart-hiccup series-seqs settings) non-goal-node (->> non-goal-hiccup (tree-seq vector? rest) (filter #(= goal-label (second %))) first)] (testing "No goal line exists when there are no goal settings." @@ -285,28 +286,27 @@ rows2 [[#t "2000-03-01T00:00:00Z" 3] [#t "2002-03-01T00:00:00Z" 4]] ;; this one needs more stuff because of stricter ts types - series-seqs [[{:name "bob" - :color "#cccccc" + series-seqs [[{:cardName "bob" :type "area" :data rows1 :yAxisPosition "left" - :column {:name "count"}} - {:name "bob2" - :color "#cccccc" + :column {:name "count" :display_name "Count"}} + {:cardName "bob" :type "line" :data rows2 :yAxisPosition "right" - :column {:name "count"}}]] + :column {:name "count2" :display_name "Count 2"}}]] labels {:left "count" :bottom "year" :right "something"} - settings {:x {:type "timeseries" - :format {:date_style "YYYY"}} - :y {:type "linear" - :format {:number_style "decimal" - :decimals 4}} - :colors {} - :labels labels}] + settings {:x {:type "timeseries" + :format {:date_style "YYYY"}} + :y {:type "linear" + :format {:number_style "decimal" + :decimals 4}} + :colors {} + :labels labels + :visualization_settings {}}] (testing "It returns bytes" (let [svg-bytes (js-svg/combo-chart series-seqs settings)] (is (bytes? svg-bytes))))