diff --git a/.changeset/30338-keep-sign-in-language-pick.md b/.changeset/30338-keep-sign-in-language-pick.md new file mode 100644 index 00000000000..021e77a4398 --- /dev/null +++ b/.changeset/30338-keep-sign-in-language-pick.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +--- + +fix: a language picked on the sign-in page is kept when the person who signs in is not the browser's last user (#30338) + +Clause-②: no +Metadata-protocol: none + +On a shared browser, signing in as someone other than the last user clears the previous user's saved state, and that sweep also removed the language just picked in the sign-in page's menu, so the person landed in the browser's language. The pick is now remembered for that page-load and put back after the sweep, so the next page opens in the chosen language. What the previous user left is still cleared when nothing was picked. A pick followed by a reload of the sign-in page, or by an identity provider's redirect, is still lost. Saving the pick as the person's own language is a separate change. diff --git a/ui/packages/app-shell/src/hooks/__tests__/userLocaleSignInPick-30338.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/userLocaleSignInPick-30338.test.tsx new file mode 100644 index 00000000000..78291fd233e --- /dev/null +++ b/ui/packages/app-shell/src/hooks/__tests__/userLocaleSignInPick-30338.test.tsx @@ -0,0 +1,245 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack#30338 — a language picked on the sign-in page is kept when the + * person who signs in is not the browser's last owner. + * + * The state pinned is the sign-in page-load: the persisted owner pointer names + * A, A's language is in both device slots, and someone picks a language in the + * sign-in page's menu (signed out, so `useLanguageSelection` is a bare local + * switch). Then B signs in and `SessionUserScope.adopt(B)` runs, exactly as + * `AuthProvider` runs it. The console then leaves by a full navigation, so + * what the next page-load boots from is the STORAGE this page-load leaves — + * that, not the live language, is what these pins read. + * + * objectui#10193 ruling B stands and is pinned beside it: what A left, with no + * pick made since, is still swept. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, waitFor, act, fireEvent } from '@testing-library/react'; +import { + I18nProvider, + LOCALE_SEED_STORAGE_KEY, + LOCALE_STORAGE_KEY, + useObjectTranslation, +} from '@object-ui/i18n'; +// Not on the `@object-ui/auth` barrel; the vitest alias maps that barrel to +// `packages/auth/src`, so this deep path is the SAME module instance the hook's +// `subscribeSessionOwnerChange` registers with. +import { SessionUserScope } from '../../../../auth/src/ActiveOrganizationStorage'; + +const session = vi.hoisted(() => ({ + user: null as { id: string } | null, + adapter: null as unknown, +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: session.user, + isAuthenticated: Boolean(session.user), + isLoading: false, + }), +})); + +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => session.adapter, +})); + +import { + __resetSignedOutPickForTests, + useLanguageSelection, + useSignedInUserLocale, +} from '../useUserLocale'; + +const USER_A = 'u_alice'; +const USER_B = 'u_bob'; +const USER_C = 'u_carol'; +const POINTER_KEY = 'auth-session-user-id'; +/** A's language, and not the browser's (`en`). */ +const A_LANGUAGE = 'ja'; +/** The language picked on the sign-in page. */ +const PICK = 'fr'; + +function userRow(id: string, locale: string | null) { + return { findOne: vi.fn(async () => ({ id, locale })), update: vi.fn(async () => ({})) }; +} + +/** The sign-in page's language menu: a switch through `useLanguageSelection`. */ +function Menu({ to }: { to: string }) { + const select = useLanguageSelection(); + const { language } = useObjectTranslation(); + return ( + <> + {language} + + + ); +} + +/** The shell, for the page-load that does not navigate away. */ +function Shell({ to }: { to: string }) { + useSignedInUserLocale(); + return ; +} + +const shownLanguage = () => screen.getByTestId('language').textContent; +const storedChoice = () => window.localStorage.getItem(LOCALE_STORAGE_KEY); + +/** The browser A left behind: pointer, explicit slot, seed. */ +function leaveAsResidue() { + window.localStorage.setItem(POINTER_KEY, USER_A); + window.localStorage.setItem(LOCALE_STORAGE_KEY, A_LANGUAGE); + window.localStorage.setItem(LOCALE_SEED_STORAGE_KEY, A_LANGUAGE); +} + +async function pickOnSignInPage(to = PICK) { + fireEvent.click(screen.getByRole('button', { name: 'pick' })); + await waitFor(() => expect(shownLanguage()).toBe(to)); + // The switch persisted itself, as every switch does. + expect(storedChoice()).toBe(to); +} + +beforeEach(() => { + window.localStorage.clear(); + SessionUserScope._resetForTests(); + __resetSignedOutPickForTests(); + session.user = null; + session.adapter = null; +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('a language picked on the sign-in page survives a change of owner (objectstack#30338)', () => { + it('B signs in after picking: the pick stays stored for the next page-load, A’s seed does not', async () => { + leaveAsResidue(); + render( + + + , + ); + expect(shownLanguage()).toBe(A_LANGUAGE); + await pickOnSignInPage(); + + let purged: unknown; + act(() => { + purged = SessionUserScope.adopt(USER_B); + }); + + // The purge really ran (A's seed is gone) — and the pick was put back. + expect(purged).toBe(true); + expect(window.localStorage.getItem(LOCALE_SEED_STORAGE_KEY)).toBeNull(); + expect(storedChoice()).toBe(PICK); + expect(window.localStorage.getItem(POINTER_KEY)).toBe(USER_B); + }); + + it('control (objectui#10193 ruling B): with no pick since A, A’s language is still swept', async () => { + leaveAsResidue(); + render( + + + , + ); + + act(() => { + SessionUserScope.adopt(USER_B); + }); + + expect(storedChoice()).toBeNull(); + expect(window.localStorage.getItem(LOCALE_SEED_STORAGE_KEY)).toBeNull(); + }); + + it('the pick goes to the next sign-in only: it does not come back at a later change of owner', async () => { + leaveAsResidue(); + render( + + + , + ); + await pickOnSignInPage(); + act(() => { + SessionUserScope.adopt(USER_B); + }); + expect(storedChoice()).toBe(PICK); + + // B's page-load ends; C signs in on the same tab without picking anything. + SessionUserScope._resetForTests(); + act(() => { + SessionUserScope.adopt(USER_C); + }); + + expect(storedChoice()).toBeNull(); + }); + + it('a signed-in switch replaces the pick, so the next owner change does not restore it', async () => { + leaveAsResidue(); + const { rerender } = render( + + + , + ); + await pickOnSignInPage(); + + // The first owner of a clean pointer is not an owner change: no purge, no + // event. B is signed in and switches to German from the globe. + window.localStorage.removeItem(POINTER_KEY); + act(() => { + SessionUserScope.adopt(USER_B); + }); + session.user = { id: USER_B }; + session.adapter = userRow(USER_B, null); + rerender( + + + , + ); + await pickOnSignInPage('de'); + + // C signs in next: neither B's German nor the earlier pick is C's. + SessionUserScope._resetForTests(); + act(() => { + SessionUserScope.adopt(USER_C); + }); + expect(storedChoice()).toBeNull(); + }); + + it('with the shell mounted (no navigation), B keeps the pick live and stored after the reset', async () => { + leaveAsResidue(); + const adapter = userRow(USER_B, null); + const { rerender } = render( + + + , + ); + await pickOnSignInPage(); + + session.user = { id: USER_B }; + session.adapter = adapter; + rerender( + + + , + ); + act(() => { + SessionUserScope.adopt(USER_B); + }); + + await waitFor(() => expect(adapter.findOne).toHaveBeenCalled()); + await new Promise((r) => setTimeout(r, 50)); + expect(shownLanguage()).toBe(PICK); + expect(storedChoice()).toBe(PICK); + }); +}); diff --git a/ui/packages/app-shell/src/hooks/useUserLocale.ts b/ui/packages/app-shell/src/hooks/useUserLocale.ts index dd0da66ed2d..67e31d981ad 100644 --- a/ui/packages/app-shell/src/hooks/useUserLocale.ts +++ b/ui/packages/app-shell/src/hooks/useUserLocale.ts @@ -60,9 +60,11 @@ * * ## Signed out * - * Nothing here applies: with no `sys_user` row there is nothing to read and - * nothing to write. {@link useLanguageSelection} performs the local switch and - * returns, so language selection for a visitor is exactly what it was. + * With no `sys_user` row there is nothing to read and nothing to write. + * {@link useLanguageSelection} performs the local switch, and it remembers the + * pick for this page-load only, so that the purge above does not take it from + * the person who signs in next (objectstack#30338, see + * {@link keepSignedOutPickThroughOwnerChange}). * * @module */ @@ -87,6 +89,78 @@ import { useAfterFirstPaint } from './sharedUserFeeds.js'; */ let ownerChangesHandled = 0; +/** + * The language picked while signed out in THIS page-load and not yet handed to + * whoever signs in next, or `null` (objectstack#30338). + */ +let signedOutPick: string | null = null; +let signedOutPickKeeperInstalled = false; + +/** + * Keep a language picked on the sign-in page through the purge that follows a + * sign-in by someone other than the browser's last owner (objectstack#30338). + * + * ## The drop this closes + * + * The sign-in page's language menu writes the pick to the device slot + * ({@link LOCALE_STORAGE_KEY}) like every switch. When the person who then + * signs in is not the one the browser last held, `SessionUserScope.adopt` + * sweeps that slot with the rest of the previous owner's state + * (objectui#5664), and the sign-in page then leaves by a full navigation. The + * next page-load boots with no choice stored, so the person lands in the + * browser's language, not the one they just picked. The reset in + * {@link useLanguageResetOnOwnerChange} is not on that path: the shell is not + * mounted on the sign-in page. + * + * ## Why this keeps objectui#10193 ruling B + * + * The purge removes what the PREVIOUS owner left, and still does. A language + * picked after they signed out is not that, and the two can be told apart + * without a stored marker: the pick and the purge happen in the same + * page-load, so the pick is known here, in memory. The owner-change event + * fires after the sweep, and this puts the pick back into the same slot, + * where the next boot reads it as it reads any choice. + * + * ⚠️ The bound, stated: memory lasts one page-load. A pick followed by a reload + * of the sign-in page, or a sign-in that leaves the page first (an identity + * provider's redirect), is still swept. + * + * Installed once, on the first signed-out pick, at module scope: the sign-in + * page's menu unmounts in the same commit that sets the user, before + * `AuthProvider`'s effect calls `adopt`, so a listener owned by a component + * would be gone when the event fires. + */ +function keepSignedOutPickThroughOwnerChange(code: string): void { + signedOutPick = code; + if (signedOutPickKeeperInstalled) return; + signedOutPickKeeperInstalled = true; + subscribeSessionOwnerChange(() => { + const pick = signedOutPick; + signedOutPick = null; + if (!pick) return; + try { + window.localStorage.setItem(LOCALE_STORAGE_KEY, pick); + } catch { + // Storage blocked — the live language still holds the pick for this + // page-load; nothing more can be kept. + } + }); +} + +/** + * The pick has reached a signed-in person, or a signed-in choice replaced it: + * either way it must not reach the NEXT owner change, which belongs to + * someone else. + */ +function forgetSignedOutPick(): void { + signedOutPick = null; +} + +/** Test seam: forget this page-load's signed-out pick. Not public. */ +export function __resetSignedOutPickForTests(): void { + signedOutPick = null; +} + /** * Serialises every language switch this module makes, so they land in the * order they were decided. The owner-change reset and the column apply are @@ -304,6 +378,9 @@ export function useSignedInUserLocale(): void { if (cancelled) return; const raw = (row as { locale?: unknown } | null)?.locale; const column = typeof raw === 'string' && raw.length > 0 ? raw : null; + // This person's session is established and read: a sign-in-page pick + // still held has reached them, and must not reach whoever is next. + forgetSignedOutPick(); setRead((prev) => ({ seq: prev.seq + 1, column })); } catch (err) { // Cosmetic and never fail-closed, the same posture the rest of the @@ -370,7 +447,9 @@ export function useSignedInUserLocale(): void { * is the divergence this card exists to end, one layer down. * * Signed out (or with no adapter): the local switch is the whole operation, no - * request is attempted and nothing is reported. + * request is attempted and nothing is reported. Signed out, the pick is also + * remembered for this page-load, so that a sign-in by someone other than the + * browser's last owner keeps it ({@link keepSignedOutPickThroughOwnerChange}). */ export function useLanguageSelection(): (code: string) => Promise { const { user } = useAuth(); @@ -389,7 +468,12 @@ export function useLanguageSelection(): (code: string) => Promise { return useCallback( async (code: string) => { await changeLanguageRef.current(code); - if (!adapter || !userId) return; + if (!userId) { + keepSignedOutPickThroughOwnerChange(code); + return; + } + forgetSignedOutPick(); + if (!adapter) return; try { await adapter.update(USER_OBJECT, userId, { [LOCALE_FIELD]: code }); } catch (err) {