Skip to content

Commit d77dc07

Browse files
test(ui-e2e): the read-only member spec names the write routes it watches; the record page read POST is not a write (#28629)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSi9paTx1Lu8ENYwwssthfK3h)_ Fixes #28618 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:只读成员的 live 用例不再把「非 GET 即写」当判据,改为按路由账本点名真正的写路由;记录页的读取 `POST …/:id/page` 不再被算作写。 - 为什么改:记录页现在通过这个 POST 读取数据,用例把成员自己打开页面误判成两次写,夜间 live 档因此常红,真回归会被这条已知红淹没。 - 风险与回滚:只动测试代码,不发布任何包;成员真的写入仍会让用例变红(已实测)。回滚即 revert 本 PR。 - 先看:`ui/e2e/live/data-writes.ts`(写路由清单与判定)、`ui/e2e/live/read-only-member-record.spec.ts`(两处调用点)。 - 两分钟验证:CI 绿即可;本地实测见下方 Verification,夜间档下一次运行应在这条用例上转绿。 - 需要你做的:无,席位落地。 ## What changes - New helper `ui/e2e/live/data-writes.ts`: `writesRecordsOf(object)` returns a predicate over Playwright requests that names the write routes on one object instead of treating every non-GET as a write. - PATCH, PUT and DELETE anywhere under `/api/v1/data/:object` are writes by method. - A POST is a write only on a named route: create (`POST /data/:object`), `clone`, `shares`, `import`, `import/jobs`, `batch`, `createMany`, `updateMany`, `deleteMany`, and the three `/actions/...` invocation shapes for the object. - The cross-object `POST /api/v1/batch` counts when one of its operations names the object, or when its body cannot be read. - The two read-shaped POSTs in the ledger, `POST /data/:object/query` and `POST /data/:object/:id/page`, are deliberately not writes. - Every route also matches under the environment-scoped mirror. - `read-only-member-record.spec.ts`: the member's write detector uses the predicate, and the "the member browser attempted no write" assertion is unchanged. The admin control step's save wait used the same "non-GET on the record URL" shape, which the record-page read also matches; it now waits on the same predicate, so only the real save resolves it. How the write set was derived: transcribed from the two route ledgers, `packages/rest/src/rest-route-ledger.ts` (families `crud`, `data-actions`, `record-shares`, `batch`) and `packages/runtime/src/route-ledger.ts` (domain `/actions`). A one-time script enumerated every non-GET ledger row whose path names `:object` (21 rows across both ledgers) and classified a URL built from each one. Nothing re-derives the list after this PR, and the helper's header says so: a write route added to a ledger later is not watched until it is added there. Sibling specs: `ui/e2e/live/*.spec.ts` was grepped for the same "every non-GET is a write" detector. No other spec has it; the others match one named method and path (`/api/v1/batch`, `PATCH`/`PUT` on a record, `PUT /meta/...`), so nothing else changed. ## Tests - No test was skipped, renamed or deleted, and the no-write assertion is not weakened. - `pnpm --dir ui type-check:e2e`: exit 0. `eslint` on both files: exit 0. - Changeset: none, `skip-changeset`. The diff is under `ui/e2e/`, which no package publishes (`ui` is the private root package), and `pnpm check:published-files` exits 0. ## Acceptance notes - What the detector watches has widened in three places: the `/actions/:object/...` invocations, the cross-object `POST /api/v1/batch`, and the environment-scoped mirror. The old detector missed all three. A neighbour object sharing the name prefix (`showcase_inquiry_x`) is no longer counted; the old substring match counted it. - Bound: a POST write route added to a ledger later goes unwatched until someone names it in `data-writes.ts`. That is the cost the card's Expected section accepts, so that a future read-shaped POST cannot red the spec. A permanent pin holding the list against the ledgers would need a cross-tree test input declaration, so it is left to the seat. ## Verification Live, on head `224a31ec93`: the workspace backend from `ui/e2e/live/ci/start-backend.sh` (this tree's CLI serving `examples/app-showcase`), with the Console built with `VITE_BASE_PATH=/` and served by `vite preview`. This is the nightly harness's stack, step for step. <details><summary>Fixed spec: 1 passed</summary> ``` ✓ 1 [chromium] › e2e/live/read-only-member-record.spec.ts:48:1 › a member who may read but not write an inquiry gets a read-only form and no edit affordance; the admin edits it (10.5s) 1 passed (12.1s) ``` </details> <details><summary>Original detector on the same stack (spec restored from d5da45b): reproduces the card's reading</summary> ``` Error: the member browser attempted no write - Array [] + Array [ + "POST http://localhost:41874/api/v1/data/showcase_inquiry/SwQodwKP5jp6hr5v/page", + "POST http://localhost:41874/api/v1/data/showcase_inquiry/SwQodwKP5jp6hr5v/page", + ] > 107 | expect(writes, 'the member browser attempted no write').toEqual([]); ``` </details> <details><summary>Ablation: a real member write still fails the spec</summary> A `PATCH /api/v1/data/showcase_inquiry/ID` sent from the member's page, injected through `scripts/ablation-replace.mjs` (anchor 1 to 0, restored to the HEAD blob, `git diff HEAD` empty): ``` Error: the member browser attempted no write - Array [] + Array [ + "PATCH http://localhost:41874/api/v1/data/showcase_inquiry/CQkKy5JDU57kmU1n", + ] ``` </details> <details><summary>The whole nightly list (`test:e2e:live:nightly`): 19 passed</summary> ``` 19 passed (1.2m) ``` </details> <details><summary>Classification check: 33 cases, 0 failures</summary> The card's two captured `POST …/H0mCRgRpfpwhhwLC/page` requests answer `false`, where the old detector answered `true`. Every one of the 21 non-GET ledger rows naming `:object` answers `true`, except `query` and `page`. Also covered: PUT, the environment-scoped mirror, `/batch` with and without the object, GET reads, the prefix neighbour and a query string. </details> Session: https://claude.ai/code/session_017ihL2gLghEqeDKTbDTC7Zd 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e78922 commit d77dc07

2 files changed

Lines changed: 91 additions & 4 deletions

File tree

‎ui/e2e/live/data-writes.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import type { Request } from '@playwright/test';
2+
3+
/**
4+
* Which browser requests WRITE the records of one object, named route by route.
5+
*
6+
* "Every non-GET under `/api/v1/data/:object` is a write" stopped being true
7+
* when the record page began loading through `POST /api/v1/data/:object/:id/page`:
8+
* a read with a request body, so a detector built on the HTTP method alone
9+
* counted a member's own page load as two writes. The write set is therefore
10+
* NAMED here, so a future read-shaped POST cannot be mistaken for a write,
11+
* and a real write still is.
12+
*
13+
* Source: the route ledgers, `packages/rest/src/rest-route-ledger.ts` (the
14+
* `crud`, `data-actions`, `record-shares` and `batch` families) and
15+
* `packages/runtime/src/route-ledger.ts` (the `/actions` domain). The rows were
16+
* transcribed from them once; nothing re-derives this list, so a write route
17+
* added to a ledger is not watched here until it is added below.
18+
*
19+
* - PATCH, PUT and DELETE anywhere under `/data/:object` are writes by method.
20+
* - A POST is a write only on a route named in `WRITE_POSTS`. The two
21+
* read-shaped POSTs in the ledger, `POST /data/:object/query` and
22+
* `POST /data/:object/:id/page`, are deliberately absent.
23+
* - `POST /batch`, the cross-object transaction, names its objects in the
24+
* body; it writes `object` when one of its operations names it, or when
25+
* the body cannot be read.
26+
*
27+
* Each route also matches under the environment-scoped mirror
28+
* (`/api/v1/environments/:environmentId/...`).
29+
*/
30+
const WRITE_POSTS = [
31+
'POST /api/v1/data/:object',
32+
'POST /api/v1/data/:object/:id/clone',
33+
'POST /api/v1/data/:object/:id/shares',
34+
'POST /api/v1/data/:object/import',
35+
'POST /api/v1/data/:object/import/jobs',
36+
'POST /api/v1/data/:object/batch',
37+
'POST /api/v1/data/:object/createMany',
38+
'POST /api/v1/data/:object/updateMany',
39+
'POST /api/v1/data/:object/deleteMany',
40+
'POST /api/v1/actions/:object/:action',
41+
'POST /api/v1/actions/:object/:action/:recordId',
42+
'POST /api/v1/actions/_activation/:object/:action',
43+
] as const;
44+
45+
const WRITE_METHODS = new Set(['PATCH', 'PUT', 'DELETE']);
46+
const ENV_MIRROR = '(?:/environments/[^/]+)?';
47+
48+
const escape = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
49+
50+
/** `/api/v1/data/:object/:id` → a pathname regex, `:object` bound to `object`. */
51+
function routePattern(path: string, object: string): RegExp {
52+
const rest = path
53+
.replace(/^\/api\/v1/, '')
54+
.split('/')
55+
.map((seg) => (seg === ':object' ? escape(object) : seg.startsWith(':') ? '[^/]+' : escape(seg)))
56+
.join('/');
57+
return new RegExp(`^/api/v1${ENV_MIRROR}${rest}/?$`);
58+
}
59+
60+
function batchNames(request: Request, object: string): boolean {
61+
let body: unknown;
62+
try {
63+
body = request.postDataJSON();
64+
} catch {
65+
return true;
66+
}
67+
const operations = (body as { operations?: unknown } | null)?.operations;
68+
if (!Array.isArray(operations)) return true;
69+
return operations.some((op) => (op as { object?: unknown } | null)?.object === object);
70+
}
71+
72+
/** A predicate over browser requests: does this one write a record of `object`? */
73+
export function writesRecordsOf(object: string): (request: Request) => boolean {
74+
const dataPrefix = new RegExp(`^/api/v1${ENV_MIRROR}/data/${escape(object)}(?:/|$)`);
75+
const posts = WRITE_POSTS.map((row) => routePattern(row.slice('POST '.length), object));
76+
const crossObjectBatch = new RegExp(`^/api/v1${ENV_MIRROR}/batch/?$`);
77+
return (request) => {
78+
const method = request.method();
79+
const path = new URL(request.url()).pathname;
80+
if (WRITE_METHODS.has(method)) return dataPrefix.test(path);
81+
if (method !== 'POST') return false;
82+
if (posts.some((p) => p.test(path))) return true;
83+
return crossObjectBatch.test(path) && batchNames(request, object);
84+
};
85+
}

‎ui/e2e/live/read-only-member-record.spec.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { test, expect, type Page } from '@playwright/test';
22
import { signInMember } from './member';
3+
import { writesRecordsOf } from './data-writes';
34

45
/**
56
* objectstack#27552 — a member who may READ a record but not WRITE it is
@@ -32,6 +33,9 @@ const API = process.env.LIVE_API_URL || 'http://localhost:3000';
3233
const OBJECT = 'showcase_inquiry';
3334
const recordPath = (id: string) => `/apps/showcase_app/${OBJECT}/record/${id}`;
3435
const formPath = (id: string) => `${recordPath(id)}?form=${encodeURIComponent(id)}`;
36+
// Named write routes, not "any non-GET": the record page itself loads through
37+
// the read `POST /api/v1/data/:object/:id/page` (see `./data-writes`).
38+
const writesInquiry = writesRecordsOf(OBJECT);
3539

3640
// The record header's Edit CTA and the record body's inline-edit pencils carry
3741
// no test id; their accessible names are the `detail.edit` and
@@ -84,7 +88,7 @@ test('a member who may read but not write an inquiry gets a read-only form and n
8488
const memberPage = await context.newPage();
8589
const writes: string[] = [];
8690
memberPage.on('request', (r) => {
87-
if (r.method() !== 'GET' && r.url().includes(`/api/v1/data/${OBJECT}`)) writes.push(`${r.method()} ${r.url()}`);
91+
if (writesInquiry(r)) writes.push(`${r.method()} ${r.url()}`);
8892
});
8993

9094
await memberPage.goto(formPath(id));
@@ -124,9 +128,7 @@ test('a member who may read but not write an inquiry gets a read-only form and n
124128

125129
const value = `Admin edit ${Date.now()}`;
126130
await company.fill(value);
127-
const saved = page.waitForResponse(
128-
(r) => r.request().method() !== 'GET' && r.url().includes(`/api/v1/data/${OBJECT}/${id}`),
129-
);
131+
const saved = page.waitForResponse((r) => writesInquiry(r.request()));
130132
await dialog.getByTestId('modal-form-footer').locator('button[type="submit"]').click();
131133
expect((await saved).ok(), 'the admin save answered 2xx').toBe(true);
132134

0 commit comments

Comments
 (0)