sadpandajoe commented on code in PR #43108:
URL: https://github.com/apache/superset/pull/43108#discussion_r3872849582
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -334,7 +404,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
}
if (id) getDataMaskApplied();
// eslint-disable-next-line react-hooks/exhaustive-deps
- }, [readyToRender]);
+ }, [readyToRender, id]);
Review Comment:
When the dashboard response arrives before charts, this new `id` dependency
executes the Rison path while `readyToRender` is false and strips matched `f=`
params before hydration. When charts then arrive, the second pass no longer
sees those filters, so the dashboard opens unfiltered. Can the URL rewrite
happen only once hydration is ready?
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -381,8 +434,24 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
}, [addDangerToast, datasets, datasetsApiError, dispatch, isNotFoundError]);
const relevantDataMask = useSelector(selectRelevantDatamask);
+ const fullDataMask = useSelector(selectDataMask);
+ const nativeFilters = useSelector(selectNativeFilters);
const activeFilters = useSelector(selectActiveFilters);
+ useEffect(() => {
+ if (!id || hydratedDashboardId !== id) return;
+ // Persist only entries that correspond to configured native filters.
+ // This avoids saving chart customization or other transient dataMask
+ // entries that are not part of the user's filter selections.
+ const nativeFilterIds = Object.keys(nativeFilters);
+ const nativeFilterMask = Object.fromEntries(
+ nativeFilterIds
+ .filter(filterId => filterId in fullDataMask)
+ .map(filterId => [filterId, fullDataMask[filterId]]),
+ );
+ saveDashboardFilters(id, userId, nativeFilterMask);
Review Comment:
The configuration can still be nonempty while `dataMask` has been cleared,
so this persists `{}` before restoration and loses saved selections. Could this
wait until restore completes or skip an empty `nativeFilterMask`?
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -226,6 +268,17 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
}
} else if (nativeFilterKeyValue) {
dataMask = await getFilterValue(id, nativeFilterKeyValue);
+ } else {
+ const savedFilters = getSavedDashboardFilters(id, userId);
+ // Guard against corrupted or unexpected localStorage data shapes
+ // (e.g. a JSON array or primitive) before assigning to dataMask.
+ if (
+ savedFilters &&
+ typeof savedFilters === 'object' &&
+ !Array.isArray(savedFilters)
+ ) {
+ dataMask = savedFilters;
Review Comment:
Keeping the same ID while changing its target still passes this check, so
the old `extraFormData` is hydrated into the new filter and can apply the prior
column or dataset selection. Can stored state be versioned against the filter
definition rather than checked by ID alone?
--
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]
