Skip to content

Commit bbd6072

Browse files
fix(console): save a language picked on the sign-in page as the person's own when they have none (#30539)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuWaHFkA9vyxuQvdUXUZnrDs)_ domain:ui#4 · os-sales · session_01L6bhgVfPeFZkN1FP93imKU ## 维护者速读 - 改了什么:在登录页选了语言、本人还没设过语言偏好的,登录时这个选择会存成本人的语言偏好(`sys_user.locale`),换设备、发邮件都跟着走。 - 为什么改:#30338 只做到「选的语言在这台浏览器上留住」,没有存进个人偏好;登录页靠整页跳转离开,跳转前没人写库。 - 风险与回滚:只有登录页选过语言才多发两次请求(读一次本人记录、写一次),已有偏好不改,写失败照常登录。回滚即撤本 PR。 - 先看:`LoginPage.tsx` Post-login effect 新增的第 0 步;`useUserLocale.ts` 的 `saveSignInLanguagePick`。 - 两分钟验证:退出登录,在登录页底部的地球图标里选 Français,用一个没设过语言的账号登录,进个人资料页,语言应为 Français;刷新仍是法语。 - 需要你做的:无,席位落地。 Fixes #30399 Clause-②: yes (widening) Metadata-protocol: none ## What changes - `@object-ui/app-shell` `useUserLocale.ts`: a language picked while signed out is now also held for one save (`signInPickToSave`, page-load memory, no storage key). New `saveSignInLanguagePick(userId, writer?)` reads the person's `sys_user` row and, only when `locale` is unset, writes the pick through the column's one write door, `update('sys_user', id, { locale })`, the call `useLanguageSelection` makes. It spends the pick on the first call, sends no request when nothing was picked, and never rejects: a failed read or write logs once at `error` (the pick stays in the device slot, so this browser still renders it). `hasSignInLanguagePick()` answers whether there is anything to save. - Barrel: `hasSignInLanguagePick`, `saveSignInLanguagePick` and the `SignInLanguagePickWriter` type are exported from `@object-ui/app-shell` (hence `Clause-②: yes (widening)`; the claim's line says `no` and names this case for the seat to correct). - `LoginPage.tsx`, the Post-login orchestration effect only: a new step 0 starts the save once (ref latch) when there is a pick, holds the spinner, and re-runs the effect when the save settles (state). Steps 1 to 3 are unchanged, so all five exits wait on it: | Exit | Applies | Why | |---|---|---| | OAuth provider hand-off (`followOauthAuthorize`) | yes | a full navigation to the relying party; same person, same page-load | | Mobile sign-in hand-off (`assign(handoff.target)`) | yes | full navigation; a session the hand-off refuses for its age is held to the form and saves nothing (it is not a sign-in yet) | | No organization (`assign(root or redirect)`) | yes | full navigation | | Several organizations (`assign('/organizations')`) | yes | full navigation | | Final redirect (`assign(redirect or root)`, after the single-org auto-select) | yes | full navigation; the auto-select itself is not an exit | The "Read public auth config" effect and the `DevAdminRow` render are untouched (#30420 is changing them in parallel). ## Why Measured on `origin/main` `f6faf751` and in a live sign-in (Verification below): the pick ends in the `objectui-locale` slot, `sys_user.locale` stays empty. Two findings the order's suggested route needed adjusting for: - **No adapter on the sign-in page.** The order suggested awaiting `adapter.update('sys_user', id, { locale })`. `useAdapter()` answers `null` on `/login`: the console mounts `AdapterProvider` inside `ConnectedShell`, and the public auth routes render outside it. So `saveSignInLanguagePick` builds the same `ObjectStackAdapter` `AdapterProvider` builds (same base URL, `createAuthenticatedFetch`) for the one save. Live, that is one `GET /api/v1/discovery`, one `GET` and one `PATCH` on `/api/v1/data/sys_user/ID`, all before the next page-load's discovery. - **The owner-change keeper spends its copy first.** #30338's `signedOutPick` is consumed by the owner-change event, which fires (inside `AuthProvider`) before the post-login effect runs, and never fires when the browser's last owner signs in again. So the save reads a second page-load copy, cleared by the same signed-in-switch path. Route B as ruled: awaited before the navigation, no fire-and-forget, no new storage marker. ## Acceptance notes - **Bound, unchanged from #30338:** memory lasts one page-load. A pick followed by a reload of the sign-in page, or by an identity provider round-trip, is not saved. - **No timeout on the save.** A request that hangs keeps the spinner up as long as the adapter's own fetch does; a refused or failed request lets the sign-in proceed. Noted, not added: a bound would be a choice of number nobody has ruled on. - **Same shape, reported, not fixed (order):** `RegisterPage.tsx` renders the same footer language menu and leaves by the same four `window.location.assign` exits after a sign-up that signs the person in, so a pick made there is kept in the slot but not saved. `SetupPage.tsx` has no language menu of its own; a pick made on another signed-out page earlier in the same page-load would reach its two exits unsaved. - **Observed, not this card's:** with no pick, the existing post-login effect can navigate twice in the test harness (once before the organizations answer, once after). The new pins assert only that the page leaves and saves nothing in that case. - An existing language is kept even when the deployment does not offer it (live row 2: a stored `de` stays `de`); what renders then is the existing reader's business, unchanged here. ## Tests - `ui/packages/app-shell/src/hooks/__tests__/userLocaleSignInPick-30338.test.tsx`, new top-level describe (8 cases): saved for a new owner; saved when the last owner signs in again (no owner change); an existing language is read and kept; CONTROL no pick, no request; spent once; a signed-in switch replaces it; a failed write resolves, logs at `error`, slot kept; a failed read writes nothing. - `ui/apps/console/src/pages/auth/__tests__/signInLanguagePick-30399.test.tsx` (new, 11 cases): for each of the five exits, nothing leaves while the save is in flight and the expected exit is taken once it settles; CONTROL for each exit with no pick; a session the mobile hand-off refuses saves nothing. <details><summary>vitest: locale hook files, the app-shell layout suite and every console auth page test (merged head 3e162ec)</summary> ``` pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/hooks/__tests__/userLocaleSignInPick-30338.test.tsx packages/app-shell/src/hooks/__tests__/userLocaleOwnerChange-10193.test.tsx packages/app-shell/src/hooks/__tests__/userLocale-10059.test.tsx packages/app-shell/src/layout/ apps/console/src/pages/auth/ Test Files 172 passed (172) Tests 1180 passed (1180) os-verify-lock: VERDICT command-exit 0 ``` </details> <details><summary>Ablations (each through scripts/ablation-replace.mjs, mutation proven on disk, restored: blob == HEAD, git diff HEAD empty)</summary> ``` A1 LoginPage: drop the step-0 return (the save starts, the exit runs beside it = route C) signInLanguagePick-30399: 5 failed | 6 passed (11) — every exit case red, every CONTROL green A2 useUserLocale: drop the "column already set" check userLocaleSignInPick-30338: 1 failed | 12 passed (13) — "a language of their own already saved is kept" A3 useUserLocale: do not hold the pick for the save userLocaleSignInPick-30338: 4 failed | 9 passed (13) — the four cases that need a pick ``` </details> <details><summary>Type check</summary> ``` pnpm --filter @object-ui/app-shell type-check (tsc --noEmit && tsc -p tsconfig.test.json) os-verify-lock: VERDICT command-exit 0 pnpm build (turbo, includes @object-ui/console tsc + vite build) on 3e162ec os-verify-lock: VERDICT command-exit 0 ``` </details> ## Verification Live: showcase `pnpm dev -- --fresh` plus the console on vite, Playwright `en-US`, 1440 wide. Users created by the seeded admin. Row 1 before = this head with step 0 disabled in place (`ablation-replace --hold`, restored after). | Row | Who | Picked | `sys_user.locale` after sign-in | Slot | Language after reload | |---|---|---|---|---|---| | 1 before | no language of their own | Français | empty | fr | fr | | 1 after | no language of their own (0 orgs, root exit) | Français | **fr** | fr | fr | | 2 after | has `de` | Français | de (unchanged) | fr | fr | | 3 after | no language of their own | nothing | empty, no PATCH sent | empty | en-US | | 4 after | seeded admin (1 org, final exit) | Français | **fr** | fr | fr | Requests in row 1 after, in order: `POST sign-in/email`, `GET discovery`, `GET sys_user/ID`, `PATCH sys_user/ID`, then the next page-load's `GET discovery`. <details><summary>Gates (merged head 3e162ec)</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 35; all 35 exit 0, codes captured before any pipe. `pnpm --dir ui exec vitest run scripts/` was killed once by the container's memory limit while other gates ran (exit 1, SIGKILL) and re-run alone: 170 passed, 2 skipped files, exit 0. Extra, outside the derivation: `pnpm --dir ui run check:spec-symbols` exit 0; `node scripts/check-sdui-manifest.mjs` exit 0 (also derived). `dispatch-gates --ran`: 35 derived, 35 run, 0 NOT-MEASURED (a derived zero). </details> Branch `claude/project-thread-3ct8gd` is the card thread's platform-assigned branch (not `claude/issue-30399-*`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgUkfsVq9sKkVSugapT --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgUkfsVq9sKkVSugapT)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bcdf8d3 commit bbd6072

7 files changed

Lines changed: 497 additions & 2 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/console': patch
4+
---
5+
6+
fix(console): a language picked on the sign-in page is saved as the person's own language when they have none (#30399)
7+
8+
Clause-②: yes (widening)
9+
Metadata-protocol: none
10+
11+
A language picked in the sign-in page's menu used to apply on that browser only: after signing in, the person's saved language stayed empty, so another device, or the language their mail is written in, did not follow the pick. The sign-in page now saves the pick to the person's own language setting before it moves on, when they have not chosen one yet. A language they already chose is kept, nothing is saved when no language was picked on the page, and a save that fails does not hold up the sign-in (the pick still applies on that browser). `@object-ui/app-shell` gains `hasSignInLanguagePick`, `saveSignInLanguagePick` and the `SignInLanguagePickWriter` type for the sign-in page. A pick followed by a reload of the sign-in page, or by an identity provider's redirect, is still not saved.

‎ui/apps/console/src/pages/auth/LoginPage.tsx‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ import {
7070
InvitationSignUpPrompt,
7171
invitationArrivalCopy,
7272
useInvitationArrival,
73+
hasSignInLanguagePick,
74+
saveSignInLanguagePick,
7375
} from '@object-ui/app-shell';
7476
import { AuthLayout } from './AuthLayout';
7577
import { AuthFooter, readLegalLinks } from './AuthFooter';
@@ -153,6 +155,11 @@ export function LoginPage() {
153155
const ssoHandoffStartedRef = useRef(false);
154156
// The same once-only latch for the mobile sign-in hand-off navigation.
155157
const mobileHandoffStartedRef = useRef(false);
158+
// #30399 — a language picked on this page, saved as the person's own before
159+
// any exit below navigates away. The ref is the once-only latch; the state
160+
// re-runs the post-login effect when the save has settled.
161+
const signInPickSaveRef = useRef<'idle' | 'saving' | 'settled'>('idle');
162+
const [signInPickSaved, setSignInPickSaved] = useState(false);
156163

157164
// `isLoading` from useAuth() is overloaded: it is true both during the
158165
// initial session check AND during every in-flight signIn. The full-page
@@ -244,6 +251,27 @@ export function LoginPage() {
244251
useEffect(() => {
245252
if (!user) return;
246253

254+
// 0. A language picked on this page while signed out (#30399): saved as
255+
// this person's own when they have none, and AWAITED before every exit
256+
// below — each leaves by a full navigation, which ends the page-load
257+
// that knows the pick was made here, and a write sent beside it races
258+
// the unload. Without a pick nothing is sent and nothing waits. A failed
259+
// save does not hold the sign-in: the pick still applies on this device.
260+
// A session the hand-off would refuse is not a sign-in yet: skipped.
261+
if (signInPickSaveRef.current === 'saving') return;
262+
if (
263+
signInPickSaveRef.current === 'idle' &&
264+
!handoff.holdForSignIn &&
265+
hasSignInLanguagePick()
266+
) {
267+
signInPickSaveRef.current = 'saving';
268+
void saveSignInLanguagePick(String(user.id)).finally(() => {
269+
signInPickSaveRef.current = 'settled';
270+
setSignInPickSaved(true);
271+
});
272+
return;
273+
}
274+
247275
// 1. OAuth provider hand-off (see file header). Replay the signed
248276
// authorize params so the IdP issues the code, then FOLLOW the redirect
249277
// it hands back. better-auth's oauth-provider answers a same-origin
@@ -310,6 +338,7 @@ export function LoginPage() {
310338
switchOrganization,
311339
handoff.target,
312340
handoff.holdForSignIn,
341+
signInPickSaved,
313342
]);
314343

315344
if ((isLoading && !hasBootstrapped) || !signInNeeded) {
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
/**
2+
* The sign-in page saves a language picked on it before it navigates away
3+
* (objectstack#30399, the saved half of objectstack#30338).
4+
*
5+
* The knowledge that the pick was made on this page is memory, and every exit
6+
* of the post-login step is a full navigation that ends it. So the page awaits
7+
* app-shell's `saveSignInLanguagePick` before each exit — a write sent beside
8+
* the navigation races the unload. What the save writes, and when it writes
9+
* nothing, is app-shell's and pinned beside the pick itself
10+
* (`userLocaleSignInPick-30338.test.tsx`); this file pins the ORDER on the
11+
* page, one case per exit:
12+
*
13+
* 1. with a pick, nothing navigates while the save is in flight, and the exit
14+
* the page would have taken is taken once it settles;
15+
* 2. without a pick, nothing is saved and nothing waits (CONTROL);
16+
* 3. a session the mobile hand-off refuses for its age is not a sign-in yet:
17+
* nothing is saved for it.
18+
*
19+
* The real `AuthProvider` runs over a mock client, as in
20+
* `signInHandoff-29971.test.tsx`.
21+
*/
22+
23+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
24+
import { render, screen, cleanup, waitFor } from '@testing-library/react';
25+
import { MemoryRouter } from 'react-router-dom';
26+
import { AuthProvider } from '@object-ui/auth';
27+
import type { AuthClient } from '@object-ui/auth';
28+
29+
const pick = vi.hoisted(() => ({
30+
held: false,
31+
save: vi.fn<(userId: string) => Promise<void>>(),
32+
}));
33+
34+
vi.mock('@object-ui/app-shell', async (importOriginal) => ({
35+
...(await importOriginal<Record<string, unknown>>()),
36+
hasSignInLanguagePick: () => pick.held,
37+
saveSignInLanguagePick: (userId: string) => pick.save(userId),
38+
}));
39+
40+
const follow = vi.hoisted(() => vi.fn());
41+
vi.mock('../followAuthorize', () => ({ followOauthAuthorize: follow }));
42+
43+
import { LoginPage } from '../LoginPage';
44+
45+
const HANDOFF =
46+
'/api/v1/auth/one-time-token/handoff?redirect_uri=ai.objectos.mobile%3A%2F%2Fauth%2Fcallback' +
47+
'&code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256';
48+
const OAUTH = '?client_id=rp&redirect_uri=https%3A%2F%2Frp.example%2Fcb&state=s';
49+
50+
const USER = { id: 'u1', name: 'Ada', email: 'ada@example.com' };
51+
const minutesAgo = (n: number) => new Date(Date.now() - n * 60_000);
52+
53+
let baseEl: HTMLBaseElement | null = null;
54+
let assign: ReturnType<typeof vi.fn>;
55+
/** Settles the save the page is awaiting. */
56+
let settleSave: () => void;
57+
58+
beforeEach(() => {
59+
window.localStorage.clear();
60+
baseEl = document.createElement('base');
61+
baseEl.setAttribute('href', '/_console/');
62+
document.head.appendChild(baseEl);
63+
vi.stubEnv('BASE_URL', './');
64+
assign = vi.fn();
65+
vi.spyOn(window.location, 'assign').mockImplementation(assign as never);
66+
vi.spyOn(console, 'warn').mockImplementation(() => {});
67+
follow.mockReset();
68+
pick.held = true;
69+
pick.save.mockReset();
70+
pick.save.mockImplementation(
71+
() =>
72+
new Promise<void>((resolve) => {
73+
settleSave = () => {
74+
pick.held = false;
75+
resolve();
76+
};
77+
}),
78+
);
79+
});
80+
81+
afterEach(() => {
82+
cleanup();
83+
baseEl?.remove();
84+
baseEl = null;
85+
vi.unstubAllEnvs();
86+
vi.restoreAllMocks();
87+
});
88+
89+
type Orgs = Array<{ id: string; name: string; slug: string }>;
90+
91+
function createMockClient(organizations?: Orgs, createdAt: Date = minutesAgo(1)) {
92+
const client = {
93+
getSession: vi.fn().mockResolvedValue({ user: USER, session: { token: 't', createdAt } }),
94+
getConfig: vi.fn().mockResolvedValue({}),
95+
signIn: vi.fn().mockResolvedValue({ user: USER, session: undefined }),
96+
signUp: vi.fn(),
97+
signInWithProvider: vi.fn(),
98+
...(organizations
99+
? {
100+
listOrganizations: vi.fn().mockResolvedValue(organizations),
101+
getActiveOrganization: vi.fn().mockResolvedValue(null),
102+
setActiveOrganization: vi.fn(async (id: string) => organizations.find((o) => o.id === id) ?? null),
103+
}
104+
: {}),
105+
};
106+
return client as typeof client & AuthClient;
107+
}
108+
109+
function renderLogin(client: AuthClient, search = '') {
110+
window.history.replaceState({}, '', `/_console/login${search}`);
111+
return render(
112+
<AuthProvider authUrl="/api/v1/auth" client={client}>
113+
<MemoryRouter initialEntries={[`/login${search}`]}>
114+
<LoginPage />
115+
</MemoryRouter>
116+
</AuthProvider>,
117+
);
118+
}
119+
120+
const lands = (target: string) => new URL(target, document.baseURI).pathname;
121+
/** Long enough for every effect and microtask the page would run before an exit. */
122+
const settle = () => new Promise((r) => setTimeout(r, 50));
123+
124+
const EXITS: Array<{
125+
exit: string;
126+
search: string;
127+
organizations?: Orgs;
128+
left: () => string | null;
129+
expected: string;
130+
}> = [
131+
{
132+
exit: 'no organization: the Console root',
133+
search: '',
134+
left: () => (assign.mock.calls[0] ? lands(assign.mock.calls[0][0] as string) : null),
135+
expected: '/_console/',
136+
},
137+
{
138+
exit: 'several organizations, none active: the picker',
139+
search: '',
140+
organizations: [
141+
{ id: 'org_1', name: 'One', slug: 'one' },
142+
{ id: 'org_2', name: 'Two', slug: 'two' },
143+
],
144+
left: () => (assign.mock.calls[0] ? lands(assign.mock.calls[0][0] as string) : null),
145+
expected: '/_console/organizations',
146+
},
147+
{
148+
exit: 'one organization: selected, then the safe redirect',
149+
search: `?redirect=${encodeURIComponent('/settings')}`,
150+
organizations: [{ id: 'org_1', name: 'One', slug: 'one' }],
151+
left: () => (assign.mock.calls[0] ? lands(assign.mock.calls[0][0] as string) : null),
152+
expected: '/_console/settings',
153+
},
154+
{
155+
exit: 'the mobile sign-in hand-off',
156+
search: `?redirect=${encodeURIComponent(HANDOFF)}`,
157+
left: () => (assign.mock.calls[0] ? (assign.mock.calls[0][0] as string) : null),
158+
expected: HANDOFF,
159+
},
160+
{
161+
exit: 'the OAuth provider hand-off',
162+
search: OAUTH,
163+
left: () => (follow.mock.calls[0] ? (follow.mock.calls[0][0] as string) : null),
164+
expected: OAUTH,
165+
},
166+
];
167+
168+
describe('LoginPage — a language picked here is saved before the page leaves (objectstack#30399)', () => {
169+
it.each(EXITS)('$exit: nothing leaves while the save is in flight, then the page leaves', async (c) => {
170+
renderLogin(createMockClient(c.organizations), c.search);
171+
172+
await waitFor(() => expect(pick.save).toHaveBeenCalledTimes(1));
173+
expect(pick.save).toHaveBeenCalledWith(USER.id);
174+
await settle();
175+
expect(assign).not.toHaveBeenCalled();
176+
expect(follow).not.toHaveBeenCalled();
177+
178+
settleSave();
179+
await waitFor(() => expect(c.left()).toBe(c.expected));
180+
expect(assign.mock.calls.length + follow.mock.calls.length).toBe(1);
181+
expect(pick.save).toHaveBeenCalledTimes(1);
182+
});
183+
184+
// Which exit, and how often, is not this file's to pin here: with nothing
185+
// to wait for, a page whose organizations are still loading can navigate
186+
// before they answer and again after — the post-login step's own behaviour,
187+
// unchanged by this card.
188+
it.each(EXITS)('CONTROL — $exit, with no pick: nothing is saved and the page leaves', async (c) => {
189+
pick.held = false;
190+
renderLogin(createMockClient(c.organizations), c.search);
191+
192+
await waitFor(() =>
193+
expect(assign.mock.calls.length + follow.mock.calls.length).toBeGreaterThanOrEqual(1),
194+
);
195+
expect(pick.save).not.toHaveBeenCalled();
196+
});
197+
198+
it('a session the mobile hand-off refuses for its age is not a sign-in yet: nothing is saved', async () => {
199+
renderLogin(createMockClient(undefined, minutesAgo(6)), `?redirect=${encodeURIComponent(HANDOFF)}`);
200+
201+
expect(await screen.findByLabelText('Email')).toBeTruthy();
202+
await settle();
203+
expect(pick.save).not.toHaveBeenCalled();
204+
expect(assign).not.toHaveBeenCalled();
205+
});
206+
});

0 commit comments

Comments
 (0)