Detect wish save failures directly instead of inferring them
Plan.tsx's handlers swallowed their rejections and onUpdate was typed to return void, so WishEditor could not observe a failed save. It inferred the outcome instead -- comparing the draft it sent against the next wishes prop, and reading "isSaving went false without a match" as failure. That coupled save status to refetch timing and could not tell a network error from a slow refresh. The handlers now return their promise, onUpdate accepts `void | Promise<void>` so void-returning callers still typecheck, and WishEditor awaits and catches the real outcome. The inference machinery (sentDraftRef, prevIsSavingRef and the comparison effect) is removed rather than left alongside, since two sources of truth can disagree. The save/refetch/save loop guard is untouched: it lives in the draft reseed effect, which only fills a row that has no draft yet, and saves only ever originate from an explicit onChange. Added a comment saying so. A failed row now keeps its edit and offers an explicit retry, rather than requiring another edit to retrigger the debounce. Per-row failures report inline only -- reporting them in the page banner too would double-report the same error. Also removes wishEditor.saveButton/savingButton, dead since the per-row Save button was replaced by auto-save. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXMhc9nT6sNz1SL4j8dznL
This commit is contained in:
@@ -34,26 +34,33 @@ export function Plan(): ReactNode {
|
||||
return <LoadingSpinner message={t('plan.loading')} />
|
||||
}
|
||||
|
||||
const handleAdd = (payload: LeaveWishCreateRequest) => {
|
||||
const handleAdd = (payload: LeaveWishCreateRequest): Promise<void> => {
|
||||
setWishError(null)
|
||||
setIsSaving(true)
|
||||
addWish(payload)
|
||||
return addWish(payload)
|
||||
.then(() => undefined)
|
||||
.catch((err: unknown) => setWishError(err instanceof Error ? err.message : t('plan.addWishError')))
|
||||
.finally(() => setIsSaving(false))
|
||||
}
|
||||
|
||||
const handleUpdate = (id: string, patch: LeaveWishUpdateRequest) => {
|
||||
setWishError(null)
|
||||
// Unlike handleAdd/handleRemove, this does NOT catch the rejection here:
|
||||
// WishEditor now awaits the promise it returns directly and drives that
|
||||
// row's own inline save-status/retry off the real outcome, so surfacing
|
||||
// the same failure again as the page-level `wishError` banner would be
|
||||
// double-reporting one failure. `isSaving` bookkeeping (which only gates
|
||||
// disabling the preset/remove buttons, never a full-page spinner) still
|
||||
// runs via `.finally`.
|
||||
const handleUpdate = (id: string, patch: LeaveWishUpdateRequest): Promise<void> => {
|
||||
setIsSaving(true)
|
||||
updateWish(id, patch)
|
||||
.catch((err: unknown) => setWishError(err instanceof Error ? err.message : t('plan.updateWishError')))
|
||||
return updateWish(id, patch)
|
||||
.then(() => undefined)
|
||||
.finally(() => setIsSaving(false))
|
||||
}
|
||||
|
||||
const handleRemove = (id: string) => {
|
||||
const handleRemove = (id: string): Promise<void> => {
|
||||
setWishError(null)
|
||||
setIsSaving(true)
|
||||
removeWish(id)
|
||||
return removeWish(id)
|
||||
.catch((err: unknown) => setWishError(err instanceof Error ? err.message : t('plan.removeWishError')))
|
||||
.finally(() => setIsSaving(false))
|
||||
}
|
||||
|
||||
@@ -237,6 +237,13 @@
|
||||
color: var(--c-danger);
|
||||
}
|
||||
|
||||
.wish-editor__retry-button {
|
||||
color: var(--c-danger);
|
||||
border-color: var(--c-danger);
|
||||
font-size: var(--fs-sm);
|
||||
padding: var(--sp-1) var(--sp-3);
|
||||
}
|
||||
|
||||
/* Plan results */
|
||||
.plan-results__preview-banner {
|
||||
background-color: var(--c-accent-weak);
|
||||
|
||||
Reference in New Issue
Block a user