Sync the dashboard layout with the server (backend #213)

[BabuPlk]

Frontend half of SprintStartProject/sprintstart-backend#213 — needs SprintStartProject/sprintstart-backend#270.

What

The dashboard layout now follows the user to other devices instead of living only in this browser.

• dashboardLayoutService.ts – GET / PUT / DELETE /api/v1/users/me/dashboard/layout
• useDashboardLayoutSync.ts – modelled on the board's useBoardStructureSync:
  • Local storage stays where the client writes first and reads from, so every gesture stays instant and a failed request costs nothing.
  • On arrival: the server's layout wins and is written into local storage. If the server has none, this browser's layout is sent up once — the migration for everybody who arranged their dashboard before this. Neither → default.
  • Afterwards: every change is sent up, debounced (1.2 s); the last pending change is flushed when leaving the page. Reset deletes it on the server too.
  • A change made before the first read settled is kept and sent instead of being overwritten by the server's copy.
  • Offline / endpoint not deployed yet → works exactly as before, from local storage.
• storage.ts – exports LAYOUT_VERSION, which is sent with every read and write; the server answers a layout of another version as the default, same as the client already does locally.
• MSW handlers for the three endpoints, so existing dashboard tests keep running without a real backend.

Tests

• useDashboardLayoutSync.test.tsx – server wins, migration, nothing stored, change before first read, debounce, reset, offline.
• Full suite passes locally (401 files, 3488 tests); tsc, eslint, prettier clean.

Note for running locally on Node 25+: NODE_OPTIONS=--no-experimental-webstorage npm test, otherwise Node's own localStorage shadows jsdom's (CI uses Node 24 and is unaffected).

[DavidLeuter]

Nice work — the hook is closer to useBoardStructureSync than a copy would be, and it's more careful in a couple of places (holding a change made before the first read, flushing the queued layout on unmount). Tests read well. Requesting changes for one behavioural bug, plus a small race.

1. A reset on one device gets undone by any other device (blocking)

dashboardLayoutService.resetLayout says it "forgets the arrangement, so every device falls back to the default". That doesn't hold, because the arrival logic can't tell "never synced" apart from "reset somewhere else":

1. PC and laptop both have layout X (server + local storage on both).
2. Reset on the PC → DELETE, the server has no row.
3. Open the dashboard on the laptop → fetchLayout answers updatedAt: null, the laptop still has X in local storage → the migration branch saveLayout(X).
4. Back on the PC, X is back.

With a multi-device feature that's a very reachable path. A reset only survives if it happens on every device where the layout was ever stored.

Suggestion: remember per browser that the local copy has already been synced once (e.g. a synced: true next to version in the stored entry, set after the first successful pull/push). On arrival with an empty server:

• local never synced → upload it (the real migration, as now)
• local already synced → someone reset it elsewhere → clearStoredLayout(userId) + onPulled()

A test for "reset elsewhere, stale local copy here → stays default" would pin it down.

2. A change made while the pending change is being flushed gets dropped (small)

In the arrival effect, pulledFor.current is only set in finally, after await saveLayout(waiting.layout) / await resetLayout(). A push() during that await still sees pulledFor !== userId and overwrites pending.current. Then finally sets pending.current = null, so that newer layout never reaches the server. Local storage keeps it, but on the next visit the server's older copy wins and overwrites it.

Fix: take waiting out of pending, set pulledFor.current = userId before awaiting the flush (later pushes then go through the debounce as usual), or re-check pending after the await.

Non-blocking notes

• "The last pending change is flushed when leaving the page" only covers unmount (SPA navigation). Closing the tab or reloading within the 1.2 s window drops the PUT, and so does any failed PUT (offline). In both cases the next arrival lets the older server copy overwrite the newer local one. So "a failed request costs nothing" is only true until the next page load. The board has the same trade-off, so I'm fine leaving it. If you want to close it cheaply: flush queued on pagehide with fetch(..., { keepalive: true }).
• The [userId] cleanup flushes the queued layout with whatever token is current when userId changes. That's harmless right now since logout reloads, but it's worth a comment if user switching ever happens without a reload.

Backend half (sprintstart-backend#270) looks good to me.

[BabuPlk]

Thanks for the thorough review! Both points are fixed in fabd4a8.

1. Reset undone by another device
This browser now remembers per user whether it has synced the layout with the server at least once (readLayoutSynced / markLayoutSynced in storage.ts). I used a separate key instead of a field on the stored layout because it has to survive a reset: a reset here clears the layout, but this browser still knows the server.

On arrival, when the server has no layout:

• local copy that was synced before → it's a stale copy of a layout reset elsewhere → clearStoredLayout + onPulled(), nothing is uploaded
• local copy that was never synced → migrated up as before, and only marked as synced once the upload succeeded (otherwise a failed upload would look like a reset on the next visit)

Tests: "drops a stale copy instead of bringing back a layout that was reset on another device" and "does not treat a failed migration as synced".

2. Change dropped while the held change is flushed
The arrival effect now takes the held change out of pending and sets pulledFor right after the read, before awaiting the flush, so a push during the flush goes through the debounce as usual. Test: "still sends a change made while the held one is being sent".

Non-blocking

• Added a comment on the [userId] cleanup about the token used for the flush.
• I left out the pagehide flush as you offered: apiClient gets the token asynchronously, so a keepalive request from pagehide often wouldn't go out anyway. Same trade-off as the board.
• One gap is left, and your suggestion has it too: a device that changed the layout offline while it was reset elsewhere drops that offline change. I think that's acceptable.

Dashboard tests: 87/87 passing locally.

[DavidLeuter]

Thanks for the quick turnaround. Point 2 (the race) is fixed cleanly and the test covers it. I agree on skipping pagehide, and the offline-change-vs-reset gap you mention is acceptable.

Point 1 fixes the resurrection, but the synced flag introduces a new failure mode that's worse than the one it replaces: the flag says "this browser has talked to the server once", but it gets read as "the local copy is what the server had". Those two diverge as soon as a PUT after the first sync doesn't go through, and then the whole local layout gets deleted.

Traced through the code:

1. First visit on a device. Server empty, nothing local → the last else branch → markLayoutSynced.
2. The user arranges their dashboard. The debounced PUT fails (backend briefly down, offline, or the tab is closed within the 1.2 s window).
3. Next visit: server empty, local && readLayoutSynced(userId) → clearStoredLayout → the arrangement is gone, on the device it was made on.

It happens the same way after a reset on this device (reset marks synced → the user rearranges → the PUT fails → next visit wipes it). Before this commit, a failed PUT cost at most the latest change (the older server copy won). Now it can cost the whole layout.

Suggested fix: make the flag mean "local == server". Clear it on every local write and only set it after a successful exchange:

• push() (or apply in useDashboardLayout) → markLayoutUnsynced(userId)
• successful pull / PUT / DELETE → markLayoutSynced(userId), as now

Then on arrival with an empty server: local && synced → reset elsewhere → drop it; local && !synced → upload it (migration, or a change that never made it up). The second case now also covers "edited offline while it was reset elsewhere" (the newer local statement wins instead of being dropped), so it closes the gap you mentioned too.

One detail with that: the .then(() => markLayoutSynced(userId)) on the debounced PUT would mark a newer local change as synced if another push happened while that PUT was in flight. Only marking when queued.current === null && !timer.current at resolve time (or comparing a small revision counter) avoids that.

A test for the scenario above would pin it: nothing anywhere → sync settles → push(LOCAL) with saveLayout rejecting → remount with the server still empty → expect LOCAL to be uploaded rather than cleared.

Rest of the diff looks good. CI was still running when I looked.

[BabuPlk]

Good catch — agreed, the flag was answering the wrong question. Fixed in 2b22472, along the lines you suggested:

• The flag now means "local == server". Every local change (push) and every reset clears it (markLayoutUnsynced); it is only set after a request confirmed both sides agree (successful pull / PUT / DELETE).
• In-flight PUTs: a small revision counter counts local changes; a PUT or DELETE only marks the layout as synced if the revision is still the one it was sent at. So a newer change made while an earlier PUT was on its way is never vouched for.
• The flush on unmount deliberately never marks synced: by the time it answers, the next mount may already have changed the layout again, and "not in sync" only ever costs one upload of what the server already has.

On arrival with an empty server: local && synced → reset elsewhere → dropped; local && !synced → uploaded (migration, or a change whose PUT failed). As you said, that also closes the "edited offline while reset elsewhere" gap — the newer local change wins now.

New tests:

• "uploads a change whose PUT failed instead of reading it as a reset elsewhere" — your scenario: nothing anywhere → settle → push(LOCAL) with the PUT rejecting → remount with an empty server → LOCAL is uploaded, not cleared
• "does not vouch for a change made while an earlier PUT was on its way"

Dashboard tests: 89/89 passing locally.

[DavidLeuter]

Thanks, this is the right shape now. Approving.

I went through the arrival paths again against 2b22472:

• Server has a layout → it wins, marked in sync, no await in between, so no revision check is needed there. ✓
• Server empty, local in sync → reset elsewhere, dropped. ✓
• Server empty, local not in sync → uploaded. That covers the old migration, a failed PUT, and an offline edit made while it was reset elsewhere. ✓
• Held change / held reset / in-flight PUT → confirmSynced(sentAt) only vouches for what was actually sent, and a newer push clears the flag first. ✓
• The unmount flush that never marks in sync is a good call. The worst case is one redundant upload.

Both new tests pin exactly the scenarios from my last review.

The one trade-off left is the same one the board has: a failed PUT, followed by a layout that a different device stored on the server, loses to the server copy on the next visit. Neither side is clearly newer there, so I'm fine with server-wins.

CI was still queued when I looked, so please merge once it's green.