sadpandajoe commented on code in PR #43108:
URL: https://github.com/apache/superset/pull/43108#discussion_r4032779346
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -226,6 +289,87 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
}
} else if (nativeFilterKeyValue) {
dataMask = await getFilterValue(id, nativeFilterKeyValue);
+ } else if (userId != null) {
Review Comment:
This localStorage fallback still runs when the URL contains an `f=` Rison
filter, and the later injection overlays only the URL's matching fields onto
this restored mask. A shared `?f=(product:Widget)` link can therefore silently
retain the viewer's saved `region=EMEA` filter and return narrower data than
the URL encodes; should any Rison filter skip the fallback like the other URL
filter keys?
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -381,8 +523,47 @@ 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(() => {
+ // Skip persistence for unauthenticated/guest users: they have no stable
+ // identity to scope the key to, and restoring filter state across
+ // guest-token sessions would leak selections between unrelated sessions.
+ // Use == null (not !userId) so a valid userId of 0 is not treated as
+ // anonymous.
+ if (
+ !id ||
+ userId == null ||
+ hydratedDashboardId !== id ||
+ !isDashboardHydrated.current
+ )
+ 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);
+ // Do not overwrite a previously saved state with an empty mask.
+ // When the store clears dataMask on unmount (SPA navigation away), the
+ // hydratedDashboardId guard may still pass briefly before the next
+ // dashboard's hydration fires; skipping empty writes prevents that race
+ // from wiping the user's last valid selection.
+ if (nativeFilterIds.length === 0) return;
+ const nativeFilterMask = Object.fromEntries(
+ nativeFilterIds
+ .filter(filterId => filterId in fullDataMask)
+ .map(filterId => [filterId, fullDataMask[filterId]]),
+ );
+ // Also skip when the configuration is non-empty but dataMask has already
+ // been cleared (e.g. during SPA unmount before the next dashboard
hydrates).
+ // The ref stays true across the switch so the hydratedDashboardId guard
+ // alone is not sufficient — a nativeFilterMask of {} would still overwrite
+ // the user's saved selection.
+ if (Object.keys(nativeFilterMask).length === 0) return;
+ saveDashboardFilters(id, userId, nativeFilterMask, nativeFilters);
Review Comment:
Opening a historical version rehydrates the same dashboard ID with that
snapshot's filter defaults, so every guard here passes and this overwrites the
user's saved live mask. If they refresh or navigate away before exiting
preview, those historical defaults are restored on the next live visit; could
persistence be disabled while a version preview is applied?
--
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]
