Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/11787-one-save-model.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@object-ui/app-shell': patch
---

Studio saves one way on a package: the permission matrix and hooks autosave to the package draft like the other pillars, every create dialog says *Save as draft*, and the Changes count keeps up with a save (objectui#11787).

- **Permission matrix, package door.** An edit is saved to the package's draft 1.5 seconds after the last change, the same autosave the Data and Interfaces pillars run, and the header shows *Saving…* and then *Saved* with the time in place of the Save button. A CEL syntax error in the row filter holds the edit and says why. An edit made while a save is in flight is kept and sent next. The set's API name can no longer be edited here, because the autosave stores the draft under that name. It is set when the set is created.
- **Permission matrix, environment-admin door.** Unchanged. It keeps its explicit Save, because it writes live access configuration (ADR-0086 D7).
- **Hooks.** A hook autosaves to its draft the same way, and the *Save hook* button is gone. A hook whose run condition does not parse is held until it does. A hook keeps the name it was created with: while its name is changed the autosave holds, and a line says to change it back. The draft is stored under the hook's name, so a rename is not a save. Creating a hook and saving one both update the Changes count.
- **Create dialogs.** *New permission set* now says *Save as draft*, like every other Studio create dialog.
- **Changes count.** After a draft save, the header no longer says "No drafts pending publish" next to an item's "Unpublished draft" chip. It counts at least the draft just saved until its next read answers, and that read is sent after the save instead of reusing a read that was already in flight.

No export, prop, type or language-pack key changes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/**
* 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.
*
* objectui#11787 — the count read after a draft save is sent after it, and
* until it answers the count is known to be at least one.
*
* Studio's header showed "No drafts pending publish" beside an item's
* "Unpublished draft" chip after a save, until the count caught up. Two causes
* meet in this hook:
* - the read a save asks for could JOIN a `_drafts` request already on the
* wire (objectui#11797's sharing), sent before the save and so unable to
* count the draft it wrote;
* - until any read answers, the count shown is the one from before the save.
*
* Counted on the wire with the same held `fetch` the objectui#11797 pins use:
* every request stays open until the test answers it.
*/
import React from 'react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { usePendingDrafts } from './usePendingDrafts.js';

interface HeldRequest {
url: string;
respond: (status: number, body: unknown) => void;
}

function heldFetch() {
const held: HeldRequest[] = [];
vi.stubGlobal(
'fetch',
vi.fn(
(input: RequestInfo | URL) =>
new Promise((resolve) => {
held.push({
url: String(input),
respond: (status, body) =>
resolve({ ok: status >= 200 && status < 300, status, json: async () => body }),
});
}),
),
);
const reads = (url: string) => held.filter((r) => r.url === url);
return { held, reads };
}

const CRM = '/api/v1/meta/_drafts?packageId=app.crm';
const draft = (name: string) => ({ type: 'object', name, packageId: 'app.crm' });

/** The Studio header's shape: the count, whether it is behind a save, and the save's refresh. */
function Header() {
const { count, refresh, behindSave } = usePendingDrafts({ packageId: 'app.crm' });
return (
<>
<span data-testid="count">{count === null ? 'unknown' : String(count)}</span>
<span data-testid="behind">{String(behindSave)}</span>
<button type="button" data-testid="saved" onClick={() => void refresh({ afterSave: true })} />
<button type="button" data-testid="refresh" onClick={() => void refresh()} />
</>
);
}

async function drain() {
for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0));
}

afterEach(() => {
vi.unstubAllGlobals();
});

describe('usePendingDrafts — the read after a draft save (objectui#11787)', () => {
it('is sent fresh while an earlier read is still on the wire, and its answer wins', async () => {
const { reads } = heldFetch();
render(<Header />);
await act(drain);
expect(reads(CRM)).toHaveLength(1);

// A draft save lands while the mount read is still pending.
await act(async () => {
fireEvent.click(screen.getByTestId('saved'));
await drain();
});
// Not joined to the request sent before the save.
expect(reads(CRM)).toHaveLength(2);
// Until a read sent after the save answers, the count is behind it.
expect(screen.getByTestId('behind').textContent).toBe('true');

// The pre-save read answers first, with the ledger as it was: ignored.
await act(async () => reads(CRM)[0].respond(200, []));
await act(drain);
expect(screen.getByTestId('count').textContent).toBe('unknown');
expect(screen.getByTestId('behind').textContent).toBe('true');

// The post-save read counts the draft the save wrote.
await act(async () => reads(CRM)[1].respond(200, [draft('crm_account')]));
await waitFor(() => expect(screen.getByTestId('count').textContent).toBe('1'));
expect(screen.getByTestId('behind').textContent).toBe('false');
});

it('CONTROL — a refresh that is not after a save still shares the pending read (objectui#11797)', async () => {
const { reads } = heldFetch();
render(<Header />);
await act(drain);
await act(async () => {
fireEvent.click(screen.getByTestId('refresh'));
await drain();
});
expect(reads(CRM)).toHaveLength(1);
expect(screen.getByTestId('behind').textContent).toBe('false');
});

it('stays behind when the read after the save fails, and a later answer clears it', async () => {
const { reads } = heldFetch();
render(<Header />);
await act(drain);
await act(async () => reads(CRM)[0].respond(200, []));
await waitFor(() => expect(screen.getByTestId('count').textContent).toBe('0'));

await act(async () => {
fireEvent.click(screen.getByTestId('saved'));
await drain();
});
await act(async () => reads(CRM)[1].respond(503, { success: false }));
await act(drain);
// The save is not in doubt: the draft it wrote exists.
expect(screen.getByTestId('count').textContent).toBe('unknown');
expect(screen.getByTestId('behind').textContent).toBe('true');

await act(async () => {
fireEvent.click(screen.getByTestId('refresh'));
await drain();
});
await act(async () => reads(CRM)[2].respond(200, [draft('crm_account')]));
await waitFor(() => expect(screen.getByTestId('count').textContent).toBe('1'));
expect(screen.getByTestId('behind').textContent).toBe('false');
});
});
50 changes: 43 additions & 7 deletions packages/app-shell/src/preview/usePendingDrafts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ export interface PendingDraftEntry {
* - the publish pulse (`emitMetadataRefresh`, the contract every publish path
* keeps — see the module doc) drops the pending requests before the hooks
* it wakes read again, so their read is never answered by a request sent
* before the publish.
* before the publish;
* - a draft save does the same for the read it asks for
* (`refresh({ afterSave: true })`, objectui#11787): a request sent before
* the save landed cannot count the draft it wrote.
*
* Keyed by the `fetch` the request went through: a request only answers
* callers of the same transport. Each caller parses the shared payload itself,
Expand All @@ -70,6 +73,10 @@ export interface PendingDraftEntry {
const pendingDraftReads = new WeakMap<typeof fetch, Map<string, Promise<unknown>>>();
let publishPulseObserved = false;

function draftsUrl(packageId?: string | null): string {
return `/api/v1/meta/_drafts${packageId ? `?packageId=${encodeURIComponent(packageId)}` : ''}`;
}

function readDraftsPayload(url: string): Promise<unknown> {
// Registered on first use rather than at import, and so before the hook's
// own pulse subscription (the hook reads first, then subscribes): the drop
Expand Down Expand Up @@ -107,8 +114,7 @@ function readDraftsPayload(url: string): Promise<unknown> {
export async function fetchPendingDrafts(
packageId?: string | null,
): Promise<PendingDraftEntry[]> {
const qs = packageId ? `?packageId=${encodeURIComponent(packageId)}` : '';
const data = (await readDraftsPayload(`/api/v1/meta/_drafts${qs}`)) as
const data = (await readDraftsPayload(draftsUrl(packageId))) as
| Array<Record<string, unknown>>
| { drafts?: Array<Record<string, unknown>>; data?: { drafts?: Array<Record<string, unknown>> } };
const list = Array.isArray(data) ? data : (data?.drafts ?? data?.data?.drafts ?? []);
Expand All @@ -132,8 +138,25 @@ export interface UsePendingDraftsResult {
/** null until the first successful read (lets callers hold rendering). */
count: number | null;
entries: PendingDraftEntry[];
/** Manual refetch — for surface-local triggers (draft saved, turn idle). */
refresh: () => Promise<void>;
/**
* Manual refetch — for surface-local triggers (draft saved, turn idle).
*
* objectui#11787 — `afterSave`: a draft save in this scope has just landed.
* The read is then sent fresh rather than joining one already on the wire,
* which was sent before the save and cannot count its draft, and
* {@link UsePendingDraftsResult.behindSave} holds until a read sent after it
* answers.
*/
refresh: (opts?: { afterSave?: boolean }) => Promise<void>;
/**
* objectui#11787 — a draft save reported through `refresh({ afterSave: true })`
* has landed, and no read sent after it has answered yet. The draft it wrote
* exists, so a surface showing the count knows it is at least one: a header
* that said "nothing to publish" beside an item's "unpublished draft" chip
* was showing a count from before the save. Stays true when that read fails,
* since the save itself is not in doubt; a later answer clears it.
*/
behindSave: boolean;
}

export function usePendingDrafts(opts: UsePendingDraftsOptions = {}): UsePendingDraftsResult {
Expand All @@ -143,15 +166,28 @@ export function usePendingDrafts(opts: UsePendingDraftsOptions = {}): UsePending
// A single in-flight guard + generation counter: a refresh started before a
// scope change must not land its stale answer on the new scope.
const genRef = useRef(0);
// objectui#11787 — draft saves reported (`savesRef`, mirrored in `saves`
// for render), and how many of them the installed answer was sent after.
const savesRef = useRef(0);
const [saves, setSaves] = useState(0);
const [counted, setCounted] = useState(0);

const refresh = useCallback(async () => {
const refresh = useCallback(async (opts?: { afterSave?: boolean }) => {
if (!enabled) return;
if (opts?.afterSave) {
savesRef.current += 1;
setSaves(savesRef.current);
// Never joined to a request sent before the save (objectui#11787).
pendingDraftReads.get(globalThis.fetch)?.delete(draftsUrl(packageId));
}
const sentAfter = savesRef.current;
const gen = ++genRef.current;
try {
const list = await fetchPendingDrafts(packageId);
if (genRef.current !== gen) return;
setEntries(list);
setCount(list.length);
setCounted(sentAfter);
} catch {
// An errored read means UNKNOWN, not zero: count stays null so surfaces
// that must fail SAFE on unknown (the preview bar keeps its Publish
Expand All @@ -176,5 +212,5 @@ export function usePendingDrafts(opts: UsePendingDraftsOptions = {}): UsePending
});
}, [refresh, enabled]);

return { count, entries, refresh };
return { count, entries, refresh, behindSave: counted < saves };
}
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ describe('PermissionMatrixEditPage — MUST NOT CHANGE: what the artifact tier m
codeLayer = { _packageId: 'com.example.showcase' };
await renderMatrix({ packageId: 'com.example.showcase' });

expect(screen.getByRole('button', { name: /^Save$/ })).toBeEnabled();
// objectui#11787 — the package door autosaves: no Save button; the live controls are the writability read.
expect(screen.queryByRole('button', { name: /^Save$/ })).toBeNull();
expect(screen.getByLabelText('a_account Read')).toBeEnabled();
const row = screen.getByText('Account').closest('tr')!;
for (const n of ['R', 'CRUD', 'All', 'None']) {
Expand All @@ -269,7 +270,8 @@ describe('PermissionMatrixEditPage — MUST NOT CHANGE: what the artifact tier m
codeLayer = null;
await renderMatrix({ packageId: 'com.example.showcase' });

expect(screen.getByRole('button', { name: /^Save$/ })).toBeEnabled();
// objectui#11787 — the package door autosaves: no Save button; the live controls are the writability read.
expect(screen.queryByRole('button', { name: /^Save$/ })).toBeNull();
expect(screen.getByLabelText('a_account Read')).toBeEnabled();
expect(lockBadge()).toBeNull();
});
Expand Down
Loading
Loading