Skip to content

Commit d1f3ac0

Browse files
fix(app-shell): a stored list sort or filter on a deleted or unreadable field is dropped when the list opens (#29027)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb6KQHAgTkT9HJRZ4Ezx4M8E)_ Fixes #28949 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:列表打开时,用户自己存下的排序和筛选现在也走链接参数已有的字段检查。排序层或筛选条件(任意嵌套层级)指向对象已不声明的字段、或当前用户已不可读的字段,就在打开时丢掉;其余存下的状态照常生效。 - **为什么改**:在 app-crm 实测:存下的排序/筛选指向已不存在的字段时,修复前整张列表显示「This view’s query was rejected」(服务端 400 `INVALID_FIELD` / `INVALID_SORT`);字段对销售代表隐藏时,服务端对同一查询答 403 `PERMISSION_DENIED`。链接里的同类字段早已被丢弃,存下的偏好却原样进查询。 - **风险与回滚**:低。只改一个文件里种子(seed)的取值,没有新键、新选项,也不改存储。打开列表时不回写存储行:被丢的部分仍留在行里,字段读权限恢复后会重新生效;用户下一次改排序或筛选时,那一部分被列表当前状态替换。回滚即回退本 PR。 - **先看**:`ui/packages/app-shell/src/views/objectViewListUrlState.ts`(`acceptStoredFilters` 的嵌套遍历、`acceptStoredSort` 对空排序的处理、`acceptField` 提到分支外);`ui/packages/app-shell/src/views/ObjectView.listUrlState-11860.test.tsx` 新增的 objectstack#28949 组。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/app-shell/src/views/ObjectView.listUrlState-11860.test.tsx`,预期全部通过;新组里「a field the object no longer declares」断言查询只带 `priority` 条件和 `due_date` 排序。 - **需要你做的**:无,席位落地。 ## What changes `useObjectViewListUrlState` seeds a list from the URL or, when the URL carries no list state, from the user's stored state (`listPrefs`). The URL seed already passed every sort level and filter condition through `acceptField`: a field the object does not declare, or one this user cannot read once field permissions have loaded, is dropped. The stored seed passed `filters` and `sort` in unchecked. - `acceptField` moves above the URL/storage branch, unchanged, so both seeds use the one gate. - `acceptStoredSort` drops a level naming a refused field and keeps the others in order. A stored `sort: []` (the user cleared the sort) is kept; a stored sort whose every level is dropped becomes no sort, so the view's own sort applies. This is the rule the URL's `uf__sort` reader already follows. - `acceptStoredFilters` walks the stored Filter panel group at any nesting. A condition naming a refused field is dropped; a nested group left with no condition is dropped with it; every other entry is kept exactly as stored, row ids included, because the panel rehydrates them. A group nothing was dropped from is returned as the same object. A group left with no condition is no filter. - No new key, option, or stored shape. `ListPreferencesProvider.tsx` is not touched; only its exported types are imported. ### What the card asks the PR to state (Target 4) - **Is the dropped part removed from the stored row?** No. Opening the list does not write the preference row. Measured on app-crm: after the fixed list opened, the `ui.list.crm_account` row still held the stale entries. The address bar's `uf__*` mirror shows the cleaned state. The user's next sort or filter change replaces that part of the row with what the list shows. Keeping it means a field whose read access comes back applies again. - **While field permissions are still loading:** the same rule as the URL seed and the list's columns. Until `perms.isLoaded`, only the declaration check applies; a field the user cannot read is dropped only once permissions have loaded. The seed is taken once per list identity, so a stored entry on an unreadable field would reach the query if the seed were taken before permissions arrive. On app-crm the permissions had loaded before the seed in 4 of 4 opens as a sales rep. Waiting for `perms.isLoaded` was not added: on a permissions error `isLoaded` stays false, so a wait would hold the list closed. ## Tests `ObjectView.listUrlState-11860.test.tsx`, beside the URL seed's "a malformed or stale param is dropped" pins, gets a new group `a stored sort or filter naming a stale field is dropped too (objectstack#28949)`. It renders the real page and `ListView` and reads the query at the data source: - a field the object no longer declares: the stale sort level and condition are gone; the valid sort and condition stored beside them reach the query and the address bar; - a field this user cannot read, once permissions are loaded: dropped; a readable field is untouched; - every stored entry stale: no filter, and the view's own declared sort; - CONTROL: while permissions are still loading, only the declaration check applies, as for a link; - the helpers directly: nested groups at any depth, ids kept, an untouched group returned as the same object, a cleared sort kept, a fully dropped sort gone. Ablation (one-time, not kept): with the two stored-seed lines put back to the unchecked values, the first three cases fail (3 failed, 25 passed); the restore read blob equal to `HEAD` with an empty `git diff HEAD`. ## Acceptance notes - **Premise, reproduced on `examples/app-crm`** (`pnpm dev:crm -- --fresh`, view `all` of `crm_account`): - *Missing field, as admin.* The stored row held a sort `ghost_field desc, annual_revenue desc` and a filter `ghost_field = x AND industry = technology`. Seeding a field the object does not declare is what a deleted field leaves behind; no field was deleted at runtime. Unfixed Console: the list query (`sort=-ghost_field,-annual_revenue`, filter on `ghost_field`) answered 400 and the list showed "This view’s query was rejected". Fixed Console: the query carried `sort=-annual_revenue` and filter `industry = technology`, and the list showed 2 records. - *Unreadable field, as a non-admin.* A sales rep holding `crm_sales_user`, with `crm_account.industry` set unreadable by a temporary local edit to the example permission set (not committed). As the rep, a filter or sort on `industry` answers 403 `PERMISSION_DENIED`, which is the query the unfixed seed sends. Fixed Console: the query carried `sort=-annual_revenue` and filter `name contains c`, and the list showed 2 records, in 4 of 4 opens. - The card expected "an empty or refused list with no hint". On the missing-field case the refusal panel does carry a hint ("Clearing the filters usually fixes it"). It does not name the stale field or mention "Reset view". - Screenshots are in the project's `issue-28949` folder: `unfixed-deleted-admin.png`, `fixed-deleted-admin.png`, `fixed-unreadable-rep.png`. ## Verification Branch base `fed3a51b`; every reading below is on head `59a34b51`. `origin/main` has moved 12 commits since, none touching a file this diff changes. <details><summary>Tests</summary> - `pnpm --dir ui exec vitest run packages/app-shell/src/views/ObjectView.listUrlState-11860.test.tsx packages/app-shell/src/views/ObjectView.userListState-12418.test.tsx`: 2 files, 54/54 passed. - All 85 `packages/app-shell/src/views/ObjectView*.test.*` files, in two batches: 43 files, 346 passed; 42 files, 359 passed; 0 failed. - `pnpm --dir ui exec vitest run scripts/`: 169 files passed, 2 skipped; 4900 tests passed. - Ablation, as described under Tests: 3 failed, 25 passed with the fix removed; restored blob equal to `HEAD`. </details> <details><summary>Type-check, lint, gates</summary> - `pnpm --filter @object-ui/app-shell run type-check`: exit 0. - `eslint` on the two changed files: 0 errors. The 13 warnings in `objectViewListUrlState.ts` are the 13 the file has on `main`. - `pnpm --dir ui run check:new-line-citations`: 0 new citations. - `node scripts/pm/dispatch-gates.mjs --commands`: 34 families derived; all 34 run, each exit 0 (`check-changeset-no-major`, `check-closing-keyword-parity` and `check-adr-0087-registration` with this body as the event). `dispatch-gates --ran`: 34 run, 0 NOT MEASURED. `node scripts/check-sdui-manifest.mjs`: exit 0, no registry change. - Dogfood battery: not run, as no `!` is in the title and `Clause-②` has no narrowing arm. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTDLaFovMzMRUHDgXU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e43df2 commit d1f3ac0

3 files changed

Lines changed: 176 additions & 9 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a stored list sort or filter on a deleted or unreadable field is dropped when the list opens
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A user's stored sort and Filter panel conditions for a list view now pass the same field check a list link already passes. A sort level or a filter condition naming a field the object no longer declares, or one the user can no longer read, is dropped when the list opens, at any nesting of filter groups. Everything else the user stored still applies, so the list opens with rows instead of failing on a field that is gone. When every stored sort level is dropped, the view's own sort applies; when every condition is dropped, the list opens unfiltered.
11+
12+
Opening the list does not rewrite the stored row: the dropped part stays stored, so a field whose read access comes back applies again. The user's next sort or filter change replaces that part with what the list shows. Field-level read permissions are applied once they have loaded, the same rule the list's columns and a list link follow.

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
* - opening and closing a panel leaves the URL alone (the triage control);
2323
* - a malformed or stale param is dropped: the list opens, nothing throws,
2424
* and the address bar loses it;
25+
* - objectstack#28949 — the same holds for the user's stored sort and
26+
* filter: a field deleted since, or one they can no longer read, is
27+
* dropped, and the rest of what they stored still applies;
2528
* - another view starts clean, and params this page wrote for one view never
2629
* read as a link to the next when the active view resolves late;
2730
* - a user the server refuses gets the same refusal with or without params.
@@ -93,6 +96,8 @@ import '@object-ui/plugin-grid';
9396
import { ExpressionProvider } from '../providers/ExpressionProvider';
9497
import { buildListFilterKey } from './listFilterStorage';
9598
import { LIST_FILTER_PARAM, LIST_GROUP_PARAM, LIST_SEARCH_PARAM, LIST_SORT_PARAM } from './userFilterUrlState';
99+
import { localListStateKey } from '../context/ListPreferencesProvider';
100+
import { acceptStoredFilters, acceptStoredSort } from './objectViewListUrlState';
96101

97102
const OBJ = 'url_task';
98103
const VIEW = `/apps/demo/${OBJ}/view`;
@@ -398,6 +403,96 @@ describe('a malformed or stale param is dropped — the list opens and nothing t
398403
});
399404
});
400405

406+
/** This user's stored state for one view (objectui#12418's per-browser cache, the shape the server row has). */
407+
function storeViewPreference(viewId: string, pref: { sort?: unknown; filters?: unknown }) {
408+
localStorage.setItem(localListStateKey('u1', OBJ), JSON.stringify({ perView: { [viewId]: pref } }));
409+
}
410+
411+
describe('a stored sort or filter naming a stale field is dropped too (objectstack#28949)', () => {
412+
const group = (...conditions: unknown[]) => ({ id: 'root', logic: 'and', conditions });
413+
414+
it('a field the object no longer declares: the list opens on the rest of the stored state', async () => {
415+
storeViewPreference('all', {
416+
sort: [{ field: 'ghost', order: 'asc' }, { field: 'due_date', order: 'desc' }],
417+
filters: group(
418+
{ id: 'g', field: 'ghost', operator: 'equals', value: 'x' },
419+
{ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' },
420+
),
421+
});
422+
const host = await mountHost([`${VIEW}/all`]);
423+
const q = lastQuery();
424+
expect(filterOf(q)).toContain('"priority","=","urgent"');
425+
expect(filterOf(q)).not.toContain('ghost');
426+
expect(q.$orderby).toEqual([{ field: 'due_date', order: 'desc' }]);
427+
const params = new URLSearchParams(host.search());
428+
expect(JSON.parse(params.get(LIST_FILTER_PARAM)!).conditions).toEqual([
429+
{ field: 'priority', operator: 'equals', value: 'urgent' },
430+
]);
431+
expect(JSON.parse(params.get(LIST_SORT_PARAM)!)).toEqual([{ field: 'due_date', order: 'desc' }]);
432+
});
433+
434+
it('a field this user cannot read is dropped once permissions are loaded; a readable one is untouched', async () => {
435+
perms.isLoaded = true;
436+
perms.checkField = (_object, field) => field !== 'priority';
437+
storeViewPreference('all', {
438+
sort: [{ field: 'priority', order: 'asc' }, { field: 'name', order: 'asc' }],
439+
filters: group(
440+
{ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' },
441+
{ id: 's', field: 'status', operator: 'equals', value: 'open' },
442+
),
443+
});
444+
await mountHost([`${VIEW}/all`]);
445+
const q = lastQuery();
446+
expect(filterOf(q)).not.toContain('priority');
447+
expect(filterOf(q)).toContain('"status","=","open"');
448+
expect(q.$orderby).toEqual([{ field: 'name', order: 'asc' }]);
449+
});
450+
451+
it('a stored state whose every entry is stale: no filter, and the view\'s own sort', async () => {
452+
storeViewPreference('sorted', {
453+
sort: [{ field: 'ghost', order: 'desc' }],
454+
filters: group({ id: 'g', field: 'ghost', operator: 'equals', value: 'x' }),
455+
});
456+
const host = await mountHost([`${VIEW}/sorted`]);
457+
expect(listQueries.length).toBeGreaterThan(0);
458+
expect(lastQuery().$filter).toBeUndefined();
459+
expect(lastQuery().$orderby).toEqual([{ field: 'name', order: 'asc' }]);
460+
const params = new URLSearchParams(host.search());
461+
expect(params.has(LIST_FILTER_PARAM)).toBe(false);
462+
expect(params.has(LIST_SORT_PARAM)).toBe(false);
463+
});
464+
465+
it('CONTROL: while permissions are still loading, only the declaration gates, as it does for a link', async () => {
466+
perms.isLoaded = false;
467+
perms.checkField = (_object, field) => field !== 'priority';
468+
storeViewPreference('all', { filters: group({ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' }) });
469+
await mountHost([`${VIEW}/all`]);
470+
expect(filterOf(lastQuery())).toContain('"priority","=","urgent"');
471+
});
472+
473+
it('nested groups are read at any depth; untouched entries keep their ids and a clean state its identity', () => {
474+
const accept = (field: string) => field !== 'ghost';
475+
const stored = group(
476+
{ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' },
477+
{ id: 'n1', logic: 'or', conditions: [
478+
{ id: 'g', field: 'ghost', operator: 'equals', value: 'x' },
479+
{ id: 's', field: 'status', operator: 'equals', value: 'open' },
480+
] },
481+
{ id: 'n2', logic: 'and', conditions: [{ id: 'g2', field: 'ghost', operator: 'equals', value: 'y' }] },
482+
);
483+
expect(acceptStoredFilters(stored, accept)).toEqual(group(
484+
{ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' },
485+
{ id: 'n1', logic: 'or', conditions: [{ id: 's', field: 'status', operator: 'equals', value: 'open' }] },
486+
));
487+
const clean = group({ id: 'p', field: 'priority', operator: 'equals', value: 'urgent' });
488+
expect(acceptStoredFilters(clean, accept)).toBe(clean);
489+
expect(acceptStoredFilters(group({ id: 'n', logic: 'or', conditions: [{ id: 'g', field: 'ghost', operator: 'equals', value: 'x' }] }), accept)).toBeUndefined();
490+
// A sort the user cleared stays cleared; one emptied by the gate is no sort.
491+
expect(acceptStoredSort([], accept)).toEqual([]);
492+
expect(acceptStoredSort([{ field: 'ghost', order: 'asc' }], accept)).toBeUndefined();
493+
});
494+
});
495+
401496
describe('the params belong to one view (objectui#11860)', () => {
402497
it('another view starts clean — the ad-hoc params do not follow the user into it', async () => {
403498
const host = await mountHost([link(`${VIEW}/all`, { filter: URGENT, search: 'acme' })]);

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

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,69 @@ import {
1818
applyListStateParams,
1919
userFilterParamsKey,
2020
type ListStatePatch,
21+
type ListUrlFieldGate,
2122
type ListUrlState,
23+
type ListSortRule,
2224
} from './userFilterUrlState.js';
2325
import { buildListFilterKey, readListFilterState } from './listFilterStorage.js';
24-
import type { useObjectListPreferences } from '../context/ListPreferencesProvider.js';
26+
import type {
27+
useObjectListPreferences,
28+
ListFilterGroupPreference,
29+
ListSortPreference,
30+
} from '../context/ListPreferencesProvider.js';
2531
import { parseUrlEqualityFilterTriples } from './drillUrlFilters.js';
2632
import { selectFilterParams } from './objectViewHelpers.js';
2733
import type { buildViewTabs } from './objectViewTabs.js';
2834
import type { ConsoleObjectViewProps } from './ObjectView.js';
2935

36+
const isRecord = (v: unknown): v is Record<string, unknown> =>
37+
!!v && typeof v === 'object' && !Array.isArray(v);
38+
39+
/**
40+
* objectstack#28949 — the stored sort through the seed's field gate: a level
41+
* naming a refused field is dropped and the levels around it keep their order.
42+
* An empty stored sort is a sort the user cleared, and it is kept; one whose
43+
* every level was dropped is no sort at all, so the view's own sort applies.
44+
*/
45+
export function acceptStoredSort(
46+
sort: readonly ListSortPreference[] | undefined,
47+
acceptField: ListUrlFieldGate,
48+
): ListSortRule[] | undefined {
49+
if (!sort) return undefined;
50+
const kept = sort.filter(level => acceptField(level.field));
51+
return sort.length === 0 || kept.length > 0 ? kept : undefined;
52+
}
53+
54+
/**
55+
* objectstack#28949 — the stored Filter panel group through the seed's field
56+
* gate, at any nesting: a condition naming a refused field is dropped, a
57+
* nested group left with no condition is dropped with it, and every other
58+
* entry is kept exactly as stored (ids included: the panel rehydrates them).
59+
* A group nothing was dropped from is returned as is; one left with no
60+
* condition at all is no filter, and the list opens unfiltered.
61+
*/
62+
export function acceptStoredFilters(
63+
group: ListFilterGroupPreference | undefined,
64+
acceptField: ListUrlFieldGate,
65+
): ListFilterGroupPreference | undefined {
66+
if (!group || !Array.isArray(group.conditions)) return group;
67+
let dropped = false;
68+
const conditions: unknown[] = [];
69+
for (const entry of group.conditions) {
70+
if (isRecord(entry) && Array.isArray(entry.conditions)) {
71+
const nested = acceptStoredFilters(entry as ListFilterGroupPreference, acceptField);
72+
if (nested !== entry) dropped = true;
73+
if (nested) conditions.push(nested);
74+
} else if (isRecord(entry) && typeof entry.field === 'string' && entry.field && !acceptField(entry.field)) {
75+
dropped = true;
76+
} else {
77+
conditions.push(entry);
78+
}
79+
}
80+
if (!dropped) return group;
81+
return conditions.length > 0 ? { ...group, conditions } : undefined;
82+
}
83+
3084
export function useObjectViewListUrlState({
3185
objectName,
3286
objectDef,
@@ -68,6 +122,11 @@ export function useObjectViewListUrlState({
68122
* seed waits until that state has been read (`listPrefs.ready`), so a
69123
* second browser opens on it rather than on an empty cache.
70124
*
125+
* Both seeds pass the same field gate (`acceptField`, objectstack#28949):
126+
* a field stored before it was deleted, or before this user lost read on
127+
* it, is dropped exactly as a stale link's is, and the rest of the state
128+
* opens as stored.
129+
*
71130
* Then the seed is written back into the URL (replace, never a new history
72131
* entry), so the address bar shows the list on screen: a restored filter
73132
* is in the link a user copies, and an entry dropped on load (a field the
@@ -93,14 +152,15 @@ export function useObjectViewListUrlState({
93152
const listSeedRef = useRef<{ key: string; source: 'url' | 'storage'; state: ListUrlState } | undefined>(undefined);
94153
const ownListParamsRef = useRef<string | undefined>(undefined);
95154
if (listIdentityKey && listPrefs.ready && listSeedRef.current?.key !== listIdentityKey) {
155+
// A field the object does not declare (a stale link, or a stored state
156+
// older than the field's deletion) or one this user cannot read is
157+
// dropped. Field-level permissions gate only once loaded — the same
158+
// gate `ListView` puts on its columns.
159+
const acceptField: ListUrlFieldGate = (field: string) =>
160+
!!objectDef.fields?.[field]
161+
&& (!perms.isLoaded || perms.checkField(objectDef.name, field, 'read'));
96162
const carried = userFilterParamsKey(searchParams);
97163
if (carried && carried !== ownListParamsRef.current) {
98-
// A field the object does not declare (a stale link) or one this
99-
// user cannot read is dropped. Field-level permissions gate only
100-
// once loaded — the same gate `ListView` puts on its columns.
101-
const acceptField = (field: string) =>
102-
!!objectDef.fields?.[field]
103-
&& (!perms.isLoaded || perms.checkField(objectDef.name, field, 'read'));
104164
listSeedRef.current = {
105165
key: listIdentityKey,
106166
source: 'url',
@@ -113,9 +173,9 @@ export function useObjectViewListUrlState({
113173
key: listIdentityKey,
114174
source: 'storage',
115175
state: {
116-
filters: (viewPref?.filters ?? undefined) as ListUrlState['filters'],
176+
filters: acceptStoredFilters(viewPref?.filters, acceptField) as ListUrlState['filters'],
117177
search: stored?.search || undefined,
118-
sort: viewPref?.sort,
178+
sort: acceptStoredSort(viewPref?.sort, acceptField),
119179
},
120180
};
121181
}

0 commit comments

Comments
 (0)