fix(plugin-form,fields): a record form refuses to save while an upload is in flight (objectui#10166)

.changeset/10166-record-form-save-during-upload.md

@@ -0,0 +1,48 @@
+---
+'@object-ui/plugin-form': minor
+'@object-ui/fields': minor
+'@object-ui/i18n': minor
+---
+
+A record form can no longer be saved while an upload is still in flight (objectui#10166).
+
+A `file` / `image` value only becomes its fileId once the presigned upload settles. Until
+now a record form had no notion of upload state at all, so a Save pressed during that
+window wrote the record WITHOUT the attachment — and reported success. The user picked the
+file, saw it listed and saved; there was no error, no warning, and the record looked saved.
+Whoever noticed did so later, looking at a record that should have a file and does not.
+
+`onUploadingChange` (ADR-0059) already carried the signal, and had exactly one consumer,
+`ActionParamDialog`. It could not have a second: that prop is per-widget, and a record form
+hands a `fields` array to the `form` node renderer and never touches a widget, so there is
+no point in the chain where it can attach a callback — and its upload controls can sit
+inside a section, a tab or a line-items subform.
+
+`@object-ui/fields` therefore publishes the AGGREGATION beside the prop: `useUploadingScope`
+(the host's "is anything below me uploading") and `UploadingScopeProvider`.
+`useUploadingSignal` — also exported now, for widgets authored outside this repo — feeds
+both sinks from the one call it already made, so the per-widget prop and the scope cannot
+disagree, and a host that mounts no provider is unaffected. A widget that unmounts
+mid-upload releases its slot, so a collapsing section cannot wedge Save shut.
+
+Nesting CHAINS rather than shadows: an inner scope gates its own Save AND reports itself to
+the scope above. The direction is forced by `MasterDetailForm`, whose Save persists parent
+and children in one batch while its rows are edited by nested `ObjectForm`s — a gate that
+saw only the parent's uploads would refuse nothing while a child's attachment was in flight
+and would still read as coverage.
+
+Every submit owner in `@object-ui/plugin-form` is gated: `ObjectForm`, `ModalForm`,
+`DrawerForm`, `SplitForm`, `TabbedForm`, `WizardForm`, `MasterDetailForm`, and
+`EmbeddableForm` through the `ObjectForm` it hosts. While an upload is in flight each
+refuses the submit (which is also the keyboard-submit guard), labels Save "Uploading…", and
+renders the reason as a sentence — `form.uploadInFlight`, new in all ten locale packs. The
+hosts that own their Save button — `ModalForm`, `DrawerForm`, `MasterDetailForm`, and
+`WizardForm`'s final step — disable it as well; the flat `ObjectForm`, `SplitForm` and
+`TabbedForm` paths submit through the `form` node renderer in `@object-ui/components`, which
+exposes no per-button disable, so there the refusal plus the label and the notice are what
+the user meets.
+
+`WizardForm` is gated on its FINAL commit only. Moving between steps writes nothing, so
+`Next` is deliberately untouched — but note that leaving a step unmounts its widgets, so an
+upload in flight is released by the unmount and its value never reaches the record. That
+loss predates this change and is not addressed by it.

packages/fields/src/index.tsx

@@ -4185,5 +4185,22 @@ export {
   fromDateTimeInputValue,
 } from './widgets/nativeDateValue.js';
 
+// The AGGREGATED half of the upload-in-flight signal (objectui#10166).
+// `onUploadingChange` answers "is THIS widget uploading" to a host that renders
+// the control itself; a record form hands a `fields` array to the `form` node
+// renderer and never touches a widget, so it has no place to attach that
+// callback. `useUploadingScope` + `UploadingScopeProvider` let such a host ask
+// "is ANYTHING below me uploading" and gate its own Save on the answer.
+// Exported because the hosts that need it live in other packages
+// (`@object-ui/plugin-form`), and because a second, host-local implementation
+// of the same aggregation would drift from the one `useUploadingSignal` feeds.
+export { useUploadingScope, UploadingScopeProvider } from './widgets/uploadingScope.js';
+// The producer side of that same signal, exported for the same reason
+// `toDomProps`/`toHostProps` are: a widget authored outside this repo publishes
+// its in-flight state through this ONE hook, and a second implementation of
+// "tell my host I am uploading" would reach only half the sinks.
+export { useUploadingSignal } from './widgets/useUploadingSignal.js';
+export type { UploadingScope } from './widgets/uploadingScope.js';
+
 // Initialize registry
 registerAllFields();

packages/fields/src/widgets/uploadingScope.test.tsx

@@ -0,0 +1,162 @@
+/**
+ * `UploadingScope` — the AGGREGATED half of the upload-in-flight signal
+ * (objectui#10166). `onUploadingChange` answers "is THIS widget uploading" to a
+ * host that renders the control; the scope answers "is ANYTHING below me
+ * uploading" to a host that does not, which is every record form.
+ *
+ * The rows that matter are the release paths. A scope that can be entered and
+ * not left wedges its host's Save for the rest of the session — a worse failure
+ * than the one the scope exists to fix, and one with no visible cause.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import React from 'react';
+import { render, screen, act } from '@testing-library/react';
+import { useUploadingScope, UploadingScopeProvider } from './uploadingScope.js';
+import { useUploadingSignal } from './useUploadingSignal.js';
+
+/** A widget that publishes through the real hook, like FileField/ImageField. */
+function Widget({ uploading, onUploadingChange }: { uploading: boolean; onUploadingChange?: (u: boolean) => void }) {
+  useUploadingSignal(uploading, onUploadingChange);
+  return null;
+}
+
+function Host({
+  children,
+  onAny,
+}: {
+  children: (scope: ReturnType<typeof useUploadingScope>) => React.ReactNode;
+  onAny?: (any: boolean) => void;
+}) {
+  const scope = useUploadingScope();
+  onAny?.(scope.anyUploading);
+  return (
+    <>
+      <span data-testid="any">{String(scope.anyUploading)}</span>
+      <UploadingScopeProvider scope={scope}>{children(scope)}</UploadingScopeProvider>
+    </>
+  );
+}
+
+const any = () => screen.getByTestId('any').textContent;
+
+describe('UploadingScope', () => {
+  it('is false with no widget uploading and true while one is', () => {
+    const { rerender } = render(<Host>{() => <Widget uploading={false} />}</Host>);
+    expect(any()).toBe('false');
+
+    rerender(<Host>{() => <Widget uploading={true} />}</Host>);
+    expect(any()).toBe('true');
+
+    rerender(<Host>{() => <Widget uploading={false} />}</Host>);
+    expect(any()).toBe('false');
+  });
+
+  it('stays true until the LAST widget settles', () => {
+    const view = (a: boolean, b: boolean) => (
+      <Host>
+        {() => (
+          <>
+            <Widget uploading={a} />
+            <Widget uploading={b} />
+          </>
+        )}
+      </Host>
+    );
+    const { rerender } = render(view(true, true));
+    expect(any()).toBe('true');
+    rerender(view(false, true));
+    expect(any()).toBe('true');
+    rerender(view(false, false));
+    expect(any()).toBe('false');
+  });
+
+  it('releases a widget that UNMOUNTS mid-upload', () => {
+    const view = (mounted: boolean) => (
+      <Host>{() => (mounted ? <Widget uploading={true} /> : null)}</Host>
+    );
+    const { rerender } = render(view(true));
+    expect(any()).toBe('true');
+    // A collapsing section / switched tab / deleted subform row. Without the
+    // release the id would stay in the set forever and Save would never come
+    // back.
+    rerender(view(false));
+    expect(any()).toBe('false');
+  });
+
+  it('keeps feeding the per-widget `onUploadingChange` prop unchanged', () => {
+    const cb = vi.fn();
+    const { rerender } = render(<Host>{() => <Widget uploading={false} onUploadingChange={cb} />}</Host>);
+    expect(cb).toHaveBeenLastCalledWith(false);
+    rerender(<Host>{() => <Widget uploading={true} onUploadingChange={cb} />}</Host>);
+    expect(cb).toHaveBeenLastCalledWith(true);
+    expect(any()).toBe('true');
+  });
+
+  it('CHAINS to an outer scope instead of shadowing it', () => {
+    // The master-detail shape: an outer host whose Save writes parent AND
+    // children, with an inner host (a row editor) owning a scope of its own.
+    // If the inner scope shadowed, the outer Save would refuse nothing while a
+    // child's attachment was still in flight — and would look gated while doing
+    // it, which is worse than no gate at all.
+    function Inner({ uploading }: { uploading: boolean }) {
+      const scope = useUploadingScope();
+      return (
+        <>
+          <span data-testid="inner-any">{String(scope.anyUploading)}</span>
+          <UploadingScopeProvider scope={scope}>
+            <Widget uploading={uploading} />
+          </UploadingScopeProvider>
+        </>
+      );
+    }
+    const view = (u: boolean) => <Host>{() => <Inner uploading={u} />}</Host>;
+    const { rerender } = render(view(false));
+    expect(any()).toBe('false');
+    expect(screen.getByTestId('inner-any').textContent).toBe('false');
+
+    rerender(view(true));
+    expect(screen.getByTestId('inner-any').textContent).toBe('true');
+    // The load-bearing half: the OUTER host sees the inner form's upload.
+    expect(any()).toBe('true');
+
+    rerender(view(false));
+    expect(any()).toBe('false');
+  });
+
+  it('releases the outer scope when a whole inner scope unmounts mid-upload', () => {
+    function Inner() {
+      const scope = useUploadingScope();
+      return (
+        <UploadingScopeProvider scope={scope}>
+          <Widget uploading={true} />
+        </UploadingScopeProvider>
+      );
+    }
+    const view = (mounted: boolean) => <Host>{() => (mounted ? <Inner /> : null)}</Host>;
+    const { rerender } = render(view(true));
+    expect(any()).toBe('true');
+    // A closed row editor must not wedge the parent's Save.
+    rerender(view(false));
+    expect(any()).toBe('false');
+  });
+
+  it('is inert for a widget with no provider above it', () => {
+    // Every host on `main` before this change — the hook must not throw and
+    // must not require a scope.
+    expect(() => render(<Widget uploading={true} />)).not.toThrow();
+  });
+
+  it('does not re-render the host when an idle widget reports no change', () => {
+    const renders: boolean[] = [];
+    const { rerender } = render(
+      <Host onAny={(a) => renders.push(a)}>{() => <Widget uploading={false} />}</Host>,
+    );
+    const before = renders.length;
+    act(() => {
+      rerender(<Host onAny={(a) => renders.push(a)}>{() => <Widget uploading={false} />}</Host>);
+    });
+    // One render for the rerender itself, and no extra render scheduled by a
+    // report that changed nothing.
+    expect(renders.length).toBe(before + 1);
+  });
+});