yousoph commented on code in PR #40929:
URL: https://github.com/apache/superset/pull/40929#discussion_r3437870889
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -223,7 +223,24 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
dataMask = await getFilterValue(id, nativeFilterKeyValue);
}
if (isOldRison) {
- dataMask = isOldRison;
+ // Normalize legacy `currentState` → `filterState`. Pre-2021 URLs
stored
+ // per-filter selections under `currentState`; modern dataMask uses
+ // `filterState`. Without this copy the filter panel shows no active
+ // selections even though extraFormData still applies the query filter.
+ if (typeof isOldRison === 'object' && isOldRison !== null) {
+ dataMask = Object.fromEntries(
+ Object.entries(
+ isOldRison as Record<string, Record<string, unknown>>,
+ ).map(([filterId, entry]) => [
+ filterId,
+ entry?.currentState && !entry?.filterState
+ ? { ...entry, filterState: entry.currentState }
+ : entry,
+ ]),
+ );
+ } else {
+ dataMask = isOldRison;
+ }
Review Comment:
Fixed — the `else { dataMask = isOldRison }` branch was removed entirely.
When `isOldRison` is truthy but not an object (e.g. a raw string from a failed
Rison decode), the assignment is now skipped and the `dataMask` previously
initialized from `nativeFilterKeyValue || {}` is preserved. No invalid state
shape can be pushed downstream.
##########
superset-frontend/src/dashboard/containers/DashboardPage.test.tsx:
##########
@@ -133,9 +134,11 @@ jest.mock('src/dashboard/util/activeDashboardFilters', ()
=> ({
}));
jest.mock('src/utils/urlUtils', () => ({
- getUrlParam: () => null,
+ getUrlParam: jest.fn().mockReturnValue(null),
}));
+const mockGetUrlParam = getUrlParam as jest.Mock;
+
Review Comment:
Addressed — `beforeEach` now calls `mockGetUrlParam.mockReset();
mockGetUrlParam.mockReturnValue(null);` explicitly so per-test
`mockImplementation` overrides are cleared before each test, regardless of
`jest.clearAllMocks()` not resetting implementations.
##########
superset-frontend/src/dashboard/containers/DashboardPage.test.tsx:
##########
@@ -443,6 +446,115 @@ test('passes null theme when Redux dashboardInfo.theme is
explicitly null (theme
);
});
+test('copies currentState to filterState for legacy native_filters URL
params', async () => {
+ // Pre-2021 URLs encode filter selections under `currentState`. The dataMask
+ // reducer uses `filterState`, so without normalization the filter panel
shows
+ // no active selections even though extraFormData still filters chart
queries.
+ mockGetUrlParam.mockImplementation((param: { name: string }) => {
+ if (param.name === 'native_filters') {
+ return {
+ 'NATIVE_FILTER-OvPTDNKc9': {
+ extraFormData: {
+ filters: [{ col: 'team_name', op: 'IN', val: ['MarginEdge'] }],
+ },
+ currentState: { value: ['MarginEdge'] },
+ },
+ };
+ }
+ return null;
+ });
Review Comment:
Addressed — `beforeEach` now calls `mockGetUrlParam.mockReset();
mockGetUrlParam.mockReturnValue(null);` so each test starts from the default
null return, preventing implementation leakage between tests.
--
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]
