Skip to content

Commit 4278c3b

Browse files
fix(studio): list an order-only field change in the publish review and pin platform fields on the form canvas (#27646)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgVPwkGeZxvbHqGDhHfetYNP)_ Fixes #27516 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:发布审阅单对「只调整了字段顺序」的对象草稿,展开后列出一行「字段顺序已变更」并保留 Update 标记,不再显示「No differences detected」;Studio 表单设计器里,与平台定义完全相同的字段(实际就是 Owner)在未分组区始终排在首位,把作者字段拖到它上方会落在它下方,拖放播报也报这个位置。 - 为什么改:发布按字段顺序计算内容哈希、纯重排会真的发布新版本,但审阅单说没有差异;画布允许把字段放在 Owner 上方,刷新后平台又把 Owner 放回最前,作者的排列悄悄失效。 - 风险与回滚:只动 Studio 两处界面与一个新文案键(10 个语言包各一行);不改元数据协议、不改服务端;回滚即 revert 本 PR。 - 先看:`DraftChangesPanel.tsx` 的 `fieldOrderChanged`(排除哪些平台注入列);`ObjectFormDesigner.tsx` 的 `settlePinned` / `dropLayout`(onDragEnd 与播报共用一套落点计算);两份新测试。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/preview/__tests__/DraftChangesPanel.fieldOrder-27516.test.tsx packages/app-shell/src/views/studio-design/ObjectFormDesigner.pinnedFields-27516.test.tsx`,预期 13 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes - **Publish review sheet (`DraftChangesPanel`)** — `computeChangeDetail` now reports `fields.reordered` for an `object` whose two versions declare their shared fields in a different order, and the drill-in prints a `~ Field order changed` row (new key `preview.changes.fieldOrderChanged`). `isEmptyChangeDetail` counts it, so such an entry keeps its Update badge instead of the "No differences detected" glyph. - The order compared is the order the publish door stores. Left out on both sides: a column the platform injects while its definition is exactly the platform's (`isInjectedColumnDefinition` over `injectedSystemColumnDefs`, both from `@objectstack/spec/data`), which the `/meta` reads prepend and the draft write strips; and a column the platform injects and hides (`isStudioHiddenSystemField`, the search companion among them). Only fields both versions declare are compared; an added or removed field is already a row. - Gated on `type === 'object'`, the one row of the door's ordered-map table; the comment on `fieldOrderChanged` names that table as the authority. - **Form designer canvas (`ObjectFormDesigner`)** — `settlePinned` places every shown ungrouped field whose definition, without its `group`, is the platform's own first in the ungrouped bucket, in the injection's order. It runs on the derived layout and on every drop. `onDragEnd` and the live-region `dropPlaceIn` now share one `dropLayout` (the arithmetic `dropPlaceIn` used to restate), so the place announced is the settled place committed. A pinned field moved into a section carries `group` and is the author's from then on; an edited one (a changed label) likewise. `FieldControlPreview` is untouched. - **i18n** — `preview.changes.fieldOrderChanged` in all ten locale packs. - Root changeset: `@object-ui/app-shell` and `@object-ui/i18n` patch. ## Tests - New `DraftChangesPanel.fieldOrder-27516.test.tsx` (9 cases): a pure reorder of authored fields, the array shape of `fields`, a same-order control, platform columns placed differently on the two reads (registry-backed vs overlay-backed) compare equal, an author-declared `owner_id` counts, added/removed fields are not a reorder, non-`object` types are not compared; on the panel, the row prints and the Update mark stays, with a same-order control still reading "No differences detected". - New `ObjectFormDesigner.pinnedFields-27516.test.tsx` (4 cases), on the real `DndContext` with the band geometry of the objectui#11802 suite, and a `reload` that replays the round trip with the spec's helpers (strip what equals the platform's definition, prepend the injected columns): Condition dropped on Owner lands at 2 of 3, is announced there, and the reloaded canvas shows the same order; author reorders below Owner survive the reload; an author-changed `owner_id` keeps the place it is given; Owner moved into a section keeps its section order through the reload. - Ablation, one leg per behaviour, via `scripts/ablation-replace.mjs` (anchor 1 → 0, blob changed, restored with `git diff HEAD` empty); app-shell tests import the source, no build in the path: | Leg | Mutation | Result | |---|---|---| | review order | `reordered` forced false | 4 of 9 red (every positive case); controls green | | canvas settle | `settlePinned` returns the layout unsettled | 1 of 4 red (the drop above Owner); the three author-order cases green, as they should be | - `pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/ packages/app-shell/src/preview/`: 293 files, 2076 tests passed (includes the objectui#11802 announcement suite, whose drop-vs-commit control now runs through the shared `dropLayout`). - `pnpm --dir ui exec vitest run packages/i18n/`: 90 files, 1379 passed, 13 skipped (locale key parity included). - `type-check` for `@object-ui/app-shell` and `@object-ui/i18n`: exit 0. eslint on the four touched source/test files: 0 errors (8 warnings, all on lines this PR does not touch). ## Acceptance notes - **Triage reading, measured.** Triage asked the review sheet to use "the same order-aware comparison publish already uses". That comparison is `canonicalize(body, 'object')` in `@objectstack/metadata-core` (its `ORDERED_MAPS` table keeps the order of an `object`'s `fields`). It cannot be imported here as is: the module imports `node:crypto` at its top, and `@object-ui/app-shell` does not depend on `@objectstack/metadata-core`. The sheet therefore restates that one row's rule, keyed on the same type, and reuses the spec's exported injection helpers rather than a second list of platform columns. The seat approved this before it was written. - **Where `owner_id` is re-injected, measured.** The `/meta` read exits call `governServedItem`, whose `applyInjectedSystemColumns` (`@objectstack/metadata-core`) builds `fields` as the injected-but-undeclared columns followed by the declared ones; the draft write's `stripInjectedSystemColumns` removes a column whose definition equals the platform's (`isInjectedColumnDefinition`). The canvas pins exactly that set, read through the same spec helpers. - **File surface.** The ten `ui/packages/i18n/src/locales/*.ts` packs (one key each) were added to the claim's file surface with the seat's approval. - The canvas does not add a visible "pinned" marker: a field dropped above Owner lands below it and the live region names that place. A marker would need new designer copy; noted, not added. - A sighted drag still previews the card above Owner while the pointer is there (dnd-kit's sortable preview); the drop settles it. Noted. - Docs-driven rule (ui/AGENTS.md #2): no docs page describes the review sheet's field rows or the designer's ungrouped order; nothing to update. ## Verification - Derived gate list (`dispatch-gates --commands`): 34 families run locally, 33 exit 0. `pnpm --dir ui exec vitest run scripts/` exited 1 on one 15 s timeout (`check-vi-mock-inherit.test.ts`, run beside a locked vitest run); that file alone under the lock: 83/83 passed. `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED. `node scripts/check-sdui-manifest.mjs` exit 0. - `origin/main` merged (`a7a422a4`); it touched neither `DraftChangesPanel.tsx` nor `ObjectFormDesigner.tsx`. After rebuilding the merged ui packages: the two new files, the objectui#11802 announcement suite and both locale parity suites, 69/69 passed; `@object-ui/app-shell` `type-check` exit 0. - Full readings are in the report comment on #27516. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CZkEeDsL4LuR4hwqTN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4d1f52c commit 4278c3b

15 files changed

Lines changed: 524 additions & 45 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+
'@object-ui/i18n': patch
4+
---
5+
6+
fix(studio): the form designer and the publish review no longer misreport an order-only field change
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **Publish review.** Expanding an object whose draft only reorders its fields used to read "No differences detected", although publishing does save the new order. The drill-in now lists a **Field order changed** row, and the entry keeps its Update badge. Only the fields both versions declare are compared, and the columns the platform injects on every read (an untouched `owner_id`, the audit columns, the search companion) are left out, because their place is the platform's, not the author's. Only an `object`'s fields are compared this way, matching the publish door.
12+
- **Form designer.** An ungrouped field whose definition is exactly the platform's own, `owner_id` among them, now stays first in the ungrouped section. A field dropped above it lands below it, and the drop announcement names that place, so the canvas shows the order a reload shows. Moving such a field into a section, or editing it, makes it the author's field, and it then keeps the place it is given.

‎ui/packages/app-shell/src/preview/DraftChangesPanel.tsx‎

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,12 @@ import { MetadataProtectionFields, stripReadDecorations } from '@objectstack/spe
8585
// objectui#12929 — the ONE discriminator of an authored default's shape, the
8686
// same function the batch door's backfill judgement calls. Moves nothing onto
8787
// the eager graph for the reason recorded above (`vendor-objectstack`).
88-
import { discriminateDefaultValueShape, provisionPrimary } from '@objectstack/spec/data';
88+
import {
89+
discriminateDefaultValueShape,
90+
injectedSystemColumnDefs,
91+
isInjectedColumnDefinition,
92+
provisionPrimary,
93+
} from '@objectstack/spec/data';
8994
import { diffFields, readFields } from '../views/metadata-admin/previews/object-fields-io.js';
9095
// objectui#13125 — the Data pillar's rule for a column the platform injected
9196
// and hides (objectui#11780), reused rather than restated. A leaf module with
@@ -313,6 +318,13 @@ export interface EntryChangeDetail {
313318
* A Map, never a plain object: `__proto__` is a legal field name.
314319
*/
315320
labels: ReadonlyMap<string, string>;
321+
/**
322+
* objectstack#27516 — publishing changes the order of the fields both
323+
* bodies declare. An `object`'s field order is content (the spec declares
324+
* it the display order), so a pure reorder publishes a new version; see
325+
* {@link fieldOrderChanged}.
326+
*/
327+
reordered: boolean;
316328
} | null;
317329
/**
318330
* Node-level diff of a flow's `nodes` (objectstack#27356): each node is
@@ -368,7 +380,9 @@ export interface FlowNodeRow {
368380
/** True when the detail lists nothing: no field row, no node row, no key row and no generated row. */
369381
function isEmptyChangeDetail(detail: EntryChangeDetail): boolean {
370382
const { fields, nodes, keys, generated } = detail;
371-
const fieldRows = fields ? fields.added.length + fields.changed.length + fields.removed.length : 0;
383+
const fieldRows = fields
384+
? fields.added.length + fields.changed.length + fields.removed.length + (fields.reordered ? 1 : 0)
385+
: 0;
372386
const nodeRows = nodes ? nodes.added.length + nodes.changed.length + nodes.removed.length : 0;
373387
return fieldRows + nodeRows + keys.added.length + keys.changed.length + keys.removed.length + generated.length === 0;
374388
}
@@ -497,6 +511,50 @@ function withoutProtectionEnvelope(body: Record<string, unknown>): Record<string
497511
return rest;
498512
}
499513

514+
/**
515+
* objectstack#27516 — does publishing `cur` over `pub` change the order of the
516+
* object's fields? The publish door already says yes: its no-op test compares
517+
* content hashes under `canonicalize(body, type)` (`@objectstack/metadata-core`),
518+
* whose `ORDERED_MAPS` table keeps the key order of exactly one map, an
519+
* `object`'s `fields`, so a pure reorder publishes. That module hashes with
520+
* `node:crypto` and is not a dependency of this package, so the comparison is
521+
* restated here for that one row, gated on the same type; a row added to that
522+
* table owes a row here.
523+
*
524+
* The door compares the bodies it STORES, and the `/meta` reads this drill-in
525+
* diffs add what the write strips again, so those fields are left out on
526+
* either side before the orders are compared:
527+
*
528+
* - a column the platform injects, while its definition is byte-for-byte the
529+
* platform's (`isInjectedColumnDefinition` over `injectedSystemColumnDefs`,
530+
* `@objectstack/spec/data`): the read puts it ahead of the declared fields
531+
* (`applyInjectedSystemColumns`) and the write takes it out
532+
* (`stripInjectedSystemColumns`), so its place is never the author's;
533+
* - a column the platform injects and hides (`isStudioHiddenSystemField`,
534+
* the search companion among them), which the registry materializes on read.
535+
*
536+
* Only fields both bodies declare are compared: an added or a removed field is
537+
* already a row of its own.
538+
*/
539+
function fieldOrderChanged(pub: Record<string, unknown>, cur: Record<string, unknown>): boolean {
540+
const storedOrder = (body: Record<string, unknown>): string[] => {
541+
const injected = injectedSystemColumnDefs(body);
542+
return readFields(body.fields)
543+
.entries.filter(({ name, def }) => {
544+
if (isStudioHiddenSystemField(def)) return false;
545+
const platform = Object.prototype.hasOwnProperty.call(injected, name) ? injected[name] : undefined;
546+
return !(platform && isInjectedColumnDefinition(def, platform));
547+
})
548+
.map((e) => e.name);
549+
};
550+
const before = storedOrder(pub);
551+
const after = storedOrder(cur);
552+
const inBefore = new Set(before);
553+
const inAfter = new Set(after);
554+
const kept = after.filter((name) => inBefore.has(name));
555+
return before.filter((name) => inAfter.has(name)).some((name, i) => name !== kept[i]);
556+
}
557+
500558
/** Each field's definition by name, in either `fields` shape the diff reads. */
501559
function fieldDefsByName(fieldsInput: unknown): Map<string, Record<string, unknown>> {
502560
return new Map(readFields(fieldsInput).entries.map((e) => [e.name, e.def] as const));
@@ -592,7 +650,9 @@ export function computeChangeDetail(
592650
const label = fieldLabelOf(curDefs.get(name) ?? pubDefs.get(name));
593651
if (label !== undefined) labels.set(name, label);
594652
}
595-
fields = { added, changed, removed, labels };
653+
// objectstack#27516 — the order the publish door hashes, for an `object` only.
654+
const reordered = type === 'object' && fieldOrderChanged(pub, cur);
655+
fields = { added, changed, removed, labels, reordered };
596656
}
597657
const isNew = published === null;
598658
const names = new Set([...Object.keys(pub), ...Object.keys(cur)]);
@@ -987,6 +1047,12 @@ function EntryDetail({
9871047
{fields?.removed.map((name) => (
9881048
<FieldRow key={`-${name}`} kind="removed" name={name} label={fields.labels.get(name)} />
9891049
))}
1050+
{fields?.reordered && (
1051+
// objectstack#27516 — a pure reorder publishes a new version, so it is a row.
1052+
<p className={`text-xs ${FIELD_ROW_TONE.changed}`} data-testid="draft-field-order-row">
1053+
{FIELD_ROW_MARK.changed} {t('preview.changes.fieldOrderChanged', { defaultValue: 'Field order changed' })}
1054+
</p>
1055+
)}
9901056
{/* objectstack#27356 — a flow's nodes, each by its label, in the field rows' shape. */}
9911057
{nodes?.added.map((n) => (
9921058
<FieldRow key={`n+${n.id}`} kind="added" name={n.id} label={n.label} testId="draft-node-row" />
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectstack#27516 — the review sheet reads an order-only field change as a
11+
* change.
12+
*
13+
* The publish door hashes an `object`'s `fields` in their declared order
14+
* (`canonicalize(body, 'object')`, `@objectstack/metadata-core`), so a draft
15+
* that only reorders its fields publishes a new version. The drill-in compared
16+
* fields by name only and read "No differences detected" for it. It now lists
17+
* a "Field order changed" row.
18+
*
19+
* The order compared is the order the door stores: the columns the `/meta`
20+
* reads inject ahead of the declared fields, and the write strips again, have
21+
* no place of the author's, so they are left out on both sides.
22+
*/
23+
24+
import '@testing-library/jest-dom/vitest';
25+
import * as React from 'react';
26+
import { afterEach, describe, expect, it, vi } from 'vitest';
27+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
28+
import { AUDIT_FIELD_DEFS, OWNER_FIELD_DEF } from '@objectstack/spec/data';
29+
30+
vi.mock('@object-ui/i18n', async (importOriginal) => ({
31+
...(await importOriginal<Record<string, unknown>>()),
32+
useObjectTranslation: () => ({
33+
t: (_k: string, o?: { defaultValue?: string }) => o?.defaultValue ?? _k,
34+
}),
35+
}));
36+
37+
import { DraftChangesPanel, computeChangeDetail } from '../DraftChangesPanel';
38+
39+
const NO_CHANGE = 'No differences detected — the draft matches the published version.';
40+
const ORDER_ROW = '~ Field order changed';
41+
42+
const condition = { type: 'select', label: 'Condition' };
43+
const notes = { type: 'textarea', label: 'Notes' };
44+
const title = { type: 'text', label: 'Title' };
45+
const search = { type: 'text', system: true, hidden: true };
46+
47+
afterEach(() => {
48+
vi.restoreAllMocks();
49+
});
50+
51+
describe('computeChangeDetail compares the order of an object’s fields (objectstack#27516)', () => {
52+
it('reports a pure reorder of two authored fields, and nothing else', () => {
53+
const d = computeChangeDetail(
54+
{ name: 'asset', fields: { title, condition, notes } },
55+
{ name: 'asset', fields: { title, notes, condition } },
56+
'object',
57+
);
58+
expect(d.fields).toMatchObject({ added: [], changed: [], removed: [], reordered: true });
59+
expect(d.keys).toEqual({ added: [], changed: [], removed: [] });
60+
});
61+
62+
it('reads the array shape of `fields` in its order too', () => {
63+
const d = computeChangeDetail(
64+
{ name: 'asset', fields: [{ name: 'title', ...title }, { name: 'condition', ...condition }] },
65+
{ name: 'asset', fields: [{ name: 'condition', ...condition }, { name: 'title', ...title }] },
66+
'object',
67+
);
68+
expect(d.fields?.reordered).toBe(true);
69+
});
70+
71+
it('CONTROL: the same order is no reorder', () => {
72+
const d = computeChangeDetail(
73+
{ name: 'asset', fields: { title, condition } },
74+
{ name: 'asset', fields: { title, condition } },
75+
'object',
76+
);
77+
expect(d.fields?.reordered).toBe(false);
78+
});
79+
80+
it('leaves out the columns the reads inject: their place is the platform’s', () => {
81+
// The registry-backed published read and the overlay-backed draft read put
82+
// the platform's columns in different places; the declared order is equal.
83+
const d = computeChangeDetail(
84+
{
85+
name: 'asset',
86+
fields: { created_at: AUDIT_FIELD_DEFS.created_at, owner_id: OWNER_FIELD_DEF, title, __search: search, condition },
87+
},
88+
{
89+
name: 'asset',
90+
fields: { owner_id: OWNER_FIELD_DEF, title, condition, created_at: AUDIT_FIELD_DEFS.created_at, __search: search },
91+
},
92+
'object',
93+
);
94+
expect(d.fields?.reordered).toBe(false);
95+
});
96+
97+
it('counts an `owner_id` the author declared: its place is theirs', () => {
98+
const owner = { ...OWNER_FIELD_DEF, label: 'Account manager' };
99+
const d = computeChangeDetail(
100+
{ name: 'asset', fields: { owner_id: owner, title } },
101+
{ name: 'asset', fields: { title, owner_id: owner } },
102+
'object',
103+
);
104+
expect(d.fields?.reordered).toBe(true);
105+
});
106+
107+
it('compares only the fields both versions declare: an added or removed field is its own row', () => {
108+
const d = computeChangeDetail(
109+
{ name: 'asset', fields: { title, notes, condition } },
110+
{ name: 'asset', fields: { serial: { type: 'text' }, title, condition } },
111+
'object',
112+
);
113+
expect(d.fields).toMatchObject({ added: ['serial'], removed: ['notes'], reordered: false });
114+
});
115+
116+
it('is the publish door’s rule for an `object` only', () => {
117+
const d = computeChangeDetail(
118+
{ name: 'asset', fields: { title, condition } },
119+
{ name: 'asset', fields: { condition, title } },
120+
'view',
121+
);
122+
expect(d.fields?.reordered).toBe(false);
123+
});
124+
});
125+
126+
/* ─────────────── the panel ─────────────── */
127+
128+
type Body = Record<string, unknown>;
129+
130+
function mockServer(published: Body, draft: Body): void {
131+
global.fetch = vi.fn(async (input: RequestInfo | URL) => {
132+
const url = String(input);
133+
const ok = (body: unknown) => ({ ok: true, status: 200, json: async () => body });
134+
if (url.includes('/_drafts')) return ok([{ type: 'object', name: 'asset', packageId: 'com.x' }]);
135+
const [type, name] = url.split('?')[0].replace('/api/v1/meta/', '').split('/');
136+
if (!name) return ok([{ name: 'asset' }]);
137+
return url.includes('state=draft') ? ok({ type, name, item: draft }) : ok(published);
138+
}) as unknown as typeof fetch;
139+
}
140+
141+
function assetRow(): HTMLElement {
142+
const hit = screen
143+
.getAllByTestId('draft-entry-toggle')
144+
.find((el) => el.querySelector('.font-mono')?.textContent === 'asset');
145+
if (!hit) throw new Error('no row for asset');
146+
return hit;
147+
}
148+
149+
describe('the drill-in lists an order-only change (objectstack#27516)', () => {
150+
it('prints a Field order changed row and keeps the Update mark', async () => {
151+
mockServer(
152+
{ name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, condition, notes } },
153+
{ name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, notes, condition }, _draft: true },
154+
);
155+
render(<DraftChangesPanel open onOpenChange={() => {}} packageId="com.x" onPublish={vi.fn()} />);
156+
await waitFor(() => expect(assetRow().textContent).toContain('Update'));
157+
fireEvent.click(assetRow());
158+
const detail = await screen.findByTestId('draft-entry-detail');
159+
expect([...detail.querySelectorAll('p')].map((p) => p.textContent)).toEqual([ORDER_ROW]);
160+
expect(screen.queryByText(NO_CHANGE)).toBeNull();
161+
expect(assetRow().textContent).toContain('Update');
162+
});
163+
164+
it('CONTROL: the same order still reads No differences detected', async () => {
165+
mockServer(
166+
{ name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, condition } },
167+
{ name: 'asset', fields: { title, owner_id: OWNER_FIELD_DEF, condition }, _draft: true },
168+
);
169+
render(<DraftChangesPanel open onOpenChange={() => {}} packageId="com.x" onPublish={vi.fn()} />);
170+
await waitFor(() => expect(assetRow().textContent).toContain('Update'));
171+
fireEvent.click(assetRow());
172+
await screen.findByText(NO_CHANGE);
173+
expect(screen.queryByText(ORDER_ROW)).toBeNull();
174+
});
175+
});

0 commit comments

Comments
 (0)