[_]: fix: update draft handling in compose message component and hooks#70
[_]: fix: update draft handling in compose message component and hooks#70jzunigax2 wants to merge 2 commits into
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.
📝 WalkthroughWalkthroughDraft autosave logic in ChangesDraft save coordination and send integration
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 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: |
d319bea
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4171a68b.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
| const handleDraftDiscard = useCallback(async () => { | ||
| if (!draftIdRef.current) return; | ||
| await discardDraft({ draftId: draftIdRef.current }).unwrap(); | ||
| }, [discardDraft]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'handleDraftDiscard|clearDraftRef|scheduleAutosave' -C3 src/components/compose-messageRepository: internxt/mail-web
Length of output: 7967
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/components/compose-message/hooks/useDraftMessage.ts --view expanded
printf '\n--- FILE (relevant sections) ---\n'
sed -n '1,260p' src/components/compose-message/hooks/useDraftMessage.ts
printf '\n--- TESTS / USAGE ---\n'
rg -n "useDraftMessage|handleDraftDiscard|clearAutosaveTimer|discardDraft|scheduleAutosave|saveDraft\(" src/components/compose-message -g '!**/node_modules/**'Repository: internxt/mail-web
Length of output: 13183
Cancel the autosave timer before discarding the draft. handleDraftDiscard() should call clearAutosaveTimer() before awaiting discardDraft(...).unwrap(). Otherwise a pending autosave can fire while draftIdRef.current still points at the draft and issue updateDraft({ draftId }) against a draft that is being deleted.
🤖 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 `@src/components/compose-message/hooks/useDraftMessage.ts` around lines 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.
2 participants
flushPendingDraftSaveto manage draft saving more effectively.draftIdwithresolveDraftIdinuseComposeSendfor better draft resolution.saveDraftlogic to prevent duplicate saves during concurrent operations.