diff --git a/.changeset/action-param-upload-guard.md b/.changeset/action-param-upload-guard.md new file mode 100644 index 000000000..149dd0dc6 --- /dev/null +++ b/.changeset/action-param-upload-guard.md @@ -0,0 +1,22 @@ +--- +"@object-ui/fields": patch +"@object-ui/app-shell": patch +"@object-ui/i18n": patch +--- + +fix(app-shell): block ActionParamDialog submit while a file/image param is uploading; map spec `autonumber` (ADR-0059 follow-ups) + +Two follow-ups to the shared-field-widget param rendering (ADR-0059): + +- **Upload-in-progress guard.** A `file`/`image` param's value only becomes its + fileId once the presigned upload settles, so confirming mid-upload sent an + empty/stale value. `FileField`/`ImageField` now surface their upload state via + an optional `onUploadingChange` prop (shared `useUploadingSignal` hook, + ignored by other widgets); `ActionParamDialog` wires it for `file`/`image` + params and disables Confirm (label → "Uploading…", new `actionDialog.uploading` + i18n key across all locales) plus blocks submit while any upload is in flight. +- **`autonumber` spelling.** `mapFieldTypeToFormType` now maps the spec + `FieldType` spelling `autonumber` (in addition to the widget-map key + `auto_number`) to the AutoNumber widget, so a spec-typed `autonumber` + field/param no longer falls through to the plain text input — fixes the object + form path as well as action params. diff --git a/content/docs/core/enhanced-actions.mdx b/content/docs/core/enhanced-actions.mdx index 2f83f04df..a498543b3 100644 --- a/content/docs/core/enhanced-actions.mdx +++ b/content/docs/core/enhanced-actions.mdx @@ -167,7 +167,9 @@ gets its real widget, not a text box (ADR-0059): File/image params upload through the ambient `UploadProvider`; lookup/user params query through the surrounding `SchemaRendererContext` data source — no -extra wiring per action. +extra wiring per action. While a file/image upload is in flight the dialog's +**Confirm** button is disabled (labelled "Uploading…"), so a param can't be +submitted before its uploaded fileId is ready. ## Action Chaining diff --git a/docs/adr/0059-action-params-shared-field-widgets.md b/docs/adr/0059-action-params-shared-field-widgets.md index 85b41ca6a..b832a8dda 100644 --- a/docs/adr/0059-action-params-shared-field-widgets.md +++ b/docs/adr/0059-action-params-shared-field-widgets.md @@ -110,3 +110,17 @@ with the shape their route expects, same as record forms. dialog work. - Bundle stays lazy: opening a param dialog loads only the widget chunks its params actually use. + +## Follow-ups (post-merge) + +- **Upload-in-progress guard.** A `file`/`image` param's value only becomes its + fileId once the presigned upload settles, so submitting mid-upload would + send an empty/stale value. The upload widgets now surface their in-progress + state via an optional `onUploadingChange` prop (shared + `useUploadingSignal` hook, ignored by non-upload widgets); the dialog wires + it for `file`/`image` params only and disables Confirm (label → "Uploading…") + and blocks submit while any upload is in flight. +- **`autonumber` spelling.** `@objectstack/spec` spells the type `autonumber` + while the widget-map key is `auto_number`; `mapFieldTypeToFormType` now folds + both so a spec-typed `autonumber` field/param resolves to the AutoNumber + widget instead of the text fallback (fixes the form path too, not just params). diff --git a/packages/app-shell/src/utils/paramToField.ts b/packages/app-shell/src/utils/paramToField.ts index 4c18b04a0..8101abb07 100644 --- a/packages/app-shell/src/utils/paramToField.ts +++ b/packages/app-shell/src/utils/paramToField.ts @@ -25,8 +25,9 @@ const PARAM_TYPE_ALIASES: Record = { checkbox: 'boolean', reference: 'lookup', 'datetime-local': 'datetime', - // Spec `FieldType` spells it `autonumber`; the widget map key is `auto_number`. - autonumber: 'auto_number', + // NOTE: spec's `autonumber` (vs the widget-map key `auto_number`) is folded + // in the shared `mapFieldTypeToFormType`, so `resolveFormWidgetType` already + // handles it — no param-only alias needed here. }; /** diff --git a/packages/app-shell/src/views/ActionParamDialog.tsx b/packages/app-shell/src/views/ActionParamDialog.tsx index 110e14313..86c6237d9 100644 --- a/packages/app-shell/src/views/ActionParamDialog.tsx +++ b/packages/app-shell/src/views/ActionParamDialog.tsx @@ -82,6 +82,11 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp const { t, language } = useObjectTranslation(); const [values, setValues] = useState>({}); const [errors, setErrors] = useState>({}); + // Params whose upload widget (file/image) is mid-upload. Confirm stays + // disabled while any is in flight so a param can't be submitted before its + // fileId resolves (the value is only the fileId once the upload settles). + const [uploading, setUploading] = useState>({}); + const anyUploading = Object.values(uploading).some(Boolean); // A param may carry a `visible` predicate (CEL) gating it on the same scope as // action visibility (features / user / app / data) — e.g. `create_user`'s @@ -102,6 +107,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp } setValues(defaults); setErrors({}); + setUploading({}); } }, [state.open, visibleParams]); @@ -115,6 +121,9 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp }; const handleSubmit = () => { + // An upload is still in flight — the param value isn't its fileId yet, so + // block the submit (Confirm is also disabled; this guards keyboard submit). + if (anyUploading) return; // Validate required fields const newErrors: Record = {}; for (const param of visibleParams) { @@ -162,6 +171,12 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp }; const field = paramToField(param); const Widget = getLazyFieldWidget(field.type); + // Only upload widgets emit upload-in-progress; wiring the callback + // to non-upload widgets would spread an unknown prop toward the DOM. + const isUploadWidget = field.type === 'file' || field.type === 'image'; + const uploadProps = isUploadWidget + ? { onUploadingChange: (u: boolean) => setUploading((prev) => ({ ...prev, [param.name]: u })) } + : {}; // A lookup-typed param that fell back to text (no referenceTo) // keeps the "paste an ID" placeholder/help hints. const isLookupParam = param.type === 'lookup' || param.type === 'reference'; @@ -215,6 +230,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp onChange={(v: unknown) => updateValue(param.name, v)} field={field} className={errors[param.name] ? 'border-destructive' : ''} + {...uploadProps} /> @@ -236,7 +252,9 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp - + diff --git a/packages/app-shell/src/views/ActionParamDialog.uploading.test.tsx b/packages/app-shell/src/views/ActionParamDialog.uploading.test.tsx new file mode 100644 index 000000000..1ed770829 --- /dev/null +++ b/packages/app-shell/src/views/ActionParamDialog.uploading.test.tsx @@ -0,0 +1,101 @@ +/** + * 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. + */ + +/** + * ActionParamDialog — Confirm is disabled while a file/image param's upload is + * in flight, so a param can't be submitted before its fileId resolves (the + * value only becomes the fileId once the presigned upload settles). The upload + * widget is stubbed here so the mid-upload window is deterministic; the real + * signal path (FileField/ImageField → useUploadingSignal → onUploadingChange) + * is covered in the fields package. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type { ActionParamDef } from '@object-ui/core'; + +vi.mock('@object-ui/fields', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + // Stub only the upload widgets with a control that drives onUploadingChange; + // every other type resolves to its real widget so paramToField resolution + // and the rest of the dialog stay authentic. + getLazyFieldWidget: (type: string) => { + if (type === 'file' || type === 'image') { + return function FakeUploadWidget({ + onChange, + onUploadingChange, + }: { + onChange: (v: unknown) => void; + onUploadingChange?: (u: boolean) => void; + }) { + return ( +
+ + +
+ ); + }; + } + return actual.getLazyFieldWidget(type); + }, + }; +}); + +// Import AFTER the mock is registered. +const { ActionParamDialog } = await import('./ActionParamDialog'); + +function openDialog(params: ActionParamDef[]) { + const resolve = vi.fn(); + render( {}} />); + return resolve; +} + +const confirmBtn = () => screen.getByText(/actionDialog\.(confirm|uploading)/).closest('button')!; + +describe('ActionParamDialog — upload-in-progress guard', () => { + it('disables Confirm while a file param is uploading and re-enables when it settles', async () => { + const resolve = openDialog([{ name: 'doc', label: 'Doc', type: 'file' }]); + await screen.findByTestId('start-upload'); + + // Idle → enabled. + expect(confirmBtn()).not.toBeDisabled(); + + // Upload starts → Confirm disabled, label switches. + fireEvent.click(screen.getByTestId('start-upload')); + expect(confirmBtn()).toBeDisabled(); + expect(screen.getByText('actionDialog.uploading')).toBeTruthy(); + + // Upload settles → Confirm enabled, value captured. + fireEvent.click(screen.getByTestId('finish-upload')); + expect(confirmBtn()).not.toBeDisabled(); + + fireEvent.click(confirmBtn()); + expect(resolve).toHaveBeenCalledWith({ doc: 'file_123' }); + }); + + it('does not submit while an upload is in flight (keyboard-submit guard)', async () => { + const resolve = openDialog([{ name: 'doc', label: 'Doc', type: 'file' }]); + await screen.findByTestId('start-upload'); + fireEvent.click(screen.getByTestId('start-upload')); + // Even a direct click on the (disabled) button must not resolve. + fireEvent.click(confirmBtn()); + expect(resolve).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/fields/src/field-type-coverage.test.ts b/packages/fields/src/field-type-coverage.test.ts index 16286a140..5c6fcc507 100644 --- a/packages/fields/src/field-type-coverage.test.ts +++ b/packages/fields/src/field-type-coverage.test.ts @@ -27,7 +27,9 @@ const FORM_WIDGET_TYPES = [ 'file', 'image', 'avatar', 'video', 'audio', 'signature', 'location', 'geolocation', 'address', 'color', 'code', 'json', 'qrcode', 'vector', 'object', 'composite', 'record', 'repeater', - 'formula', 'summary', 'auto_number', + // `autonumber` is the spec `FieldType` spelling; `auto_number` is the widget + // map key. Both must resolve to the AutoNumber widget, not text. + 'formula', 'summary', 'auto_number', 'autonumber', ]; /** diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index a32f8fd31..51a281485 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -1907,6 +1907,10 @@ export function mapFieldTypeToFormType(fieldType: string): string { // Auto-generated/computed fields (typically read-only) formula: 'field:formula', summary: 'field:summary', + // `@objectstack/spec` spells this `autonumber`; the widget map key is + // `auto_number`. Map both spellings so a spec-typed field/param doesn't + // fall through to the plain text input. + autonumber: 'field:auto_number', auto_number: 'field:auto_number', }; diff --git a/packages/fields/src/widgets/FileField.tsx b/packages/fields/src/widgets/FileField.tsx index 2eff69896..e3ced74bd 100644 --- a/packages/fields/src/widgets/FileField.tsx +++ b/packages/fields/src/widgets/FileField.tsx @@ -3,6 +3,7 @@ import { Button, EmptyValue } from '@object-ui/components'; import { useUpload } from '@object-ui/providers'; import { Upload, X, File as FileIcon, ImageIcon, Camera, Loader2 } from 'lucide-react'; import { FieldWidgetProps } from './types'; +import { useUploadingSignal } from './useUploadingSignal'; /** * Shared upload pipeline for the file widgets: validates size, uploads through @@ -84,7 +85,7 @@ function useFileUploads(opts: { * Supports single and multiple file uploads with configurable accepted file types. * L2: File size validation, per-file progress indicators, error messages. */ -export function FileField({ value, onChange, field, readonly, ...props }: FieldWidgetProps) { +export function FileField({ value, onChange, field, readonly, onUploadingChange, ...props }: FieldWidgetProps) { const inputRef = useRef(null); const cameraRef = useRef(null); const fileField = (field || (props as any).schema) as any; @@ -111,6 +112,7 @@ export function FileField({ value, onChange, field, readonly, ...props }: FieldW const { processFiles, errors, uploading, uploadProgress } = useFileUploads({ files, multiple, maxSize, onChange, }); + useUploadingSignal(uploading, onUploadingChange); const handleDragOver = useCallback((e: React.DragEvent) => { e.preventDefault(); diff --git a/packages/fields/src/widgets/ImageField.tsx b/packages/fields/src/widgets/ImageField.tsx index df9aaafde..c73d33f5e 100644 --- a/packages/fields/src/widgets/ImageField.tsx +++ b/packages/fields/src/widgets/ImageField.tsx @@ -3,6 +3,7 @@ import { Button, EmptyValue } from '@object-ui/components'; import { useUpload } from '@object-ui/providers'; import { Upload, X, Image as ImageIcon, Crop as CropIcon, Loader2 } from 'lucide-react'; import { FieldWidgetProps } from './types'; +import { useUploadingSignal } from './useUploadingSignal'; // Lazy-load the cropper so the dialog (canvas + crop logic) is not in the initial // ImageField bundle. Consumers that never crop pay zero cost. @@ -14,7 +15,7 @@ const ImageCropperDialog = lazy(() => * ImageField - Image upload widget with preview thumbnails * Supports single and multiple image uploads with drag-and-drop and preview display */ -export function ImageField({ value, onChange, field, readonly, ...props }: FieldWidgetProps) { +export function ImageField({ value, onChange, field, readonly, onUploadingChange, ...props }: FieldWidgetProps) { const inputRef = useRef(null); const imageField = (field || (props as any).schema) as any; const multiple = imageField?.multiple || false; @@ -26,6 +27,7 @@ export function ImageField({ value, onChange, field, readonly, ...props }: Field const [cropTarget, setCropTarget] = useState<{ index: number; src: string; name: string } | null>(null); const { upload } = useUpload(); const [uploading, setUploading] = useState(false); + useUploadingSignal(uploading, onUploadingChange); if (readonly) { if (!value) return ; diff --git a/packages/fields/src/widgets/types.ts b/packages/fields/src/widgets/types.ts index c8c886464..c457591df 100644 --- a/packages/fields/src/widgets/types.ts +++ b/packages/fields/src/widgets/types.ts @@ -9,5 +9,11 @@ export type FieldWidgetProps = { disabled?: boolean; className?: string; errorMessage?: string; + /** + * Upload widgets (`file`/`image`) fire this when their in-progress state + * flips, so a host can block submit until a presigned upload settles. Other + * widgets ignore it. + */ + onUploadingChange?: (uploading: boolean) => void; [key: string]: any; } diff --git a/packages/fields/src/widgets/useUploadingSignal.test.tsx b/packages/fields/src/widgets/useUploadingSignal.test.tsx new file mode 100644 index 000000000..fff0c5280 --- /dev/null +++ b/packages/fields/src/widgets/useUploadingSignal.test.tsx @@ -0,0 +1,62 @@ +import { describe, it, expect, vi } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useUploadingSignal } from './useUploadingSignal'; + +/** + * useUploadingSignal — surfaces an upload widget's in-progress flips to a host + * (ActionParamDialog blocks Confirm while a file/image param uploads). It must + * fire only when `uploading` changes and always call the latest callback. + */ +describe('useUploadingSignal', () => { + it('fires with the initial value on mount', () => { + const cb = vi.fn(); + renderHook(() => useUploadingSignal(false, cb)); + expect(cb).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenLastCalledWith(false); + }); + + it('fires again only when `uploading` changes', () => { + const cb = vi.fn(); + const { rerender } = renderHook(({ u }) => useUploadingSignal(u, cb), { + initialProps: { u: false }, + }); + cb.mockClear(); + + rerender({ u: true }); + expect(cb).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenLastCalledWith(true); + + // Same value → no extra fire. + rerender({ u: true }); + expect(cb).toHaveBeenCalledTimes(1); + + rerender({ u: false }); + expect(cb).toHaveBeenCalledTimes(2); + expect(cb).toHaveBeenLastCalledWith(false); + }); + + it('always invokes the latest callback (no stale closure)', () => { + const first = vi.fn(); + const second = vi.fn(); + const { rerender } = renderHook( + ({ cb, u }) => useUploadingSignal(u, cb), + { initialProps: { cb: first, u: false } }, + ); + // Swap the callback without changing `uploading` → no fire yet. + rerender({ cb: second, u: false }); + expect(second).not.toHaveBeenCalled(); + // Now flip `uploading` → the NEW callback fires, not the old one. + rerender({ cb: second, u: true }); + expect(second).toHaveBeenCalledWith(true); + expect(first).not.toHaveBeenCalledWith(true); + }); + + it('is a no-op when no callback is provided', () => { + expect(() => { + const { rerender } = renderHook(({ u }) => useUploadingSignal(u), { + initialProps: { u: false }, + }); + rerender({ u: true }); + }).not.toThrow(); + }); +}); diff --git a/packages/fields/src/widgets/useUploadingSignal.ts b/packages/fields/src/widgets/useUploadingSignal.ts new file mode 100644 index 000000000..098393d38 --- /dev/null +++ b/packages/fields/src/widgets/useUploadingSignal.ts @@ -0,0 +1,20 @@ +import { useEffect, useRef } from 'react'; + +/** + * Notify a parent whenever an upload widget's in-progress state flips. Used by + * hosts that must block a submit until the presigned upload resolves — e.g. + * `ActionParamDialog` disables Confirm so a `file`/`image` param can't be + * submitted mid-upload (the value only becomes the fileId once the upload + * settles). Fires with the latest callback and only when `uploading` actually + * changes, so an inline arrow prop doesn't thrash the parent every render. + */ +export function useUploadingSignal( + uploading: boolean, + onUploadingChange?: (uploading: boolean) => void, +): void { + const ref = useRef(onUploadingChange); + ref.current = onUploadingChange; + useEffect(() => { + ref.current?.(uploading); + }, [uploading]); +} diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 30c411276..6659c06de 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -1492,6 +1492,7 @@ const ar = { requiredError: "{{label}} مطلوب", cancel: "إلغاء", confirm: "تأكيد", + uploading: "جارٍ الرفع…", defaultActionTitle: "إجراء", ok: "موافق", lookupPlaceholder: "لصق معرف السجل (UUID) لـ {{label}}", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 0faf2a127..bd14c0557 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -1494,6 +1494,7 @@ const de = { requiredError: "{{label}} ist erforderlich", cancel: "Abbrechen", confirm: "Bestätigen", + uploading: "Wird hochgeladen…", defaultActionTitle: "Aktion", ok: "OK", lookupPlaceholder: "Datensatz-ID (UUID) für {{label}} einfügen", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 6bc4271ea..ec98c2b23 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -2131,6 +2131,7 @@ const en = { lookupHelpText: 'Enter the record id of the referenced object. A picker is coming soon.', cancel: 'Cancel', confirm: 'Confirm', + uploading: 'Uploading…', defaultActionTitle: 'Action', ok: 'OK', }, diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index d4e1c6549..081e812e0 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -1494,6 +1494,7 @@ const es = { requiredError: "{{label}} es obligatorio", cancel: "Cancelar", confirm: "Confirmar", + uploading: "Subiendo…", defaultActionTitle: "Acción", ok: "Aceptar", lookupPlaceholder: "Pegar ID de registro (UUID) para {{label}}", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index a2ec84b49..4245d9c13 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -1492,6 +1492,7 @@ const fr = { requiredError: "{{label}} est obligatoire", cancel: "Annuler", confirm: "Confirmer", + uploading: "Téléversement…", defaultActionTitle: "Action", ok: "OK", lookupPlaceholder: "Coller l'ID d'enregistrement (UUID) pour {{label}}", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 150544d8b..1eb1374a8 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -1494,6 +1494,7 @@ const ja = { requiredError: "{{label}} は必須です", cancel: "キャンセル", confirm: "確認", + uploading: "アップロード中…", defaultActionTitle: "アクション", ok: "OK", lookupPlaceholder: "{{label}} のレコードID(UUID)を貼り付け", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index a81c618aa..798c793e1 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -1494,6 +1494,7 @@ const ko = { requiredError: "{{label}}은(는) 필수입니다", cancel: "취소", confirm: "확인", + uploading: "업로드 중…", defaultActionTitle: "작업", ok: "확인", lookupPlaceholder: "{{label}}의 레코드 ID(UUID) 붙여넣기", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 608b09227..b1551ff60 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -1492,6 +1492,7 @@ const pt = { requiredError: "{{label}} é obrigatório", cancel: "Cancelar", confirm: "Confirmar", + uploading: "Enviando…", defaultActionTitle: "Ação", ok: "OK", lookupPlaceholder: "Colar ID do registro (UUID) para {{label}}", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 3a8723ecd..27c1a84b7 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -1492,6 +1492,7 @@ const ru = { requiredError: "{{label}} обязательно", cancel: "Отмена", confirm: "Подтвердить", + uploading: "Загрузка…", defaultActionTitle: "Действие", ok: "ОК", lookupPlaceholder: "Вставить ID записи (UUID) для {{label}}", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 4abe38ac2..a16fbc223 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -2211,6 +2211,7 @@ const zh = { lookupHelpText: '请输入引用记录的 ID。可视化选择器即将上线。', cancel: '取消', confirm: '确认', + uploading: '上传中…', defaultActionTitle: '操作', ok: '确定', },