From e9babf65e7f9083cbd18de884e19941dec1900f0 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 08:25:46 +0200 Subject: [PATCH] fix(guests): rebuild consumers on identity switch; repair fallback reads Codex review round 3 on #1268. Three of these were defects in the round 1-2 fixes themselves. Consumers holding local feedback state are now rebuilt on an identity switch. Invalidating queries was not enough: six gallery layouts seed their liked set behind a mount-only likedSeededRef ('so refetches don't clobber in-session optimistic toggles') and PhotoLightbox keeps its own copy, so a refetch left the previous guest's hearts on screen. The provider re-keys its subtree, which covers all seven without touching them. Deliberately only on a switch away from an established identity -- remounting on first sign-in would tear down the gallery under the click that triggered the prompt and drop the pending action. The storage fallback now repoints reads. storeGuestIdentity wrote to sessionStorage when localStorage rejected the real write but left resolvedStorage on localStorage, so every later read missed: x-guest-token was never sent and the identity vanished on reload. The fallback looked like it worked while achieving nothing. Clearing an identity now notifies this tab. Native storage events fire only in other documents, so the interceptor dropping a server-rejected identity left the provider still showing that guest and ensureIdentity() still handing it out. A same-tab event completes the loop. Cross-tab adoption resolves pending callers. A tab parked on the prompt awaiting ensureIdentity() while another tab registers now completes exactly as register() does, instead of hanging forever and registering a second guest if the visitor submits the still-open prompt. --- .../src/contexts/GuestIdentityContext.tsx | 59 +++++++++++++++++-- frontend/src/utils/guestIdentityStorage.ts | 24 +++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/frontend/src/contexts/GuestIdentityContext.tsx b/frontend/src/contexts/GuestIdentityContext.tsx index 36bac6c9..7efa8cc5 100644 --- a/frontend/src/contexts/GuestIdentityContext.tsx +++ b/frontend/src/contexts/GuestIdentityContext.tsx @@ -2,6 +2,7 @@ import React, { createContext, useCallback, useContext, useEffect, useMemo, useR import { useQueryClient } from '@tanstack/react-query'; import { guestsService, GuestIdentity } from '../services/guests.service'; import { + GUEST_IDENTITY_CLEARED_EVENT, clearGuestIdentity, getGuestIdentity, storeGuestIdentity, @@ -80,14 +81,43 @@ export const GuestIdentityProvider: React.FC = ({ // needs to catch up. A null key means the whole store was cleared. useEffect(() => { if (typeof window === 'undefined') return; + + const adopt = () => { + const next = getGuestIdentity(slug); + setIdentity(next); + if (next) { + // A caller may be parked on the prompt waiting for ensureIdentity(). + // Another tab just answered the question, so complete them exactly as + // register() does — otherwise the action hangs forever and submitting + // the still-open prompt registers a second guest. + setPromptOpen(false); + setRecoveryOpen(false); + pendingResolvers.current.forEach((r) => r(next)); + pendingResolvers.current = []; + pendingRejecters.current = []; + } + }; + const onStorage = (event: StorageEvent) => { if (event.key && event.key !== `guest_token_${slug}` && event.key !== `guest_identity_${slug}`) { return; } - setIdentity(getGuestIdentity(slug)); + adopt(); }; + // Fires in THIS tab, where `storage` does not — e.g. the axios interceptor + // dropping an identity the server has rejected. + const onLocalClear = (event: Event) => { + const detail = (event as CustomEvent<{ slug?: string }>).detail; + if (detail?.slug && detail.slug !== slug) return; + adopt(); + }; + window.addEventListener('storage', onStorage); - return () => window.removeEventListener('storage', onStorage); + window.addEventListener(GUEST_IDENTITY_CLEARED_EVENT, onLocalClear); + return () => { + window.removeEventListener('storage', onStorage); + window.removeEventListener(GUEST_IDENTITY_CLEARED_EVENT, onLocalClear); + }; }, [slug]); // Guest-scoped caches are keyed by slug and photo id, never by guest id, so @@ -96,14 +126,29 @@ export const GuestIdentityProvider: React.FC = ({ // token. Reachable three ways now: another tab signing in or out, "Not // you?", and a ?invite= redemption over an existing identity. const previousIdentityId = useRef(identity?.id ?? null); + // Invalidating queries is not enough on its own: six gallery layouts seed + // their liked-photo set behind a `likedSeededRef` that is deliberately + // mount-only ("so refetches don't clobber in-session optimistic toggles"), + // and PhotoLightbox holds its own copy. A refetch therefore leaves the + // previous guest's hearts on screen. Bumping this generation re-keys the + // subtree so every such consumer is rebuilt. + const [identityGeneration, setIdentityGeneration] = useState(0); useEffect(() => { const currentId = identity?.id ?? null; - if (previousIdentityId.current === currentId) return; + const previousId = previousIdentityId.current; + if (previousId === currentId) return; previousIdentityId.current = currentId; + queryClient.invalidateQueries({ queryKey: ['my-feedback', slug] }); queryClient.invalidateQueries({ queryKey: ['gallery-photos', slug] }); // Every mounted photo, regardless of id. queryClient.invalidateQueries({ queryKey: ['photo-feedback', slug] }); + + // Only on a SWITCH away from an established identity — signing in for the + // first time (null -> A) must not remount, or registering from the prompt + // would tear down the gallery under the very click that triggered it and + // drop the pending action. + if (previousId !== null) setIdentityGeneration((g) => g + 1); }, [identity, slug, queryClient]); // When an invite token is present on the URL (?invite=xxx), redeem it once @@ -271,7 +316,13 @@ export const GuestIdentityProvider: React.FC = ({ ] ); - return {children}; + return ( + + {/* Re-keyed on an identity switch so consumers holding local feedback + state are rebuilt rather than showing the previous guest's. */} + {children} + + ); }; export function useGuestIdentity(): GuestIdentityContextValue { diff --git a/frontend/src/utils/guestIdentityStorage.ts b/frontend/src/utils/guestIdentityStorage.ts index f1881312..bc8e3b59 100644 --- a/frontend/src/utils/guestIdentityStorage.ts +++ b/frontend/src/utils/guestIdentityStorage.ts @@ -28,6 +28,9 @@ import type { GuestIdentity } from '../services/guests.service'; const TOKEN_KEY_PREFIX = 'guest_token_'; const IDENTITY_KEY_PREFIX = 'guest_identity_'; +/** Same-tab counterpart to the native cross-tab `storage` event. */ +export const GUEST_IDENTITY_CLEARED_EVENT = 'picpeak:guest-identity-cleared'; + const isBrowser = typeof window !== 'undefined'; /** @@ -169,7 +172,14 @@ export function storeGuestIdentity(slug: string, identity: GuestIdentity, token: if (isBrowser) { try { if (window.sessionStorage && window.sessionStorage !== storage) { - write(window.sessionStorage); + if (write(window.sessionStorage)) { + // Repoint reads at the store that actually accepted the write. + // Without this the identity is written to sessionStorage while + // getGuestToken() keeps reading localStorage, so x-guest-token is + // never sent and the identity is lost on reload — the fallback + // would look like it worked while achieving nothing. + resolvedStorage = window.sessionStorage; + } } } catch { // No store will take it. The identity lasts as long as this page does, @@ -235,6 +245,18 @@ export function clearGuestIdentity(slug: string): void { // Best-effort. } } + + // `storage` events fire only in OTHER documents, so a clear triggered inside + // this tab (the axios interceptor dropping a server-rejected identity) would + // leave the provider still showing the guest and ensureIdentity() still + // handing it out. Announce it locally too. + if (isBrowser) { + try { + window.dispatchEvent(new CustomEvent(GUEST_IDENTITY_CLEARED_EVENT, { detail: { slug } })); + } catch { + // CustomEvent unavailable — the provider simply refreshes on next mount. + } + } } /**