omsn2 commented on code in PR #43108:
URL: https://github.com/apache/superset/pull/43108#discussion_r3781915389
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -81,6 +81,44 @@ import {
type NativeFilterConfigEntry = Partial<Filter> & { id: string };
+const DASHBOARD_FILTERS_STORAGE_PREFIX = 'superset_dashboard_filters_';
+
+function getStorageKey(dashboardId: number, userId: number | undefined) {
+ // Scope the key to userId to prevent one user's filter state from
+ // leaking into another user's session on the same browser profile.
+ // Guest users (no userId) are not scoped — guest sessions are ephemeral.
+ return userId
+ ? `${DASHBOARD_FILTERS_STORAGE_PREFIX}${userId}_${dashboardId}`
+ : `${DASHBOARD_FILTERS_STORAGE_PREFIX}${dashboardId}`;
+}
+
+function getSavedDashboardFilters(
+ dashboardId: number,
+ userId: number | undefined,
+) {
+ try {
+ const raw = localStorage.getItem(getStorageKey(dashboardId, userId));
+ return raw ? JSON.parse(raw) : null;
+ } catch {
+ return null;
+ }
+}
+
+function saveDashboardFilters(
+ dashboardId: number,
+ userId: number | undefined,
+ nativeFilterMask: Record<string, unknown>,
+) {
+ try {
+ localStorage.setItem(
+ getStorageKey(dashboardId, userId),
+ JSON.stringify(nativeFilterMask),
+ );
+ } catch {
Review Comment:
Fixed in the latest commit. Added a read-before-write guard:
`getSavedDashboardFilters` now reads the existing value and compares it to the
new serialized value before calling `setItem`. If the value hasn't changed, the
write is skipped entirely, avoiding unnecessary synchronous main-thread work on
every `dataMask` update.
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -226,6 +267,11 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }:
PageProps) => {
}
} else if (nativeFilterKeyValue) {
dataMask = await getFilterValue(id, nativeFilterKeyValue);
+ } else {
+ const savedFilters = getSavedDashboardFilters(id, userId);
+ if (savedFilters) {
+ dataMask = savedFilters;
+ }
}
Review Comment:
> Fixed in the latest commit. Added a shape guard before assigning the
parsed `localStorage` value to `dataMask`:
> ```ts
> if (savedFilters && typeof savedFilters === 'object' &&
!Array.isArray(savedFilters))
> ```
> This rejects any corrupted, tampered, or unexpectedly shaped data (arrays,
primitives, `null`) before it can reach downstream code that assumes `dataMask`
is a plain object map.
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -81,6 +81,44 @@ import {
type NativeFilterConfigEntry = Partial<Filter> & { id: string };
+const DASHBOARD_FILTERS_STORAGE_PREFIX = 'superset_dashboard_filters_';
+
+function getStorageKey(dashboardId: number, userId: number | undefined) {
+ // Scope the key to userId to prevent one user's filter state from
+ // leaking into another user's session on the same browser profile.
+ // Guest users (no userId) are not scoped — guest sessions are ephemeral.
+ return userId
+ ? `${DASHBOARD_FILTERS_STORAGE_PREFIX}${userId}_${dashboardId}`
+ : `${DASHBOARD_FILTERS_STORAGE_PREFIX}${dashboardId}`;
+}
Review Comment:
Fixed in the latest commit. The storage key is now scoped to the
authenticated user ID. The actual key format used is
dashboard__native_filters__{userId}__{dashboardId} — this ensures that logging
out and signing in as a different user in the same browser profile will not
expose or reuse the previous user's filter state. Guest users (who have no
userId) gracefully fall back to a dashboard-only key
(dashboard__native_filters__{dashboardId}), since guest sessions are inherently
ephemeral and not tied to a persistent account.
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -381,8 +427,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.
Review Comment:
> Fixed in the latest commit. Added 4 unit tests to `DashboardPage.test.tsx`
covering:
> - **Load path:** Restores saved filters from `localStorage` when no
`permalinkKey` or `nativeFiltersKey` is present in the URL
> - **userId scoping:** Reads from the correct user-scoped key
(`dashboard__native_filters__{userId}__{dashboardId}`) when the user is
authenticated
> - **URL key priority:** URL-based filter key (`nativeFiltersKey`) takes
priority over any stored `localStorage` state
> - **Corrupted data rejection:** An array or primitive stored under the key
is ignored and does not get assigned to `dataMask`
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -81,6 +81,44 @@ import {
type NativeFilterConfigEntry = Partial<Filter> & { id: string };
+const DASHBOARD_FILTERS_STORAGE_PREFIX = 'superset_dashboard_filters_';
+
+function getStorageKey(dashboardId: number, userId: number | undefined) {
+ // Scope the key to userId to prevent one user's filter state from
+ // leaking into another user's session on the same browser profile.
+ // Guest users (no userId) are not scoped — guest sessions are ephemeral.
+ return userId
+ ? `${DASHBOARD_FILTERS_STORAGE_PREFIX}${userId}_${dashboardId}`
Review Comment:
> Fixed — updated the PR description to accurately reflect the
implementation. The feature does scope by `userId` when the authenticated user
has one (key format: `dashboard__native_filters__{userId}__{dashboardId}`).
Guest users (no `userId`) fall back to a dashboard-only key since their
sessions are ephemeral. The "No per-user scoping" entry has been removed from
the known limitations and replaced with an accurate description.
--
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]
