Skip to content

Commit 61b487b

Browse files
fix(app-shell): a personal view opened by its address no longer carries the default view's filter and sort in the URL (#30085)
<!-- claude-projects-attribution: {"github_login":"os-support-ai"} --> _Requested by **Yibo Wang** · [project thread](https://claude.ai/code/project/chan_01BVUyyPie8gHwoPM18iABwB?thread=cmsg_01BVUyyPie8gHwoPM18iABwB91foStSJ4BheWaVcVqRJRd)_ Fixes #30034 ## 维护者速读 - 改了什么:直接打开个人视图时,在「我的视图」读取完成、所请求的视图解析出来之前,列表不再取种子,也不再把任何状态写进地址栏;解析完成后只用该视图自己的状态。 - 为什么改:偏好先到、个人视图后到时,默认视图的筛选和排序会被写进 URL 并留在那里,刷新或分享链接后个人视图带上谁也没选过的条件,还显示「已个性化」。 - 风险与回滚:只改 app-shell 两个文件里的一个门禁(种子条件和写回 effect 的依赖),回滚即还原此 PR;带 `uf_*` 参数的链接行为不变(有对照测试)。 - 先看:`ui/packages/app-shell/src/views/objectViewListUrlState.ts` 的种子条件与 effect 依赖;`ObjectView.tsx` 传入 `awaitingPersonalViews` 的一行;新测试 `ObjectView.personalViews-29635.test.tsx` 末尾的 describe。 - 两分钟验证:`cd ui && pnpm exec vitest run packages/app-shell/src/views/ObjectView.personalViews-29635.test.tsx -t 30034`,预期 3 例全绿;在 main 上同一命令「偏好先到」那例变红,地址栏多出 `uf__filter`/`uf__sort`。 - 需要你做的:无,席位落地。 Clause-②: no Metadata-protocol: none ## What changes - `useObjectViewListUrlState` takes `viewResolving`, which `ObjectView` passes as its existing `awaitingPersonalViews` (a requested or pinned view the list does not hold yet, while the reader's own views are still being read). While it is true, no seed is taken, so the write-back effect has nothing to write: the fall-back view's stored filter, sort and search never reach the address bar. The effect now also lists `viewResolving` in its dependencies, so a resolution that ends on the fall-back view (a pin naming a deleted personal view) still seeds and writes once. - When the view resolves, the seed is taken for that view alone, by the unchanged rules: the `uf_*` params a link carried, or else the view's own stored state. - No delay or timer was added. The gate is on the resolved identity. ## Why: the race, measured The card's reading holds, with the mechanism made concrete. React Router 7's `BrowserRouter` (the Console's router; `MemoryRouter` and `HistoryRouter` too) commits each location change inside `startTransition`. So: 1. The preferences land first. The page is still on the default view (the fall-back while `awaitingPersonalViews`), takes the default view's stored seed and writes it with `setSearchParams` (replace). That location update is a pending transition. 2. The personal views read lands before the transition commits. Its state update is urgent, so React renders it first, against the address as it was before step 1's write. 3. The identity moves to the personal view. Its seed compares against that old, empty address, finds nothing to change, and records the empty key as the page's own write. 4. The transition commits, and the default view's `uf__filter` and `uf__sort` stay in the address bar. The list's own query is the personal view's (its seed was clean), so nothing on screen shows it until a reload or a shared link opens the polluted address. That matches the card's evidence: the list request carried only the personal view's filter, and reloading the polluted URL drew the filter badge and the personalized marker. On the dev boot below, the pending location update was still not committed when the personal views read landed, about 0.8 s after the write, on every load (6 of 6). ## Tests New top-level describe in the shared personal-views harness `ui/packages/app-shell/src/views/ObjectView.personalViews-29635.test.tsx` (real `ObjectView`, real `ListPreferencesProvider` with an attached family, real router through `unstable_HistoryRouter` over a memory history). The two reads are deferred and land in the order each case names. The address bar is read from the history the router wrote. - **Personal views read lands AFTER the preferences read** (the personal views read is landed inside the first `history.replace`, which is exactly the window above): the address bar ends with no `uf_*` param, and the list's last query carries the personal view's `Paris` filter and not the default view's `Rome`. - **The reverse order** (personal views first, then preferences): the same clean end state. - **CONTROL**: a link carrying `uf__sort` for the personal view keeps that param and the list sorts by it. <details><summary>Red on main, green on the branch, ablation (head 1cf9cef)</summary> - Red on main (`origin/main` blobs of `ObjectView.tsx` and `objectViewListUrlState.ts` checked out over the final test, restored after, `git diff HEAD` empty): `Tests 1 failed | 2 passed`. The failing case is the AFTER order; received `uf__filter` = the default view's `city equals Rome` and `uf__sort` = `status desc`. - Mechanism probe on main: the same test with `useTransitions={false}` on the router passes all 3. The red is the transition window, not some other write. - Ablation (fix committed first, `scripts/ablation-replace.mjs` removing `!viewResolving &&` from the seed condition, restored blob == HEAD): `Tests 1 failed | 2 passed`, the same AFTER case red. - Branch: the harness file `Tests 14 passed (14)`. The 88 app-shell test files that import `ObjectView` or `objectViewListUrlState`: `Test Files 88 passed (88)`, `Tests 833 passed (833)`. - `pnpm --filter @object-ui/app-shell run type-check` (both tsconfigs): exit 0. `npx eslint` on the 3 changed files from the package dir: 0 errors (80 existing `react-hooks/refs` warnings, none new in kind). </details> ## Acceptance notes - Acceptance 1 and 2 are pinned by the two order cases, and acceptance 4 by the CONTROL. Acceptance 3 ("every load") was measured live: 0 of 6 loads polluted after the fix, against 6 of 6 before (Verification). - The gate also covers a pinned personal view opened from the bare address, since `awaitingPersonalViews` already includes the pin arm. - `listIdentityKey` itself still names the fall-back view while resolving. Nothing renders the list then (`ObjectView` draws no list while `awaitingPersonalViews`), and the chip selections are re-read per identity, so only the seed needed the gate. ## Verification <details><summary>Local gates at head 1cf9cef: 35 derived, 35 exit 0</summary> - `node scripts/pm/dispatch-gates.mjs --commands` at `1cf9cef8` (merge base `c23501b2b`, 4 paths): 35 families, each run serially. All 35 exited 0, including `node scripts/check-sdui-manifest.mjs` and its `--self-test`, `pnpm check:type-check-debt` (it took the verify lock for its own build), and `pnpm --dir ui exec vitest run scripts/` (run under the lock). - `--ran` reconciliation exit 0: 35 derived, 35 run, 0 NOT-MEASURED, 0 UNRUN. - CI families: this pull's check-runs had not concluded when it was opened. The seat cites them at ACCEPT. </details> <details><summary>Live check on a dev boot: 6 of 6 loads polluted before, 0 of 6 after</summary> - Stack: full `turbo run build --filter='!@objectstack/docs'` under the lock (exit 0), the showcase boot `pnpm dev -- --fresh -p 4317`, and the Console's vite from `ui/apps/console` serving ui src. Vite was restarted between legs with `--force`: in this container it did not pick up a src change made under a running server. - Seeded by API as the admin: a writable package, an object with 3 rows, its default list view `pvrace_item.default`, an app, the admin's stored list state for the default view (`city equals Rome`, `status desc`, in `sys_user_preference` key `ui.list.pvrace_item`), and one personal view filtered to `city equals Paris`. - CONTROL, both legs: the bare default view address restores the stored state into the address bar (`uf__filter` Rome + `uf__sort` status desc). So the stored state was there to leak. - Before (`origin/main` src of both files): 6 hard loads of `/apps/com.qa.pvrace/pvrace_item/view/PV_ID`. All 6 ended with the default view's `uf__filter` and `uf__sort` in the address bar. In a traced main load, the list's request carried only the personal view's `city equals Paris`, as the card measured. - After (head `1cf9cef8` src): the same 6 loads all ended with no `uf_*` param. - The boot and vite were torn down (health 000). </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WGufziPRtMejsYzERkQhwd --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f35b361 commit 61b487b

4 files changed

Lines changed: 200 additions & 5 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+
---
4+
5+
fix(app-shell): a personal view opened by its address no longer carries the default view's filter and sort in the address bar
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- While the list waits for the reader's own views to resolve a requested or pinned view it does not hold yet, the list's URL state is neither seeded nor written. The default view the page falls back to meanwhile is not the view the address names, so its stored filter, sort and search no longer reach the address bar. Before, when the list preferences landed first, that default seed was written to the URL, and a personal views read landing before the write committed left it there: a reload or a shared link then opened the personal view with conditions nobody chose for it and marked it personalized.
11+
- When the view resolves, the seed is taken for it alone: from its own stored state, or from the `uf_*` params a link carried, as before.

‎ui/packages/app-shell/src/views/ObjectView.personalViews-29635.test.tsx‎

Lines changed: 166 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,22 @@
2525
* view it does not hold yet.
2626
* - A refused read (no grant on the object) draws neither the section nor
2727
* Save as my view, and the list is unaffected.
28+
* - objectstack#30034 — a personal view opened by its address is not seeded
29+
* from, and its address bar never carries, the default view's stored list
30+
* state, whichever of the two reads (the reader's own views, the list
31+
* preferences) lands first; a link's own `uf_*` params still open it.
2832
*/
2933

3034
import * as React from 'react';
3135
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
3236
import { render, cleanup, screen, waitFor, within, fireEvent } from '@testing-library/react';
33-
import { MemoryRouter, Routes, Route } from 'react-router-dom';
37+
import {
38+
MemoryRouter,
39+
Routes,
40+
Route,
41+
unstable_HistoryRouter as HistoryRouter,
42+
UNSAFE_createMemoryHistory as createMemoryHistory,
43+
} from 'react-router-dom';
3444
import { createI18n, I18nProvider } from '@object-ui/i18n';
3545
import { ListViewSchema } from '@objectstack/spec/ui';
3646

@@ -80,6 +90,9 @@ vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null }));
8090

8191
import { ObjectView } from './ObjectView';
8292
import { ExpressionProvider } from '../providers/ExpressionProvider';
93+
import { UserStateAdaptersProvider, useAttachListStateFamily } from '../context/UserStateAdapters';
94+
import { ListPreferencesProvider, listPreferenceKey } from '../context/ListPreferencesProvider';
95+
import { applyListStateParams } from './userFilterUrlState';
8396

8497
/** Every request the page sends through `fetch`: the owner's door among them. */
8598
const fetchCalls: Array<{ url: string; method: string }> = [];
@@ -378,3 +391,155 @@ describe('objectstack#29635 — My views in the list view switcher', () => {
378391
expect(within(menu).getByTestId('list-view-switcher-save-mine')).toBeInTheDocument();
379392
});
380393
});
394+
395+
/**
396+
* objectstack#30034 — the two reads a personal view opened by its address
397+
* waits on: the reader's own views (`sys_personal_view`) and the list
398+
* preferences (the attached family's first load). Each lands when the case
399+
* says, and the address bar is the page's history, read where the router
400+
* wrote it.
401+
*
402+
* The race the card measured: the location the router hands the page moves in
403+
* a transition, so a read landing between the page's URL write and its commit
404+
* renders against the address as it was before that write. `landPersonalOn`
405+
* `'firstReplace'` lands the personal views read exactly there.
406+
*/
407+
describe('objectstack#30034 — a personal view opened by its address never carries the default view\'s list state', () => {
408+
/** The reader's stored list state for the DEFAULT view, `v1`: a filter and a sort. */
409+
const DEFAULT_VIEW_STATE = {
410+
perView: {
411+
v1: {
412+
sort: [{ field: 'status', order: 'desc' }],
413+
filters: { id: 'root', logic: 'and', conditions: [{ id: 'c1', field: 'city', operator: 'equals', value: 'Rome' }] },
414+
},
415+
},
416+
};
417+
418+
function deferred<T>() {
419+
let resolve: (value: T) => void = () => {};
420+
const promise = new Promise<T>((r) => { resolve = r; });
421+
return { promise, resolve };
422+
}
423+
424+
function AttachFamily({ family }: { family: unknown }) {
425+
const attach = useAttachListStateFamily();
426+
React.useEffect(() => {
427+
attach(family as never);
428+
return () => attach(null);
429+
}, [attach, family]);
430+
return null;
431+
}
432+
433+
function mountPersonal(
434+
entry: string,
435+
{ landPersonalOn }: { landPersonalOn: 'firstReplace' | 'beforePreferences' },
436+
) {
437+
const objects = [LEAD];
438+
const i18n = createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false });
439+
const personalRead = deferred<{ data: unknown[]; total: number }>();
440+
const preferencesRead = deferred<Record<string, unknown>>();
441+
const landPersonal = () => personalRead.resolve({ data: [OWN], total: 1 });
442+
const landPreferences = () => preferencesRead.resolve({ [listPreferenceKey('crm_lead')]: structuredClone(DEFAULT_VIEW_STATE) });
443+
const family = { prefix: 'ui.list.', load: vi.fn(() => preferencesRead.promise), save: vi.fn(async () => {}) };
444+
const history = createMemoryHistory({ initialEntries: [entry], v5Compat: true });
445+
const replace = history.replace.bind(history);
446+
let replaced = 0;
447+
history.replace = (...args: Parameters<typeof history.replace>) => {
448+
replace(...args);
449+
replaced += 1;
450+
if (replaced === 1 && landPersonalOn === 'firstReplace') landPersonal();
451+
};
452+
const dataSource = {
453+
find: vi.fn(async (resource: string, _params?: unknown) => (resource === 'sys_personal_view' ? personalRead.promise : { data: ROWS, total: ROWS.length })),
454+
findOne: vi.fn(async () => null),
455+
getObjectSchema: vi.fn(async (name: string) => objects.find((o) => o.name === name) ?? null),
456+
create: vi.fn(async () => ({})),
457+
update: vi.fn(async () => ({})),
458+
delete: vi.fn(async () => ({})),
459+
};
460+
render(
461+
<UserStateAdaptersProvider>
462+
<AttachFamily family={family} />
463+
<ListPreferencesProvider>
464+
<I18nProvider instance={i18n}>
465+
<ExpressionProvider user={{ id: 'u1', name: 'Sales Rep' }}>
466+
<HistoryRouter history={history as never}>
467+
<Routes>
468+
<Route
469+
path="/apps/:appName/:objectName/view/:viewId"
470+
element={<ObjectView dataSource={dataSource as never} objects={objects} onEdit={() => {}} />}
471+
/>
472+
</Routes>
473+
</HistoryRouter>
474+
</ExpressionProvider>
475+
</I18nProvider>
476+
</ListPreferencesProvider>
477+
</UserStateAdaptersProvider>,
478+
);
479+
return {
480+
dataSource,
481+
landPersonal,
482+
landPreferences,
483+
/** The `uf_*` params of the address bar, by name. */
484+
ufParams: () => Object.fromEntries([...new URLSearchParams(history.location.search)].filter(([key]) => key.startsWith('uf_'))),
485+
};
486+
}
487+
488+
const settle = () => new Promise((r) => setTimeout(r, 300));
489+
490+
/** The `crm_lead` reads the list sent, in order (the footer's `$top: 0` count read is not one). */
491+
const listReads = (dataSource: { find: { mock: { calls: unknown[][] } } }) =>
492+
dataSource.find.mock.calls
493+
.filter(([resource, params]) => resource === 'crm_lead' && (params as { $top?: number } | undefined)?.$top !== 0)
494+
.map(([, params]) => params as Record<string, unknown>);
495+
496+
it('the personal views read landing AFTER the preferences read: the address bar ends clean, holding only the personal view\'s own state', async () => {
497+
const page = mountPersonal('/apps/crm/crm_lead/view/pv_mine', { landPersonalOn: 'firstReplace' });
498+
// The preferences land first; the personal views read is still out.
499+
await waitFor(() => expect(page.dataSource.find.mock.calls.some(([r]) => r === 'sys_personal_view')).toBe(true));
500+
page.landPreferences();
501+
await settle();
502+
// Nothing wrote the address while the requested view was unresolved, so
503+
// the personal views read is still out: land it now.
504+
page.landPersonal();
505+
await listToolbar();
506+
expect(switcher()).toHaveTextContent('Paris leads');
507+
await settle();
508+
expect(page.ufParams()).toEqual({});
509+
// The list's query carries the personal view's own filter, and none of the default view's.
510+
const last = JSON.stringify(listReads(page.dataSource).at(-1));
511+
expect(last).toContain('Paris');
512+
expect(last).not.toContain('Rome');
513+
});
514+
515+
it('the reverse order — the personal views read landing BEFORE the preferences read: the address bar ends clean too', async () => {
516+
const page = mountPersonal('/apps/crm/crm_lead/view/pv_mine', { landPersonalOn: 'beforePreferences' });
517+
await waitFor(() => expect(page.dataSource.find.mock.calls.some(([r]) => r === 'sys_personal_view')).toBe(true));
518+
page.landPersonal();
519+
await settle();
520+
page.landPreferences();
521+
await listToolbar();
522+
expect(switcher()).toHaveTextContent('Paris leads');
523+
await settle();
524+
expect(page.ufParams()).toEqual({});
525+
const last = JSON.stringify(listReads(page.dataSource).at(-1));
526+
expect(last).toContain('Paris');
527+
expect(last).not.toContain('Rome');
528+
});
529+
530+
it('CONTROL: a link carrying `uf_*` params the reader sent opens the personal view with those params applied', async () => {
531+
const sort = [{ field: 'name', order: 'asc' as const }];
532+
const query = applyListStateParams(new URLSearchParams(), { sort }).toString();
533+
const page = mountPersonal(`/apps/crm/crm_lead/view/pv_mine?${query}`, { landPersonalOn: 'firstReplace' });
534+
await waitFor(() => expect(page.dataSource.find.mock.calls.some(([r]) => r === 'sys_personal_view')).toBe(true));
535+
page.landPreferences();
536+
await settle();
537+
page.landPersonal();
538+
await listToolbar();
539+
expect(switcher()).toHaveTextContent('Paris leads');
540+
await settle();
541+
expect(page.ufParams()).toEqual(Object.fromEntries(new URLSearchParams(query)));
542+
await waitFor(() => expect(listReads(page.dataSource).at(-1)?.$orderby).toEqual(sort));
543+
expect(JSON.stringify(listReads(page.dataSource).at(-1))).not.toContain('Rome');
544+
});
545+
});

‎ui/packages/app-shell/src/views/ObjectView.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co
679679
objectName,
680680
objectDef,
681681
activeView,
682+
viewResolving: awaitingPersonalViews,
682683
listPrefs,
683684
perms,
684685
user,

‎ui/packages/app-shell/src/views/objectViewListUrlState.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export function useObjectViewListUrlState({
101101
objectName,
102102
objectDef,
103103
activeView,
104+
viewResolving,
104105
listPrefs,
105106
perms,
106107
user,
@@ -110,6 +111,12 @@ export function useObjectViewListUrlState({
110111
objectName: string | undefined;
111112
objectDef: ConsoleObjectViewProps['objects'][number];
112113
activeView: ReturnType<typeof buildViewTabs>[number];
114+
/**
115+
* objectstack#30034 — the view the address (or the reader's pin) names is
116+
* still being resolved (`ObjectView`'s `awaitingPersonalViews`), so
117+
* `activeView` is only the view the page falls back to meanwhile.
118+
*/
119+
viewResolving: boolean;
113120
listPrefs: ReturnType<typeof useObjectListPreferences>;
114121
perms: ReturnType<typeof usePermissions>;
115122
user: ReturnType<typeof useAuth>['user'];
@@ -138,6 +145,15 @@ export function useObjectViewListUrlState({
138145
* seed waits until that state has been read (`listPrefs.ready`), so a
139146
* second browser opens on it rather than on an empty cache.
140147
*
148+
* Neither seed is taken, and nothing is written, while the view the
149+
* address names is still being resolved (`viewResolving`, objectstack#30034):
150+
* the view on hand then is the fall-back, not the one asked for, and its
151+
* stored state written into the address would outlive it. The location
152+
* the router hands this page moves in a transition, so a read that lands
153+
* between that write and its commit renders against the old address,
154+
* finds nothing to change for the resolved view, and the fall-back's
155+
* filter and sort stay in a link that a reload or a colleague then opens.
156+
*
141157
* Both seeds pass the same field gate (`acceptField`, objectstack#28949):
142158
* a field stored before it was deleted, or before this user lost read on
143159
* it, is dropped exactly as a stale link's is, and the rest of the state
@@ -171,7 +187,7 @@ export function useObjectViewListUrlState({
171187
// that list opens from the (just cleared) stored state, never from the
172188
// `uf_*` params, whoever wrote them, and its write-back strips them all.
173189
const resetPendingRef = useRef(false);
174-
if (listIdentityKey && listPrefs.ready && listSeedRef.current?.key !== listIdentityKey) {
190+
if (listIdentityKey && listPrefs.ready && !viewResolving && listSeedRef.current?.key !== listIdentityKey) {
175191
// A field the object does not declare (a stale link, or a stored state
176192
// older than the field's deletion) or one this user cannot read is
177193
// dropped. Field-level permissions gate only once loaded — the same
@@ -218,10 +234,12 @@ export function useObjectViewListUrlState({
218234
if (seed.source === 'storage') ownListParamsRef.current = userFilterParamsKey(next);
219235
if (next.toString() !== searchParams.toString()) setSearchParams(next, { replace: true });
220236
// Once per list identity, by design: later URL writes are the list's
221-
// own changes, already on screen. (`listPrefs.ready` because the seed
222-
// is taken only once the user's own state has been read.)
237+
// own changes, already on screen. (`listPrefs.ready` and
238+
// `viewResolving` because the seed is taken only once the user's own
239+
// state has been read and the view the address names is resolved; a
240+
// resolution that ends on the fall-back keeps the identity unchanged.)
223241
// eslint-disable-next-line react-hooks/exhaustive-deps -- see above
224-
}, [listIdentityKey, listPrefs.ready]);
242+
}, [listIdentityKey, listPrefs.ready, viewResolving]);
225243
/** Write the list's own change into the URL — replace, never a history entry per keystroke. */
226244
const writeListUrlState = useCallback(
227245
(patch: ListStatePatch) => {

0 commit comments

Comments
 (0)