fix(test): make optional chains explicit for tsc strictness

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

superset-frontend/playwright/tests/dashboard/clear-all-filters.spec.ts

@@ -0,0 +1,220 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { testWithAssets, expect } from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import { apiPostDashboard } from '../../helpers/api/dashboard';
+import { DashboardPage } from '../../pages/DashboardPage';
+
+const DATASET_NAME = 'birth_names';
+const FILTER_COLUMN = 'gender';
+
+async function findDatasetIdByName(page: any, name: string): Promise<number> {
+  const rison = `(filters:!((col:table_name,opr:eq,value:'${name}')))`;
+  const resp = await page.request.get(`api/v1/dataset/?q=${rison}`);
+  const body = await resp.json();
+  if (!body.result?.length) {
+    throw new Error(`Dataset ${name} not found`);
+  }
+  return body.result[0].id;
+}
+
+testWithAssets(
+  'Clear all filters waits for Apply (sc-105059)',
+  async ({ page, testAssets }) => {
+    const datasetId = await findDatasetIdByName(page, DATASET_NAME);
+
+    // Create a chart that the dashboard filter will target
+    const chartParams = {
+      datasource: `${datasetId}__table`,
+      viz_type: 'big_number_total',
+      metric: 'count',
+      adhoc_filters: [],
+      header_font_size: 0.4,
+      subheader_font_size: 0.15,
+    };
+    const chartResp = await apiPost(page, 'api/v1/chart/', {
+      slice_name: `clear_all_repro_${Date.now()}`,
+      viz_type: 'big_number_total',
+      datasource_id: datasetId,
+      datasource_type: 'table',
+      params: JSON.stringify(chartParams),
+    });
+    expect(chartResp.ok()).toBe(true);
+    const chart = await chartResp.json();
+    const chartId: number = chart.id ?? chart.result?.id;
+    testAssets.trackChart(chartId);
+
+    // Create dashboard with chart in position_json and a native filter in json_metadata
+    const filterId = `NATIVE_FILTER-${Math.random().toString(36).slice(2, 10)}`;
+    const chartLayoutKey = `CHART-${chartId}`;
+    const positionJson = {
+      DASHBOARD_VERSION_KEY: 'v2',
+      ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] },
+      GRID_ID: {
+        type: 'GRID',
+        id: 'GRID_ID',
+        children: ['ROW-1'],
+        parents: ['ROOT_ID'],
+      },
+      'ROW-1': {
+        type: 'ROW',
+        id: 'ROW-1',
+        children: [chartLayoutKey],
+        parents: ['ROOT_ID', 'GRID_ID'],
+        meta: { background: 'BACKGROUND_TRANSPARENT' },
+      },
+      [chartLayoutKey]: {
+        type: 'CHART',
+        id: chartLayoutKey,
+        children: [],
+        parents: ['ROOT_ID', 'GRID_ID', 'ROW-1'],
+        meta: {
+          chartId,
+          width: 6,
+          height: 50,
+          sliceName: 'clear_all_repro',
+        },
+      },
+    };
+
+    const jsonMetadata = {
+      native_filter_configuration: [
+        {
+          id: filterId,
+          name: 'Gender',
+          filterType: 'filter_select',
+          type: 'NATIVE_FILTER',
+          targets: [
+            {
+              datasetId,
+              column: { name: FILTER_COLUMN },
+            },
+          ],
+          controlValues: {
+            multiSelect: false,
+            enableEmptyFilter: false,
+            defaultToFirstItem: false,
+            inverseSelection: false,
+            searchAllOptions: false,
+          },
+          defaultDataMask: { filterState: {}, extraFormData: {} },
+          cascadeParentIds: [],
+          scope: { rootPath: ['ROOT_ID'], excluded: [] },
+          chartsInScope: [chartId],
+        },
+      ],
+      chart_configuration: {},
+      cross_filters_enabled: false,
+      global_chart_configuration: {
+        scope: { rootPath: ['ROOT_ID'], excluded: [] },
+        chartsInScope: [chartId],
+      },
+    };
+
+    const dashResp = await apiPostDashboard(page, {
+      dashboard_title: `clear_all_repro_${Date.now()}`,
+      published: true,
+      position_json: JSON.stringify(positionJson),
+      json_metadata: JSON.stringify(jsonMetadata),
+    });
+    expect(dashResp.ok()).toBe(true);
+    const dashBody = await dashResp.json();
+    const dashboardId: number = dashBody.result?.id ?? dashBody.id;
+    testAssets.trackDashboard(dashboardId);
+
+    // Associate chart with the dashboard so it actually renders
+    const linkResp = await apiPut(page, `api/v1/chart/${chartId}`, {
+      dashboards: [dashboardId],
+    });
+    expect(linkResp.ok()).toBe(true);
+
+    // Visit dashboard
+    const dashboardPage = new DashboardPage(page);
+    await dashboardPage.gotoById(dashboardId);
+    await dashboardPage.waitForLoad();
+    await dashboardPage.waitForChartsToLoad();
+
+    // The Gender select should be visible in the filter bar
+    const filterCombobox = page
+      .locator('[data-test="form-item-value"]')
+      .first()
+      .locator('[role="combobox"]');
+    await filterCombobox.click();
+    await page
+      .locator('.ant-select-item-option', { hasText: /^boy$/ })
+      .first()
+      .click();
+    // Close the dropdown
+    await page.keyboard.press('Escape');
+
+    const applyBtn = page.locator(
+      '[data-test="filter-bar__apply-button"], [data-test="filterbar-action-buttons"] button[type="submit"]',
+    );
+
+    // Wait for chart data to come back after Apply
+    const firstApplyResponse = page.waitForResponse(
+      r =>
+        r.url().includes('/api/v1/chart/data') &&
+        r.request().method() === 'POST',
+      { timeout: 10_000 },
+    );
+    await applyBtn.first().click();
+    await firstApplyResponse;
+    await dashboardPage.waitForChartsToLoad();
+
+    // Now track POST /api/v1/chart/data requests around Clear All
+    const postsAfterClearAll: string[] = [];
+    const handler = (req: any) => {
+      if (
+        req.url().includes('/api/v1/chart/data') &&
+        req.method() === 'POST'
+      ) {
+        postsAfterClearAll.push(req.url());
+      }
+    };
+    page.on('request', handler);
+
+    const clearBtn = page.locator('[data-test="filter-bar__clear-button"]');
+    await clearBtn.click();
+
+    // Allow time for any debounced reload to fire if the bug is present
+    await page.waitForTimeout(2000);
+
+    page.off('request', handler);
+
+    // BUG: on master, the Clear All triggers an immediate dispatch which
+    // re-runs the chart query before the user clicks Apply. After the fix,
+    // no chart/data request should fire until Apply is clicked.
+    expect(
+      postsAfterClearAll,
+      'Clear All must not reload charts until Apply is clicked',
+    ).toEqual([]);
+
+    // After Apply, the chart should reload
+    const applyAfterClearPromise = page.waitForResponse(
+      r =>
+        r.url().includes('/api/v1/chart/data') &&
+        r.request().method() === 'POST',
+      { timeout: 10_000 },
+    );
+    await applyBtn.first().click();
+    await applyAfterClearPromise;
+  },
+);

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx

@@ -486,7 +486,7 @@ test('FilterBar renders correctly when filter has complete extraFormData', async
   expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
 });
 
-test('handleClearAll dispatches updateDataMask with value undefined for filter_select', async () => {
+test('Clear All stages filter_select clear without dispatching until Apply', async () => {
   const filterId = 'NATIVE_FILTER-clear-select';
   const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
   const selectFilter = createFilter({
@@ -513,7 +513,9 @@ test('handleClearAll dispatches updateDataMask with value undefined for filter_s
       activeTabs: ['ROOT_ID'],
     },
     dataMask: {
-      [filterId]: createDataMask(filterId, ['East']),
+      [filterId]: createDataMask(filterId, ['East'], {
+        filters: [{ col: 'region', op: 'IN', val: ['East'] }],
+      }),
     },
     nativeFilters: {
       filters: { [filterId]: selectFilter },
@@ -533,14 +535,24 @@ test('handleClearAll dispatches updateDataMask with value undefined for filter_s
     userEvent.click(clearBtn);
   });
 
+  // Clear All must not dispatch — staging only
+  expect(updateDataMaskSpy).not.toHaveBeenCalled();
+
+  // Apply commits the staged clear
+  const applyBtn = screen.getByTestId(getTestId('apply-button'));
+  expect(applyBtn).not.toBeDisabled();
+  await act(async () => {
+    userEvent.click(applyBtn);
+  });
   expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
-    filterState: { value: undefined },
+    id: filterId,
+    filterState: { value: undefined, validateStatus: undefined },
     extraFormData: {},
   });
   updateDataMaskSpy.mockRestore();
 });
 
-test('handleClearAll dispatches updateDataMask with [null, null] for filter_range', async () => {
+test('Clear All stages filter_range clear with [null, null], dispatched on Apply', async () => {
   fetchMock.post('glob:*/api/v1/chart/data', {
     result: [{ data: [{ min: 0, max: 100 }] }],
   });
@@ -570,7 +582,9 @@ test('handleClearAll dispatches updateDataMask with [null, null] for filter_rang
       activeTabs: ['ROOT_ID'],
     },
     dataMask: {
-      [filterId]: createDataMask(filterId, [10, 50]),
+      [filterId]: createDataMask(filterId, [10, 50], {
+        filters: [{ col: 'age', op: '>=', val: 10 }],
+      }),
     },
     nativeFilters: {
       filters: { [filterId]: rangeFilter },
@@ -590,14 +604,21 @@ test('handleClearAll dispatches updateDataMask with [null, null] for filter_rang
     userEvent.click(clearBtn);
   });
 
+  expect(updateDataMaskSpy).not.toHaveBeenCalled();
+
+  const applyBtn = screen.getByTestId(getTestId('apply-button'));
+  await act(async () => {
+    userEvent.click(applyBtn);
+  });
   expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, {
-    filterState: { value: [null, null] },
+    id: filterId,
+    filterState: { value: [null, null], validateStatus: undefined },
     extraFormData: {},
   });
   updateDataMaskSpy.mockRestore();
 });
 
-test('handleClearAll only dispatches for filters present in dataMask', async () => {
+test('Clear All + Apply only dispatches for filters present in dataMask', async () => {
   const idInMask = 'NATIVE_FILTER-has-value';
   const idNotInMask = 'NATIVE_FILTER-no-value';
   const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
@@ -631,7 +652,9 @@ test('handleClearAll only dispatches for filters present in dataMask', async ()
       activeTabs: ['ROOT_ID'],
     },
     dataMask: {
-      [idInMask]: createDataMask(idInMask, ['v']),
+      [idInMask]: createDataMask(idInMask, ['v'], {
+        filters: [{ col: 'x', op: 'IN', val: ['v'] }],
+      }),
     },
     nativeFilters: {
       filters: {
@@ -652,10 +675,16 @@ test('handleClearAll only dispatches for filters present in dataMask', async ()
   await act(async () => {
     userEvent.click(clearBtn);
   });
+  expect(updateDataMaskSpy).not.toHaveBeenCalled();
 
+  const applyBtn = screen.getByTestId(getTestId('apply-button'));
+  await act(async () => {
+    userEvent.click(applyBtn);
+  });
   expect(updateDataMaskSpy).toHaveBeenCalledTimes(1);
   expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, {
-    filterState: { value: undefined },
+    id: idInMask,
+    filterState: { value: undefined, validateStatus: undefined },
     extraFormData: {},
   });
   updateDataMaskSpy.mockRestore();
@@ -790,18 +819,86 @@ test('FilterBar Clear All only clears in-scope filters, not out-of-scope ones',
   await act(async () => {
     userEvent.click(clearButton);
   });
+  expect(updateDataMaskSpy).not.toHaveBeenCalled();
 
-  // Verify only the in-scope filter was cleared, not the out-of-scope ones
-  const clearedFilterIds = updateDataMaskSpy.mock.calls.map(call => call[0]);
-  expect(clearedFilterIds).toContain(inScopeFilterId);
-  expect(clearedFilterIds).not.toContain(outOfScopeRequiredFilterId);
-  expect(clearedFilterIds).not.toContain(outOfScopeNonRequiredFilterId);
+  // After Apply: only the in-scope filter was cleared. Out-of-scope filters
+  // retain their original values (Apply re-dispatches them unchanged).
+  const applyButton = screen.getByTestId(getTestId('apply-button'));
+  await act(async () => {
+    userEvent.click(applyButton);
+  });
 
-  // Verify the in-scope filter was cleared with the correct value
   expect(updateDataMaskSpy).toHaveBeenCalledWith(inScopeFilterId, {
-    filterState: { value: undefined },
+    id: inScopeFilterId,
+    filterState: { value: undefined, validateStatus: undefined },
     extraFormData: {},
   });
 
+  // Out-of-scope filters keep their existing values; not cleared
+  const outOfScopeRequiredCall = updateDataMaskSpy.mock.calls.find(
+    call => call[0] === outOfScopeRequiredFilterId,
+  );
+  expect(outOfScopeRequiredCall?.[1]?.filterState?.value).toEqual(['value2']);
+  const outOfScopeNonRequiredCall = updateDataMaskSpy.mock.calls.find(
+    call => call[0] === outOfScopeNonRequiredFilterId,
+  );
+  expect(outOfScopeNonRequiredCall?.[1]?.filterState?.value).toEqual([
+    'value3',
+  ]);
+
+  updateDataMaskSpy.mockRestore();
+});
+
+test('Clear All on a required filter disables Apply via validateStatus', async () => {
+  const filterId = 'NATIVE_FILTER-required-clear';
+  const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
+  const requiredFilter = createFilter({
+    id: filterId,
+    name: 'Required Region',
+    filterType: 'filter_select',
+    targets: [{ datasetId: 7, column: { name: 'region' } }],
+    controlValues: { enableEmptyFilter: true },
+    chartsInScope: [18],
+  });
+  const state = {
+    ...stateWithoutNativeFilters,
+    dashboardInfo: {
+      id: 1,
+      dash_edit_perm: true,
+      filterBarOrientation: FilterBarOrientation.Vertical,
+      metadata: {
+        native_filter_configuration: [requiredFilter],
+        chart_configuration: {},
+      },
+    },
+    dashboardState: {
+      ...stateWithoutNativeFilters.dashboardState,
+      activeTabs: ['ROOT_ID'],
+    },
+    dataMask: {
+      [filterId]: createDataMask(filterId, ['East'], {
+        filters: [{ col: 'region', op: 'IN', val: ['East'] }],
+      }),
+    },
+    nativeFilters: {
+      filters: { [filterId]: requiredFilter },
+      filtersState: {},
+    },
+  };
+
+  const props = createOpenedBarProps();
+  renderFilterBar(props, state);
+  await act(async () => {
+    jest.advanceTimersByTime(300);
+  });
+
+  const clearBtn = screen.getByTestId(getTestId('clear-button'));
+  await act(async () => {
+    userEvent.click(clearBtn);
+  });
+
+  // No dispatch yet; Apply should be disabled because the required filter is empty
+  expect(updateDataMaskSpy).not.toHaveBeenCalled();
+  expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
   updateDataMaskSpy.mockRestore();
 });

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx

@@ -498,17 +498,20 @@ const FilterBar: FC<FiltersBarProps> = ({
       // Range filters use [null, null] as the cleared value; others use undefined
       const clearedValue =
         filterType === 'filter_range' ? [null, null] : undefined;
-      const clearedDataMask = {
-        filterState: { value: clearedValue },
-        extraFormData: {},
-      };
+      const isRequired = !!filter.controlValues?.enableEmptyFilter;
       if (dataMaskSelected[id]) {
-        dispatch(updateDataMask(id, clearedDataMask));
+        // Stage the cleared value locally; do NOT dispatch to Redux here.
+        // Persistence happens when the user clicks Apply.
         setDataMaskSelected(draft => {
           if (draft[id].filterState?.value !== undefined) {
             draft[id].filterState!.value = clearedValue;
           }
           draft[id].extraFormData = {};
+          if (draft[id].filterState) {
+            draft[id].filterState!.validateStatus = isRequired
+              ? 'error'
+              : undefined;
+          }
         });
         newClearAllTriggers[id] = true;
       }