Skip to content

Commit 0318118

Browse files
os-zhuangclaude
andauthored
fix(app-shell): guard ActionParamDialog submit during file upload + map spec autonumber (ADR-0059 follow-ups) (#2707)
Two follow-ups to the shared field-widget param rendering (#2700 / ADR-0059): - Upload-in-progress guard: file/image params only carry their fileId once the presigned upload settles, so confirming mid-upload sent an empty/stale value. FileField/ImageField now surface upload state via an optional onUploadingChange prop (shared useUploadingSignal hook, ignored by other widgets); the dialog wires it for file/image params, disables Confirm (label -> "Uploading…", new actionDialog.uploading key in all 10 locales) and blocks submit while any upload is in flight. - autonumber spelling: mapFieldTypeToFormType now folds the spec FieldType spelling `autonumber` (alongside the widget-map key `auto_number`) to the AutoNumber widget, so a spec-typed autonumber field/param no longer falls through to the text input — fixes the object form path too. The redundant param-side alias is dropped (the shared map now covers it). Tests: useUploadingSignal unit test (fires-on-change, latest-callback, no-op); ActionParamDialog upload-guard test (Confirm disabled/re-enabled, submit blocked) via a stubbed upload widget; field-type-coverage asserts autonumber -> non-text. Docs: ADR-0059 follow-ups section, enhanced-actions params note; changeset (patch). Claude-Session: https://claude.ai/code/session_01Rg7oavKSUdb2KzYEEYrCHX Co-authored-by: Claude <noreply@anthropic.com>
1 parent 94d4876 commit 0318118

23 files changed

Lines changed: 273 additions & 7 deletions
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@object-ui/fields": patch
3+
"@object-ui/app-shell": patch
4+
"@object-ui/i18n": patch
5+
---
6+
7+
fix(app-shell): block ActionParamDialog submit while a file/image param is uploading; map spec `autonumber` (ADR-0059 follow-ups)
8+
9+
Two follow-ups to the shared-field-widget param rendering (ADR-0059):
10+
11+
- **Upload-in-progress guard.** A `file`/`image` param's value only becomes its
12+
fileId once the presigned upload settles, so confirming mid-upload sent an
13+
empty/stale value. `FileField`/`ImageField` now surface their upload state via
14+
an optional `onUploadingChange` prop (shared `useUploadingSignal` hook,
15+
ignored by other widgets); `ActionParamDialog` wires it for `file`/`image`
16+
params and disables Confirm (label → "Uploading…", new `actionDialog.uploading`
17+
i18n key across all locales) plus blocks submit while any upload is in flight.
18+
- **`autonumber` spelling.** `mapFieldTypeToFormType` now maps the spec
19+
`FieldType` spelling `autonumber` (in addition to the widget-map key
20+
`auto_number`) to the AutoNumber widget, so a spec-typed `autonumber`
21+
field/param no longer falls through to the plain text input — fixes the object
22+
form path as well as action params.

content/docs/core/enhanced-actions.mdx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ gets its real widget, not a text box (ADR-0059):
167167

168168
File/image params upload through the ambient `UploadProvider`; lookup/user
169169
params query through the surrounding `SchemaRendererContext` data source — no
170-
extra wiring per action.
170+
extra wiring per action. While a file/image upload is in flight the dialog's
171+
**Confirm** button is disabled (labelled "Uploading…"), so a param can't be
172+
submitted before its uploaded fileId is ready.
171173

172174
## Action Chaining
173175

docs/adr/0059-action-params-shared-field-widgets.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,17 @@ with the shape their route expects, same as record forms.
110110
dialog work.
111111
- Bundle stays lazy: opening a param dialog loads only the widget chunks its
112112
params actually use.
113+
114+
## Follow-ups (post-merge)
115+
116+
- **Upload-in-progress guard.** A `file`/`image` param's value only becomes its
117+
fileId once the presigned upload settles, so submitting mid-upload would
118+
send an empty/stale value. The upload widgets now surface their in-progress
119+
state via an optional `onUploadingChange` prop (shared
120+
`useUploadingSignal` hook, ignored by non-upload widgets); the dialog wires
121+
it for `file`/`image` params only and disables Confirm (label → "Uploading…")
122+
and blocks submit while any upload is in flight.
123+
- **`autonumber` spelling.** `@objectstack/spec` spells the type `autonumber`
124+
while the widget-map key is `auto_number`; `mapFieldTypeToFormType` now folds
125+
both so a spec-typed `autonumber` field/param resolves to the AutoNumber
126+
widget instead of the text fallback (fixes the form path too, not just params).

packages/app-shell/src/utils/paramToField.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ const PARAM_TYPE_ALIASES: Record<string, string> = {
2525
checkbox: 'boolean',
2626
reference: 'lookup',
2727
'datetime-local': 'datetime',
28-
// Spec `FieldType` spells it `autonumber`; the widget map key is `auto_number`.
29-
autonumber: 'auto_number',
28+
// NOTE: spec's `autonumber` (vs the widget-map key `auto_number`) is folded
29+
// in the shared `mapFieldTypeToFormType`, so `resolveFormWidgetType` already
30+
// handles it — no param-only alias needed here.
3031
};
3132

3233
/**

packages/app-shell/src/views/ActionParamDialog.tsx

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
8282
const { t, language } = useObjectTranslation();
8383
const [values, setValues] = useState<Record<string, any>>({});
8484
const [errors, setErrors] = useState<Record<string, boolean>>({});
85+
// Params whose upload widget (file/image) is mid-upload. Confirm stays
86+
// disabled while any is in flight so a param can't be submitted before its
87+
// fileId resolves (the value is only the fileId once the upload settles).
88+
const [uploading, setUploading] = useState<Record<string, boolean>>({});
89+
const anyUploading = Object.values(uploading).some(Boolean);
8590

8691
// A param may carry a `visible` predicate (CEL) gating it on the same scope as
8792
// action visibility (features / user / app / data) — e.g. `create_user`'s
@@ -102,6 +107,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
102107
}
103108
setValues(defaults);
104109
setErrors({});
110+
setUploading({});
105111
}
106112
}, [state.open, visibleParams]);
107113

@@ -115,6 +121,9 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
115121
};
116122

117123
const handleSubmit = () => {
124+
// An upload is still in flight — the param value isn't its fileId yet, so
125+
// block the submit (Confirm is also disabled; this guards keyboard submit).
126+
if (anyUploading) return;
118127
// Validate required fields
119128
const newErrors: Record<string, boolean> = {};
120129
for (const param of visibleParams) {
@@ -162,6 +171,12 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
162171
};
163172
const field = paramToField(param);
164173
const Widget = getLazyFieldWidget(field.type);
174+
// Only upload widgets emit upload-in-progress; wiring the callback
175+
// to non-upload widgets would spread an unknown prop toward the DOM.
176+
const isUploadWidget = field.type === 'file' || field.type === 'image';
177+
const uploadProps = isUploadWidget
178+
? { onUploadingChange: (u: boolean) => setUploading((prev) => ({ ...prev, [param.name]: u })) }
179+
: {};
165180
// A lookup-typed param that fell back to text (no referenceTo)
166181
// keeps the "paste an ID" placeholder/help hints.
167182
const isLookupParam = param.type === 'lookup' || param.type === 'reference';
@@ -215,6 +230,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
215230
onChange={(v: unknown) => updateValue(param.name, v)}
216231
field={field}
217232
className={errors[param.name] ? 'border-destructive' : ''}
233+
{...uploadProps}
218234
/>
219235
</Suspense>
220236

@@ -236,7 +252,9 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
236252

237253
<DialogFooter>
238254
<Button variant="outline" onClick={handleCancel}>{t('actionDialog.cancel')}</Button>
239-
<Button onClick={handleSubmit}>{t('actionDialog.confirm')}</Button>
255+
<Button onClick={handleSubmit} disabled={anyUploading}>
256+
{anyUploading ? t('actionDialog.uploading') : t('actionDialog.confirm')}
257+
</Button>
240258
</DialogFooter>
241259
</DialogContent>
242260
</Dialog>
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* ActionParamDialog — Confirm is disabled while a file/image param's upload is
11+
* in flight, so a param can't be submitted before its fileId resolves (the
12+
* value only becomes the fileId once the presigned upload settles). The upload
13+
* widget is stubbed here so the mid-upload window is deterministic; the real
14+
* signal path (FileField/ImageField → useUploadingSignal → onUploadingChange)
15+
* is covered in the fields package.
16+
*/
17+
import { describe, it, expect, vi } from 'vitest';
18+
import { render, screen, fireEvent } from '@testing-library/react';
19+
import type { ActionParamDef } from '@object-ui/core';
20+
21+
vi.mock('@object-ui/fields', async (importActual) => {
22+
const actual = await importActual<typeof import('@object-ui/fields')>();
23+
return {
24+
...actual,
25+
// Stub only the upload widgets with a control that drives onUploadingChange;
26+
// every other type resolves to its real widget so paramToField resolution
27+
// and the rest of the dialog stay authentic.
28+
getLazyFieldWidget: (type: string) => {
29+
if (type === 'file' || type === 'image') {
30+
return function FakeUploadWidget({
31+
onChange,
32+
onUploadingChange,
33+
}: {
34+
onChange: (v: unknown) => void;
35+
onUploadingChange?: (u: boolean) => void;
36+
}) {
37+
return (
38+
<div>
39+
<button type="button" data-testid="start-upload" onClick={() => onUploadingChange?.(true)}>
40+
start
41+
</button>
42+
<button
43+
type="button"
44+
data-testid="finish-upload"
45+
onClick={() => {
46+
onChange('file_123');
47+
onUploadingChange?.(false);
48+
}}
49+
>
50+
finish
51+
</button>
52+
</div>
53+
);
54+
};
55+
}
56+
return actual.getLazyFieldWidget(type);
57+
},
58+
};
59+
});
60+
61+
// Import AFTER the mock is registered.
62+
const { ActionParamDialog } = await import('./ActionParamDialog');
63+
64+
function openDialog(params: ActionParamDef[]) {
65+
const resolve = vi.fn();
66+
render(<ActionParamDialog state={{ open: true, params, resolve }} onOpenChange={() => {}} />);
67+
return resolve;
68+
}
69+
70+
const confirmBtn = () => screen.getByText(/actionDialog\.(confirm|uploading)/).closest('button')!;
71+
72+
describe('ActionParamDialog — upload-in-progress guard', () => {
73+
it('disables Confirm while a file param is uploading and re-enables when it settles', async () => {
74+
const resolve = openDialog([{ name: 'doc', label: 'Doc', type: 'file' }]);
75+
await screen.findByTestId('start-upload');
76+
77+
// Idle → enabled.
78+
expect(confirmBtn()).not.toBeDisabled();
79+
80+
// Upload starts → Confirm disabled, label switches.
81+
fireEvent.click(screen.getByTestId('start-upload'));
82+
expect(confirmBtn()).toBeDisabled();
83+
expect(screen.getByText('actionDialog.uploading')).toBeTruthy();
84+
85+
// Upload settles → Confirm enabled, value captured.
86+
fireEvent.click(screen.getByTestId('finish-upload'));
87+
expect(confirmBtn()).not.toBeDisabled();
88+
89+
fireEvent.click(confirmBtn());
90+
expect(resolve).toHaveBeenCalledWith({ doc: 'file_123' });
91+
});
92+
93+
it('does not submit while an upload is in flight (keyboard-submit guard)', async () => {
94+
const resolve = openDialog([{ name: 'doc', label: 'Doc', type: 'file' }]);
95+
await screen.findByTestId('start-upload');
96+
fireEvent.click(screen.getByTestId('start-upload'));
97+
// Even a direct click on the (disabled) button must not resolve.
98+
fireEvent.click(confirmBtn());
99+
expect(resolve).not.toHaveBeenCalled();
100+
});
101+
});

packages/fields/src/field-type-coverage.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ const FORM_WIDGET_TYPES = [
2727
'file', 'image', 'avatar', 'video', 'audio', 'signature',
2828
'location', 'geolocation', 'address', 'color', 'code', 'json', 'qrcode', 'vector',
2929
'object', 'composite', 'record', 'repeater',
30-
'formula', 'summary', 'auto_number',
30+
// `autonumber` is the spec `FieldType` spelling; `auto_number` is the widget
31+
// map key. Both must resolve to the AutoNumber widget, not text.
32+
'formula', 'summary', 'auto_number', 'autonumber',
3133
];
3234

3335
/**

packages/fields/src/index.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1907,6 +1907,10 @@ export function mapFieldTypeToFormType(fieldType: string): string {
19071907
// Auto-generated/computed fields (typically read-only)
19081908
formula: 'field:formula',
19091909
summary: 'field:summary',
1910+
// `@objectstack/spec` spells this `autonumber`; the widget map key is
1911+
// `auto_number`. Map both spellings so a spec-typed field/param doesn't
1912+
// fall through to the plain text input.
1913+
autonumber: 'field:auto_number',
19101914
auto_number: 'field:auto_number',
19111915
};
19121916

packages/fields/src/widgets/FileField.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Button, EmptyValue } from '@object-ui/components';
33
import { useUpload } from '@object-ui/providers';
44
import { Upload, X, File as FileIcon, ImageIcon, Camera, Loader2 } from 'lucide-react';
55
import { FieldWidgetProps } from './types';
6+
import { useUploadingSignal } from './useUploadingSignal';
67

78
/**
89
* Shared upload pipeline for the file widgets: validates size, uploads through
@@ -84,7 +85,7 @@ function useFileUploads(opts: {
8485
* Supports single and multiple file uploads with configurable accepted file types.
8586
* L2: File size validation, per-file progress indicators, error messages.
8687
*/
87-
export function FileField({ value, onChange, field, readonly, ...props }: FieldWidgetProps<any>) {
88+
export function FileField({ value, onChange, field, readonly, onUploadingChange, ...props }: FieldWidgetProps<any>) {
8889
const inputRef = useRef<HTMLInputElement>(null);
8990
const cameraRef = useRef<HTMLInputElement>(null);
9091
const fileField = (field || (props as any).schema) as any;
@@ -111,6 +112,7 @@ export function FileField({ value, onChange, field, readonly, ...props }: FieldW
111112
const { processFiles, errors, uploading, uploadProgress } = useFileUploads({
112113
files, multiple, maxSize, onChange,
113114
});
115+
useUploadingSignal(uploading, onUploadingChange);
114116

115117
const handleDragOver = useCallback((e: React.DragEvent) => {
116118
e.preventDefault();

packages/fields/src/widgets/ImageField.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Button, EmptyValue } from '@object-ui/components';
33
import { useUpload } from '@object-ui/providers';
44
import { Upload, X, Image as ImageIcon, Crop as CropIcon, Loader2 } from 'lucide-react';
55
import { FieldWidgetProps } from './types';
6+
import { useUploadingSignal } from './useUploadingSignal';
67

78
// Lazy-load the cropper so the dialog (canvas + crop logic) is not in the initial
89
// ImageField bundle. Consumers that never crop pay zero cost.
@@ -14,7 +15,7 @@ const ImageCropperDialog = lazy(() =>
1415
* ImageField - Image upload widget with preview thumbnails
1516
* Supports single and multiple image uploads with drag-and-drop and preview display
1617
*/
17-
export function ImageField({ value, onChange, field, readonly, ...props }: FieldWidgetProps<any>) {
18+
export function ImageField({ value, onChange, field, readonly, onUploadingChange, ...props }: FieldWidgetProps<any>) {
1819
const inputRef = useRef<HTMLInputElement>(null);
1920
const imageField = (field || (props as any).schema) as any;
2021
const multiple = imageField?.multiple || false;
@@ -26,6 +27,7 @@ export function ImageField({ value, onChange, field, readonly, ...props }: Field
2627
const [cropTarget, setCropTarget] = useState<{ index: number; src: string; name: string } | null>(null);
2728
const { upload } = useUpload();
2829
const [uploading, setUploading] = useState(false);
30+
useUploadingSignal(uploading, onUploadingChange);
2931

3032
if (readonly) {
3133
if (!value) return <EmptyValue />;

0 commit comments

Comments
 (0)