feat(email): shift+e marks thread not done; undoable archive toasts#5033
Conversation
- Add shift+e hotkey in the email thread view to mark a done thread as not done (inverse of 'e') - Route the thread view's direct archive/unarchive fallback through an undoable mutation so both directions get a Marked as done / Marked as not done toast with an Undo button, matching the soup mark-done action - On undo of a soup mark-done that navigated to the next item, navigate back to the restored entity via the caller's navigation callback - Fix archive mutationFn not awaiting the request (void throwOnErr): the settle-time refetch raced the in-flight PATCH and could clobber the optimistic inbox_visible flip, making 'e' mark as done twice in a row instead of toggling; onError also never fired Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughEmail threads now support undoable archive and unarchive mutations with optimistic cache updates, rollback, query invalidation, and undo/redo toast actions. The email context exposes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/src/lib/queries/email/thread.ts (1)
274-404: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSignificant duplication between
useArchiveThreadMutation,useUndoableArchiveThreadMutation, andreplayThreadArchive.All three implement the same
throwOnErr(emailClient.flagArchived(...))call, the same rollback-on-error againstcontext.previousData, and the same pair ofinvalidateQueriescalls. Extracting a shared "perform archive + rollback + invalidate" primitive would reduce the risk of the three copies drifting (e.g. the recent await-fix only touched two of the three call sites).♻️ Sketch of a shared helper
+async function performArchiveRequest(params: ArchiveThreadParams) { + await throwOnErr( + async () => + await emailClient.flagArchived( + { id: params.threadId, value: params.archive }, + params.linkId + ) + ); +}Then
mutationFnin both mutations andreplayThreadArchive's try-block can callperformArchiveRequest(params).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/queries/email/thread.ts` around lines 274 - 404, Extract the shared archive operation used by useArchiveThreadMutation, useUndoableArchiveThreadMutation, and replayThreadArchive into a single performArchiveRequest helper. Centralize the emailClient.flagArchived call, throwOnErr handling, rollback using the prior thread data on failure, and both query invalidations in that helper, then update all three callers to use it while preserving their existing callback behavior and error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/features/block-email/component/EmailContext.tsx`:
- Around line 404-406: Update the onError handling in EmailContext to
distinguish archive and unarchive failures, using the mutation parameters
exposed by useUndoableArchiveThreadMutation (or the consolidated signature
change) to select the matching archive or mark-not-done error message. Preserve
the existing direction-specific success behavior.
In `@apps/web/src/lib/queries/email/thread.ts`:
- Around line 361-390: The archive mutation’s onError callback discards the
mutation parameters, preventing callers from distinguishing archive versus
unarchive failures. Update the onError option type and the useUndoableMutation
onError handler around threadArchiveOnMutate to pass the relevant
ArchiveThreadParams to options.onError, and update its callers to use
params.archive when selecting the failure message.
- Around line 355-404: Update useUndoableArchiveThreadMutation so the undoLabel
is determined from params.archive, using the appropriate action label for
archive versus unarchive instead of always returning “Mark Done.” Keep the
existing undo and redo behavior unchanged.
---
Nitpick comments:
In `@apps/web/src/lib/queries/email/thread.ts`:
- Around line 274-404: Extract the shared archive operation used by
useArchiveThreadMutation, useUndoableArchiveThreadMutation, and
replayThreadArchive into a single performArchiveRequest helper. Centralize the
emailClient.flagArchived call, throwOnErr handling, rollback using the prior
thread data on failure, and both query invalidations in that helper, then update
all three callers to use it while preserving their existing callback behavior
and error propagation.
🪄 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: CHILL
Plan: Pro
Run ID: cd7f8691-fbb0-45b6-b62e-b3d7fc3ca474
📒 Files selected for processing (6)
apps/web/src/features/block-email/component/Email.tsxapps/web/src/features/block-email/component/EmailContext.tsxapps/web/src/features/block-email/util/emailHotkeys.tsapps/web/src/features/next-soup/actions/make-mark-done-action.tsapps/web/src/lib/core/hotkey/tokens.tsapps/web/src/lib/queries/email/thread.ts
Direction-specific failure toast and undo label for the archive mutation: onError now receives the mutation params, and undoLabel accepts a per-entry function of the variables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Adds a dedicated
shift+e"mark not done" hotkey to the email thread view and fixes feedback gaps + a race in the thread archive flow.Changes
shift+emarks a thread as not done in the email thread view — the explicit inverse ofe. No-op when the thread is already in the inbox. (Newentity.action.markNotDonehotkey token.)archiveMutation. That path now uses a newuseUndoableArchiveThreadMutation, so both directions show the same "Marked as done" / "Marked as not done" toast with an Undo button as the soup mark-done action, and entries land on the mutation undo stack (cmd+zworks). Undo/redo replays the oppositeflagArchivedcall with the same optimisticinbox_visiblehandling, preserving theX-Email-Link-Idheader for non-primary inboxes.mutationFnwasvoid throwOnErr(...), so the mutation "succeeded" instantly: the settle-time thread refetch raced the in-flight PATCH and, when the GET won, clobbered the optimisticinbox_visibleflip — makingemark the same thread as done twice in a row instead of toggling. It also meantonErrornever fired on real failures. Both the existinguseArchiveThreadMutationand the new undoable variant now await the request.