Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { NativeFilterType } from '@superset-ui/core';
import { CHART_TYPE } from '../../util/componentTypes';
import DashboardContainer from './DashboardContainer';
import * as nativeFiltersActions from '../../actions/nativeFilters';
import * as chartCustomizationActions from '../../actions/chartCustomizationActions';

fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {});
fetchMock.put('glob:*/api/v1/dashboard/*/colors*', {});
Expand All @@ -43,6 +44,16 @@ jest.mock('src/dashboard/containers/DashboardGrid', () => ({
default: () => <div data-test="mock-dashboard-grid" />,
}));

// DashboardContainer dispatches these on mount, so unit tests stub them.
jest.mock('src/dashboard/actions/dashboardState', () => ({
...jest.requireActual('src/dashboard/actions/dashboardState'),
applyDashboardLabelsColorOnLoad: jest.fn(() => () => Promise.resolve()),
updateDashboardLabelsColor: jest.fn(() => () => Promise.resolve()),
persistDashboardLabelsColor: jest.fn(() => () => Promise.resolve()),
ensureSyncedSharedLabelsColors: jest.fn(() => () => Promise.resolve()),
ensureSyncedLabelsColorMap: jest.fn(() => () => Promise.resolve()),
}));

const defaultTestFilter = {
id: 'FILTER-1',
name: 'Test Filter',
Expand Down Expand Up @@ -438,3 +449,146 @@ test('calculates tabsInScope for filters with tab-scoped charts', async () => {
]),
);
});

// Chart customization scope tests.

test('calculates chartsInScope correctly for new-format chart customizations', async () => {
const customizationId = 'CHART_CUSTOMIZATION-1';
const originalFn = chartCustomizationActions.setInScopeStatusOfCustomizations;
const spy = jest.spyOn(
chartCustomizationActions,
'setInScopeStatusOfCustomizations',
);
spy.mockImplementation(args => originalFn(args));

try {
const state = {
dashboardInfo: {
...mockState.dashboardInfo,
metadata: {
...mockState.dashboardInfo.metadata,
native_filter_configuration: [],
chart_customization_config: [
{
id: customizationId,
type: 'CHART_CUSTOMIZATION',
name: 'Dynamic Group By',
filterType: 'chart_customization_dynamic_groupby',
targets: [{ datasetId: 1, column: { name: 'status' } }],
scope: { rootPath: ['ROOT_ID'], excluded: [] },
chartsInScope: [],
defaultDataMask: {},
controlValues: {},
},
],
},
},
nativeFilters: {
filters: {
[customizationId]: {
id: customizationId,
type: 'CHART_CUSTOMIZATION',
chartsInScope: [],
},
},
},
};
setup(state);

await waitFor(() => {
expect(spy).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
customizationId,
chartsInScope: [sliceId],
}),
]),
);
});
} finally {
spy.mockRestore();
}
});

test('migrates legacy-format customizations before scope calculation for scope-less items', async () => {
const legacyCustomizationId = 'CHART_CUSTOMIZATION-legacy-1';
const originalFn = chartCustomizationActions.setInScopeStatusOfCustomizations;
const spy = jest.spyOn(
chartCustomizationActions,
'setInScopeStatusOfCustomizations',
);
spy.mockImplementation(args => originalFn(args));

try {
const state = {
dashboardInfo: {
...mockState.dashboardInfo,
metadata: {
...mockState.dashboardInfo.metadata,
native_filter_configuration: [],
chart_customization_config: [
{
id: legacyCustomizationId,
customization: {
dataset: 1,
column: 'status',
filterType: 'chart_customization_dynamic_groupby',
name: 'Legacy Group By',
},
},
],
},
},
nativeFilters: {
filters: {
[legacyCustomizationId]: {
id: legacyCustomizationId,
chartsInScope: [],
},
},
},
};
setup(state);

await waitFor(() => {
expect(spy).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
customizationId: legacyCustomizationId,
chartsInScope: [sliceId],
}),
]),
);
});
} finally {
spy.mockRestore();
}
});

test('does not dispatch setInScopeStatusOfCustomizations when chart_customization_config is empty', async () => {
const spy = jest.spyOn(
chartCustomizationActions,
'setInScopeStatusOfCustomizations',
);

try {
const state = {
dashboardInfo: {
...mockState.dashboardInfo,
metadata: {
...mockState.dashboardInfo.metadata,
native_filter_configuration: [],
chart_customization_config: [],
},
},
nativeFilters: { filters: {} },
};
setup(state);

await waitFor(() => {
expect(spy).not.toHaveBeenCalled();
});
} finally {
spy.mockRestore();
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ import {
} from 'src/dashboard/actions/dashboardState';
import { getColorNamespace, resetColors } from 'src/utils/colorScheme';
import { calculateScopes } from 'src/dashboard/util/calculateScopes';
import {
isLegacyChartCustomizationFormat,
migrateChartCustomizationArray,
} from 'src/dashboard/util/migrateChartCustomization';
import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
import { selectFilterConfiguration } from '../nativeFilters/state';
Expand Down Expand Up @@ -192,8 +196,16 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
return;
}

// Normalize legacy chart customizations before scope calculation.
const hasLegacy = chartCustomizations.some(
isLegacyChartCustomizationFormat,
);
const normalizedCustomizations = hasLegacy
? migrateChartCustomizationArray(chartCustomizations)
: chartCustomizations;

const scopes = calculateScopes(
chartCustomizations,
normalizedCustomizations,
chartIds,
chartLayoutItems,
item => item.type === ChartCustomizationType.Divider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
ensureIsArray,
JsonObject,
PartialFilters,
QueryFormExtraFilter,
ChartCustomization,
} from '@superset-ui/core';
import {
Expand Down Expand Up @@ -135,9 +134,6 @@ function buildExistingColumnsSet(chart: ChartQueryPayload): Set<string> {
const existingColumns = new Set<string>();
const chartType = chart.form_data?.viz_type;

const existingGroupBy = ensureIsArray(chart.form_data?.groupby);
existingGroupBy.forEach((col: string) => existingColumns.add(col));

const xAxisColumn = chart.form_data?.x_axis;
if (xAxisColumn && chartType !== 'heatmap' && chartType !== 'heatmap_v2') {
existingColumns.add(xAxisColumn);
Expand Down Expand Up @@ -251,7 +247,9 @@ function applyChartSpecificGroupBy(
groupByFormData.target = limitedColumns[1];
}
} else if (['chord'].includes(chartType)) {
groupByFormData.groupby = [...existingGroupBy, ...groupByColumns];
groupByFormData.groupby = [
...new Set([...existingGroupBy, ...groupByColumns]),
];
} else if (chartType === 'bubble_v2') {
const { limitedColumns } = limitColumnsForChartType(
chartType,
Expand Down Expand Up @@ -283,7 +281,6 @@ function processGroupByCustomizations(
): {
groupby?: string[];
order_by_cols?: string[];
filters?: QueryFormExtraFilter[];
x_axis?: string;
series?: string;
columns?: string[];
Expand Down Expand Up @@ -328,7 +325,6 @@ function processGroupByCustomizations(
const xAxisColumn = chart.form_data?.x_axis;

const groupByColumns: string[] = [];
const allFilters: QueryFormExtraFilter[] = [];
let orderByConfig: string[] | undefined;
let heatmapColumnAdded = false;

Expand Down Expand Up @@ -376,16 +372,6 @@ function processGroupByCustomizations(
});
}

columnNames.forEach(columnName => {
if (selectedValues.length > 0) {
allFilters.push({
col: columnName,
op: 'IN',
val: selectedValues,
});
}
});

const sortMetric = item.controlValues?.sortMetric;
const sortAscending = item.controlValues?.sortAscending;
if (sortMetric) {
Expand All @@ -400,10 +386,6 @@ function processGroupByCustomizations(
xAxisColumn,
);

if (allFilters.length > 0) {
groupByFormData.filters = allFilters;
}

if (orderByConfig) {
groupByFormData.order_by_cols = orderByConfig;
}
Expand Down Expand Up @@ -548,7 +530,11 @@ export default function getFormDataWithExtraFilters({

const selectedValues = mask.filterState?.value;
groupByState[key] = {
selectedValues: Array.isArray(selectedValues) ? selectedValues : [],
selectedValues: Array.isArray(selectedValues)
? selectedValues
: typeof selectedValues === 'string'
? [selectedValues]
: [],
hasInteracted: mask.filterState?.value !== undefined,
};
}
Expand Down
Loading
Loading