fix(app-shell): keep a language picked on the sign-in page when someone else last used the browser

.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.

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<Record<string, unknown>>()),
+  useAuth: () => ({
+    user: session.user,
+    isAuthenticated: Boolean(session.user),
+    isLoading: false,
+  }),
+}));
+
+vi.mock('@object-ui/react', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  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 (
+    <>
+      <span data-testid="language">{language}</span>
+      <button type="button" onClick={() => void select(to)}>
+        pick
+      </button>
+    </>
+  );
+}
+
+/** The shell, for the page-load that does not navigate away. */
+function Shell({ to }: { to: string }) {
+  useSignedInUserLocale();
+  return <Menu to={to} />;
+}
+
+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(
+      <I18nProvider>
+        <Menu to={PICK} />
+      </I18nProvider>,
+    );
+    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(
+      <I18nProvider>
+        <Menu to={PICK} />
+      </I18nProvider>,
+    );
+
+    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(
+      <I18nProvider>
+        <Menu to={PICK} />
+      </I18nProvider>,
+    );
+    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(
+      <I18nProvider>
+        <Menu to={PICK} />
+      </I18nProvider>,
+    );
+    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(
+      <I18nProvider>
+        <Menu to="de" />
+      </I18nProvider>,
+    );
+    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(
+      <I18nProvider>
+        <Shell to={PICK} />
+      </I18nProvider>,
+    );
+    await pickOnSignInPage();
+
+    session.user = { id: USER_B };
+    session.adapter = adapter;
+    rerender(
+      <I18nProvider>
+        <Shell to={PICK} />
+      </I18nProvider>,
+    );
+    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);
+  });
+});

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<void> {
   const { user } = useAuth();
@@ -389,7 +468,12 @@ export function useLanguageSelection(): (code: string) => Promise<void> {
   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) {