[_]: fix: update draft handling in compose message component and hooks#70
Conversation
- Introduced `flushPendingDraftSave` to manage draft saving more effectively. - Replaced `draftId` with `resolveDraftId` in `useComposeSend` for better draft resolution. - Enhanced `saveDraft` logic to prevent duplicate saves during concurrent operations. - Updated tests to reflect changes in draft handling and ensure correct functionality.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDraft autosave now queues concurrent saves and exposes asynchronous draft ID resolution. Sending awaits that resolver, and the compose dialog passes it through. Tests cover pending saves, autosave re-arming, resolved draft IDs, and new-email sends. ChangesDraft save coordination and send integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 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 `@src/components/compose-message/hooks/useComposeSend.test.ts`:
- Around line 314-315: The affected test descriptions in useComposeSend.test
should be rewritten to follow the “When…then…” style without naming
implementation details like sendEmail or draftId. Update the descriptions for
the related tests in the useComposeSend test suite to describe the behavior in
user-facing terms, while keeping the assertions and setup unchanged.
In `@src/components/compose-message/hooks/useDraftMessage.ts`:
- Around line 130-164: Extract the repeated autosave timer cleanup in
useDraftMessage into a shared helper (for example a clearAutosaveTimer-style
function) and reuse it in saveDraft, flushPendingDraftSave, clearDraftRef, and
scheduleAutosave. Make sure the helper still nulls timerRef.current after
clearTimeout, and update each useCallback’s dependency array to include the new
helper so the teardown logic stays consistent and DRY.
🪄 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: 680672d6-f875-4472-8563-a33674b6e4d6
📒 Files selected for processing (5)
src/components/compose-message/hooks/useComposeSend.test.tssrc/components/compose-message/hooks/useComposeSend.tssrc/components/compose-message/hooks/useDraftMessage.test.tssrc/components/compose-message/hooks/useDraftMessage.tssrc/components/compose-message/index.tsx
| toRecipients, | ||
| inReplyTo: inReplyItemId, | ||
| draftId: draftId ?? undefined, | ||
| resolveDraftId: flushPendingDraftSave, |
There was a problem hiding this comment.
Would be better resolvePendingDrafts or smth like that. What do you think?
… clarity - Updated the draft handling logic in the ComposeMessage component and related hooks. - Replaced `flushPendingDraftSave` with `resolveDraftId` to better reflect its purpose. - Adjusted tests to align with the new draft resolution approach and ensure accurate functionality.
Deploying mail-web with
|
| Latest commit: |
38cc752
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe8f0220.mail-web-ea0.pages.dev |
| Branch Preview URL: | https://fix-draft-handling.mail-web-ea0.pages.dev |
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 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 `@src/components/compose-message/hooks/useDraftMessage.ts`:
- Around line 167-170: Cancel the autosave timer in handleDraftDiscard before
the draft deletion request is awaited, so a pending autosave cannot race with
discardDraft and call updateDraft on the same draft; update useDraftMessage to
invoke clearAutosaveTimer() at the start of handleDraftDiscard, before
discardDraft({ draftId: draftIdRef.current }).unwrap(), and keep the guard on
draftIdRef.current intact.
🪄 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: 18d0b740-5dfb-44bf-8e7c-7f4785c4b5a6
📒 Files selected for processing (4)
src/components/compose-message/hooks/useComposeSend.test.tssrc/components/compose-message/hooks/useDraftMessage.test.tssrc/components/compose-message/hooks/useDraftMessage.tssrc/components/compose-message/index.tsx
- Added clearAutosaveTimer to the handleDraftDiscard function to ensure proper cleanup when discarding drafts. - Updated dependencies in the useCallback hook to reflect the new logic.
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 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 `@src/components/compose-message/hooks/useDraftMessage.ts`:
- Around line 167-171: Update handleDraftDiscard to await pendingSaveRef.current
immediately after clearAutosaveTimer(), before checking draftIdRef.current,
mirroring resolveDraftId(). Then discard the resolved draft ID so an in-flight
createDraft() completes before the discard request.
🪄 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: 6db74d60-8ab4-46b4-942e-a9302ac4c86a
📒 Files selected for processing (1)
src/components/compose-message/hooks/useDraftMessage.ts
- Implemented a test to verify that discarding a draft while a save is in progress waits for the save to complete before discarding. - Updated the `handleDraftDiscard` function to resolve the draft ID before attempting to discard, ensuring proper draft management.
|
2 participants
flushPendingDraftSaveto manage draft saving more effectively.draftIdwithresolveDraftIdinuseComposeSendfor better draft resolution.saveDraftlogic to prevent duplicate saves during concurrent operations.