fix(frontend): preserve unsaved card drafts across archive recovery and completion - #3015
Conversation
Refresh card state and a completed archive/restore were the two close paths in the card editor that emitted close unconditionally, so an unsaved draft typed after an archive failure or during an in-flight archive was discarded without a confirmation. Refresh card state now funnels through handleClose, so it inherits both the dirty-close confirmation and the in-flight assignment-save refusal (#2997); the board refetch it owes is carried with the close and runs only when the editor actually closes. A completed archive over a dirty editor keeps the editor open with its draft, records the state the request produced, refetches the board and says so, instead of emitting updated/close - both hosts treat updated as "this editor is finished". Refs #2969, #2997
Seven specs in CardModal.spec.ts cover the guarded close funnels: archive error then edit then Refresh card state (confirmation shown, no close, no refetch), cancelling that confirmation (draft and failed state kept, the refresh dropped), a clean recovery close (one close plus the refetch), an archive that succeeds over a newer draft (no close, no updated, draft kept, state notice, refetch, save refused), a clean completion (updated + close), a rejected request followed by a clean close, a late success after a clean close, and an ordinary untouched close. One spec in CardModalAssignmentSave.spec.ts pins #2997: archive recovery now refuses while an assignment PUT is unanswered. Verified red against the pre-fix component: the two draft-loss specs and the #2997 spec fail there and pass with the fix; the rest pin unchanged behaviour. Refs #2969, #2997
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38e672b3e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <p v-else-if="assignmentDirty" class="text-sm">Save or cancel assignment changes before saving other card fields.</p> | ||
| <CardModalActions | ||
| :is-form-valid="isFormValid && !card.isArchived && !isSaving && !assignmentDirty" | ||
| :is-form-valid="isFormValid && !cardIsArchived && !isSaving && !assignmentDirty" |
There was a problem hiding this comment.
Apply the settled archive state to every control
When an archive finishes over a draft, cardIsArchived reflects the server state but only this Save gate consumes it. If the user dismisses the busy archive confirmation, edits the card, and the archive then succeeds, the assignment field and archive action still read the snapshot's card.isArchived === false; assignments remain submittable even though CardAssignmentService.StageReplaceAsync rejects archived cards, and reverting the draft re-enables a stale “Archive card” action instead of Restore. Propagate the settled state to every archive-sensitive control.
AGENTS.md reference: frontend/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in round 2 (478b8c5): every archive-state gate now reads the settled state.
| if (hasUnsavedChanges.value) { | ||
| archiveStateAfterChange.value = !cardIsArchived.value | ||
| archiveCompletedWithDraft.value = true | ||
| void boardStore.fetchBoard(props.card.boardId) |
There was a problem hiding this comment.
Reload comments after the kept-open refresh
On this kept-draft path, a successful fetchBoard clears cardCommentsByCardId in boardCrudStore, but the modal remains open and useCardModal reloads comments only when the editor opens or switches cards. Thus, if the user dismisses an in-flight archive confirmation and starts editing an existing comment before the archive settles, this refresh removes the comment row and makes its unsaved edit inaccessible until the editor is closed, defeating the draft-preservation behavior. Preserve the comment cache or explicitly reload this card's comments after the refresh.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Triaged once: real, narrow, non-blocking (round ceiling reached; no committed data lost). Tracked as #3023.
|
Fresh-context independent review (round 1) at exact head Refuted by the reviewer from source (read-only):
Fix-now (MEDIUM, both inside the new editor-open-over-an-archived-card state):
Non-blocking (law 2c):
|
Round 2 review, two MEDIUMs in the "editor kept open over an archived card"
state this PR introduces.
1. Only the save gate had moved to the settled state; the form fieldset, the
parent field, the assignment field's read-only gate and the type selector
still read the host's snapshot, whose isArchived stays permanently false
there. The assignment field therefore stayed writable and a Save assignments
PUT would have been rejected server-side with a misleading explanation. All
of them now read cardIsArchived, so the editor is consistently read-only
over an archived card while the typed draft stays visible.
2. The notice claimed the card could be restored to save the draft, but the
archive control is disabled while the editor is dirty, and its label and
operation came from the stale snapshot ("Archive card", archive=true with a
pre-archive updatedAt). The notice now states what is actually possible, and
CardArchiveAction takes an optional `archived` override so its label and
operation follow the settled state. Archive/restore stays refused while
dirty: the explicit lifecycle confirmation contract is unchanged, and the
override defaults to `undefined` (not Vue's Boolean cast to false) so every
existing caller keeps reading its own card.
Also: a close refused by the in-flight assignment-save guard now drops the
board refetch "Refresh card state" had armed, so it cannot fire behind a later
unrelated close.
Specs: the kept-draft case asserts the new notice text, the disabled form
fieldset, the read-only assignment field and the "Restore card" label; a new
case pins the deliberate no-op when the host suppresses the discard prompt;
the #2997 case now also proves no stale refetch after the refusal.
Refs #2969, #2997
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478b8c5fce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hasUnsavedChanges.value) { | ||
| archiveStateAfterChange.value = !cardIsArchived.value | ||
| archiveCompletedWithDraft.value = true | ||
| void boardStore.fetchBoard(props.card.boardId) |
There was a problem hiding this comment.
Handle rejected board refreshes
When the post-archive refresh encounters a current network/5xx failure, fetchBoard uses the default explicit intent and rethrows after updating the store (boardCrudStore.ts:488-498), but this detached promise has no rejection handler. The same pattern appears in the recovery close path at line 286, so either flow produces a global unhandledrejection and erroneous Sentry event instead of treating the refresh failure as an expected UI error; await/catch it or use the non-rejecting background refresh contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Triaged once: real, cosmetic in effect (unhandled rejection on a failed refresh). Tracked as #3023.
|
Fix round 2 (last round) pushed: MEDIUM 1 — half-migrated gates. Fixed. Every gate that turns on the archive state now reads MEDIUM 2 — contradictory notice. Fixed, minimally. The notice now reads:
and One detail worth flagging for the next reader: the override prop is declared One-liners, both done. A close refused by the in-flight assignment-save guard now clears LOWs — recorded, not fixed (law 2c; none of them is a merge blocker under 2a):
Checks at Red-checked against the reviewed head Still NOT verified, unchanged from the PR body: no browser, E2E or screen-reader run; no live backend; the new archive cases run in the |
|
Round 2 (fix round) review at exact head Scoped fresh-context verification of the fix, with the two candidate HIGHs refuted from source and the installed Vue 3.5 runtime:
Triage of everything raised since round 1 (law 2c), all non-blocking and tracked as #3023:
Author-run proof at this head: 145 passed across the five specs, typecheck, build (exit 0), eslint; red-checked against the round-1 head. The hosted rollup at this head is the executable proof. |
What
Two close paths in the Legacy card editor emitted
closewithout asking the dirty-close guard anything, so an unsaved draft could disappear without a confirmation:refreshArchiveState(therefreshhandler behind "Refresh card state" on a failed archive) emittedcloseand refetched the board. Edit the title after an archive fails, press it, and the draft is gone.@changed="emit('updated'); emit('close')"closed unconditionally, so a draft typed while an archive request was in flight was discarded the moment that request succeeded.Both now funnel through the guards PR #2977 established:
refreshArchiveStatecallshandleClose. A clean editor closes and refetches exactly as before; a dirty one gets the save/discard confirmation; an unanswered assignment PUT gets Add multiple card assignments and explicit import mapping #2977's truthful "already sent to the server" notice instead of a close. The board refetch it owes is carried with the close (pendingArchiveRefresh) and runs only if the close actually happens — cancel the confirmation and neither the draft nor the failed archive state moves.handleArchiveChangedkeeps the editor open when the draft is dirty: it records the state the completed request produced, refetches the board, and renders arole="status"notice saying what happened and that the unsaved changes are still there. A clean editor behaves as today (updatedthen the normal close funnel). A request that settles after the editor already closed emits nothing, so one user action can never produce two closes.Two supporting details, both deliberate:
updated. That event is not a neutral "something changed" signal in this codebase —ColumnLanemaps it straight to its close handler andPaperBoardViewresolves a held navigation with it — so emitting it would close the editor through the host and reintroduce the same loss. The committed archive still reaches the board through the store mutation plus the refetch.ref, soprops.cardnever learns the card is now archived while the editor stays open.cardIsArchivedprefers the state the settled request produced, and the Save gate uses it, so the editor stops offering a save that could only fail against an archived card.CardArchiveAction.vueis untouched: its confirmation flow, in-dialog recovery and expected-version/fingerprint checks are exactly as #2995 left them, and no prop or emit signature changed.Closes #2969
Closes #2997
Checks run (worktree
C:/wt/td-2969, Node 24, head 38e672b)npx vitest --run --maxWorkers=2 src/tests/components/CardModal.spec.ts src/tests/composables/useCardModal.spec.ts src/tests/components/CardModalAssignmentSave.spec.ts src/tests/components/CardArchiveAction.spec.ts src/tests/components/BoardCardArchive.spec.ts— 144 passed (135 before this change).npm run typecheck— clean.npm run build— clean.npx eslinton the three touched files — clean.CardModal.vuereverted to base4c479a7ffand the new specs in place, 3 of the 7 newCardModal.spec.tscases fail (the two draft-loss paths) and the [Frontend][Assignments] Refresh-card-state closes the editor behind an in-flight assignment save (from PR #2977 round-2 review) #2997 case inCardModalAssignmentSave.spec.tsfails; all pass with the fix. The other four new cases pin behaviour that is unchanged on both sides (clean recovery close, clean completion, rejected request then clean close, late success after a clean close, ordinary untouched close).NOT verified
role="status"in the DOM.presentation: 'inspector') was exercised only through the existingCardModalAssignmentSave.spec.tsmatrix; the new archive cases run in the modal presentation.PaperBoardView.spec.tsand the wider frontend suite were not run — the suite needs a live backend on this box.setCardArchivedremoving the card fromcurrentBoardCards) is exercised only through mocks here.