fix(studio): list an order-only field change in the publish review and pin platform fields on the form canvas

.changeset/27516-studio-field-order-reorder.md

@@ -0,0 +1,12 @@
+---
+'@object-ui/app-shell': patch
+'@object-ui/i18n': patch
+---
+
+fix(studio): the form designer and the publish review no longer misreport an order-only field change
+
+Clause-②: no
+Metadata-protocol: none
+
+- **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.
+- **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

@@ -85,7 +85,12 @@ import { MetadataProtectionFields, stripReadDecorations } from '@objectstack/spe
 // objectui#12929 — the ONE discriminator of an authored default's shape, the
 // same function the batch door's backfill judgement calls. Moves nothing onto
 // the eager graph for the reason recorded above (`vendor-objectstack`).
-import { discriminateDefaultValueShape, provisionPrimary } from '@objectstack/spec/data';
+import {
+  discriminateDefaultValueShape,
+  injectedSystemColumnDefs,
+  isInjectedColumnDefinition,
+  provisionPrimary,
+} from '@objectstack/spec/data';
 import { diffFields, readFields } from '../views/metadata-admin/previews/object-fields-io.js';
 // objectui#13125 — the Data pillar's rule for a column the platform injected
 // and hides (objectui#11780), reused rather than restated. A leaf module with
@@ -313,6 +318,13 @@ export interface EntryChangeDetail {
      * A Map, never a plain object: `__proto__` is a legal field name.
      */
     labels: ReadonlyMap<string, string>;
+    /**
+     * objectstack#27516 — publishing changes the order of the fields both
+     * bodies declare. An `object`'s field order is content (the spec declares
+     * it the display order), so a pure reorder publishes a new version; see
+     * {@link fieldOrderChanged}.
+     */
+    reordered: boolean;
   } | null;
   /**
    * Node-level diff of a flow's `nodes` (objectstack#27356): each node is
@@ -368,7 +380,9 @@ export interface FlowNodeRow {
 /** True when the detail lists nothing: no field row, no node row, no key row and no generated row. */
 function isEmptyChangeDetail(detail: EntryChangeDetail): boolean {
   const { fields, nodes, keys, generated } = detail;
-  const fieldRows = fields ? fields.added.length + fields.changed.length + fields.removed.length : 0;
+  const fieldRows = fields
+    ? fields.added.length + fields.changed.length + fields.removed.length + (fields.reordered ? 1 : 0)
+    : 0;
   const nodeRows = nodes ? nodes.added.length + nodes.changed.length + nodes.removed.length : 0;
   return fieldRows + nodeRows + keys.added.length + keys.changed.length + keys.removed.length + generated.length === 0;
 }
@@ -497,6 +511,50 @@ function withoutProtectionEnvelope(body: Record<string, unknown>): Record<string
   return rest;
 }
 
+/**
+ * objectstack#27516 — does publishing `cur` over `pub` change the order of the
+ * object's fields? The publish door already says yes: its no-op test compares
+ * content hashes under `canonicalize(body, type)` (`@objectstack/metadata-core`),
+ * whose `ORDERED_MAPS` table keeps the key order of exactly one map, an
+ * `object`'s `fields`, so a pure reorder publishes. That module hashes with
+ * `node:crypto` and is not a dependency of this package, so the comparison is
+ * restated here for that one row, gated on the same type; a row added to that
+ * table owes a row here.
+ *
+ * The door compares the bodies it STORES, and the `/meta` reads this drill-in
+ * diffs add what the write strips again, so those fields are left out on
+ * either side before the orders are compared:
+ *
+ * - a column the platform injects, while its definition is byte-for-byte the
+ *   platform's (`isInjectedColumnDefinition` over `injectedSystemColumnDefs`,
+ *   `@objectstack/spec/data`): the read puts it ahead of the declared fields
+ *   (`applyInjectedSystemColumns`) and the write takes it out
+ *   (`stripInjectedSystemColumns`), so its place is never the author's;
+ * - a column the platform injects and hides (`isStudioHiddenSystemField`,
+ *   the search companion among them), which the registry materializes on read.
+ *
+ * Only fields both bodies declare are compared: an added or a removed field is
+ * already a row of its own.
+ */
+function fieldOrderChanged(pub: Record<string, unknown>, cur: Record<string, unknown>): boolean {
+  const storedOrder = (body: Record<string, unknown>): string[] => {
+    const injected = injectedSystemColumnDefs(body);
+    return readFields(body.fields)
+      .entries.filter(({ name, def }) => {
+        if (isStudioHiddenSystemField(def)) return false;
+        const platform = Object.prototype.hasOwnProperty.call(injected, name) ? injected[name] : undefined;
+        return !(platform && isInjectedColumnDefinition(def, platform));
+      })
+      .map((e) => e.name);
+  };
+  const before = storedOrder(pub);
+  const after = storedOrder(cur);
+  const inBefore = new Set(before);
+  const inAfter = new Set(after);
+  const kept = after.filter((name) => inBefore.has(name));
+  return before.filter((name) => inAfter.has(name)).some((name, i) => name !== kept[i]);
+}
+
 /** Each field's definition by name, in either `fields` shape the diff reads. */
 function fieldDefsByName(fieldsInput: unknown): Map<string, Record<string, unknown>> {
   return new Map(readFields(fieldsInput).entries.map((e) => [e.name, e.def] as const));
@@ -592,7 +650,9 @@ export function computeChangeDetail(
       const label = fieldLabelOf(curDefs.get(name) ?? pubDefs.get(name));
       if (label !== undefined) labels.set(name, label);
     }
-    fields = { added, changed, removed, labels };
+    // objectstack#27516 — the order the publish door hashes, for an `object` only.
+    const reordered = type === 'object' && fieldOrderChanged(pub, cur);
+    fields = { added, changed, removed, labels, reordered };
   }
   const isNew = published === null;
   const names = new Set([...Object.keys(pub), ...Object.keys(cur)]);
@@ -987,6 +1047,12 @@ function EntryDetail({
       {fields?.removed.map((name) => (
         <FieldRow key={`-${name}`} kind="removed" name={name} label={fields.labels.get(name)} />
       ))}
+      {fields?.reordered && (
+        // objectstack#27516 — a pure reorder publishes a new version, so it is a row.
+        <p className={`text-xs ${FIELD_ROW_TONE.changed}`} data-testid="draft-field-order-row">
+          {FIELD_ROW_MARK.changed} {t('preview.changes.fieldOrderChanged', { defaultValue: 'Field order changed' })}
+        </p>
+      )}
       {/* objectstack#27356 — a flow's nodes, each by its label, in the field rows' shape. */}
       {nodes?.added.map((n) => (
         <FieldRow key={`n+${n.id}`} kind="added" name={n.id} label={n.label} testId="draft-node-row" />

ui/packages/app-shell/src/preview/__tests__/DraftChangesPanel.fieldOrder-27516.test.tsx

@@ -0,0 +1,175 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectstack#27516 — the review sheet reads an order-only field change as a
+ * change.
+ *
+ * The publish door hashes an `object`'s `fields` in their declared order
+ * (`canonicalize(body, 'object')`, `@objectstack/metadata-core`), so a draft
+ * that only reorders its fields publishes a new version. The drill-in compared
+ * fields by name only and read "No differences detected" for it. It now lists
+ * a "Field order changed" row.
+ *
+ * The order compared is the order the door stores: the columns the `/meta`
+ * reads inject ahead of the declared fields, and the write strips again, have
+ * no place of the author's, so they are left out on both sides.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import * as React from 'react';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { AUDIT_FIELD_DEFS, OWNER_FIELD_DEF } from '@objectstack/spec/data';
+
+vi.mock('@object-ui/i18n', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useObjectTranslation: () => ({
+    t: (_k: string, o?: { defaultValue?: string }) => o?.defaultValue ?? _k,
+  }),
+}));
+
+import { DraftChangesPanel, computeChangeDetail } from '../DraftChangesPanel';
+
+const NO_CHANGE = 'No differences detected — the draft matches the published version.';
+const ORDER_ROW = '~ Field order changed';
+
+const condition = { type: 'select', label: 'Condition' };
+const notes = { type: 'textarea', label: 'Notes' };
+const title = { type: 'text', label: 'Title' };
+const search = { type: 'text', system: true, hidden: true };
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe('computeChangeDetail compares the order of an object’s fields (objectstack#27516)', () => {
+  it('reports a pure reorder of two authored fields, and nothing else', () => {
+    const d = computeChangeDetail(
+      { name: 'asset', fields: { title, condition, notes } },
+      { name: 'asset', fields: { title, notes, condition } },
+      'object',
+    );
+    expect(d.fields).toMatchObject({ added: [], changed: [], removed: [], reordered: true });
+    expect(d.keys).toEqual({ added: [], changed: [], removed: [] });
+  });
+
+  it('reads the array shape of `fields` in its order too', () => {
+    const d = computeChangeDetail(
+      { name: 'asset', fields: [{ name: 'title', ...title }, { name: 'condition', ...condition }] },
+      { name: 'asset', fields: [{ name: 'condition', ...condition }, { name: 'title', ...title }] },
+      'object',
+    );
+    expect(d.fields?.reordered).toBe(true);
+  });
+
+  it('CONTROL: the same order is no reorder', () => {
+    const d = computeChangeDetail(
+      { name: 'asset', fields: { title, condition } },
+      { name: 'asset', fields: { title, condition } },
+      'object',
+    );
+    expect(d.fields?.reordered).toBe(false);
+  });
+
+  it('leaves out the columns the reads inject: their place is the platform’s', () => {
+    // The registry-backed published read and the overlay-backed draft read put
+    // the platform's columns in different places; the declared order is equal.
+    const d = computeChangeDetail(
+      {
+        name: 'asset',
+        fields: { created_at: AUDIT_FIELD_DEFS.created_at, owner_id: OWNER_FIELD_DEF, title, __search: search, condition },
+      },
+      {
+        name: 'asset',
+        fields: { owner_id: OWNER_FIELD_DEF, title, condition, created_at: AUDIT_FIELD_DEFS.created_at, __search: search },
+      },
+      'object',
+    );
+    expect(d.fields?.reordered).toBe(false);
+  });
+
+  it('counts an `owner_id` the author declared: its place is theirs', () => {
+    const owner = { ...OWNER_FIELD_DEF, label: 'Account manager' };
+    const d = computeChangeDetail(
+      { name: 'asset', fields: { owner_id: owner, title } },
+      { name: 'asset', fields: { title, owner_id: owner } },
+      'object',
+    );
+    expect(d.fields?.reordered).toBe(true);
+  });
+
+  it('compares only the fields both versions declare: an added or removed field is its own row', () => {
+    const d = computeChangeDetail(
+      { name: 'asset', fields: { title, notes, condition } },
+      { name: 'asset', fields: { serial: { type: 'text' }, title, condition } },
+      'object',
+    );
+    expect(d.fields).toMatchObject({ added: ['serial'], removed: ['notes'], reordered: false });
+  });
+
+  it('is the publish door’s rule for an `object` only', () => {
+    const d = computeChangeDetail(
+      { name: 'asset', fields: { title, condition } },
+      { name: 'asset', fields: { condition, title } },
+      'view',
+    );
+    expect(d.fields?.reordered).toBe(false);
+  });
+});
+
+/* ─────────────── the panel ─────────────── */
+
+type Body = Record<string, unknown>;
+
+function mockServer(published: Body, draft: Body): void {
+  global.fetch = vi.fn(async (input: RequestInfo | URL) => {
+    const url = String(input);
+    const ok = (body: unknown) => ({ ok: true, status: 200, json: async () => body });
+    if (url.includes('/_drafts')) return ok([{ type: 'object', name: 'asset', packageId: 'com.x' }]);
+    const [type, name] = url.split('?')[0].replace('/api/v1/meta/', '').split('/');
+    if (!name) return ok([{ name: 'asset' }]);
+    return url.includes('state=draft') ? ok({ type, name, item: draft }) : ok(published);
+  }) as unknown as typeof fetch;
+}
+
+function assetRow(): HTMLElement {
+  const hit = screen
+    .getAllByTestId('draft-entry-toggle')
+    .find((el) => el.querySelector('.font-mono')?.textContent === 'asset');
+  if (!hit) throw new Error('no row for asset');
+  return hit;
+}
+
+describe('the drill-in lists an order-only change (objectstack#27516)', () => {
+  it('prints a Field order changed row and keeps the Update mark', async () => {
+    mockServer(
+      { name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, condition, notes } },
+      { name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, notes, condition }, _draft: true },
+    );
+    render(<DraftChangesPanel open onOpenChange={() => {}} packageId="com.x" onPublish={vi.fn()} />);
+    await waitFor(() => expect(assetRow().textContent).toContain('Update'));
+    fireEvent.click(assetRow());
+    const detail = await screen.findByTestId('draft-entry-detail');
+    expect([...detail.querySelectorAll('p')].map((p) => p.textContent)).toEqual([ORDER_ROW]);
+    expect(screen.queryByText(NO_CHANGE)).toBeNull();
+    expect(assetRow().textContent).toContain('Update');
+  });
+
+  it('CONTROL: the same order still reads No differences detected', async () => {
+    mockServer(
+      { name: 'asset', fields: { owner_id: OWNER_FIELD_DEF, title, condition } },
+      { name: 'asset', fields: { title, owner_id: OWNER_FIELD_DEF, condition }, _draft: true },
+    );
+    render(<DraftChangesPanel open onOpenChange={() => {}} packageId="com.x" onPublish={vi.fn()} />);
+    await waitFor(() => expect(assetRow().textContent).toContain('Update'));
+    fireEvent.click(assetRow());
+    await screen.findByText(NO_CHANGE);
+    expect(screen.queryByText(ORDER_ROW)).toBeNull();
+  });
+});