feat: undo mark-as-done + notification refetch cleanup#2701
Merged
Conversation
Shows a toast with an "Undo" button for 10s after marking items done, restoring notifications and unarchiving emails when clicked.
Moves action buttons into the header row between the title and close button instead of a separate row below, making toasts more compact when they have a single action like "Undo".
Exposes markNotificationsDone() returning a reversible {done, undo}
handle. Desktop stack rows and mobile swipe-left now show a toast with
an Undo button for 10s after marking a stack done.
invalidateQueries on the infinite notification query refetches every cached page, which can be 15+ pages of 500 items each. The optimistic update already keeps the cache consistent, so pass refetchType: 'none' — matching the existing useMarkNotificationsAsDoneMutation pattern.
The "Undone" confirmation toast stacked on top of the "Marked as done" toast, adding noise. The caller already saw the previous toast get dismissed and the item reappear — surface a toast only if undo fails.
optimisticInsertNotification already prepends the incoming notification to the cache via setQueriesData. The follow-up invalidateUserNotifications call then refetches every cached page of the infinite query -- 15+ requests for a single incoming message. Drop the refetch with refetchType: none. The other invalidate call sites (mobile visibilitychange, push notification tap) still refetch fully where it is actually needed.
Previous rollback did `setQueryData(key, previousData)`, wholesale restoring the snapshot captured before mark-done. Any notifications inserted via websocket in between (e.g. a newly-sent message) would be wiped on undo. Now mark/unmark just flips `done` on the targeted items in place. The rest of the cache is untouched, so concurrent inserts survive the undo window.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces undo/redo functionality for "mark as done" actions across emails and notifications. Changes include adding a Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mounts MutationUndoProvider and pushes each mark-done handle via pushUndo so the action participates in the global undo stack. The toast Undo button now drives ctx.undo() instead of calling handle.undo() directly, which is the groundwork for a Cmd+Z hotkey.
Replaces manual pushUndo calls with useUndoableMutation — onMutate kicks off the optimistic handle, mutationFn is a no-op so onSuccess runs immediately, and the undoFn handler is wired into the stack automatically. Toast + undo-entry creation now live in one place per action, so the desktop stack and mobile swipe share the hook's flow.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/next-soup/actions/make-mark-done-action.ts`:
- Around line 29-66: The current implementation starts the real network call in
onMutate and keeps mutationFn as a no-op, which causes onSuccess/undo entries to
fire before the API resolves; move the real work into mutationFn by calling
await markEntitiesDone({ entities: variables.entities, notificationSource:
notificationSource() }) inside mutationFn (remove the handle.done.catch usage),
keep onMutate only for optimistic local changes and return any minimal context
needed for undo, and implement onError to dismiss the success toast and remove
the undo entry if the network call fails; update undoFn to call the server undo
using the mutation result/context; also add a short JSDoc to makeMarkDoneAction
noting it must be used under MutationUndoProvider (useMutationUndoContext
dependency).
In `@js/app/packages/app/component/next-soup/utils.ts`:
- Around line 667-725: The emailClient.flagArchived calls in the done and undo
flows return result objects instead of throwing, so success paths may ignore API
failures; wrap or replace the map entries using the existing throwOnErr helper
so each flagArchived call throws on an error result (similar to how
notificationServiceClient.bulkMarkNotificationAsDone/Undone are wrapped), e.g.
map each id to throwOnErr(async () => await emailClient.flagArchived({...})) so
failures trigger rollback and are propagated from done/undo; keep the
notification bulk calls unchanged and preserve the rollback/invalidateSoupEntity
behavior.
- Around line 611-631: Await the in-flight query cancellations before taking the
snapshot and performing optimistic cache mutations: change the two non-awaited
calls to queryClient.cancelQueries({ queryKey: queryKeys.all.email }) and
queryClient.cancelQueries({ queryKey: notificationKeys.user._def }) so they are
awaited (await queryClient.cancelQueries(...)) before reading previousEmail
(queryClient.getQueriesData) and before calling
removeSoupEntities/emailIdSet-based mutations and queryClient.setQueryData; this
ensures no in-flight email/notification queries can resolve and overwrite the
optimistic update.
In `@js/app/packages/entity/src/extractors-notification/notification-actions.tsx`:
- Around line 42-55: The current onSuccess handler in notification-actions.tsx
calls undoCtx.undo() (global latest) when the toast "Undo" is clicked, causing
older toasts to undo the newest mutation; instead capture a mutation-specific
undo handle/token when performing the mutation (or have the mutation response
return an undoId) and store it in the onSuccess closure (e.g., map toastId ->
undoId or capture undoId local to the toast), then invoke undoCtx.undo(undoId)
from the toast's onClick; alternatively mark older toasts as non-actionable by
disabling the Undo button when a newer undoable mutation is pushed. Ensure you
stop calling the global undoCtx.undo() with no args and use the
mutation-specific identifier tied to the toast created by toast.success.
In `@js/app/packages/notifications/notification-helpers.ts`:
- Around line 277-298: The optimistic undo handler (the undo async arrow
function) flips the cache with setDoneFlag(false) before the server confirms;
modify it so that if notificationServiceClient.bulkMarkNotificationAsUndone
(wrapped by throwOnErr) throws, you revert the optimistic change by calling
setDoneFlag(true) in the catch path before rethrowing or returning, then keep
the final queryClient.invalidateQueries call (notificationKeys.user._def) as-is
so the cache is revalidated; basically ensure the undo block around
throwOnErr/bulkMarkNotificationAsUndone re-applies setDoneFlag(true) on failure
to roll back the UI state.
- Around line 227-249: The call to queryClient.cancelQueries({ queryKey:
notificationKeys.user._def }) is not awaited, so in-flight fetches can complete
and overwrite the optimistic update; change the flow to await
queryClient.cancelQueries(...) before running the optimistic cache mutation (the
setDoneFlag logic that calls queryClient.setQueriesData) so cancellation
completes first, then apply the in-place update and proceed with the rest of the
mutation flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ef86609-df88-4040-9627-365734b24570
📒 Files selected for processing (11)
js/app/packages/app/component/Root.tsxjs/app/packages/app/component/next-soup/actions/make-mark-done-action.tsjs/app/packages/app/component/next-soup/utils.tsjs/app/packages/core/component/Toast/Toast.tsxjs/app/packages/entity/src/extractors-notification/mobile-notification-stacks.tsxjs/app/packages/entity/src/extractors-notification/notification-actions.tsxjs/app/packages/entity/src/extractors-notification/notification-stacks.tsxjs/app/packages/notifications/index.tsjs/app/packages/notifications/notification-helpers.tsjs/app/packages/queries/notification/user-notifications.tsjs/app/packages/service-clients/service-notification/client.ts
- await cancelQueries up front before taking the optimistic snapshot so a stale in-flight fetch cannot clobber the flip (TanStack v5 pattern). markEntitiesDone and markNotificationsDone are now async. - Wrap emailClient.flagArchived with throwOnErr — it returns a result object, not a throwing promise, so failures were being swallowed. - Re-apply the optimistic done state when undo fails, keeping UI and server in sync when bulk/undone or unarchive errors. - Toast Undo button now calls this action's own handle.undo() rather than undoCtx.undo() (which pops the global latest entry). Older toasts no longer end up undoing newer actions. - When handle.done rejects, dismiss the stale success toast before surfacing the failure toast. - Factor the optimistic email/soup work so it can be re-run on undo failure with a fresh soup transaction. - Document that makeMarkDoneAction must run inside MutationUndoProvider.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/next-soup/actions/make-mark-done-action.ts`:
- Around line 24-86: The optimistic flow currently pushes an undo entry
immediately (via useUndoableMutation's onSuccess) but never removes it if the
background API (handle.done) fails; update makeMarkDoneAction so
handle.done.catch also removes the stale undo entry from the global undo stack:
in onMutate capture a removal/cancel callback (or the mutation's pushUndo
handle) alongside the handle and in handle.done.catch invoke that removal, or
instead delay pushing the undo (move the pushUndo/onSuccess work into
handle.done.then) so the undo entry is only pushed when handle.done resolves;
reference useUndoableMutation, onMutate, handle.done.catch, onSuccess and undoFn
when making the change.
In `@js/app/packages/app/component/next-soup/utils.ts`:
- Around line 755-759: The catch block that currently calls applyOptimistic()
indiscriminately causes re-archiving of items even when some unarchive/undo
calls succeeded; change the undo flow to use Promise.allSettled when invoking
flagArchived({ value: false }) and bulkMarkNotificationAsUndone, inspect the
settled results, and build a list of only the IDs whose corresponding API
promise rejected, then call applyOptimistic() (or a new helper) with that subset
so you only re-apply optimistic "done/archived" state for items that actually
failed; update functions flagArchived, bulkMarkNotificationAsUndone, and the
catch handling around them to correlate promises to IDs and use the
rejected-results to drive the re-apply.
- Around line 683-719: The current done async IIFE uses Promise.all which aborts
on first failure and then always invalidates queries with refetchType: 'none',
causing UI/server divergence; change the implementation in the done block (the
Promise.all over emailIds → throwOnErr wrapping emailClient.flagArchived and the
optional notificationServiceClient.bulkMarkNotificationAsDone call) to either:
A) use Promise.allSettled to collect per-id success/failure, then apply a
partial rollback via rollback only for the ids that failed (and only
optimistic-unmark those ids), or B) on any rejection force a foreground refetch
by calling queryClient.invalidateQueries without refetchType: 'none' (or
explicitly triggering a refetch) so the UI reconciles with server state; ensure
you update the finally/invalidation logic (invalidateSoupEntity(emailId) and
notificationKeys/user invalidation) to reflect the chosen approach and reference
the emailIds/notificationIds results when computing partial rollback.
- Around line 618-641: previousEmail snapshot/restore causes lost concurrent
email inserts; replace full snapshot logic in
previousEmail/filterEmailCache/restoreEmailCache with an in-place per-id
approach: when applying (applyOptimistic) iterate current queries
(queryClient.getQueriesData) and remove only items whose ids are in emailIdSet
while simultaneously storing a Map of removed EntityData by id (not whole
pages); on rollback/undo use that Map to re-insert the exact removed entities
into the current pages (merge/splice into pages items in-place) rather than
calling setQueryData with the old snapshot, mirroring the markEntitiesDone /
setNotificationDoneFlag pattern so websocket/fetched arrivals between apply and
undo are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52da462a-ba6c-4722-a789-24d5102e06a1
📒 Files selected for processing (4)
js/app/packages/app/component/next-soup/actions/make-mark-done-action.tsjs/app/packages/app/component/next-soup/utils.tsjs/app/packages/entity/src/extractors-notification/notification-actions.tsxjs/app/packages/notifications/notification-helpers.ts
Clicking the toast Undo button moved focus onto the toast action button (rendered in a portal outside the soup view), so the soup view's focusin listener never refired and the hotkey scope stayed unset — j/k navigation silently stopped working. Call restoreSoupFocus() at the end of the toast's onClick so focus lands back on an entity row and the scope reactivates.
…pFocus Entity rows have no explicit tabindex, so `.focus()` on them was a silent no-op — restoreSoupFocus "succeeded" without moving DOM focus and the hotkey scope stayed wherever the previous focus left it (typically BODY after a toast dismiss). j/k navigation was dead until the next soup interaction. Only the soup container itself has tabindex=-1, which is what actually reactivates the scope. Fall back to focusing the container if neither the target entity nor the first entity accepts focus.
An in-flight page-1 fetch could land after the optimistic cache flip and overwrite `done=true` back to `done=false`, making the stack disappear then reappear right after mark-as-done. The race was easy to hit during initial page load while the notification infinite query was still cascading through its pages. Move the optimistic state out of the query cache entirely: a module- level Map tracks id -> expected done value, and the notifications memo in notification-source applies the override when reading cache data. Stale fetch writes land in the cache but the UI reads through the memo, so they no longer clobber the optimistic state.
Addresses the remaining PR feedback on markEntitiesDone: - Email cache used a full snapshot/restore; emails arriving via websocket during the undo window were lost on rollback. Now we snapshot only the specific emails we filter out (keyed by query) and re-insert just those on rollback, leaving concurrent inserts intact. - Promise.all over the archive + notification bulk call was all-or-nothing: a single failure caused full rollback even when some API calls had succeeded server-side, leaving the server and UI diverged. Switch to Promise.allSettled, classify each result, and roll back only the failed ids. Same approach on the undo path. - On any partial failure, force a refetch (not refetchType: 'none') so server state reconciles against the mixed rollback.
gbirman
force-pushed
the
macro-czfK6ViJkGUXhMhaoX9gp-undo-mark-as-done
branch
from
19096b9 to
1fc2937
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/next-soup/utils.ts`:
- Around line 596-600: The JSDoc for markEntitiesDone is stale: it states the
function "Returns synchronously" but markEntitiesDone is async and awaits
Promise.all([cancelQueries, cancelQueries]) before returning; either update the
comment to remove the synchronous-return guarantee, or preserve the sync
behavior by moving the cancelQueries awaits into the inner done IIFE so that the
function produces the handle and calls applyOptimistic() synchronously (matching
trashEmails) while the background awaits happen inside the IIFE; locate symbols
markEntitiesDone, cancelQueries, applyOptimistic, and the done IIFE to apply the
change.
- Around line 669-685: The pages.map callback inside the
queryClient.setQueryData in restoreEmailCache uses a nested ternary that reduces
readability; replace the nested ternary in the pages.map((page, idx) => ...)
callback with an explicit if/else chain: compute filtered and then if idx === 0
return { ...page, items: [...toRestore, ...filtered] }, else if filtered.length
=== page.items.length return page, else return { ...page, items: filtered }.
Keep the existing variables (toRestore, restoredIds, EmailCacheData) and
preserve all other logic and return shapes.
In `@js/app/packages/entity/src/extractors-notification/notification-actions.tsx`:
- Around line 36-89: The mutation created with useUndoableMutation (see
mutation, onMutate and markStackAsDone) needs an onError handler to catch
failures from onMutate (e.g., markNotificationsDone rejecting) so the UI/logs
show a failure and we don't leave the mutation in an unhandled error state; add
an onError callback alongside onMutate/onSuccess/undoFn/redoFn that logs or
shows a failure toast (consistent with the make-mark-done-action pattern), and
ensure it safely handles a missing context.handle (i.e., don't assume handle
exists when onMutate fails).
In `@js/app/packages/notifications/notification-source.ts`:
- Around line 89-110: doneOverrides (created by createRoot and mutated by
setDoneOverride) accumulates forever because entries are only removed when
callers pass done: undefined; fix by pruning redundant overrides after
successful reconciliation: in the success path of markNotificationsDone (after
invalidateQueries/reconciliation) remove ids from doneOverrides whose cache
entry already matches the expected done state, or alternatively prune entries
during the notifications memo by removing keys not present in
notificationsQuery.data; update setDoneOverride or add a helper (e.g.,
pruneDoneOverrides) to accept an array of ids to delete and call it from
markNotificationsDone or the notifications memo so doneOverrides no longer grows
unbounded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3df8b1e9-521e-4c09-8064-8ca44bccb178
📒 Files selected for processing (9)
js/app/packages/app/component/GlobalUndoHotkeys.tsxjs/app/packages/app/component/Root.tsxjs/app/packages/app/component/next-soup/actions/make-mark-done-action.tsjs/app/packages/app/component/next-soup/utils.tsjs/app/packages/core/hotkey/tokens.tsjs/app/packages/entity/src/extractors-notification/notification-actions.tsxjs/app/packages/notifications/index.tsjs/app/packages/notifications/notification-helpers.tsjs/app/packages/notifications/notification-source.ts
- Update stale JSDoc on markEntitiesDone (function is async since the cancelQueries await was added). - Flatten the nested ternary inside restoreEmailCache's page map to if/else per the repo coding guideline. - Add onError handlers to the mark-done and mark-stack-done mutations so failures in onMutate (e.g. markNotificationsDone rejecting before returning the handle) surface a toast instead of landing silently. - Prune doneOverrides when the underlying notification is no longer in the query cache, or when the cache has caught up to the override value. Keeps the map bounded for long-lived sessions.
…n notifications-only markEntitiesDone writes directly to the email cache and soup cache (both clobberable by a stale in-flight fetch), so it keeps its async signature and awaits cancelQueries before the optimistic writes. markNotificationsDone only mutates the done-override map — which lives outside any query cache and is applied in the `notifications` memo — so it can return synchronously and move its cancelQueries inside the `done` IIFE without risking a race. Also updates the JSDoc on both functions to accurately describe their new shapes.
`toast.success` de-dupes by default via a 3s throttle keyed on
`${type}:${message}`, so two rapid "Marked as done" toasts collapsed
into one and the second click's Undo button replaced the first — users
lost the ability to undo an earlier action.
Add a `stack` param to `toast.success` / `createToast` that bypasses
the `recentToasts` tracking entirely. Mark-done toasts pass `true` so
each action keeps its own toast and its own Undo handle.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/next-soup/actions/make-mark-done-action.ts`:
- Around line 66-84: The toast onClick currently calls handle?.undo() directly
which bypasses the mutation's undo/redo bookkeeping (leaving a stale global undo
entry); instead, have the toast call the same mutation-provider undo
token/handle used by the mutation (or remove/update the global entry) so the
stack stays consistent: use the mutation's provided context.handle token rather
than calling handle?.undo() directly in the onClick, ensure you update/remove
the pushed undo entry via the provider API when the toast performs the undo, and
add a corresponding redoFn on the mutation that reissues the mark-done mutation
and refreshes the context handle so redo (Shift+Cmd+Z) works correctly; keep
restoreSoupFocus(firstEntityId, inPreview) after the guarded provider undo call.
In `@js/app/packages/app/component/next-soup/utils.ts`:
- Around line 695-710: The rollback function currently clears notification
overrides by calling setDoneOverride(notificationsToRoll, undefined) which is
correct for a failed mark-done but not for an undo; change rollback so that when
it's used for an undo it sets the override to false (call
setDoneOverride(notificationsToRoll, false)) instead of undefined, while
preserving the undefined behavior for failed-mark-done paths; update the other
occurrence noted (the similar call around the block referenced at lines 789-790)
to the same conditional behavior; locate the rollback implementation and the
other setDoneOverride call and make the change using the
notificationsToRoll/emailIds variables and restoreEmailCache/soupTxn logic
already present to determine the proper undo vs failure path.
In `@js/app/packages/entity/src/extractors-notification/notification-actions.tsx`:
- Around line 63-82: The toast's onClick currently calls handle?.undo() directly
which bypasses the mutation's undo bookkeeping and leaves the global undo stack
inconsistent; instead, route the toast action through the same undo entry
created by useUndoableMutation by invoking the mutation's undo handler/token
(the undo path that corresponds to undoFn) so the provider stack is updated (or
remove the pushed entry) when the toast performs the undo. Concretely: change
the onClick in the toast to call the mutation-managed undo entry (use the same
context/handle provided by the useUndoableMutation result rather than calling
handle?.undo() directly), ensure the mutation registers a matching redoFn or
removes its global undo entry when consumed, and keep usage of
context?.setSuccessToastId/toastId intact so the toast and provider remain in
sync.
In `@js/app/packages/notifications/notification-helpers.ts`:
- Around line 231-235: The direct calls to
notificationServiceClient.bulkMarkNotificationAsDone/Undone in
notification-helpers.ts must be moved behind the queries layer: create wrapper
functions (e.g., bulkMarkNotificationAsDone and bulkMarkNotificationAsUndone) in
the queries package module `@queries/notification/user-notifications` that perform
the serviceClient call inside a TanStack Query/mutation, export those functions,
and then replace the direct notificationServiceClient.* invocations in
notification-helpers.ts (the await throwOnErr(...) blocks around
bulkMarkNotificationAsDone and the corresponding undone code at lines ~260-264)
with calls to the new query wrappers and their mutation runners; update imports
to use the new `@queries` module and remove direct service client usage so all
network calls flow through the queries package.
In `@js/app/packages/notifications/notification-source.ts`:
- Around line 141-152: The current createEffect prunes doneOverrides eagerly by
comparing overrides to notificationsQuery.data (in createEffect,
notificationsQuery, doneOverrides, setDoneOverride), which can race with
in-flight mutations; stop removing overrides here and instead have the owning
mutation clear its override only after a successful reconciliation (i.e., remove
the setDoneOverride(toPrune, undefined) call from this createEffect), or
implement an override timestamp/version on doneOverrides and only prune an
override when notificationsQuery's fetch timestamp/version is newer than the
override's timestamp (compare notificationsQuery.fetchTime/lastUpdated with the
override timestamp before calling setDoneOverride).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b46ba1f4-d3f4-461a-9f58-5f067a29b780
📒 Files selected for processing (6)
js/app/packages/app/component/next-soup/actions/make-mark-done-action.tsjs/app/packages/app/component/next-soup/utils.tsjs/app/packages/core/component/Toast/Toast.tsxjs/app/packages/entity/src/extractors-notification/notification-actions.tsxjs/app/packages/notifications/notification-helpers.tsjs/app/packages/notifications/notification-source.ts
synoet
reviewed
Apr 21, 2026
…+ executeUndone
Replaces the handle-based markEntitiesDone / markNotificationsDone API
with pure functions that slot into useUndoableMutation's expected
shape:
- applyEntitiesDoneOptimistic(vars): returns { rollback, reapply }
— called from onMutate for synchronous optimistic UI updates.
- executeMarkEntitiesDone / executeMarkEntitiesUndone: pure API
calls — go into mutationFn / undoFn / redoFn.
- resolveMarkEntitiesDoneVariables: captures email + notification ids
at mutation time so all three handlers see the same snapshot.
Removes the no-op mutationFn + handle.done.catch dance, the
setSuccessToastId back-channel context, and the handle.undo / handle.redo
plumbing. mutation.isPending / isError now reflect real API state;
onError fires on real API failure.
…semantics Addresses CodeRabbit review feedback: - Move service-client calls into @queries: add bulkMarkNotificationsAsDone / bulkMarkNotificationsAsUndone wrappers in user-notifications.ts and call them from the execute helpers and utils.ts. All network calls to notificationServiceClient now go through the queries layer. - Split the mark-entities-done context into rollback / reapply / applyUndone. undoFn and the toast's Undo path now use applyUndone (override=false) instead of clearing the override, so if the cache has already reconciled to done=true the UI still flips back to not-done. - Toast Undo button routes through undoCtx.undo() so the undo stack stays synced — Cmd+Z and shift+Cmd+Z no longer see a stale entry. - Only prune overrides for notifications that have aged out of the query cache; matching-value pruning was too eager and could drop an override mid-mutation while the pre-mutation cache value still lived in the infinite query pages.
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
2 participants
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.
Summary
invalidateQuerieswas re-fetching every cached page (15+ requests for a single incoming message).doneflag on targeted items, so newer notifications delivered by the websocket during the undo window are preserved.Notes
ehotkey, soup action menu) and to individual notification stacks (desktop check button / context menu /eon focused row, mobile swipe-left).bulkMarkNotificationAsUndoneclient method hits the existingPATCH /v2/user_notifications/bulk/undoneendpoint.