fix(useNotifications): persist interaction state, not notification snapshots - #893
Open
johnleider wants to merge 1 commit into
Open
fix(useNotifications): persist interaction state, not notification snapshots#893johnleider wants to merge 1 commit into
johnleider wants to merge 1 commit into
Conversation
…apshots
The merged persist option stored whole notifications (subject, body,
severity, arbitrary data) and restore re-registered them wholesale —
deleted notifications resurrected forever, content duplicated the code
source of truth, payloads landed in storage, and expired snoozes
restored unfiltered. Persistence is now a map of id → { readAt, seenAt,
archivedAt, snoozedUntil } merged onto notifications the app actually
registers: restore never creates tickets, a register:ticket
subscription merges saved state onto late arrivals (adapter and
runtime), expired snoozes drop at both coercion and write, unregistered
ids prune naturally on the next write, and the old array format is
rejected by the shape guard. Storage wins for interaction fields on
merge; adapters win for content.
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix-forward for the persist option merged in #384, completing the persist-doctrine pair with #892 (never duplicate a source of truth code recreates on boot).
Was: whole-notification snapshots (subject/body/severity/arbitrary
data) re-registered wholesale on restore — deleted notifications resurrected forever, content duplicated code, payloads hit storage, expired snoozes came back.Now:
Record<id, { readAt?, seenAt?, archivedAt?, snoozedUntil? }>— interaction state only, merged onto notifications the app actually registers. Restore never creates tickets; aregister:ticketsubscription merges saved state onto late arrivals (adapter setup, runtime sends, and the AppBanner's boot registration all ride the same path); expired snoozes drop at coercion and write; unregistered ids GC naturally on the next write;UNSAFE_KEYSguarded; numeric ids round-trip; the old array format is rejected outright (dev never shipped, so a hard schema break was cleared).Judgment call (flag for review): on merge, storage wins for interaction fields, adapters win for content — a snooze is a local user choice; server-side read-state sync would be a different feature. One-line change in
mergeif you want adapter interaction state to win instead.Browser-proven on the real consumer: banner snooze → storage holds only the interaction map (zero content) → reload holds → hand-expired snooze reappears. AppBanner needed no changes.
Changeset edited in place (one changelog entry at the minor). useNotifications 124/124; full sweep 4,765 green; typecheck clean.