fix(dashboard): apply dynamic groupby display controls to scoped charts (#39356)

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx

@@ -21,10 +21,11 @@ import fetchMock from 'fetch-mock';
 import { storeWithState } from 'spec/fixtures/mockStore';
 import mockState from 'spec/fixtures/mockState';
 import { sliceId } from 'spec/fixtures/mockChartQueries';
-import { NativeFilterType } from '@superset-ui/core';
+import { ChartCustomizationType, 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*', {});
@@ -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(() => () => undefined),
+  updateDashboardLabelsColor: jest.fn(() => () => undefined),
+  persistDashboardLabelsColor: jest.fn(() => () => undefined),
+  ensureSyncedSharedLabelsColors: jest.fn(() => () => undefined),
+  ensureSyncedLabelsColorMap: jest.fn(() => () => undefined),
+}));
+
 const defaultTestFilter = {
   id: 'FILTER-1',
   name: 'Test Filter',
@@ -438,3 +449,281 @@ 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('preserves legacy chart-specific customizations during scope calculation', async () => {
+  const legacyCustomizationId = 'CHART_CUSTOMIZATION-legacy-chart-1';
+  const originalFn = chartCustomizationActions.setInScopeStatusOfCustomizations;
+  const spy = jest.spyOn(
+    chartCustomizationActions,
+    'setInScopeStatusOfCustomizations',
+  );
+  spy.mockImplementation(args => originalFn(args));
+
+  const baseDashboardLayout = mockState.dashboardLayout.present;
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { CHART_ID: _removed, ...cleanLayout } = baseDashboardLayout;
+
+  try {
+    const state = {
+      dashboardState: {
+        ...mockState.dashboardState,
+        sliceIds: [18, 19],
+      },
+      dashboardLayout: {
+        ...mockState.dashboardLayout,
+        present: {
+          ...cleanLayout,
+          CHART_ID_1: {
+            id: 'CHART_ID_1',
+            type: CHART_TYPE,
+            meta: { chartId: 18, width: 4, height: 10 },
+            parents: ['ROOT_ID', 'GRID_ID', 'ROW_ID'],
+          },
+          CHART_ID_2: {
+            id: 'CHART_ID_2',
+            type: CHART_TYPE,
+            meta: { chartId: 19, width: 4, height: 10 },
+            parents: ['ROOT_ID', 'GRID_ID', 'ROW_ID'],
+          },
+        },
+      },
+      dashboardInfo: {
+        ...mockState.dashboardInfo,
+        metadata: {
+          ...mockState.dashboardInfo.metadata,
+          native_filter_configuration: [],
+          chart_customization_config: [
+            {
+              id: legacyCustomizationId,
+              chartId: 18,
+              customization: {
+                dataset: 1,
+                column: 'status',
+                filterType: 'chart_customization_dynamic_groupby',
+                name: 'Legacy Chart Scoped Group By',
+              },
+            },
+          ],
+        },
+      },
+      nativeFilters: {
+        filters: {
+          [legacyCustomizationId]: {
+            id: legacyCustomizationId,
+            chartsInScope: [],
+          },
+        },
+      },
+    };
+    setup(state);
+
+    await waitFor(() => {
+      expect(spy).toHaveBeenCalledWith(
+        expect.arrayContaining([
+          expect.objectContaining({
+            customizationId: legacyCustomizationId,
+            chartsInScope: [18],
+          }),
+        ]),
+      );
+    });
+  } finally {
+    spy.mockRestore();
+  }
+});
+
+test('returns empty scope data for chart customization dividers', async () => {
+  const dividerId = 'CHART_CUSTOMIZATION_DIVIDER-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: dividerId,
+              type: ChartCustomizationType.Divider,
+              title: 'Divider',
+              description: 'Section divider',
+            },
+          ],
+        },
+      },
+      nativeFilters: {
+        filters: {
+          [dividerId]: {
+            id: dividerId,
+            type: ChartCustomizationType.Divider,
+          },
+        },
+      },
+    };
+    setup(state);
+
+    await waitFor(() => {
+      expect(spy).toHaveBeenCalledWith(
+        expect.arrayContaining([
+          expect.objectContaining({
+            customizationId: dividerId,
+            chartsInScope: [],
+            tabsInScope: [],
+          }),
+        ]),
+      );
+    });
+  } 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();
+  }
+});

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx

@@ -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,
+  migrateChartCustomization,
+} 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';
@@ -85,6 +89,37 @@ interface CustomizationScopeData extends ScopeData {
   customizationId: string;
 }
 
+function normalizeChartCustomizationsForScopeCalculation(
+  chartCustomizations: ChartCustomizationConfiguration,
+  chartIds: number[],
+): ChartCustomizationConfiguration {
+  if (!chartCustomizations.some(isLegacyChartCustomizationFormat)) {
+    return chartCustomizations;
+  }
+
+  return chartCustomizations.map(item => {
+    if (!isLegacyChartCustomizationFormat(item)) {
+      return item;
+    }
+
+    const migratedCustomization = migrateChartCustomization(item);
+
+    if (!item.chartId) {
+      return migratedCustomization;
+    }
+
+    return {
+      ...migratedCustomization,
+      // Legacy items could target a single chart without an explicit scope.
+      // Preserve that targeting before calculateScopes recomputes chartsInScope.
+      scope: {
+        ...migratedCustomization.scope,
+        excluded: chartIds.filter(chartId => chartId !== item.chartId),
+      },
+    };
+  });
+}
+
 export const renderedChartIdsSelector: (state: RootState) => number[] =
   createSelector([(state: RootState) => state.charts], charts =>
     Object.values(charts)
@@ -192,8 +227,14 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
       return;
     }
 
+    const normalizedCustomizations =
+      normalizeChartCustomizationsForScopeCalculation(
+        chartCustomizations,
+        chartIds,
+      );
+
     const scopes = calculateScopes(
-      chartCustomizations,
+      normalizedCustomizations,
       chartIds,
       chartLayoutItems,
       item => item.type === ChartCustomizationType.Divider,

superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts

@@ -24,7 +24,6 @@ import {
   ensureIsArray,
   JsonObject,
   PartialFilters,
-  QueryFormExtraFilter,
   ChartCustomization,
 } from '@superset-ui/core';
 import {
@@ -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);
@@ -251,7 +247,8 @@ function applyChartSpecificGroupBy(
       groupByFormData.target = limitedColumns[1];
     }
   } else if (['chord'].includes(chartType)) {
-    groupByFormData.groupby = [...existingGroupBy, ...groupByColumns];
+    groupByFormData.groupby =
+      groupByColumns.length > 0 ? [groupByColumns[0]] : existingGroupBy;
   } else if (chartType === 'bubble_v2') {
     const { limitedColumns } = limitColumnsForChartType(
       chartType,
@@ -283,7 +280,6 @@ function processGroupByCustomizations(
 ): {
   groupby?: string[];
   order_by_cols?: string[];
-  filters?: QueryFormExtraFilter[];
   x_axis?: string;
   series?: string;
   columns?: string[];
@@ -319,7 +315,7 @@ function processGroupByCustomizations(
   });
 
   const chartType = chart.form_data?.viz_type;
-  if (isChartWithoutGroupBy(chartType)) {
+  if (isChartWithoutGroupBy(chartType) || chartType === 'chord') {
     return {};
   }
 
@@ -328,7 +324,6 @@ function processGroupByCustomizations(
   const xAxisColumn = chart.form_data?.x_axis;
 
   const groupByColumns: string[] = [];
-  const allFilters: QueryFormExtraFilter[] = [];
   let orderByConfig: string[] | undefined;
   let heatmapColumnAdded = false;
 
@@ -342,7 +337,9 @@ function processGroupByCustomizations(
       return;
     }
 
-    const selectedValues = groupByInfo.selectedValues || [];
+    const selectedValues = (groupByInfo.selectedValues || []).filter(
+      (value): value is string => typeof value === 'string' && value.length > 0,
+    );
     const columnNames = selectedValues;
 
     if (columnNames.length === 0) {
@@ -376,16 +373,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) {
@@ -400,10 +387,6 @@ function processGroupByCustomizations(
     xAxisColumn,
   );
 
-  if (allFilters.length > 0) {
-    groupByFormData.filters = allFilters;
-  }
-
   if (orderByConfig) {
     groupByFormData.order_by_cols = orderByConfig;
   }
@@ -548,7 +531,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,
       };
     }

superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts

@@ -20,191 +20,449 @@ import getFormDataWithExtraFilters, {
   CachedFormDataWithExtraControls,
   GetFormDataWithExtraFiltersArguments,
 } from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
+import { ChartCustomizationType } from '@superset-ui/core';
 import { sliceId as chartId } from 'spec/fixtures/mockChartQueries';
 
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
-describe('getFormDataWithExtraFilters', () => {
-  const filterId = 'native-filter-1';
-  const mockChart = {
-    id: chartId,
-    chartAlert: null,
-    chartStatus: null,
-    chartUpdateEndTime: null,
-    chartUpdateStartTime: 1,
-    lastRendered: 1,
-    latestQueryFormData: {},
-    sliceFormData: null,
-    queryController: null,
-    queriesResponse: null,
-    triggerQuery: false,
-    form_data: {
-      viz_type: 'filter_select',
-      filters: [
-        {
-          col: 'country_name',
-          op: 'IN',
-          val: ['United States'],
-        },
-      ],
-      datasource: '123',
-      url_params: {},
-    },
+type ChartCustomizationItem = NonNullable<
+  GetFormDataWithExtraFiltersArguments['chartCustomizationItems']
+>[number];
+
+function createChartCustomization(
+  overrides: Partial<ChartCustomizationItem> = {},
+): ChartCustomizationItem {
+  return {
+    id: 'CHART_CUSTOMIZATION-1',
+    type: ChartCustomizationType.ChartCustomization,
+    name: 'Dynamic Group By',
+    filterType: 'chart_customization_dynamic_groupby',
+    targets: [{ datasetId: 3, column: { name: 'status' } }],
+    scope: { rootPath: [], excluded: [] },
+    chartsInScope: [chartId],
+    defaultDataMask: {},
+    controlValues: {},
+    ...overrides,
   };
-  const mockArgs: GetFormDataWithExtraFiltersArguments = {
-    chartConfiguration: {},
-    chart: mockChart,
-    filters: {
-      region: ['Spain'],
-      color: ['pink', 'purple'],
+}
+
+const expectGroupBy = (
+  result: CachedFormDataWithExtraControls,
+  expected: unknown,
+) => {
+  expect('groupby' in result).toBe(true);
+  if (!('groupby' in result)) {
+    throw new Error('Expected groupby to be present in form data');
+  }
+  expect(result.groupby).toEqual(expected);
+};
+
+const expectGroupByLength = (
+  result: CachedFormDataWithExtraControls,
+  length: number,
+) => {
+  expect('groupby' in result).toBe(true);
+  if (!('groupby' in result)) {
+    throw new Error('Expected groupby to be present in form data');
+  }
+  expect(result.groupby).toHaveLength(length);
+};
+
+const getResultFilters = (result: CachedFormDataWithExtraControls) => {
+  if (!('filters' in result) || !Array.isArray(result.filters)) {
+    return [];
+  }
+
+  return result.filters.filter(
+    (
+      filter,
+    ): filter is {
+      col: string;
+      val: unknown[];
+    } =>
+      typeof filter === 'object' &&
+      filter !== null &&
+      'col' in filter &&
+      'val' in filter &&
+      typeof filter.col === 'string' &&
+      Array.isArray(filter.val),
+  );
+};
+
+const filterId = 'native-filter-1';
+const mockChart = {
+  id: chartId,
+  chartAlert: null,
+  chartStatus: null,
+  chartUpdateEndTime: null,
+  chartUpdateStartTime: 1,
+  lastRendered: 1,
+  latestQueryFormData: {},
+  sliceFormData: null,
+  queryController: null,
+  queriesResponse: null,
+  triggerQuery: false,
+  form_data: {
+    viz_type: 'filter_select',
+    filters: [
+      {
+        col: 'country_name',
+        op: 'IN',
+        val: ['United States'],
+      },
+    ],
+    datasource: '123',
+    url_params: {},
+  },
+};
+const mockArgs: GetFormDataWithExtraFiltersArguments = {
+  chartConfiguration: {},
+  chart: mockChart,
+  filters: {
+    region: ['Spain'],
+    color: ['pink', 'purple'],
+  },
+  sliceId: chartId,
+  nativeFilters: {},
+  dataMask: {
+    [filterId]: {
+      id: filterId,
+      extraFormData: {},
+      filterState: {},
+      ownState: {},
     },
-    sliceId: chartId,
-    nativeFilters: {},
+  },
+  extraControls: {
+    stack: 'Stacked',
+  },
+  allSliceIds: [chartId],
+};
+
+test('should include filters from the passed filters', () => {
+  const result = getFormDataWithExtraFilters(mockArgs);
+  expect(result.extra_filters).toHaveLength(2);
+  expect(result.extra_filters[0]).toEqual({
+    col: 'region',
+    op: 'IN',
+    val: ['Spain'],
+  });
+  expect(result.extra_filters[1]).toEqual({
+    col: 'color',
+    op: 'IN',
+    val: ['pink', 'purple'],
+  });
+});
+
+test('should compose extra control', () => {
+  const result: CachedFormDataWithExtraControls =
+    getFormDataWithExtraFilters(mockArgs);
+  expect(result.stack).toEqual('Stacked');
+});
+
+test('should merge extraFormData from chart customizations', () => {
+  const customizationId = 'CHART_CUSTOMIZATION-1';
+  const argsWithCustomization: GetFormDataWithExtraFiltersArguments = {
+    ...mockArgs,
     dataMask: {
-      [filterId]: {
-        id: filterId,
-        extraFormData: {},
-        filterState: {},
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {
+          time_grain_sqla: 'PT1H',
+        },
+        filterState: {
+          value: ['category1', 'category2'],
+        },
         ownState: {},
       },
     },
-    extraControls: {
-      stack: 'Stacked',
-    },
-    allSliceIds: [chartId],
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+        name: 'Time Grain Customization',
+        filterType: 'chart_customization_time_grain',
+        targets: [
+          {
+            datasetId: 123,
+            column: { name: 'time_column' },
+          },
+        ],
+        scope: {
+          rootPath: [],
+          excluded: [],
+        },
+        chartsInScope: [chartId],
+        defaultDataMask: {},
+        controlValues: {},
+      }),
+    ],
   };
 
-  test('should include filters from the passed filters', () => {
-    const result = getFormDataWithExtraFilters(mockArgs);
-    expect(result.extra_filters).toHaveLength(2);
-    expect(result.extra_filters[0]).toEqual({
-      col: 'region',
-      op: 'IN',
-      val: ['Spain'],
-    });
-    expect(result.extra_filters[1]).toEqual({
-      col: 'color',
-      op: 'IN',
-      val: ['pink', 'purple'],
-    });
-  });
-
-  test('should compose extra control', () => {
-    const result: CachedFormDataWithExtraControls =
-      getFormDataWithExtraFilters(mockArgs);
-    expect(result.stack).toEqual('Stacked');
-  });
-
-  test('should merge extraFormData from chart customizations', () => {
-    const customizationId = 'CHART_CUSTOMIZATION-1';
-    const argsWithCustomization: GetFormDataWithExtraFiltersArguments = {
-      ...mockArgs,
-      dataMask: {
-        [customizationId]: {
-          id: customizationId,
-          extraFormData: {
-            time_grain_sqla: 'PT1H',
-          },
-          filterState: {
-            value: ['category1', 'category2'],
-          },
-          ownState: {},
-        },
-      },
-      chartCustomizationItems: [
-        {
-          id: customizationId,
-          type: 'CHART_CUSTOMIZATION' as any,
-          name: 'Time Grain Customization',
-          filterType: 'chart_customization_time_grain',
-          targets: [
-            {
-              datasetId: 123,
-              column: { name: 'time_column' },
-            },
-          ],
-          scope: {
-            rootPath: [],
-            excluded: [],
-          },
-          chartsInScope: [chartId],
-          defaultDataMask: {},
-          controlValues: {},
-        },
-      ],
-    };
-
-    const result = getFormDataWithExtraFilters(argsWithCustomization);
-    expect((result as any).time_grain_sqla).toEqual('PT1H');
-  });
-
-  test('should merge both filters and customization extraFormData', () => {
-    const customizationId = 'CHART_CUSTOMIZATION-1';
-    const argsWithBoth: GetFormDataWithExtraFiltersArguments = {
-      ...mockArgs,
-      chartConfiguration: {
-        [filterId]: {
-          id: filterId,
-          targets: [
-            {
-              datasetId: 123,
-              column: { name: 'country_name' },
-            },
-          ],
-          scope: { rootPath: ['ROOT_ID'], excluded: [] },
-          cascadeParentIds: [],
-        },
-      } as any,
-      dataMask: {
-        [filterId]: {
-          id: filterId,
-          extraFormData: {
-            filters: [
-              {
-                col: 'country_name',
-                op: 'IN',
-                val: ['United States'],
-              },
-            ],
-          },
-          filterState: {},
-          ownState: {},
-        },
-        [customizationId]: {
-          id: customizationId,
-          extraFormData: {
-            time_grain_sqla: 'PT1H',
-          },
-          filterState: {
-            value: ['category1'],
-          },
-          ownState: {},
-        },
-      },
-      chartCustomizationItems: [
-        {
-          id: customizationId,
-          type: 'CHART_CUSTOMIZATION' as any,
-          name: 'Time Grain Customization',
-          filterType: 'chart_customization_time_grain',
-          targets: [
-            {
-              datasetId: 123,
-              column: { name: 'time_column' },
-            },
-          ],
-          scope: {
-            rootPath: [],
-            excluded: [],
-          },
-          chartsInScope: [chartId],
-          defaultDataMask: {},
-          controlValues: {},
-        },
-      ],
-    };
-
-    const result = getFormDataWithExtraFilters(argsWithBoth);
-    expect((result as any).time_grain_sqla).toEqual('PT1H');
-    expect(result.extra_form_data).toBeDefined();
-  });
+  const result = getFormDataWithExtraFilters(argsWithCustomization);
+  expect(result).toEqual(expect.objectContaining({ time_grain_sqla: 'PT1H' }));
+});
+
+test('should merge both filters and customization extraFormData', () => {
+  const customizationId = 'CHART_CUSTOMIZATION-1';
+  const argsWithBoth: GetFormDataWithExtraFiltersArguments = {
+    ...mockArgs,
+    activeFilters: {
+      [filterId]: {
+        targets: [
+          {
+            datasetId: 123,
+            column: { name: 'country_name' },
+          },
+        ],
+        scope: [chartId],
+        values: {
+          filters: [
+            {
+              col: 'country_name',
+              op: 'IN',
+              val: ['United States'],
+            },
+          ],
+        },
+      },
+    },
+    dataMask: {
+      [filterId]: {
+        id: filterId,
+        extraFormData: {
+          filters: [
+            {
+              col: 'country_name',
+              op: 'IN',
+              val: ['United States'],
+            },
+          ],
+        },
+        filterState: {},
+        ownState: {},
+      },
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {
+          time_grain_sqla: 'PT1H',
+        },
+        filterState: {
+          value: ['category1'],
+        },
+        ownState: {},
+      },
+    },
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+        name: 'Time Grain Customization',
+        filterType: 'chart_customization_time_grain',
+        targets: [
+          {
+            datasetId: 123,
+            column: { name: 'time_column' },
+          },
+        ],
+        scope: {
+          rootPath: [],
+          excluded: [],
+        },
+        chartsInScope: [chartId],
+        defaultDataMask: {},
+        controlValues: {},
+      }),
+    ],
+  };
+
+  const result = getFormDataWithExtraFilters(argsWithBoth);
+  expect(result).toEqual(expect.objectContaining({ time_grain_sqla: 'PT1H' }));
+  expect(result.extra_form_data).toBeDefined();
+});
+
+const makeGroupByArgs = (
+  selectedValue: string | string[],
+  baseGroupby: string[] = [],
+): GetFormDataWithExtraFiltersArguments => {
+  const customizationId = 'CHART_CUSTOMIZATION-groupby-1';
+  return {
+    ...mockArgs,
+    chart: {
+      ...mockChart,
+      form_data: {
+        ...mockChart.form_data,
+        viz_type: 'table',
+        datasource: '3__table',
+        groupby: baseGroupby,
+      },
+    },
+    dataMask: {
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {},
+        filterState: { value: selectedValue },
+        ownState: {},
+      },
+    },
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+      }),
+    ],
+  };
+};
+
+test('dynamic group by does not inject a filter using the selected column name as a value', () => {
+  const result = getFormDataWithExtraFilters(makeGroupByArgs(['status']));
+  const spuriousFilter = getResultFilters(result).find(
+    filter => filter.col === 'status' && filter.val.includes('status'),
+  );
+  expect(spuriousFilter).toBeUndefined();
+  expectGroupBy(result, ['status']);
+});
+
+test('dynamic group by still applies when the selected column is already in the base groupby', () => {
+  const result = getFormDataWithExtraFilters(
+    makeGroupByArgs(['status'], ['status']),
+  );
+  expectGroupBy(result, ['status']);
+});
+
+test('dynamic group by with no selection leaves the base groupby unchanged', () => {
+  const result = getFormDataWithExtraFilters(makeGroupByArgs([], ['status']));
+  expectGroupBy(result, ['status']);
+});
+
+test('dynamic group by ignores empty-string selections and keeps the base groupby', () => {
+  const result = getFormDataWithExtraFilters(
+    makeGroupByArgs(['', 'status'], ['original_column']),
+  );
+  expectGroupBy(result, ['status']);
+});
+
+test('chord chart ignores dynamic group by selections and keeps the existing source unchanged', () => {
+  const result = getFormDataWithExtraFilters({
+    ...makeGroupByArgs(['payment_method'], ['status']),
+    chart: {
+      ...mockChart,
+      form_data: {
+        ...mockChart.form_data,
+        viz_type: 'chord',
+        datasource: '3__table',
+        groupby: ['status'],
+      },
+    },
+  });
+
+  expectGroupBy(result, ['status']);
+});
+
+test('dynamic group by normalizes a single-select string value into a one-item groupby array', () => {
+  const result = getFormDataWithExtraFilters(makeGroupByArgs('status'));
+  expectGroupBy(result, ['status']);
+});
+
+test('structural conflict: metric column blocks groupby override (nonConflictingColumns guard)', () => {
+  const customizationId = 'CHART_CUSTOMIZATION-groupby-conflict';
+  const argsWithMetricConflict: GetFormDataWithExtraFiltersArguments = {
+    ...mockArgs,
+    chart: {
+      ...mockChart,
+      form_data: {
+        ...mockChart.form_data,
+        viz_type: 'table',
+        datasource: '3__table',
+        groupby: ['original_column'],
+        metrics: ['revenue'],
+      },
+    },
+    dataMask: {
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {},
+        filterState: { value: ['revenue'] },
+        ownState: {},
+      },
+    },
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+        targets: [{ datasetId: 3, column: { name: 'revenue' } }],
+      }),
+    ],
+  };
+
+  const result = getFormDataWithExtraFilters(argsWithMetricConflict);
+  expectGroupBy(result, ['original_column']);
+});
+
+test('multi-column selection: all selected columns appear in result groupby', () => {
+  const result = getFormDataWithExtraFilters(
+    makeGroupByArgs(['status', 'product_line']),
+  );
+  expectGroupBy(result, expect.arrayContaining(['status', 'product_line']));
+  expectGroupByLength(result, 2);
+});
+
+test('dataset mismatch: display control for a different dataset does not affect the chart', () => {
+  const customizationId = 'CHART_CUSTOMIZATION-wrong-dataset';
+  const argsWrongDataset: GetFormDataWithExtraFiltersArguments = {
+    ...mockArgs,
+    chart: {
+      ...mockChart,
+      form_data: {
+        ...mockChart.form_data,
+        viz_type: 'table',
+        datasource: '3__table',
+        groupby: ['original_column'],
+      },
+    },
+    dataMask: {
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {},
+        filterState: { value: ['status'] },
+        ownState: {},
+      },
+    },
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+        targets: [{ datasetId: 999, column: { name: 'status' } }],
+      }),
+    ],
+  };
+
+  const result = getFormDataWithExtraFilters(argsWrongDataset);
+  expectGroupBy(result, ['original_column']);
+});
+
+test('Scope boundary: display control with chartsInScope:[] does not affect the chart', () => {
+  const customizationId = 'CHART_CUSTOMIZATION-groupby-out-of-scope';
+  const argsOutOfScope: GetFormDataWithExtraFiltersArguments = {
+    ...mockArgs,
+    chart: {
+      ...mockChart,
+      form_data: {
+        ...mockChart.form_data,
+        viz_type: 'table',
+        datasource: '3__table',
+        groupby: ['original_column'],
+      },
+    },
+    dataMask: {
+      [customizationId]: {
+        id: customizationId,
+        extraFormData: {},
+        filterState: { value: ['replacement_column'] },
+        ownState: {},
+      },
+    },
+    chartCustomizationItems: [
+      createChartCustomization({
+        id: customizationId,
+        name: 'Out Of Scope Group By',
+        chartsInScope: [],
+      }),
+    ],
+  };
+
+  const result = getFormDataWithExtraFilters(argsOutOfScope);
+  expectGroupBy(result, ['original_column']);
 });