diff --git a/.changeset/10585-hook-object-roster-state.md b/.changeset/10585-hook-object-roster-state.md new file mode 100644 index 0000000000..50299564b6 --- /dev/null +++ b/.changeset/10585-hook-object-roster-state.md @@ -0,0 +1,34 @@ +--- +'@object-ui/app-shell': patch +--- + +The Hook inspector's object picker stops calling a selected object "not +published", and stops telling the author to publish one, while its object +roster is still loading or after the fetch failed (objectui#10585). + +The picker made two claims from the object catalog: a selected object the +catalog does not list was flagged `(not published)`, and an empty list printed +"No objects found — publish an object, then pick it here." Both were read off +the list alone, and the list is empty while the fetch is in flight and after it +failed, exactly as it is for a catalog with no objects. So every selected +object was called unpublished, and the author was told to publish one, before +the catalog had answered; on a failed fetch the claims never cleared. + +`useObjectOptions` now reports a failure as its own fact: beside `options` and +`loading` it returns `error`, spelled the way `useObjectFields` spells it, and +`null` while in flight and after an answer. `options` and `loading` are +unchanged, so the action and page-block object pickers, which read only +`options` and fall back to a free-text input while it is empty, behave as +before. + +The Hook inspector folds `loading` and `error` through `rosterFrom`, the rule +the objectui#8862 / objectui#9651 family set for the designer's pickers: only +an answered roster may make a claim. + +- Answered: unchanged — a selected object the catalog does not list is flagged, + and an empty catalog prints the "publish an object" copy. +- In flight: a selected object is shown bare, and the empty list says it is + loading. +- Failed: a selected object is shown bare and stays toggleable, and a status + notice says the options could not be loaded and names the cause, with the + same localized title the designer's other pickers use for a failed catalog. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.rosterFailure.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.rosterFailure.test.tsx new file mode 100644 index 0000000000..fe0ea38506 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.rosterFailure.test.tsx @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10585 — the Hook inspector's object picker makes its two claims + * only from a roster that ANSWERED. + * + * The picker makes two claims from the object catalog `useObjectOptions` + * fetches: a selected object the catalog does not list is flagged + * `(not published)`, and an empty list prints "No objects found — publish an + * object, then pick it here." Both used to be read off `options` alone, and + * `options` is `[]` while the fetch is in flight and after it failed — exactly + * what a catalog with no objects leaves. So every selected object was called + * unpublished, and the author was told to publish one, before the catalog had + * spoken and, on a failed fetch, permanently. + * + * The rule is the one objectui#8862 / objectui#9651 set for the designer's + * pickers, and the arms are the ones `ViewColumnInspector.rosterFailure.test.tsx` + * pins for that inspector: + * + * • in flight → no flag, no "publish" copy, no failure notice + * • FAILED → no flag, no "publish" copy, plus a notice naming the cause, + * and the selection stays editable + * • answered, object genuinely absent → flag, and an empty answered catalog + * still prints the "publish" copy (the controls that the + * claims still fire when they are true) + * + * The real `useObjectOptions` runs; only the metadata client is stubbed, and + * its `list` is settled by the test. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, act, waitFor, fireEvent } from '@testing-library/react'; + +const state = vi.hoisted(() => { + let settle = { + resolve: (_d: unknown) => {}, + reject: (_e: unknown) => {}, + }; + const pending = { current: new Promise(() => {}) }; + return { + metadataClient: { + get: vi.fn(async () => undefined), + list: vi.fn(() => pending.current), + }, + /** Arm a fresh unsettled `list` response; call before mounting. */ + hold() { + pending.current = new Promise((resolve, reject) => { + settle = { resolve, reject }; + }); + // The hook attaches its own handler only after mount; without this the + // rejection is "unhandled" for one turn and fails the test itself. + pending.current.catch(() => {}); + }, + land(docs: unknown) { + settle.resolve(docs); + }, + fail(err: unknown) { + settle.reject(err); + }, + }; +}); + +vi.mock('../useMetadata', () => ({ + useMetadataClient: () => state.metadataClient, +})); + +import { HookDefaultInspector } from './HookDefaultInspector'; + +afterEach(cleanup); + +const FLAGGED = 'ghost_object (not published)'; +const PUBLISH_COPY = /publish an object/; + +function mount(object?: string) { + state.hold(); + const onPatch = vi.fn(); + render( + , + ); + return { onPatch }; +} + +/** The failure notice the picker renders for a failed roster, if any. */ +const failureNotice = () => screen.queryByTestId('hook-object-roster-failure'); + +/** + * Reject the in-flight request and flush the hook's `catch` plus the re-render + * it schedules — explicitly, not by waiting for the notice, so the flag and + * empty-copy rows do not depend on the notice existing. + */ +async function failRequest(err: unknown): Promise { + await act(async () => { + state.fail(err); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +describe('HookDefaultInspector — the object roster FAILED to load (objectui#10585)', () => { + it('does not call a selected object "not published" after the fetch failed', async () => { + mount('ghost_object'); + await failRequest(new Error('503 Service Unavailable')); + + expect( + screen.getByRole('checkbox', { name: 'ghost_object' }), + 'the selection stays on screen, bare', + ).toBeTruthy(); + expect( + screen.queryByText(FLAGGED), + 'a roster that never answered cannot testify that the object is unpublished', + ).toBeNull(); + }); + + it('does not tell the author to publish an object after the fetch failed', async () => { + mount(); + await failRequest(new Error('503 Service Unavailable')); + + expect(screen.queryByText(PUBLISH_COPY)).toBeNull(); + expect( + screen.queryByText('Loading options…'), + 'a failed request is no longer in flight — it must not read as one', + ).toBeNull(); + }); + + it('tells the author the roster failed, and names the cause', async () => { + mount('ghost_object'); + await failRequest(new Error('503 Service Unavailable')); + + expect(failureNotice(), 'the failure reached the picker').not.toBeNull(); + expect(failureNotice()?.getAttribute('role')).toBe('status'); + expect(failureNotice()?.textContent).toContain('Options could not be loaded'); + expect(failureNotice()?.textContent).toContain('503 Service Unavailable'); + }); + + it('leaves the selection EDITABLE after the failure — un-ticking it reaches the draft', async () => { + const { onPatch } = mount('ghost_object'); + await failRequest(new Error('boom')); + + fireEvent.click(screen.getByRole('checkbox', { name: 'ghost_object' })); + expect( + onPatch, + 'a failed catalog must not also block authoring', + ).toHaveBeenCalledWith({ object: [] }); + }); +}); + +describe('HookDefaultInspector — the object roster is IN FLIGHT (objectui#10585)', () => { + it('shows a selected object bare, unflagged, with no failure notice', () => { + // No `waitFor`: waiting is what hides the in-flight window. + mount('ghost_object'); + expect(screen.getByRole('checkbox', { name: 'ghost_object' })).toBeTruthy(); + expect(screen.queryByText(FLAGGED)).toBeNull(); + expect(failureNotice(), 'an unanswered request is not a failed one').toBeNull(); + }); + + it('does not tell the author to publish an object while loading', () => { + mount(); + expect(screen.queryByText(PUBLISH_COPY)).toBeNull(); + expect(screen.getByText('Loading options…')).toBeTruthy(); + }); +}); + +describe('HookDefaultInspector — CONTROLS: an ANSWERED roster still makes its claims', () => { + it('flags a selected object the answered catalog does not list, with no notice', async () => { + mount('ghost_object'); + await act(async () => { + state.land([{ name: 'account', label: 'Account' }]); + }); + + await waitFor(() => { + expect(screen.getByRole('checkbox', { name: FLAGGED })).toBeTruthy(); + }); + expect(failureNotice(), 'a completed load is not a failure').toBeNull(); + }); + + it('prints the "publish an object" copy for a catalog that answered empty', async () => { + mount(); + await act(async () => { + state.land([]); + }); + + await waitFor(() => { + expect(screen.getByText(PUBLISH_COPY)).toBeTruthy(); + }); + expect(failureNotice()).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx index f2c158277e..6f96a68592 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.tsx @@ -20,7 +20,8 @@ * `object` is spec `string | string[]` (or `'*'` for every object). The picker * normalises to/from that: `'*'` ⇒ all, one selection ⇒ a string, many ⇒ an * array. Any already-selected object not in the live catalog is preserved as a - * synthesized option so a value is never silently dropped. + * synthesized option so a value is never silently dropped; it is flagged as not + * published only once the catalog has answered (objectui#10585). */ import * as React from 'react'; @@ -35,6 +36,7 @@ import { InspectorNumberField, InspectorCheckboxField, flagUnknownValue, + rosterFrom, } from './_shared.js'; import { useObjectOptions } from '../previews/useObjectOptions.js'; import { ConditionBuilder, RECORD_CONDITION_SUBJECTS } from './ConditionBuilder.js'; @@ -125,18 +127,32 @@ export function HookDefaultInspector({ const events: string[] = Array.isArray(draft.events) ? (draft.events as string[]) : []; const { all: allObjects, names: objectNames } = readObjects(draft.object); - const { options: objectOptions } = useObjectOptions(); + const { + options: objectOptions, + loading: objectsLoading, + error: objectsError, + } = useObjectOptions(); + // objectui#10585 — the roster's ONE state, read the way the objectui#8862 / + // objectui#9651 family reads it: only an ANSWERED roster may say an object is + // missing. While the fetch is in flight, or after it failed, `objectOptions` + // is `[]` exactly as it is for a catalog with no objects, so a claim made + // from the list alone told the author that every selected object was + // unpublished and that they should publish one. + const roster = rosterFrom({ loading: objectsLoading, error: objectsError }); + const rosterAnswered = roster.status === 'loaded'; // Preserve any selected object missing from the live catalog (draft-only / // cross-package) so it is never dropped from the picker. Its flag reads in - // the designer's locale (objectui#10448). + // the designer's locale (objectui#10448), and it is drawn only once the + // roster answered — until then the name is shown bare, which asserts nothing. const pickerOptions = React.useMemo(() => { const known = new Set(objectOptions.map((o) => o.value)); const notPublished = t('engine.form.notPublished', locale); const extra = objectNames .filter((n) => !known.has(n)) - .map((n) => ({ value: n, label: flagUnknownValue(n, notPublished, locale) })); + .map((n) => ({ value: n, label: rosterAnswered ? flagUnknownValue(n, notPublished, locale) : n })); return [...extra, ...objectOptions]; - }, [objectOptions, objectNames, locale]); + }, [objectOptions, objectNames, locale, rosterAnswered]); + const rosterFailure = roster.status === 'error' ? roster.message : undefined; const patchBody = (p: Record) => onPatch({ body: { ...body, ...p } }); @@ -206,10 +222,18 @@ export function HookDefaultInspector({ onCommit={(on) => onPatch({ object: on ? ALL_OBJECTS : writeObjects(false, objectNames) })} disabled={readOnly} /> - {!allObjects && ( + {/* The empty-list copy is a MEASUREMENT ("no objects found"), so only + an answered roster may print it; in flight it says it is loading. + A failed roster with nothing selected draws no list at all — the + notice below is the whole answer there. */} + {!allObjects && (pickerOptions.length > 0 || rosterFailure === undefined) && (
{pickerOptions.length === 0 ? ( -

No objects found — publish an object, then pick it here.

+

+ {rosterAnswered + ? 'No objects found — publish an object, then pick it here.' + : tr('engine.form.loadingOptions')} +

) : ( pickerOptions.map((o) => ( )} + {/* objectui#10585 — a failed roster says so, with its cause. This is + the notice `InspectorSelectField` renders for the same fact + (objectui#9651): the same role, tone, localized title and cause + span. That notice lives inside the primitive and is not exported, + and this picker is a checkbox list rather than a select, so the + markup is repeated here; the wording is the shared catalogue key. */} + {!allObjects && rosterFailure !== undefined && ( +

+ {tr('engine.form.optionsLoadFailedTitle')} + {rosterFailure ? ( + <> + {' '} + {rosterFailure} + + ) : null} +

+ )} {!allObjects && objectNames.length === 0 && (

Pick at least one object (or All objects).

)} diff --git a/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.test.tsx new file mode 100644 index 0000000000..2153b7098c --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.test.tsx @@ -0,0 +1,59 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10585 — `useObjectOptions` tells a failed fetch apart from a catalog + * that answered with no objects. + * + * Both leave `options: []` with `loading: false`; before this card nothing + * else was returned, so a caller could not tell the two apart and made the + * claims of an answered roster ("not published", "publish an object") on a + * failure. The hook now publishes `error`, spelled the way `useObjectFields` + * spells it, so `rosterFrom({ loading, error })` reads both hooks alike. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { renderHook, waitFor, cleanup } from '@testing-library/react'; + +const state = vi.hoisted(() => ({ + metadataClient: { + list: vi.fn(async (): Promise => []), + }, +})); + +vi.mock('../useMetadata', () => ({ + useMetadataClient: () => state.metadataClient, +})); + +import { useObjectOptions } from './useObjectOptions'; + +afterEach(cleanup); + +describe('useObjectOptions — failure is its own fact (objectui#10585)', () => { + it('a rejected `client.list` sets `error` to the cause, with `loading` false and no options', async () => { + state.metadataClient.list.mockImplementationOnce(async () => { + throw new Error('503 Service Unavailable'); + }); + const { result } = renderHook(() => useObjectOptions()); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.error).toBe('503 Service Unavailable'); + expect(result.current.options).toEqual([]); + }); + + it('a resolved EMPTY list leaves `error` null — an answer, not a failure', async () => { + state.metadataClient.list.mockImplementationOnce(async () => []); + const { result } = renderHook(() => useObjectOptions()); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.error).toBeNull(); + expect(result.current.options).toEqual([]); + }); + + it('in flight, `error` is null and `loading` is true', () => { + state.metadataClient.list.mockImplementationOnce(() => new Promise(() => {})); + const { result } = renderHook(() => useObjectOptions()); + + expect(result.current.loading).toBe(true); + expect(result.current.error).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.ts b/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.ts index 0525f4de28..10c46410db 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/useObjectOptions.ts @@ -1,9 +1,18 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * useObjectOptions — the list of objects (name + label) for object pickers in - * the page-editor block inspector. Async; degrades to an empty list (the - * picker then falls back to a free-text input). + * useObjectOptions — the list of objects (name + label) for the object pickers + * in the designer's default inspectors (action, hook, page block). Async. + * + * It publishes three facts, the same three `useObjectFields` does: `options`, + * `loading` and `error`. A failed fetch still leaves `options` empty with + * nothing in flight — the action and page-block pickers then fall back to a + * free-text input — but it also sets `error`, so a failure is no longer + * indistinguishable from a catalog that answered with no objects + * (objectui#10585). A caller that makes a claim from the list — flagging a + * selected object as missing, or telling the author to publish one — must read + * all three, not `options` alone: hand `{ loading, error }` to `rosterFrom` + * (`inspectors/_shared.tsx`) and make the claim only when the roster answered. */ import * as React from 'react'; import { useMetadataClient } from '../useMetadata.js'; @@ -13,14 +22,23 @@ export interface ObjectOption { label: string; } -export function useObjectOptions(): { options: ObjectOption[]; loading: boolean } { +export interface UseObjectOptionsResult { + options: ObjectOption[]; + loading: boolean; + /** Why the last fetch failed; `null` while in flight and after an answer. */ + error: string | null; +} + +export function useObjectOptions(): UseObjectOptionsResult { const client = useMetadataClient(); const [options, setOptions] = React.useState([]); const [loading, setLoading] = React.useState(true); + const [error, setError] = React.useState(null); React.useEffect(() => { let cancelled = false; setLoading(true); + setError(null); client .list<{ name?: string; label?: string }>('object') .then((items) => { @@ -36,10 +54,13 @@ export function useObjectOptions(): { options: ObjectOption[]; loading: boolean setOptions(mapped); setLoading(false); }) - .catch(() => { + .catch((err) => { if (!cancelled) { setOptions([]); setLoading(false); + // The same spelling `useObjectFields` uses, so `rosterFrom` reads + // both hooks alike: a fault with an empty message is still a fault. + setError(err?.message ?? String(err)); } }); return () => { @@ -47,5 +68,5 @@ export function useObjectOptions(): { options: ObjectOption[]; loading: boolean }; }, [client]); - return { options, loading }; + return { options, loading, error }; }