Update metric with new config

This commit is contained in:
Nate Kelley 2025-09-25 22:16:45 -06:00
parent efe668c405
commit 8ca932b60c
No known key found for this signature in database
GPG Key ID: FD90372AB8D98B4F
5 changed files with 25 additions and 203 deletions

View File

@ -339,10 +339,8 @@ export const useUpdateMetric = (params: {
const { mutateAsync: saveMetric } = useSaveMetric({ updateOnSave });
const saveMetricToServer = async (newMetric: BusterMetric, prevMetric: BusterMetric) => {
const changedValues = prepareMetricUpdateMetric(newMetric, prevMetric);
if (changedValues) {
const changedValues = prepareMetricUpdateMetric(newMetric, prevMetric); //why do I do this now?
await saveMetric({ ...changedValues, update_version: updateVersion });
}
};
const combineAndUpdateMetric = ({

View File

@ -91,7 +91,7 @@ describe('useDatasetOptions', () => {
x: ['month'],
y: ['sales'],
tooltip: ['level'], // Include the level field in tooltip so it's available for coloring
colorBy: { columnId: 'level' },
colorBy: ['level'],
},
selectedChartType: 'bar' as ChartType,
columnLabelFormats: mockColumnLabelFormats,
@ -176,7 +176,7 @@ describe('useDatasetOptions', () => {
y: ['sales'],
y2: ['profit'],
tooltip: ['type'],
colorBy: { columnId: 'type' },
colorBy: ['type'],
},
selectedChartType: 'combo' as ChartType,
columnLabelFormats: mockComboColumnLabelFormats,

View File

@ -20,7 +20,7 @@ describe('getChartTypeDropZones', () => {
barLayout: 'vertical',
});
expect(result).toHaveLength(4);
expect(result).toHaveLength(5);
expect(result[0]).toEqual({
id: SelectAxisContainerId.XAxis,
title: 'X-Axis',
@ -32,11 +32,16 @@ describe('getChartTypeDropZones', () => {
items: ['value1'],
});
expect(result[2]).toEqual({
id: SelectAxisContainerId.ColorBy,
title: 'Color By',
items: [],
});
expect(result[3]).toEqual({
id: SelectAxisContainerId.CategoryAxis,
title: 'Category',
items: ['category1'],
});
expect(result[3]).toEqual({
expect(result[4]).toEqual({
id: SelectAxisContainerId.Tooltip,
title: 'Tooltip',
items: ['tooltip1', 'tooltip2'],
@ -58,7 +63,7 @@ describe('getChartTypeDropZones', () => {
barLayout: 'horizontal',
});
expect(result).toHaveLength(4);
expect(result).toHaveLength(5);
// For horizontal bar, Y axis shows as X-Axis title but uses Y axis ID
expect(result[0]).toEqual({
id: SelectAxisContainerId.YAxis,
@ -72,11 +77,16 @@ describe('getChartTypeDropZones', () => {
items: ['column1'],
});
expect(result[2]).toEqual({
id: SelectAxisContainerId.ColorBy,
title: 'Color By',
items: [],
});
expect(result[3]).toEqual({
id: SelectAxisContainerId.CategoryAxis,
title: 'Category',
items: ['category1'],
});
expect(result[3]).toEqual({
expect(result[4]).toEqual({
id: SelectAxisContainerId.Tooltip,
title: 'Tooltip',
items: [],
@ -98,7 +108,7 @@ describe('getChartTypeDropZones', () => {
barLayout: 'horizontal',
});
expect(result).toHaveLength(4);
expect(result).toHaveLength(5);
// Even with empty arrays, the zones should still be created with swapped titles
expect(result[0]).toEqual({
id: SelectAxisContainerId.YAxis,
@ -110,12 +120,12 @@ describe('getChartTypeDropZones', () => {
title: 'Y-Axis', // X data appears as Y-Axis in horizontal mode
items: [],
});
expect(result[2]).toEqual({
expect(result[3]).toEqual({
id: SelectAxisContainerId.CategoryAxis,
title: 'Category',
items: [],
});
expect(result[3]).toEqual({
expect(result[4]).toEqual({
id: SelectAxisContainerId.Tooltip,
title: 'Tooltip',
items: ['hover_info', 'extra_data'],

View File

@ -1,10 +1,6 @@
import type { ChartType } from '@buster/server-shared/metrics';
import { describe, expect, it } from 'vitest';
import type { BusterMetric } from '@/api/asset_interfaces/metric';
import {
getChangedTopLevelMessageValues,
getChangesFromDefaultChartConfig,
} from './saveToServerHelpers';
import { getChangedTopLevelMessageValues } from './saveToServerHelpers';
// Mock minimal metric objects for testing
const createMockMetric = (overrides?: Partial<BusterMetric>): BusterMetric =>
@ -147,74 +143,3 @@ describe('getChangedTopLevelMessageValues', () => {
expect(result).toEqual({ name: 'Updated Metric Name is a good name' });
});
});
// Tests for getChangesFromDefaultChartConfig
describe('getChangesFromDefaultChartConfig', () => {
it('should return empty object when chart_config is undefined', () => {
const metric = createMockMetric({ chart_config: undefined });
const result = getChangesFromDefaultChartConfig(metric);
expect(result).toEqual({});
});
it('should detect changes from default chart config', () => {
// Create a metric with partial chart config that will be merged with defaults
const metric = createMockMetric();
// Assert on a non-typed partial chart_config to avoid TypeScript errors
// @ts-expect-error - We're testing runtime behavior, TypeScript doesn't need to validate this test data
metric.chart_config = {
selectedChartType: 'bar',
showLegend: true,
xAxisShowAxisLabel: false,
colors: ['#FF0000', '#00FF00', '#0000FF'], // Custom colors
};
const result = getChangesFromDefaultChartConfig(metric);
expect(result).toEqual({
selectedChartType: 'bar',
showLegend: true,
xAxisShowAxisLabel: false,
colors: ['#FF0000', '#00FF00', '#0000FF'],
});
});
it('should detect and filter changes in column label formats', () => {
const metric = createMockMetric();
// Create the chart config manually to avoid type issues
const chartConfig = {} as any;
chartConfig.columnLabelFormats = {
revenue: {
style: 'currency',
currency: 'EUR',
compactNumbers: true,
makeLabelHumanReadable: true,
},
date: {
columnType: 'date',
isUTC: true,
},
};
metric.chart_config = chartConfig;
const result = getChangesFromDefaultChartConfig(metric);
// Should only include the properties that differ from defaults
expect(result).toEqual({
columnLabelFormats: {
revenue: {
style: 'currency',
currency: 'EUR',
compactNumbers: true,
},
date: {
columnType: 'date',
isUTC: true,
},
},
});
});
});

View File

@ -1,25 +1,9 @@
import {
type BarAndLineAxis,
type ChartConfigProps,
type ColumnLabelFormat,
type ColumnSettings,
type ComboChartAxis,
type DataMetadata,
DEFAULT_CHART_CONFIG_ENTRIES,
DEFAULT_COLUMN_LABEL_FORMAT,
DEFAULT_COLUMN_SETTINGS,
type PieChartAxis,
type ScatterAxis,
} from '@buster/server-shared/metrics';
import isEqual from 'lodash/isEqual';
import type { DataMetadata } from '@buster/server-shared/metrics';
import type { BusterMetric } from '@/api/asset_interfaces/metric';
import type { updateMetric } from '@/api/buster_rest/metrics';
import { getChangedValues } from '@/lib/objects';
import { createDefaultChartConfig } from './messageAutoChartHandler';
const DEFAULT_COLUMN_SETTINGS_ENTRIES = Object.entries(DEFAULT_COLUMN_SETTINGS);
const DEFAULT_COLUMN_LABEL_FORMATS_ENTRIES = Object.entries(DEFAULT_COLUMN_LABEL_FORMAT);
export const getChangedTopLevelMessageValues = (
newMetric: BusterMetric,
oldMetric: BusterMetric
@ -28,115 +12,20 @@ export const getChangedTopLevelMessageValues = (
return changes;
};
const keySpecificHandlers: Partial<Record<keyof ChartConfigProps, (value: unknown) => unknown>> = {
barAndLineAxis: (value: unknown) => value as BarAndLineAxis,
scatterAxis: (value: unknown) => value as ScatterAxis,
pieChartAxis: (value: unknown) => value as PieChartAxis,
comboChartAxis: (value: unknown) => value as ComboChartAxis,
colors: (value: unknown) => value as string[],
columnSettings: (columnSettings: unknown) => {
const typedColumnSettings = columnSettings as ChartConfigProps['columnSettings'];
// Early return if no column settings
if (!typedColumnSettings) return {};
const diff: Record<string, ColumnSettings> = {};
// Single loop through column settings
for (const [key, value] of Object.entries(typedColumnSettings)) {
const changedSettings: Partial<ColumnSettings> = {};
let hasChanges = false;
// Check each default setting
for (const [settingKey, defaultValue] of DEFAULT_COLUMN_SETTINGS_ENTRIES) {
const columnSettingValue = value?.[settingKey as keyof ColumnSettings];
if (!isEqual(defaultValue, columnSettingValue)) {
(changedSettings as Record<string, unknown>)[settingKey] = columnSettingValue;
hasChanges = true;
}
}
if (hasChanges) {
diff[key] = changedSettings as ColumnSettings;
}
}
return diff;
},
columnLabelFormats: (columnLabelFormats: unknown) => {
const typedColumnLabelFormats = columnLabelFormats as Record<string, ColumnLabelFormat>;
// Early return if no column settings
if (!typedColumnLabelFormats) return {};
const diff: Record<string, ColumnLabelFormat> = {};
// Single loop through column label formats
for (const [key, value] of Object.entries(typedColumnLabelFormats)) {
const changedSettings: Partial<ColumnLabelFormat> = {};
let hasChanges = false;
// Check each default setting
for (const [settingKey, defaultValue] of DEFAULT_COLUMN_LABEL_FORMATS_ENTRIES) {
const columnSettingValue = value[settingKey as keyof ColumnLabelFormat];
if (!isEqual(defaultValue, columnSettingValue)) {
// biome-ignore lint/suspicious/noExplicitAny: this is a workaround to avoid type errors
changedSettings[settingKey as keyof ColumnLabelFormat] = columnSettingValue as any;
hasChanges = true;
}
}
if (hasChanges) {
diff[key] = changedSettings as ColumnLabelFormat;
}
}
return diff;
},
};
export const getChangesFromDefaultChartConfig = (newMetric: BusterMetric) => {
const chartConfig = newMetric.chart_config;
if (!chartConfig) return {} as ChartConfigProps;
const diff: Partial<ChartConfigProps> = {};
for (const [_key, defaultValue] of DEFAULT_CHART_CONFIG_ENTRIES) {
const key = _key as keyof ChartConfigProps;
const chartConfigValue = chartConfig[key];
const handler = keySpecificHandlers[key];
if (handler) {
const valueToUse = handler(chartConfigValue);
if (valueToUse && Object.keys(valueToUse).length > 0) {
// biome-ignore lint/suspicious/noExplicitAny: this is a workaround to avoid type errors
diff[key] = valueToUse as any;
}
continue;
}
if (!isEqual(chartConfigValue, defaultValue)) {
// biome-ignore lint/suspicious/noExplicitAny: this is a workaround to avoid type errors
diff[key] = chartConfigValue as any;
}
}
return diff as ChartConfigProps;
};
export const combineChangeFromDefaultChartConfig = (
newMetric: BusterMetric,
dataMetadata: DataMetadata
) => {
const chartConfig = createDefaultChartConfig({
return createDefaultChartConfig({
chart_config: newMetric.chart_config,
data_metadata: dataMetadata,
});
return chartConfig;
};
export const prepareMetricUpdateMetric = (
newMetric: BusterMetric,
prevMetric: BusterMetric
): Parameters<typeof updateMetric>[0] | null => {
): Parameters<typeof updateMetric>[0] => {
const changedTopLevelValues = getChangedTopLevelMessageValues(
newMetric,
prevMetric