Re: [PR] fix(native-filters): preserve selected filter state [superset]

["sadpandajoe (via GitHub)" <gi...@apache.org>]

sadpandajoe commented on code in PR #44803:
URL: https://github.com/apache/superset/pull/44803#discussion_r4159026430


##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx:
##########
@@ -1411,3 +1413,73 @@ test('FilterBar with orientation=Vertical renders Vertical layout (sanity counte
     screen.queryByRole('img', { name: 'setting' }),
   ).not.toBeInTheDocument();
 });
+
+test('FilterBar keeps a configured filter selected when its applied data mask is removed', async () => {
+  fetchMock.post(
+    'glob:*/api/v1/chart/data',
+    {
+      result: [
+        {
+          data: [{ region: 'East' }, { region: 'West' }],
+          colnames: ['region'],
+          coltypes: [1],
+          applied_filters: [],
+        },
+      ],
+    },
+    { name: 'configured-filter-selected-chart-data' },
+  );
+
+  const filterId = 'NATIVE_FILTER-keep-selected';
+  const filter = createFilter({
+    id: filterId,
+    name: 'Region',
+    filterType: 'filter_select',
+    targets: [{ datasetId: 7, column: { name: 'region' } }],
+    chartsInScope: [18],
+  });
+
+  const state = createStateWithFilter(
+    filter,
+    createDataMask(filterId, ['East'], {
+      filters: [{ col: 'region', op: 'IN', val: ['East'] }],
+    }),
+    {
+      filterBarOrientation: FilterBarOrientation.Vertical,
+      metadata: {
+        native_filter_configuration: [filter],
+        chart_configuration: {},
+      },
+    },
+  );
+
+  const store = createStore(state, reducerIndex);
+
+  render(
+    <FilterBar
+      orientation={FilterBarOrientation.Vertical}
+      verticalConfig={{
+        width: 280,
+        height: 400,
+        offset: 0,
+        ...createOpenedBarProps(),
+      }}
+    />,
+    { store, useDnd: true, useRouter: true },
+  );
+
+  await act(async () => {
+    jest.advanceTimersByTime(1000);
+  });
+
+  expect(screen.getByText('Region')).toBeInTheDocument();
+  expect(screen.getByTitle('East')).toBeInTheDocument();
+
+  await act(async () => {

Review Comment:
   This test can pass without fixing #44680’s horizontal time-filter flow: it removes an applied Select mask, whereas the reported failure loses the displayed range while the applied time mask remains present. Could we add a real-store test selecting Current month through the time popup, checking that dashboard Apply enables, then asserting the applied Redux range, retained label, and Clear All behavior, with fail-on-base/pass-on-head verification?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org