bito-code-review[bot] commented on code in PR #43108:
URL: https://github.com/apache/superset/pull/43108#discussion_r4174691773
##########
superset-frontend/src/dashboard/containers/DashboardPage.test.tsx:
##########
@@ -630,3 +631,628 @@ test('clears undo history after hydrating the dashboard',
async () => {
.invocationCallOrder[0];
expect(clearOrder).toBeGreaterThan(hydrateOrder);
});
+
+// ---------------------------------------------------------------------------
+// localStorage filter persistence
+// ---------------------------------------------------------------------------
+
+test('restores native filter state from localStorage when no URL key is
present', async () => {
+ // Versioned format: { dataMask, filterDefinitions }. The current code
rejects
+ // unversioned entries (ID-only check) since the key has not shipped yet.
+ const savedVersioned = {
+ dataMask: {
+ 'NATIVE_FILTER-abc123': {
+ filterState: { value: ['California'] },
+ extraFormData: {
+ filters: [{ col: 'state', op: 'IN', val: ['California'] }],
+ },
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-abc123': {
+ targets: [{ column: { name: 'state' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+ // Authenticated user — key format:
dashboard__native_filters__{userId}__{dashboardId}
+ localStorage.setItem(
+ 'dashboard__native_filters__42__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ // Include the filter ID (with matching targets/type) so versioned
validation passes.
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-abc123',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'state' } }],
+ },
+ ],
+ },
+ },
+ error: null,
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 42 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-abc123': expect.objectContaining({
+ filterState: { value: ['California'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('skips localStorage restore for guest/embedded users (userId is
undefined)', async () => {
+ const FILTER_ID = 'NATIVE_FILTER-guest';
+ const filterConfig = {
+ id: FILTER_ID,
+ name: 'Team',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'col' } }],
+ defaultDataMask: {},
+ };
+
+ // Give the dashboard a matching native filter definition so that it would
be restored if not a guest
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [filterConfig],
+ },
+ json_metadata: JSON.stringify({
+ native_filter_configuration: [filterConfig],
+ }),
+ },
+ error: null,
+ });
+
+ const nativeFilters = {
+ filters: {
+ [FILTER_ID]: filterConfig,
+ },
+ };
+
+ const savedVersioned = {
+ dataMask: {
+ [FILTER_ID]: {
+ id: FILTER_ID,
+ filterState: { value: ['SomeValue'] },
+ extraFormData: {},
+ },
+ },
+ filterDefinitions: {
+ [FILTER_ID]: {
+ id: FILTER_ID,
+ targets: filterConfig.targets,
+ type: filterConfig.filterType,
+ defaultDataMask: filterConfig.defaultDataMask,
+ },
+ },
+ };
+
+ // Write under the guest (dashboard-only) key
+ localStorage.setItem(
+ 'dashboard__native_filters__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: {
+ id: 1,
+ metadata: { native_filter_configuration: [filterConfig] },
+ },
+ dashboardState: { sliceIds: [] },
+ nativeFilters,
+ dataMask: {},
+ user: { userId: undefined },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // dataMask should be empty — guest users skip localStorage restoration
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({ dataMask: {} }),
+ );
+
+ window.localStorage.clear();
+});
+
+test('scopes localStorage key to userId when user is authenticated', async ()
=> {
+ const savedVersioned = {
+ dataMask: {
+ 'NATIVE_FILTER-xyz': {
+ filterState: { value: ['2024'] },
+ extraFormData: {},
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-xyz': {
+ targets: [{ column: { name: 'year' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+ // key is scoped to userId=7 and dashboardId=1
+ localStorage.setItem(
+ 'dashboard__native_filters__7__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ // Include the filter ID in native_filter_configuration so versioned
validation passes.
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-xyz',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'year' } }],
+ },
+ ],
+ },
+ },
+ error: null,
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 7 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-xyz': expect.objectContaining({
+ filterState: { value: ['2024'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('does not restore localStorage filters when a nativeFiltersKey is in the
URL', async () => {
+ // Put something in localStorage that should be ignored because the URL key
takes priority
+ localStorage.setItem(
+ 'dashboard__native_filters__1',
+ JSON.stringify({ 'NATIVE_FILTER-abc': { filterState: { value: ['X'] } } }),
+ );
+
+ const { getFilterValue } = jest.requireMock(
+ 'src/dashboard/components/nativeFilters/FilterBar/keyValue',
+ );
+ (getFilterValue as jest.Mock).mockResolvedValueOnce({
+ 'NATIVE_FILTER-abc': { filterState: { value: ['FromURL'] } },
+ });
+
+ mockGetUrlParam.mockImplementation((param: { name: string }) => {
+ if (param.name === 'native_filters_key') return 'some-key';
+ return null;
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: undefined },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // hydrateDashboard should use the URL-resolved value, not localStorage
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-abc': expect.objectContaining({
+ filterState: { value: ['FromURL'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('ignores corrupted localStorage data (array) and uses empty dataMask',
async () => {
+ // An array is not a valid dataMask shape and must be rejected
+ localStorage.setItem(
+ 'dashboard__native_filters__42__1',
+ JSON.stringify([1, 2, 3]),
+ );
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 42 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // dataMask should be empty — the corrupted array value must not be used
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({ dataMask: {} }),
+ );
+});
+
+test('restores versioned localStorage filters and drops them if targets
change', async () => {
+ const savedVersionedData = {
+ dataMask: {
+ 'NATIVE_FILTER-versioned': {
+ filterState: { value: ['California'] },
+ extraFormData: {
+ filters: [{ col: 'state', op: 'IN', val: ['California'] }],
+ },
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-versioned': {
+ targets: [{ column: { name: 'state' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+
+ // Use an authenticated user key — restoration skips unauthenticated users.
+ localStorage.setItem(
+ 'dashboard__native_filters__5__1',
+ JSON.stringify(savedVersionedData),
+ );
+
+ // 1. Simulate the dashboard where the target matches
+ mockUseDashboard.mockReturnValueOnce({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-versioned',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'state' } }],
+ },
+ ],
+ },
+ },
+ });
+
+ const { render } = jest.requireActual('spec/helpers/testing-library');
+ const { unmount } = render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 5 },
+ },
+ },
+ );
+
+ // hydrateDashboard should be called with the restored value since target
matches
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-versioned': expect.anything(),
+ }),
+ }),
+ );
+
+ unmount();
+ (hydrateDashboard as jest.Mock).mockClear();
+
+ // 2. Simulate the dashboard where the target has changed
+ mockUseDashboard.mockReturnValueOnce({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-versioned',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'country' } }],
+ },
+ ],
+ },
+ },
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 5 },
+ },
+ },
+ );
+
+ // hydrateDashboard should NOT have the dropped filter since target mismatch
+ expect(hydrateDashboard).not.toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-versioned': expect.anything(),
+ }),
+ }),
+ );
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-670: Vacuous negative assertion</b></div>
<div id="fix">
The negative assertion `not.toHaveBeenCalledWith` also passes if
`hydrateDashboard` is never called in scenario 2, so the test can pass without
exercising the target-mismatch drop logic in `getDataMaskApplied`. Add a
positive control first (e.g. `toHaveBeenCalledWith(expect.objectContaining({
dataMask: {} }))`) to prove hydration ran.
([CWE-670](https://cwe.mitre.org/data/definitions/670.html))
</div>
</div>
<small><i>Code Review Run #c42069</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/containers/DashboardPage.test.tsx:
##########
@@ -630,3 +631,628 @@ test('clears undo history after hydrating the dashboard',
async () => {
.invocationCallOrder[0];
expect(clearOrder).toBeGreaterThan(hydrateOrder);
});
+
+// ---------------------------------------------------------------------------
+// localStorage filter persistence
+// ---------------------------------------------------------------------------
+
+test('restores native filter state from localStorage when no URL key is
present', async () => {
+ // Versioned format: { dataMask, filterDefinitions }. The current code
rejects
+ // unversioned entries (ID-only check) since the key has not shipped yet.
+ const savedVersioned = {
+ dataMask: {
+ 'NATIVE_FILTER-abc123': {
+ filterState: { value: ['California'] },
+ extraFormData: {
+ filters: [{ col: 'state', op: 'IN', val: ['California'] }],
+ },
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-abc123': {
+ targets: [{ column: { name: 'state' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+ // Authenticated user — key format:
dashboard__native_filters__{userId}__{dashboardId}
+ localStorage.setItem(
+ 'dashboard__native_filters__42__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ // Include the filter ID (with matching targets/type) so versioned
validation passes.
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-abc123',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'state' } }],
+ },
+ ],
+ },
+ },
+ error: null,
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 42 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-abc123': expect.objectContaining({
+ filterState: { value: ['California'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('skips localStorage restore for guest/embedded users (userId is
undefined)', async () => {
+ const FILTER_ID = 'NATIVE_FILTER-guest';
+ const filterConfig = {
+ id: FILTER_ID,
+ name: 'Team',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'col' } }],
+ defaultDataMask: {},
+ };
+
+ // Give the dashboard a matching native filter definition so that it would
be restored if not a guest
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [filterConfig],
+ },
+ json_metadata: JSON.stringify({
+ native_filter_configuration: [filterConfig],
+ }),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Dead mock fixture field</b></div>
<div id="fix">
The `json_metadata` field set on the `useDashboard` mock result is never
consumed: `DashboardPage.tsx` reads filter config from
`dashboard.metadata.native_filter_configuration`, and `hydrateDashboard` is
mocked in this suite. This dead setup implies `json_metadata` drives behavior
when it does not — remove it to keep the fixture minimal.
</div>
</div>
<small><i>Code Review Run #c42069</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/containers/DashboardPage.test.tsx:
##########
@@ -630,3 +631,628 @@ test('clears undo history after hydrating the dashboard',
async () => {
.invocationCallOrder[0];
expect(clearOrder).toBeGreaterThan(hydrateOrder);
});
+
+// ---------------------------------------------------------------------------
+// localStorage filter persistence
+// ---------------------------------------------------------------------------
+
+test('restores native filter state from localStorage when no URL key is
present', async () => {
+ // Versioned format: { dataMask, filterDefinitions }. The current code
rejects
+ // unversioned entries (ID-only check) since the key has not shipped yet.
+ const savedVersioned = {
+ dataMask: {
+ 'NATIVE_FILTER-abc123': {
+ filterState: { value: ['California'] },
+ extraFormData: {
+ filters: [{ col: 'state', op: 'IN', val: ['California'] }],
+ },
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-abc123': {
+ targets: [{ column: { name: 'state' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+ // Authenticated user — key format:
dashboard__native_filters__{userId}__{dashboardId}
+ localStorage.setItem(
+ 'dashboard__native_filters__42__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ // Include the filter ID (with matching targets/type) so versioned
validation passes.
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-abc123',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'state' } }],
+ },
+ ],
+ },
+ },
+ error: null,
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 42 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-abc123': expect.objectContaining({
+ filterState: { value: ['California'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('skips localStorage restore for guest/embedded users (userId is
undefined)', async () => {
+ const FILTER_ID = 'NATIVE_FILTER-guest';
+ const filterConfig = {
+ id: FILTER_ID,
+ name: 'Team',
+ filterType: 'filter_select',
+ targets: [{ datasetId: 1, column: { name: 'col' } }],
+ defaultDataMask: {},
+ };
+
+ // Give the dashboard a matching native filter definition so that it would
be restored if not a guest
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [filterConfig],
+ },
+ json_metadata: JSON.stringify({
+ native_filter_configuration: [filterConfig],
+ }),
+ },
+ error: null,
+ });
+
+ const nativeFilters = {
+ filters: {
+ [FILTER_ID]: filterConfig,
+ },
+ };
+
+ const savedVersioned = {
+ dataMask: {
+ [FILTER_ID]: {
+ id: FILTER_ID,
+ filterState: { value: ['SomeValue'] },
+ extraFormData: {},
+ },
+ },
+ filterDefinitions: {
+ [FILTER_ID]: {
+ id: FILTER_ID,
+ targets: filterConfig.targets,
+ type: filterConfig.filterType,
+ defaultDataMask: filterConfig.defaultDataMask,
+ },
+ },
+ };
+
+ // Write under the guest (dashboard-only) key
+ localStorage.setItem(
+ 'dashboard__native_filters__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: {
+ id: 1,
+ metadata: { native_filter_configuration: [filterConfig] },
+ },
+ dashboardState: { sliceIds: [] },
+ nativeFilters,
+ dataMask: {},
+ user: { userId: undefined },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // dataMask should be empty — guest users skip localStorage restoration
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({ dataMask: {} }),
+ );
+
+ window.localStorage.clear();
+});
+
+test('scopes localStorage key to userId when user is authenticated', async ()
=> {
+ const savedVersioned = {
+ dataMask: {
+ 'NATIVE_FILTER-xyz': {
+ filterState: { value: ['2024'] },
+ extraFormData: {},
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-xyz': {
+ targets: [{ column: { name: 'year' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+ // key is scoped to userId=7 and dashboardId=1
+ localStorage.setItem(
+ 'dashboard__native_filters__7__1',
+ JSON.stringify(savedVersioned),
+ );
+
+ // Include the filter ID in native_filter_configuration so versioned
validation passes.
+ mockUseDashboard.mockReturnValue({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-xyz',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'year' } }],
+ },
+ ],
+ },
+ },
+ error: null,
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 7 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-xyz': expect.objectContaining({
+ filterState: { value: ['2024'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('does not restore localStorage filters when a nativeFiltersKey is in the
URL', async () => {
+ // Put something in localStorage that should be ignored because the URL key
takes priority
+ localStorage.setItem(
+ 'dashboard__native_filters__1',
+ JSON.stringify({ 'NATIVE_FILTER-abc': { filterState: { value: ['X'] } } }),
+ );
+
+ const { getFilterValue } = jest.requireMock(
+ 'src/dashboard/components/nativeFilters/FilterBar/keyValue',
+ );
+ (getFilterValue as jest.Mock).mockResolvedValueOnce({
+ 'NATIVE_FILTER-abc': { filterState: { value: ['FromURL'] } },
+ });
+
+ mockGetUrlParam.mockImplementation((param: { name: string }) => {
+ if (param.name === 'native_filters_key') return 'some-key';
+ return null;
+ });
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: undefined },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // hydrateDashboard should use the URL-resolved value, not localStorage
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({
+ dataMask: expect.objectContaining({
+ 'NATIVE_FILTER-abc': expect.objectContaining({
+ filterState: { value: ['FromURL'] },
+ }),
+ }),
+ }),
+ );
+});
+
+test('ignores corrupted localStorage data (array) and uses empty dataMask',
async () => {
+ // An array is not a valid dataMask shape and must be rejected
+ localStorage.setItem(
+ 'dashboard__native_filters__42__1',
+ JSON.stringify([1, 2, 3]),
+ );
+
+ render(
+ <Suspense fallback="loading">
+ <DashboardPage idOrSlug="1" />
+ </Suspense>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: {
+ dashboardInfo: { id: 1, metadata: {} },
+ dashboardState: { sliceIds: [] },
+ nativeFilters: { filters: {} },
+ dataMask: {},
+ user: { userId: 42 },
+ },
+ },
+ );
+
+ await waitFor(() => {
+ expect(screen.queryByText('loading')).not.toBeInTheDocument();
+ });
+
+ // dataMask should be empty — the corrupted array value must not be used
+ expect(hydrateDashboard).toHaveBeenCalledWith(
+ expect.objectContaining({ dataMask: {} }),
+ );
+});
+
+test('restores versioned localStorage filters and drops them if targets
change', async () => {
+ const savedVersionedData = {
+ dataMask: {
+ 'NATIVE_FILTER-versioned': {
+ filterState: { value: ['California'] },
+ extraFormData: {
+ filters: [{ col: 'state', op: 'IN', val: ['California'] }],
+ },
+ },
+ },
+ filterDefinitions: {
+ 'NATIVE_FILTER-versioned': {
+ targets: [{ column: { name: 'state' } }],
+ type: 'filter_select',
+ },
+ },
+ };
+
+ // Use an authenticated user key — restoration skips unauthenticated users.
+ localStorage.setItem(
+ 'dashboard__native_filters__5__1',
+ JSON.stringify(savedVersionedData),
+ );
+
+ // 1. Simulate the dashboard where the target matches
+ mockUseDashboard.mockReturnValueOnce({
+ result: {
+ ...mockDashboard,
+ metadata: {
+ native_filter_configuration: [
+ {
+ id: 'NATIVE_FILTER-versioned',
+ filterType: 'filter_select',
+ targets: [{ column: { name: 'state' } }],
+ },
+ ],
+ },
+ },
+ });
+
+ const { render } = jest.requireActual('spec/helpers/testing-library');
+ const { unmount } = render(
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Redundant requireActual render</b></div>
<div id="fix">
`jest.requireActual('spec/helpers/testing-library')` re-resolves the same
module already imported at the top of the file (no `jest.mock` targets it
here), and the second mount at line 1041 uses the top-level `render`. Two
different `render` references for the same operation is redundant and
misleading — use the imported `render` for both.
</div>
</div>
<small><i>Code Review Run #c42069</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
