fix(12.2): close code-review blockers on uploads, JSON caps, and pivot fill
Keep form save behind in-flight uploads, make retries idempotent via X-Upload-Id, cap remaining JSON bodies, and surface pending pivot type errors instead of zeroing them. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -37,9 +37,12 @@ export interface UploadHandle {
|
||||
export type UploadProgress = (loaded: number, total: number) => void
|
||||
|
||||
/** Everything a fileupload control does with its files. */
|
||||
/** Client upload id sent as X-Upload-Id so a retry returns the stored file. */
|
||||
export const UPLOAD_ID_HEADER = 'X-Upload-Id'
|
||||
|
||||
export interface FileRoutes {
|
||||
list(): Promise<FileCallResult<FileItem[]>>
|
||||
upload(file: File, onProgress?: UploadProgress): UploadHandle
|
||||
upload(file: File, onProgress?: UploadProgress, uploadId?: string): UploadHandle
|
||||
update(file: number, body: AdminFileCaptionRequest): Promise<FileCallResult<FileItem>>
|
||||
remove(file: number): Promise<FileCallResult<FileMutationResult>>
|
||||
reorder(ids: number[]): Promise<FileCallResult<FileItem[]>>
|
||||
@@ -137,6 +140,7 @@ export function uploadWithProgress(
|
||||
file: File,
|
||||
headers: Record<string, string>,
|
||||
onProgress?: UploadProgress,
|
||||
uploadId?: string,
|
||||
): UploadHandle {
|
||||
let current: XMLHttpRequest | null = null
|
||||
let aborted = false
|
||||
@@ -151,6 +155,9 @@ export function uploadWithProgress(
|
||||
for (const [name, value] of Object.entries(headers)) {
|
||||
request.setRequestHeader(name, value)
|
||||
}
|
||||
if (uploadId) {
|
||||
request.setRequestHeader(UPLOAD_ID_HEADER, uploadId)
|
||||
}
|
||||
request.upload.onprogress = (event: ProgressEvent) => {
|
||||
if (event.lengthComputable) {
|
||||
onProgress?.(event.loaded, event.total)
|
||||
@@ -208,7 +215,7 @@ export function parentFileRoutes(
|
||||
const url = `${controllerUrl(source)}/${segment(recordId)}/files/${segment(field)}`
|
||||
return {
|
||||
list: () => settle(() => api.GET('/{vendor}/{plugin}/{controller}/{id}/files/{field}', { params: { path, header } })),
|
||||
upload: (file, onProgress) => uploadWithProgress(url, file, { ...header }, onProgress),
|
||||
upload: (file, onProgress, uploadId) => uploadWithProgress(url, file, { ...header }, onProgress, uploadId),
|
||||
update: (file, body) =>
|
||||
settle(() =>
|
||||
api.PUT('/{vendor}/{plugin}/{controller}/{id}/files/{field}/{file}', {
|
||||
@@ -273,7 +280,7 @@ export function childFileRoutes(
|
||||
params: { path, header },
|
||||
}),
|
||||
),
|
||||
upload: (file, onProgress) => uploadWithProgress(url, file, { ...header }, onProgress),
|
||||
upload: (file, onProgress, uploadId) => uploadWithProgress(url, file, { ...header }, onProgress, uploadId),
|
||||
update: (file, body) =>
|
||||
settle(() =>
|
||||
api.PUT('/{vendor}/{plugin}/{controller}/{id}/relations/{name}/records/{child}/files/{field}/{file}', {
|
||||
|
||||
@@ -287,7 +287,13 @@ export function boundValue(
|
||||
return day
|
||||
}
|
||||
const wall = new CalendarDateTime(day.year, day.month, day.day, end ? 23 : 0, end ? 59 : 0, end ? 59 : 0)
|
||||
return ignoreTimezone ? wall : toZoned(wall, getLocalTimeZone())
|
||||
if (ignoreTimezone) {
|
||||
return wall
|
||||
}
|
||||
// The server checks the saved instant's UTC calendar date. Build the
|
||||
// bound as a UTC instant so a local midnight just after the UTC day
|
||||
// change is refused, and a local evening still on that UTC day is allowed.
|
||||
return toTimeZone(toZoned(wall, 'UTC'), getLocalTimeZone())
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -55,6 +55,8 @@ interface Item {
|
||||
file: FileItem | null
|
||||
source: File | null
|
||||
handle: UploadHandle | null
|
||||
/** Stable client id sent on every attempt of this queued file. */
|
||||
uploadId: string
|
||||
}
|
||||
|
||||
let nextUid = 1
|
||||
@@ -78,6 +80,9 @@ const transient: string[] = []
|
||||
const routes: FileRoutes | null = session ? session.routes(props.field.name) : null
|
||||
let reorderTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let orderBefore: number[] | null = null
|
||||
let orderInFlight = false
|
||||
let pendingOrder: number[] | null = null
|
||||
let orderGen = 0
|
||||
let unmounted = false
|
||||
|
||||
const readOnly = computed(() => {
|
||||
@@ -240,9 +245,14 @@ function itemFor(file: FileItem): Item {
|
||||
file,
|
||||
source: null,
|
||||
handle: null,
|
||||
uploadId: '',
|
||||
}) as Item
|
||||
}
|
||||
|
||||
function newUploadId(): string {
|
||||
return globalThis.crypto?.randomUUID?.() ?? `u${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`
|
||||
}
|
||||
|
||||
function localItem(source: File, error: string): Item {
|
||||
return reactive<Item>({
|
||||
uid: nextUid++,
|
||||
@@ -255,6 +265,7 @@ function localItem(source: File, error: string): Item {
|
||||
file: null,
|
||||
source: markRaw(source),
|
||||
handle: null,
|
||||
uploadId: newUploadId(),
|
||||
}) as Item
|
||||
}
|
||||
|
||||
@@ -380,48 +391,86 @@ async function upload(item: Item): Promise<void> {
|
||||
item.progress = 0
|
||||
item.error = ''
|
||||
item.retryable = false
|
||||
const handle = routes.upload(item.source, (loaded, total) => {
|
||||
item.progress = total > 0 ? Math.min(100, Math.round((loaded / total) * 100)) : 0
|
||||
})
|
||||
item.handle = markRaw(handle)
|
||||
const result = await handle.promise
|
||||
item.handle = null
|
||||
const kept = !!find(item.uid)
|
||||
if (result.ok) {
|
||||
if (!kept || unmounted) {
|
||||
// Cancelled after the server stored it: drop the pending upload.
|
||||
void routes.remove(result.item.id)
|
||||
} else {
|
||||
item.state = 'done'
|
||||
item.progress = 100
|
||||
item.file = result.item
|
||||
item.name = result.item.file_name
|
||||
item.size = result.item.file_size
|
||||
session?.markDirty()
|
||||
}
|
||||
} else if (kept && result.reason !== 'aborted') {
|
||||
item.state = 'failed'
|
||||
switch (result.reason) {
|
||||
case 'too_large':
|
||||
item.error = maxSizeLabel.value
|
||||
? t('backend::lang.fileupload.too_large', { name: item.name, size: maxSizeLabel.value })
|
||||
: result.message || t('backend::lang.fileupload.upload_failed')
|
||||
break
|
||||
case 'invalid':
|
||||
item.error = result.message || t('backend::lang.fileupload.upload_failed')
|
||||
break
|
||||
case 'network':
|
||||
const known = new Set(items.value.filter((entry) => entry.file).map((entry) => entry.file!.id))
|
||||
const release = session?.beginUpload()
|
||||
try {
|
||||
const handle = routes.upload(
|
||||
item.source,
|
||||
(loaded, total) => {
|
||||
item.progress = total > 0 ? Math.min(100, Math.round((loaded / total) * 100)) : 0
|
||||
},
|
||||
item.uploadId || undefined,
|
||||
)
|
||||
item.handle = markRaw(handle)
|
||||
const result = await handle.promise
|
||||
item.handle = null
|
||||
const kept = !!find(item.uid)
|
||||
if (result.ok) {
|
||||
if (!kept || unmounted) {
|
||||
// Cancelled after the server stored it: drop the pending upload.
|
||||
void routes.remove(result.item.id)
|
||||
} else {
|
||||
item.state = 'done'
|
||||
item.progress = 100
|
||||
item.file = result.item
|
||||
item.name = result.item.file_name
|
||||
item.size = result.item.file_size
|
||||
session?.markDirty()
|
||||
}
|
||||
} else if (result.reason === 'aborted' || result.reason === 'network') {
|
||||
const extras = await lostUploads(known)
|
||||
if (result.reason === 'aborted') {
|
||||
for (const extra of extras) {
|
||||
void routes.remove(extra.id)
|
||||
}
|
||||
items.value = items.value.filter((entry) => entry.uid !== item.uid)
|
||||
forgetThumb(item.uid)
|
||||
} else if (kept && extras[0] && extras.length === 1) {
|
||||
item.state = 'done'
|
||||
item.progress = 100
|
||||
item.file = extras[0]
|
||||
item.name = extras[0].file_name
|
||||
item.size = extras[0].file_size
|
||||
session?.markDirty()
|
||||
} else {
|
||||
item.state = 'failed'
|
||||
item.error = t('backend::lang.fileupload.upload_failed')
|
||||
item.retryable = true
|
||||
break
|
||||
default:
|
||||
item.error = result.message || t('backend::lang.fileupload.upload_failed')
|
||||
item.retryable = result.status >= 500
|
||||
}
|
||||
} else if (kept) {
|
||||
item.state = 'failed'
|
||||
switch (result.reason) {
|
||||
case 'too_large':
|
||||
item.error = maxSizeLabel.value
|
||||
? t('backend::lang.fileupload.too_large', { name: item.name, size: maxSizeLabel.value })
|
||||
: result.message || t('backend::lang.fileupload.upload_failed')
|
||||
break
|
||||
case 'invalid':
|
||||
item.error = result.message || t('backend::lang.fileupload.upload_failed')
|
||||
break
|
||||
default:
|
||||
item.error = result.message || t('backend::lang.fileupload.upload_failed')
|
||||
item.retryable = result.status >= 500
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
release?.()
|
||||
if (!unmounted) {
|
||||
pump()
|
||||
}
|
||||
}
|
||||
if (!unmounted) {
|
||||
pump()
|
||||
}
|
||||
|
||||
/** Pending files that appeared after this upload started (response lost). */
|
||||
async function lostUploads(known: Set<number>): Promise<FileItem[]> {
|
||||
if (!routes) {
|
||||
return []
|
||||
}
|
||||
const listed = await routes.list()
|
||||
if (!listed.data) {
|
||||
return []
|
||||
}
|
||||
return listed.data.filter((file) => file.pending && !known.has(file.id))
|
||||
}
|
||||
|
||||
function retry(item: Item): void {
|
||||
@@ -488,19 +537,40 @@ function scheduleReorder(): void {
|
||||
}, REORDER_DEBOUNCE)
|
||||
}
|
||||
|
||||
function currentFileIds(): number[] {
|
||||
return items.value.filter((item) => item.state === 'done' && item.file).map((item) => item.file!.id)
|
||||
}
|
||||
|
||||
async function sendOrder(): Promise<void> {
|
||||
const before = orderBefore
|
||||
orderBefore = null
|
||||
if (!routes) {
|
||||
return
|
||||
}
|
||||
const ids = items.value.filter((item) => item.state === 'done' && item.file).map((item) => item.file!.id)
|
||||
const result = await routes.reorder(ids)
|
||||
if (!result.data && !unmounted) {
|
||||
if (before) {
|
||||
restore(before)
|
||||
const ids = currentFileIds()
|
||||
if (orderInFlight) {
|
||||
pendingOrder = ids
|
||||
return
|
||||
}
|
||||
const before = orderBefore
|
||||
orderBefore = null
|
||||
const gen = ++orderGen
|
||||
orderInFlight = true
|
||||
try {
|
||||
const result = await routes.reorder(ids)
|
||||
if (pendingOrder) {
|
||||
return
|
||||
}
|
||||
if (!result.data && !unmounted && gen === orderGen) {
|
||||
if (before) {
|
||||
restore(before)
|
||||
}
|
||||
showToast(t('backend::lang.fileupload.reorder_failed'), 'danger')
|
||||
}
|
||||
} finally {
|
||||
orderInFlight = false
|
||||
if (pendingOrder) {
|
||||
pendingOrder = null
|
||||
void sendOrder()
|
||||
}
|
||||
showToast(t('backend::lang.fileupload.reorder_failed'), 'danger')
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
// writes action results back through patch, which marks the form dirty and
|
||||
// clears that field's errors exactly like typing into it. A form without a
|
||||
// provider (settings pages) gets the defaults of the injecting control.
|
||||
import type { InjectionKey, Ref } from 'vue'
|
||||
import { ref, type InjectionKey, type Ref } from 'vue'
|
||||
import type { FileRoutes } from '../../api/files'
|
||||
import type { AdminRecord } from '../../api/types'
|
||||
|
||||
@@ -46,6 +46,35 @@ export interface FormSession {
|
||||
pendingChanges: Readonly<Ref<number>>
|
||||
/** Goes up after every successful save, so fields can reload their files. */
|
||||
revision: Readonly<Ref<number>>
|
||||
/**
|
||||
* Registers an in-flight upload. Call the returned function in `finally`
|
||||
* when the request reaches a terminal state. The form will not submit
|
||||
* while any upload is still open.
|
||||
*/
|
||||
beginUpload(): () => void
|
||||
/** How many uploads have not yet reached a terminal state. */
|
||||
activeUploads: Readonly<Ref<number>>
|
||||
}
|
||||
|
||||
/** Counter and register/release pair used by record and child forms. */
|
||||
export function createUploadGate(): { activeUploads: Ref<number>; beginUpload: () => () => void } {
|
||||
const activeUploads = ref(0)
|
||||
return {
|
||||
activeUploads,
|
||||
beginUpload() {
|
||||
activeUploads.value++
|
||||
let released = false
|
||||
return () => {
|
||||
if (released) {
|
||||
return
|
||||
}
|
||||
released = true
|
||||
if (activeUploads.value > 0) {
|
||||
activeUploads.value--
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
export const FORM_SESSION: InjectionKey<FormSession> = Symbol('summer.form.session')
|
||||
|
||||
@@ -17,7 +17,7 @@ import { CHILD_SESSION_HEADER, SESSION_HEADER, newSessionKey } from '../../app/s
|
||||
import FormErrorBanner from '../form/FormErrorBanner.vue'
|
||||
import FormGrid from '../form/FormGrid.vue'
|
||||
import FormTabs from '../form/FormTabs.vue'
|
||||
import { FORM_PATCH, FORM_SESSION, FORM_VALUES, type FormSession } from '../form/formContext'
|
||||
import { FORM_PATCH, FORM_SESSION, FORM_VALUES, createUploadGate, type FormSession } from '../form/formContext'
|
||||
import {
|
||||
DEFAULT_TAB,
|
||||
contextAllows,
|
||||
@@ -79,6 +79,7 @@ const activeTab = ref(DEFAULT_TAB)
|
||||
const childKey = ref(newSessionKey())
|
||||
const pendingChanges = ref(0)
|
||||
const revision = ref(0)
|
||||
const { activeUploads, beginUpload } = createUploadGate()
|
||||
const body = ref<HTMLElement | null>(null)
|
||||
const confirm = useConfirm()
|
||||
let generation = 0
|
||||
@@ -148,7 +149,9 @@ const dirty = computed(
|
||||
!preview.value &&
|
||||
!loading.value &&
|
||||
!failed.value &&
|
||||
(pendingChanges.value > 0 || snapshot(editablePayload(fields.value, values.value)) !== saved.value),
|
||||
(pendingChanges.value > 0 ||
|
||||
activeUploads.value > 0 ||
|
||||
snapshot(editablePayload(fields.value, values.value)) !== saved.value),
|
||||
)
|
||||
|
||||
function headers() {
|
||||
@@ -189,6 +192,8 @@ const session: FormSession = {
|
||||
},
|
||||
pendingChanges: readonly(pendingChanges),
|
||||
revision: readonly(revision),
|
||||
beginUpload,
|
||||
activeUploads: readonly(activeUploads),
|
||||
}
|
||||
provide(FORM_SESSION, session)
|
||||
provide(FORM_VALUES, computed(() => values.value))
|
||||
@@ -319,7 +324,7 @@ async function showErrors(details: Record<string, unknown> | undefined): Promise
|
||||
}
|
||||
|
||||
async function onSubmit(): Promise<void> {
|
||||
if (busy.value || preview.value || loading.value || failed.value) {
|
||||
if (busy.value || preview.value || loading.value || failed.value || activeUploads.value > 0) {
|
||||
return
|
||||
}
|
||||
busy.value = true
|
||||
@@ -506,7 +511,7 @@ function onCloseAutoFocus(event: Event): void {
|
||||
v-if="!failed"
|
||||
variant="primary"
|
||||
data-action="save-child"
|
||||
:disabled="busy || loading"
|
||||
:disabled="busy || loading || activeUploads > 0"
|
||||
@click="onSubmit"
|
||||
>
|
||||
{{ busy ? t('backend::lang.form.saving') : submitLabel }}
|
||||
|
||||
@@ -13,7 +13,7 @@ import { mapWinterUrl } from '../app/winterUrl'
|
||||
import FormErrorBanner from '../components/form/FormErrorBanner.vue'
|
||||
import FormGrid from '../components/form/FormGrid.vue'
|
||||
import FormTabs from '../components/form/FormTabs.vue'
|
||||
import { FORM_ASSETS, FORM_LOCALE, FORM_PATCH, FORM_SESSION, FORM_VALUES } from '../components/form/formContext'
|
||||
import { FORM_ASSETS, FORM_LOCALE, FORM_PATCH, FORM_SESSION, FORM_VALUES, createUploadGate } from '../components/form/formContext'
|
||||
import { RELATION_MANAGER, needsRecord } from '../components/form/registry'
|
||||
import {
|
||||
DEFAULT_TAB,
|
||||
@@ -79,6 +79,7 @@ const sessionHeader = { [SESSION_HEADER]: sessionKey } as { 'X-Session-Key': str
|
||||
const pendingChanges = ref(0)
|
||||
// Bumped after each successful save, so file fields reload their lists.
|
||||
const revision = ref(0)
|
||||
const { activeUploads, beginUpload } = createUploadGate()
|
||||
|
||||
// A relation manager needs a saved record: on create it is dropped with its
|
||||
// tab even when the YAML forgets `context: update` (D-05, design screen 5).
|
||||
@@ -151,7 +152,9 @@ const subtitle = computed(() =>
|
||||
const dirty = computed(
|
||||
() =>
|
||||
!loading.value &&
|
||||
(pendingChanges.value > 0 || snapshot(editablePayload(fields.value, values.value)) !== saved.value),
|
||||
(pendingChanges.value > 0 ||
|
||||
activeUploads.value > 0 ||
|
||||
snapshot(editablePayload(fields.value, values.value)) !== saved.value),
|
||||
)
|
||||
|
||||
function adopt(record: RecordEnvelope | undefined): void {
|
||||
@@ -211,6 +214,8 @@ provide(FORM_SESSION, {
|
||||
},
|
||||
pendingChanges: readonly(pendingChanges),
|
||||
revision: readonly(revision),
|
||||
beginUpload,
|
||||
activeUploads: readonly(activeUploads),
|
||||
})
|
||||
|
||||
/** 422: messages under fields, the first invalid field (schema order) focused. */
|
||||
@@ -236,7 +241,7 @@ function redirectTarget(kind: 'redirect' | 'redirectClose', id: unknown): string
|
||||
|
||||
/** Saves the record; returns the saved envelope, or null after an error. */
|
||||
async function save(): Promise<RecordEnvelope | null> {
|
||||
if (busy.value || !schema.value) {
|
||||
if (busy.value || !schema.value || activeUploads.value > 0) {
|
||||
return null
|
||||
}
|
||||
busy.value = true
|
||||
@@ -420,10 +425,10 @@ void load()
|
||||
</Button>
|
||||
<div class="ml-auto flex items-center gap-2.5">
|
||||
<Button variant="ghost" data-action="cancel" :to="listPath">{{ t('backend::lang.form.cancel') }}</Button>
|
||||
<Button variant="outline" data-action="save-close" :disabled="busy || loading || !schema" @click="onSaveAndClose">
|
||||
<Button variant="outline" data-action="save-close" :disabled="busy || loading || !schema || activeUploads > 0" @click="onSaveAndClose">
|
||||
{{ t('backend::lang.form.save_and_close') }}
|
||||
</Button>
|
||||
<Button variant="primary" data-action="save" :disabled="busy || loading || !schema" @click="onSave">
|
||||
<Button variant="primary" data-action="save" :disabled="busy || loading || !schema || activeUploads > 0" @click="onSave">
|
||||
{{ busy ? t('backend::lang.form.saving') : t('backend::lang.form.save') }}
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user