fix(frontend): prevent API tokens page crash on delete modal close by Marfa · Pull Request #3417 · go-vikunja/vikunja

Marfa · 2026-08-03T10:20:11Z

Summary

Keep tokenToDelete until the delete modal finishes closing, and guard the modal slot with v-if=tokenToDelete. Clearing the ref immediately raced with Modal's ~150ms close animation and threw Cannot read properties of null (reading 'title'), which then surfaced as Vue's follow-up emitsOptions error on later updates.
Call useFlatpickrLanguage() once at setup in ApiTokenForm instead of inside a computed getter.
Add unit coverage for create-form edits and delete-modal close.

Test plan

pnpm exec vitest run src/views/user/settings/ApiTokens.test.ts src/components/token/ApiTokenForm.test.ts
On /user/settings/api-tokens, create a token, then delete it and confirm no error toast
After deleting, continue editing the create form (title / checkboxes / expiry) without emitsOptions errors

Clearing tokenToDelete while Modal still renders its slot caused a null title read and a follow-up Vue emitsOptions crash on the API tokens page. Co-authored-by: Cursor <cursoragent@cursor.com>

Marfa · 2026-08-03T10:37:44Z

Test plan results (live instance)

Verified against https://todo.themarfa.name with patched SPA (assets/index-CEBgTEYM.js).

Unit: ApiTokens.test.ts + ApiTokenForm.test.ts (3/3 pass)
Create API token in UI — success toast, row appears
Delete token via modal confirm — row gone, no emitsOptions / reading 'title' / null toast
Edit create form after delete (title + permissions) — no crash toast

Deploy note: frontend overlay at /opt/vikunja/frontend-dist via nginx; API still vikunja/vikunja:latest (v2.4.0).

Marfa · 2026-08-03T11:35:58Z

CI failures

Both failing checks look unrelated to this PR’s changes:

classify — LLM action error: this model is not supported MaxTokens, please use MaxCompletionTokens (model gpt-5.6-luna). Maintainer-side workflow/API mismatch.
docker (Preview) — checkout of fork PR code from pull_request_target is refused (allow-unsafe-pr-checkout: false). Expected for PRs from forks unless the workflow is adjusted or the branch lives in the base repo.

Happy to re-trigger once those are fixed on the repo side. Local unit tests and a live create/delete flow on a patched instance are green (see earlier comment).

…te modal test

tink-bot

🤖 PR Swarm — automated review, round 1. See inline comments.

tink-bot · 2026-08-03T17:20:45Z

🤖 PR Swarm — automated review. Panel: bughunter · security · conventions · design · verifier — round 3 (final) @ 8c5de61 (+2/−7 this round, PR +177/−44)

Verdict: ✅ APPROVE — 2 questions open for maintainer

3 rounds done. All actionable findings fixed, runtime-verified. Diff shrank from author's +262/−9 to +177/−44 while gaining real regression coverage. Two ❓ threads await your reply — answer in-thread, re-run swarm to implement.

❓ Needs your input (2)

ApiTokens.vue:52 — deleteToken shape trade-off (round cap hit, panel split): current clear-before-await kills double-submit (verified 8/8 trials, one DELETE each) but dismisses modal even when DELETE rejects (main kept it open for retry; error still surfaces via global handler, row stays). Alternative: separate deleting flag — keeps modal open on failure, kills the (measured-imperceptible) name-blank flash. Same double-submit hole exists pre-existing in BotUsers.vue:108. (a) keep current, (b) apply deleting flag here, (c) uniform follow-up PR for both pages?
ApiTokenForm.vue — pre-existing expiry crash, reproduces on main: Custom → preset switch throws Vue insertBefore NotFoundError (flatpickr mutates v-if-owned DOM), corrupts vnode tree, later resurfaces emitsOptions — the error class this PR's title targets. Fix here (stable-keyed wrapper / CSS-hide) or follow-up issue?

✅ Fixed round 3 (2)

Modal.test.ts:14 — round-2 spy annotation didn't compile (TS2344, masked by continue-on-error typecheck job); now MockInstance<HTMLDialogElement['showModal']>, vue-tsc clean. f88bbed
ApiTokens.test.ts — dead mock line + mutation-verified zero-signal assertions dropped. 8c5de61

💤 Resolved as nit, round 3 (4)

deleting-flag redesign as immediate action — round cap; escalated to ❓ above instead of a fourth reshape.
BotUsers.vue:108 double-submit hole — pre-existing, out of diff; folded into ❓ Home assistant Integration #1.
runtimeErrorMessages helper inlining — kept, shapes failure output.
Verifier one-off double-DELETE observation — attributed to Vite HMR-stale module (stack named superseded module version); guard held 8/8 after clean load.

🔎 Runtime verification (round 3 — delete flow re-drive)

Full sentences for the record:

✅ confirm-delete end-to-end — Exactly one DELETE /api/v1/tokens/{id} per confirm across all trials; rows and API state matched at every step down to an empty list.
✅ rapid double-click — Eight clean trials (real dblclick, two synchronous programmatic clicks in one JS task, post-HMR) each fired exactly one DELETE, no 403s, no console errors.
✅ close animation — The fading dialog renders the token name empty for ~150ms, but at opacity 0.075 by 60ms it is not perceptible; dialog unmounts cleanly, no stuck backdrop.
➖ console noise — The onUnmounted ... no active component instance warning trio is pre-existing (fires via formatDateSince/useGlobalNow whenever the table renders on main too).

Reviewer summaries (round 3)

Reviewer	Assessment
🐛 bughunter	Round-2 changes sound (mobileInput timing, stubGlobal, spy semantics all verified); flagged error-path trade-off → ❓, and a type annotation → fixed.
🛡 security	Clean; deleteToken rewrite strictly improves revocation flow, failure path still surfaces to user.
📏 conventions	Zero findings — style, comments, i18n all clean.
📐 design	Both tests mutation-verified non-vacuous; helpers earn their keep; remaining `?.` debate → ❓.
🔎 verifier	Delete flow re-verified 8/8 incl. adversarial double-click; flash imperceptible; sole anomaly HMR-attributed.

Previous rounds (2)

round 1 @ 17a48dc — ⚠️ REQUEST CHANGES: 31-line restructure replaced with sibling BotUsers.vue ?. idiom; 2 of 3 new tests deleted (passed on pre-fix code); dialog stubs deduped; expiry input labeled; full E2E verification vs crashing baseline. +91/−207.
round 2 @ b11524e — 💬 APPROVE WITH NITS: round-1 stub helper deleted (happy-dom implements dialog natively), double-submit guard + test (red/green verified), mobile a11y label, two-token fixture. +82/−81.

…uble submit

tink-bot

🤖 PR Swarm — automated review, round 2. See inline comments.

…tions

tink-bot

🤖 PR Swarm — automated review, round 3 (final). See inline comments.

tink-bot · 2026-08-03T18:09:47Z

 		return
 	}
-	tokens.value.splice(index, 1)
+	tokenToDelete.value = undefined


[convergent: design + bughunter] 🟡 MEDIUM — ❓ needs your input

Clear-before-await trade-off, panel split, round cap hit — your call. Current shape (ships now): ref cleared + modal closed before await service.delete; double-submit dead (verified 8/8 runtime trials, exactly one DELETE incl. two synchronous clicks in one JS task); token name blanks in fading modal for ~150ms but measured imperceptible (opacity 0.075 at 60ms). Cost vs main: on DELETE rejection modal is already dismissed (main kept it open for retry); error still surfaces via global errorHandler, row stays. Alternative both reviewers converged on: keep tokenToDelete set, separate const deleting = ref(false) guard — keeps modal open on failure, kills flash, ?. still needed for close-fade. Same hole also exists pre-existing in BotUsers.vue:108 (two clicks = two DELETEs there, no crash). Options: (a) keep current shape, (b) swarm applies deleting-flag shape here, (c) follow-up PR covering ApiTokens + BotUsers uniformly. Reply here; re-run swarm to implement.

(a) keep the current shape for this PR.

Double-submit is verified dead, the blank-name flash is imperceptible, and DELETE failures still surface via the global error handler with the row intact. A deleting flag / unifying with BotUsers.vue is a reasonable follow-up if maintainers want that pattern everywhere — I’d rather not reshape this PR again for it.

tink-bot · 2026-08-03T18:09:47Z


+// altInput (or mobileInput on the mobile path) is a fresh element inheriting no attributes from the
+// input we render, so label it here
+const labelDateInput: Hook = (_dates, _str, instance) => {


[verifier] 🟡 MEDIUM — ❓ needs your input (anchored nearest — affected markup not in diff)

Pre-existing crash on this page, reproduces on main identically: switch Expires "Custom" → preset throws Vue insertBefore NotFoundError (flatpickr mutates DOM owned by v-if), corrupts vnode tree; next create submit throws, then emitsOptions — the very error class this PR's title targets. Root cause separate from this PR (flatpickr DOM ownership); candidate fix: stable-keyed wrapper element around <flat-pickr> or CSS-hide instead of v-if. Fix in this PR or file follow-up issue? Reply here; re-run swarm to implement.

Follow-up issue, please — leave it out of this PR.

Agreed it’s a real crash class on main (flatpickr mutating v-if-owned DOM), but it’s separate from the delete-modal race this PR targets. Happy to open/track a dedicated issue if that helps.

fix(frontend): keep API token delete target during modal close animation

2da5a9b

Clearing tokenToDelete while Modal still renders its slot caused a null title read and a follow-up Vue emitsOptions crash on the API tokens page. Co-authored-by: Cursor <cursoragent@cursor.com>

tink-bot added the pr-swarm/started PR Swarm run is currently active on this PR label Aug 3, 2026

kolaente added 4 commits August 3, 2026 19:18

refactor(frontend): simplify delete modal race fix to optional chaining

6084d09

test(frontend): extract shared HTMLDialogElement stub helper

75c0ca0

test(frontend): drop tests that pass without the fix and tighten dele…

ef721a8

…te modal test

fix(frontend): add accessible name to custom expiry date input

17a48dc

tink-bot reviewed Aug 3, 2026

View reviewed changes

kolaente added 3 commits August 3, 2026 19:51

fix(frontend): guard token delete flow against double submit

32bafd5

fix(frontend): label mobile expiry input via stable onReady hook

f373cf8

test(frontend): drop dialog stubs happy-dom makes redundant, cover do…

b11524e

…uble submit

tink-bot reviewed Aug 3, 2026

View reviewed changes

Comment thread frontend/src/views/user/settings/ApiTokens.vue

Comment thread frontend/src/components/misc/Modal.test.ts

Comment thread frontend/src/views/user/settings/ApiTokens.test.ts

Comment thread frontend/src/components/token/ApiTokenForm.vue

kolaente added 2 commits August 3, 2026 20:09

fix(test): use MockInstance type for showModal spy

f88bbed

test(frontend): drop dead mock line and redundant double-submit asser…

8c5de61

…tions

tink-bot reviewed Aug 3, 2026

View reviewed changes

tink-bot added pr-swarm/needs-input PR Swarm finished, deferred items are waiting on maintainer input and removed pr-swarm/started PR Swarm run is currently active on this PR labels Aug 3, 2026

Uh oh!

Uh oh!

Conversation

Marfa commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Marfa commented Aug 3, 2026

Test plan results (live instance)

Uh oh!

Marfa commented Aug 3, 2026

CI failures

Uh oh!

tink-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tink-bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verdict: ✅ APPROVE — 2 questions open for maintainer

❓ Needs your input (2)

✅ Fixed round 3 (2)

💤 Resolved as nit, round 3 (4)

🔎 Runtime verification (round 3 — delete flow re-drive)

Reviewer summaries (round 3)

Uh oh!

tink-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tink-bot left a comment

Choose a reason for hiding this comment

Uh oh!

tink-bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Marfa Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

tink-bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Marfa Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Marfa commented Aug 3, 2026 •

edited

Loading

tink-bot commented Aug 3, 2026 •

edited

Loading