Skip to content

Commit 2fec2e0

Browse files
fix(app-shell): Studio's field pickers read the draft-overlaid object, so a draft-only object offers its fields (objectui#11895) (#11952)
Fixes #11895 Clause-②: no ## What changes Studio and the metadata-admin designers read an object's fields through one hook, `useObjectFields`. That hook asked for the published object only (`GET /meta/object/NAME`). For an object built in Studio and not yet published, the read answers 404, so every field picker on the hook offered no fields. The flow Start node's entry-condition builder listed only `previous`, and an author had to switch to Expression mode and type CEL. The hook now reads the draft-overlaid object through `client.withPreviewDrafts(true).get('object', name)` (`?preview=draft`). This is the same read objectui#11783's object picker already makes for its list. The hook's signature, its `override` short-circuit, its result shape and its not-found sentence are unchanged, and every consumer keeps its code. - **One request, no second fallback read.** In the framework (17.7.0), `getMetaItem({ previewDrafts: true })` serves the pending draft when there is one and falls back to the active object otherwise; it never answers `no_draft`. Both transports do this: the runtime dispatcher's object branch and `RestServer`'s uncached arm. It is pinned in objectstack `packages/objectql/src/protocol-meta.test.ts` ("falls back to active when previewDrafts but no draft exists (no no_draft 404)"). A 404 now means neither a draft nor a published object exists. - **No package needed.** With no package, the draft lookup takes any row (`servedOverlayRowCandidates`), so a package-bound draft is found. - **Who can see drafts.** Every caller that reaches the fetch is an authoring surface, under `views/metadata-admin` or `views/studio-design`. The one runtime host, `ViewConfigPanel`, always passes `objectFieldsOverride` and never sends this read. The server also answers anyone it does not admit to drafts (`mayReadPendingDrafts`: system, `studio.access`, `setup.access`, `manage_metadata`) with the published object, as if the switch had not been sent. ## The 39 test doubles (claim amendment, seat ruling A) 39 existing test files mock `../useMetadata` with a metadata-client double that predates this read. With the fix, each one goes red: 37 doubles had no `withPreviewDrafts`, so the hook threw inside its effect, and 2 had a `withPreviewDrafts` that returned only `list`. All 39 were green with the fix ablated and red with it in place, so the doubles were the only cause. Each double gains the one member it lacked, with no assertion, fixture or test-name change: - **Shape 1 (37 files).** `withPreviewDrafts() { return this; }`, which returns the same double, so the test's own `get` stub keeps answering. In `StudioDesignSurface.autosaveSwitch-11232` it is `withPreviewDrafts: vi.fn((): unknown => mockClient)` instead, because that suite's `beforeEach` calls `mockClear` on every member. - **Shape 2 (2 files).** In `FlowReferenceField.objectPicker-11783` and `ObjectFieldInspector.objectPicker-11783`, the object their existing `withPreviewDrafts` returns also carries `get`. - **Two comments.** Two double comments that named the read as `client.get` now say `client.withPreviewDrafts(true).get`. The 39 files, all under `packages/app-shell/src/views/`: - `metadata-admin/`: `ResourceEditPage.defaultGate`, `ResourceEditPage.requiredInGatedSection-6900`, `capabilityGateChannel-7234` - `metadata-admin/inspectors/`: - `ConditionBuilder` (`.test`, `.celGate`, `.clientMountRoots`, `.contextSubjects`, `.mountRoots`, `.mountScope`, `.placeholderRoots`, `.referenceValue`, `.subjectVocabulary`) - `FlowReferenceField.objectPicker-11783` - `HookDefaultInspector.flag.i18n-10448`, `HookDefaultInspector.rosterFailure`, `HookDefaultInspector.reach-11820` - `ObjectFieldInspector.objectPicker-11783` - `PageBlockInspector` (`.colorLabelling`, `.definitionListItems-8279`, `.i18n`, `.pageHeaderBreadcrumb-11173`, `.retiredBlockProps`, `.sectionName`) - `ViewColumnInspector.rosterFailure`, `ViewColumnInspector.rosterPending` - `ViewVariantInspector` (`.celGate`, `.homeGate`, `.requiredInGatedSection-6900`) - `unknownValueFlag.i18n-9652` - `metadata-admin/previews/`: `flowFieldChrome.i18n-10862-s4` - `studio-design/`: - `DataPillar.designerRegistryPopulated` - `ObjectHooksPanel.celGate`, `ObjectHooksPanel.newHookTarget-11820` - `StudioDesignSurface` (`.autosaveSwitch-11232`, `.listViewInspector-11823`, `.navEntryIdentity-11774`, `.selectionLeafScope`, `.studioCanvasLeaf`, `.zhPillarNames-11801`) **Overlap with objectui#11894 (seat 3, in flight, no PR when this opened).** The nine `ConditionBuilder.*.test.tsx` files above are among objectui#11894's "tests beside them". This change adds one property to each file's `vi.mock` double and nothing else. Whoever lands second merges `main`; on a conflict, keep objectui#11894's changes and re-add only the `withPreviewDrafts` member. objectui#11894 had not landed when this branch last merged `main`. ## Pins New file `previews/useObjectFields.draftOverlay-11895.test.tsx`, with 7 tests. It runs the REAL `MetadataClient` over a stub `fetch` that answers the way the framework's item read does: - A draft-only object: the hook answers its draft fields, in 1 request that carries `preview=draft`. - A published object with a pending draft: the draft's fields (the overlay). - CONTROL: a published object with no draft reads as before, in 1 request. - A name with neither: `fields: []` and the not-found sentence (`engine.form.objectNotFound`), in 1 request. - CONTROL: an `override` still short-circuits the read (0 requests). - The flow Start node's entry-condition builder (real `FlowNodeConfigField`, real descriptor, real scope) offers a draft-only object's fields (`issue_summary`, `severity`, `previous.issue_summary`), not only `previous`. - CONTROL: the same builder offers a published object's fields as before. ## Verification Every result below comes from a run on the branch HEAD `811fef2` (`git rev-parse --short HEAD`, after merging `main` `f1781be`), the pushed tip. - **Dependency closure.** `pnpm --filter '@object-ui/app-shell^...' build`: exit 0 (`VERDICT command-exit 0`). - **Type-check.** `pnpm --filter @object-ui/app-shell type-check` (`tsc --noEmit && tsc -p tsconfig.test.json`): exit 0. `--listFilesOnly` confirms the new pin file is in the test program. - **Family sweep.** This covers every test file that imports `useObjectFields.ts` directly or transitively: 504 files, re-derived on this HEAD, in 6 lock holds. Each hold reported `Test Files 84 passed (84)`, 4522 tests passed in all. That includes the 39 doubles, the pin, the flow designer's entry-condition suites, and `StudioDesignSurface.accessGuard` / `pillarNavGuard`. - **View-cache guard and pin.** `viewCacheInvalidation.guard.test.tsx` plus the pin: `Test Files 2 passed (2)`, `Tests 16 passed (16)`. - **Reverse check, on this HEAD.** Done with the fix committed, via objectstack `scripts/ablation-replace.mjs` in wrap mode. - The mutation replaced `client .withPreviewDrafts(true) .get` with the published-only `client .get`. The anchor count went 1 to 0, the blob changed `f6b903e3` to `72d5be5f`, and the on-disk count read 0. - Expected beforehand: 3 red, 4 green. Observed: `Tests 3 failed | 4 passed (7)`. The red ones were the draft-only pin, the overlay pin and the flow entry-condition pin; the controls stayed green. - The restore was proven: the blob equals the HEAD blob and `git diff HEAD` is empty. - An earlier leg on the pre-merge tree ran the 39 double files under the same mutation, and all were green. So the doubles were the only cause of their red. - **Check gates.** All exit 0, each printing its OK/VERDICT line: - `pnpm check:control-bytes`, `check:new-line-citations` (0 new citations), `check:changeset-claims` and `check:pending-changeset-literals`; - `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:test-path-roots`, `check:metadata-write-doors` and `check:unreferenced-sources`; - `node scripts/check-changeset-presence.mjs`. - **Lint, narrowed.** `eslint --format json` over the 41 touched ts/tsx files, run from `packages/app-shell` as its `lint` script runs: 41 files, 0 errors, 5 warnings. - Every pre-existing file has the same error/warning counts as its merge-base blob, linted through stdin. The new pin file has 0/0. - `eslint.config.js` declares no `parserOptions.project` / `projectService`, so the linting is not type-aware, and this diff cannot move the verdict on an untouched file. - The repo-wide `pnpm lint` and the full test farm are CI's. - **After the union.** `main` moved to `f3a0488`, touching no `ConditionBuilder` or `useObjectFields` file. Its 6 new or changed test files were run against this fix in an uncommitted merge, which was then aborted: `Test Files 6 passed (6)`. ## Acceptance notes - The order's gate lead (`git grep -l useObjectFields -- '*.test.*'`) misses most of the double breakage, because the red files mount a consumer and never name the hook. The test family here is every test file that imports `useObjectFields.ts`, directly or transitively, derived by a reverse import-graph walk. - Observation, not filed: `ConditionBuilder` calls `useObjectFields(objectName)` even when a `fields` prop is supplied, so a mount passing both sends a read whose answer it discards. That is a wasted read, not a wrong answer, and it is outside this card's surface. --- _Generated by [Claude Code](https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 077d198 commit 2fec2e0

42 files changed

Lines changed: 303 additions & 22 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
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+
Studio's field pickers offer the fields of an object that is still an unpublished draft (objectui#11895).
6+
7+
Studio and the metadata-admin designers read an object's fields through one hook, and that hook asked for the published object only. For an object built in Studio and not yet published, that read answers "not found", so every picker on it offered no fields: the flow Start node's entry-condition builder listed only `previous`, and an author had to switch to Expression mode and type the condition by hand.
8+
9+
The hook now reads the draft-overlaid object (`?preview=draft`), the same read the object picker already makes for its list. A pending draft's fields are offered before the first publish; an object with a pending draft offers the draft's fields; an object with no draft reads as before, in the same single request. A name with neither a draft nor a published object still shows the not-found message. Only callers that may read drafts see them: the server answers anyone else the published object. The runtime view configuration panel passes its own field list and does not send this read.
10+
11+
No export, prop, type member or language-pack key changes.

‎packages/app-shell/src/views/metadata-admin/ResourceEditPage.defaultGate.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const mockClient = {
5050
layered: vi.fn(async () => ({ effective: viewDef, code: viewDef, editable: true })),
5151
getDraft: vi.fn(async () => null),
5252
get: vi.fn(async () => null),
53+
withPreviewDrafts() { return this; },
5354
saveDraft: vi.fn(async () => ({})),
5455
};
5556

‎packages/app-shell/src/views/metadata-admin/ResourceEditPage.requiredInGatedSection-6900.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const mockClient = {
9090
type === 'object' && name === 'employee' ? employeeObject() : null,
9191
),
9292
saveDraft: vi.fn(async () => ({})),
93+
withPreviewDrafts() { return this; },
9394
};
9495

9596
vi.mock('./useMetadata', async (importOriginal) => {

‎packages/app-shell/src/views/metadata-admin/capabilityGateChannel-7234.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ vi.mock('@object-ui/permissions', async (importOriginal) => {
8282
// ConditionBuilders, which call the shared metadata client at mount. Stub it so
8383
// no fetch escapes; same mechanism ActionDefaultInspector.celGate.test.tsx uses.
8484
const state = vi.hoisted(() => ({
85-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
85+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
8686
}));
8787
vi.mock('./useMetadata', () => ({
8888
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.celGate.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/re
3737
// the shared client so that mount-time fetch doesn't escape to the real
3838
// network; see PageBlockInspector.i18n.test.tsx for the full mechanism.
3939
const state = vi.hoisted(() => ({
40-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
40+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
4141
}));
4242
vi.mock('../useMetadata', () => ({
4343
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.clientMountRoots.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import '@objectstack/formula';
5353
// `useObjectOptions()` unconditionally, so a mount-time fetch would escape to
5454
// the real network.
5555
const state = vi.hoisted(() => ({
56-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
56+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
5757
}));
5858
vi.mock('../useMetadata', () => ({
5959
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.contextSubjects.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ import '@objectstack/formula';
6767
// unconditionally even when `fields` is supplied, so a mount-time fetch would
6868
// otherwise escape to the real network.
6969
const state = vi.hoisted(() => ({
70-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
70+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
7171
}));
7272
vi.mock('../useMetadata', () => ({
7373
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.mountRoots.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ import '@objectstack/formula';
5959
// objectui#4697 — these inspectors call `useObjectFields(objectName)`
6060
// unconditionally, so a mount-time fetch would escape to the real network.
6161
const state = vi.hoisted(() => ({
62-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
62+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
6363
}));
6464
vi.mock('../useMetadata', () => ({
6565
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.mountScope.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ import '@objectstack/formula';
7979
// engine reports a bare reference from the SCOPE, not from the field list
8080
// (measured with `fields: []` and with `fields: undefined` — identical).
8181
const state = vi.hoisted(() => ({
82-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
82+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
8383
}));
8484
vi.mock('../useMetadata', () => ({
8585
useMetadataClient: () => state.metadataClient,

‎packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.placeholderRoots.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import '@objectstack/formula';
5555
// objectui#4697 — these inspectors call `useObjectFields(objectName)`
5656
// unconditionally, so a mount-time fetch would escape to the real network.
5757
const state = vi.hoisted(() => ({
58-
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
58+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
5959
}));
6060
vi.mock('../useMetadata', () => ({
6161
useMetadataClient: () => state.metadataClient,

0 commit comments

Comments
 (0)