refactor(frontend): migrate project backgrounds to v2 client - #3694
refactor(frontend): migrate project backgrounds to v2 client#3694tink-bot wants to merge 18 commits into
Conversation
Preview DeploymentPreview deployments for this PR are available at:
The preview environment will start automatically on first visit. Subsequent pushes to this PR will update the Run locally with Dockerdocker pull ghcr.io/go-vikunja/vikunja:pr-3694
docker run -p 3456:3456 ghcr.io/go-vikunja/vikunja:pr-3694Last updated for commit 3138f06 |
d8f5e0d to
02b64c5
Compare
02b64c5 to
f3d21d4
Compare
Address pr-swarm findings: remove the uncalled backgroundLoading export and the blurHashLoading bookkeeping it required, log blur hash decode failures instead of surfacing them as global error toasts, and narrow the parameter back to ProjectResponse.
Address pr-swarm security finding: the auth store clears the query cache when the signed-in identity changes, but the base store kept the previous account's background object URLs and current project id alive across an SPA logout, so the next user on the same tab could briefly see them.
Address pr-swarm findings: background mutations now capture and assert the client request context like every sibling mutation, resolve to the merged cached project instead of the sparse wire response, flatten Unsplash search results to the fields the UI needs, guard the blob responses, and drop the unused unsplash key.
… settings Address pr-swarm findings: render each Unsplash result through a small component that owns its thumbnail query and object URLs instead of hand rolled pending sets and revoke bookkeeping, pass the merged project from the mutations straight to the base store, ignore mutation results after the user moved to another project, encode the attribution handle and skip the link when unknown, and hide write affordances without write access.
Address pr-swarm runtime findings: a failed Unsplash search rendered an empty grid with no message or retry, the image tiles had no accessible name, and the attribution link was focusable but invisible without hover.
tink-bot
left a comment
There was a problem hiding this comment.
🤖 PR Swarm — automated review, round 1. See inline comments.
|
🤖 PR Swarm — automated review. Panel: bughunter · security · conventions · design · verifier — round 3 (final) @ 3138f06 (+43/−80 this round, PR +1448/−1266) Verdict: ✅ APPROVEThree rounds, 50 findings, 43 fixed, 7 nit-resolved, 0 deferred. Round 3 found no regressions from round 2; verifier re-drove every changed flow on a local build of ❓ Needs your input (0)none ✅ Fixed this round (4)
💤 Resolved as nit (3)
🔎 Runtime verification (round 3, local build of
|
| Reviewer | Assessment |
|---|---|
| 🐛 bughunter | No regression from round 2; traced null-heuristic, cancel interplay, useObjectUrl, enabled reactivity, identity watchers. Three LOW judgment calls, two declined with reasons. |
| 🛡 security | All controls hold; identityKey collision-free for every reachable transition; canWrite fail-closed with server CanWrite independent; attribution XSS-safe. One logging nit. |
| 📏 conventions | Clean. |
| 📐 design | Consolidations justified; flagged triplicated handler tail, deleted negative test, test mocking a pure helper — all addressed; one placement disagreement declined. |
| 🔎 verifier | All eight re-driven flows pass; no findings. |
Out-of-scope notes for follow-ups
- Backend:
backgroundRemove(v2) andRemoveProjectBackground(v1) callDeleteBackgroundFileIfExistswithBackgroundFileIDalways 0 — stored files orphaned on removal (pre-existing, bughunter r1). - Backend:
Info interface{}on background images generatesinfo?: unknown; typing it would remove the runtime narrowing inunsplashAuthor. - Backend:
backgroundRemovecould returnproject.ReadOneso the frontend merge dance is unnecessary. - Base store still owns current-project background URLs per plan Phase 2 step 6; deriving from
useCurrentProjectis a later phase.
Previous rounds (2)
round 2 @ 41e9e23 — identityKey, getCachedProject, useBlurHashUrl/useObjectUrl; Unsplash flatten reverted per plan. Runtime: local build of 98e7c13, deep-link bail observed then fixed; screenshots under round-2. Duration 24m35s.
round 1 @ 98e7c13 —
Address pr-swarm finding: the identity-change watcher's tuple-and-guard logic was about to be duplicated in the base store, so derive a single identity key on the auth store and watch that instead.
Address pr-swarm finding: background mutations need the same detail-or-list cache lookup that updateProject already does, so share it.
Address pr-swarm findings: derive the cache update and blob eviction from the response instead of per-call mode arguments, fetch the project when nothing is cached instead of fabricating a blank one, type the background fields from the generated schema, and return raw Unsplash results so the optional fields are narrowed at the use site as the migration plan asks.
Address pr-swarm findings: watch the auth store's identity key instead of re-implementing its guard, and include the has-tasks flag in the reset so it cannot leak between accounts on the same tab.
Address pr-swarm findings: replace the hand-rolled object URL lifecycle in useProjectBackground with useObjectUrl and a shared useBlurHashUrl composable, which also recreates the URL when a project regains its background while the blob is still cached.
…r project Address pr-swarm findings: the stale-route bail swallowed the success toast and the modal close whenever background settings were opened from the sidebar or by URL for a project that is not the current one, so only the base store sync is gated on that now. Also narrow Unsplash results at the use site, move the attribution into the thumbnail component, show the project query's loading and error state instead of hiding controls, and stop running the Unsplash search for users who cannot apply a background.
Address pr-swarm runtime findings: read-only users saw an empty background dialog, and thumbnail load failures logged a bare reactive proxy.
tink-bot
left a comment
There was a problem hiding this comment.
🤖 PR Swarm — automated review, round 2. See inline comments.
Address pr-swarm finding: the route recheck, base store gate and toast were copied into all three background handlers.
Address pr-swarm finding: the generated client throws the parsed problem body on HTTP failures, so error.message logged undefined.
Address pr-swarm findings: restore the negative identity-reset case as a table row and let the thumbnail test exercise the real unsplashAuthor.
tink-bot
left a comment
There was a problem hiding this comment.
🤖 PR Swarm — automated review, round 3 (final). See inline comments.








Project backgrounds still used legacy services and split cache ownership.
How to verify