Skip to content

[_]: fix: update draft handling in compose message component and hooks #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jzunigax2 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[_]: fix: update draft handling in compose message component and hooks #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f5c841
fix: update draft handling in compose message component and hooks
jzunigax2 Jul 8, 2026
d319bea
refactor: rename flushPendingDraftSave to resolveDraftId for improved…
jzunigax2 Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/components/compose-message/hooks/useComposeSend.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,22 +311,25 @@ describe('useComposeSend', () => {
vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock);
});

test('When sending an email opened from a draft, then the draftId is forwarded to sendEmail so the server consumes the draft', async () => {
test('When sending an email opened from a draft, then the saved draft identifier is included in the send request so the server can consume it', async () => {
const resolveDraftId = vi.fn().mockResolvedValue('draft-42');
const { result, onSent } = renderSend({
toRecipients: [recipient('bob@inxt.me')],
draftId: 'draft-42',
resolveDraftId,
});

await act(async () => {
await result.current.send();
});

expect(resolveDraftId).toHaveBeenCalled();
expect(mocks.sendEmail).toHaveBeenCalledWith(expect.objectContaining({ draftId: 'draft-42' }));
expect(onSent).toHaveBeenCalled();
});

test('When sending a brand-new email (no draftId), then sendEmail is called without a draftId', async () => {
const { result, onSent } = renderSend({ toRecipients: [recipient('bob@inxt.me')] });
test('When sending a brand-new email with no saved draft, then the send request omits any draft identifier', async () => {
const resolveDraftId = vi.fn().mockResolvedValue(null);
const { result, onSent } = renderSend({ toRecipients: [recipient('bob@inxt.me')], resolveDraftId });

await act(async () => {
await result.current.send();
Expand Down
8 changes: 5 additions & 3 deletions src/components/compose-message/hooks/useComposeSend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ interface UseComposeSendParams {
attachments: AttachmentTask[];
attachmentsSessionKey: Uint8Array;
inReplyTo?: string;
draftId?: string;
resolveDraftId?: () => Promise<string | null>;
onSent: () => void;
markResolvingInherited: (id: string) => void;
markInheritedResolved: (id: string, blobId: string) => void;
Expand Down Expand Up @@ -59,7 +59,7 @@ export const useComposeSend = ({
attachments,
attachmentsSessionKey,
inReplyTo,
draftId,
resolveDraftId,
onSent,
markResolvingInherited,
markInheritedResolved,
Expand Down Expand Up @@ -224,6 +224,8 @@ export const useComposeSend = ({

const deliveryMode = (encryptionState === 'internxt' ? 'INTERNXT' : 'EXTERNAL') as DeliveryMode;

const draftId = (await resolveDraftId?.()) ?? undefined;

await sendEmail({
to: toRecipients.map(toEmailAddress),
cc: ccRecipients.length ? ccRecipients.map(toEmailAddress) : undefined,
Expand Down Expand Up @@ -254,7 +256,7 @@ export const useComposeSend = ({
attachments,
attachmentsSessionKey,
inReplyTo,
draftId,
resolveDraftId,
triggerLookup,
sendEmail,
onSent,
Expand Down
86 changes: 84 additions & 2 deletions src/components/compose-message/hooks/useDraftMessage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ vi.mock('@/store/api/mail', () => ({
useGetMailAccountKeysQuery: () => ({ data: mocks.senderKeys }),
}));

const editor = { getHTML: () => '<p>hi</p>', getText: () => 'hi' } as unknown as Editor;
const editor = { getHTML: () => '<p>hi</p>', getText: () => 'hi', on: vi.fn(), off: vi.fn() } as unknown as Editor;
const recipient = (email: string): Recipient => ({ id: email, email });

const mockEncryptionBlock = {
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('Draft Message', () => {

test('When a new compose is empty, then the draft is not created', async () => {
vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock);
const emptyEditor = { getHTML: () => '<p></p>', getText: () => '' } as unknown as Editor;
const emptyEditor = { getHTML: () => '<p></p>', getText: () => '', on: vi.fn(), off: vi.fn() } as unknown as Editor;

const { result } = renderDraft({ editor: emptyEditor });

Expand Down Expand Up @@ -190,4 +190,86 @@ describe('Draft Message', () => {
expect(mocks.createDraft).toHaveBeenCalledTimes(1);
expect(mocks.updateDraft).toHaveBeenCalledWith(expect.objectContaining({ draftId: 'new-draft-1' }));
});

test('When a save is triggered while another is in flight, then it waits and updates instead of creating a duplicate', async () => {
vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock);

let resolveCreate!: (value: { id: string; receivedAt: string }) => void;
mocks.createDraft.mockReturnValue({
unwrap: () =>
new Promise((resolve) => {
resolveCreate = resolve;
}),
});

const { result } = renderDraft({ subject: 'Racy', toRecipients: [recipient('bob@inxt.me')] });

await act(async () => {
const first = result.current.saveDraft();
const second = result.current.saveDraft();

await vi.advanceTimersByTimeAsync(0);

resolveCreate({ id: 'new-draft-1', receivedAt: '2026-06-22T13:42:30Z' });
await Promise.all([first, second]);
});

expect(mocks.createDraft).toHaveBeenCalledTimes(1);
expect(mocks.updateDraft).toHaveBeenCalledTimes(1);
expect(mocks.updateDraft).toHaveBeenCalledWith(expect.objectContaining({ draftId: 'new-draft-1' }));
});

test('When resolving the draft id before send, then it waits for the in-flight save and returns the fresh draft id', async () => {
vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock);

let resolveCreate!: (value: { id: string; receivedAt: string }) => void;
mocks.createDraft.mockReturnValue({
unwrap: () =>
new Promise((resolve) => {
resolveCreate = resolve;
}),
});

const { result } = renderDraft({ subject: 'Send race', toRecipients: [recipient('bob@inxt.me')] });

let resolvedId: string | null = null;
await act(async () => {
const save = result.current.saveDraft();
await vi.advanceTimersByTimeAsync(0);
const resolve = result.current.resolveDraftId();
resolveCreate({ id: 'new-draft-1', receivedAt: '2026-06-22T13:42:30Z' });
[, resolvedId] = await Promise.all([save, resolve]);
});

expect(resolvedId).toBe('new-draft-1');
});

test('When the body is edited, then autosave re-arms from the editor update event', async () => {
vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock);

let updateHandler: (() => void) | undefined;
const typedEditor = {
getHTML: () => '<p>hi</p>',
getText: () => 'hi',
on: (event: string, cb: () => void) => {
if (event === 'update') updateHandler = cb;
},
off: vi.fn(),
} as unknown as Editor;

renderDraft({ editor: typedEditor, subject: 'Typing' });
expect(updateHandler).toBeDefined();

await act(async () => {
await vi.advanceTimersByTimeAsync(6000);
updateHandler!();
await vi.advanceTimersByTimeAsync(6000);
});
expect(mocks.createDraft).not.toHaveBeenCalled();

await act(async () => {
await vi.advanceTimersByTimeAsync(4000);
});
expect(mocks.createDraft).toHaveBeenCalledTimes(1);
});
});
91 changes: 66 additions & 25 deletions src/components/compose-message/hooks/useDraftMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface UseDraftMessageResult {
isDiscarding: boolean;
handleDraftDiscard: () => Promise<void>;
saveDraft: () => Promise<void>;
resolveDraftId: () => Promise<string | null>;
clearDraftRef: () => void;
}

Expand All @@ -63,6 +64,8 @@ export const useDraftMessage = ({
// Using a ref instead an state to avoid losing data on re-renders
const draftIdRef = useRef<string | null>(existentDraftId ?? null);
const draftReceivedAtRef = useRef<string | null>(draftReceivedAt ?? null);
const pendingSaveRef = useRef<Promise<void> | null>(null);
const activeSavesRef = useRef(0);

useEffect(() => {
if (existentDraftId && draftIdRef.current === null) {
Expand Down Expand Up @@ -107,60 +110,97 @@ export const useDraftMessage = ({
};
}, [toRecipients, ccRecipients, bccRecipients, subject, editor, attachments, attachmentsSessionKey, senderKeys]);

const saveDraft = useCallback(async () => {
const clearAutosaveTimer = useCallback(() => {
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = null;
}
}, []);

const performSave = useCallback(async () => {
const payload = await buildPayload();
if (!payload) return;

if (!draftIdRef.current && isPayloadEmpty(payload, editor)) return;

if (draftIdRef.current) {
const { id: newDraftId, receivedAt } = await updateDraft({ draftId: draftIdRef.current, payload }).unwrap();
draftIdRef.current = newDraftId;
draftReceivedAtRef.current = receivedAt;
} else {
const { id, receivedAt } = await createDraft(payload).unwrap();
draftIdRef.current = id;
draftReceivedAtRef.current = receivedAt;
}
}, [buildPayload, createDraft, updateDraft, editor]);

const saveDraft = useCallback(async () => {
clearAutosaveTimer();

const previous = pendingSaveRef.current ?? Promise.resolve();
const current = previous.catch(() => undefined).then(performSave);
pendingSaveRef.current = current;

activeSavesRef.current += 1;
setIsSaving(true);
try {
const payload = await buildPayload();
if (!payload) return;

if (!draftIdRef.current && isPayloadEmpty(payload, editor)) return;

if (draftIdRef.current) {
const { id: newDraftId, receivedAt } = await updateDraft({ draftId: draftIdRef.current, payload }).unwrap();
draftIdRef.current = newDraftId;
draftReceivedAtRef.current = receivedAt;
} else {
const { id, receivedAt } = await createDraft(payload).unwrap();
draftIdRef.current = id;
draftReceivedAtRef.current = receivedAt;
}
await current;
} finally {
setIsSaving(false);
activeSavesRef.current -= 1;
if (activeSavesRef.current === 0) setIsSaving(false);
if (pendingSaveRef.current === current) pendingSaveRef.current = null;
}
}, [buildPayload, createDraft, updateDraft, editor]);
}, [performSave, clearAutosaveTimer]);

const resolveDraftId = useCallback(async (): Promise<string | null> => {
clearAutosaveTimer();
if (pendingSaveRef.current) {
try {
await pendingSaveRef.current;
} catch {
// Save failed; proceed with the last id that reached the server. no op
}
}
return draftIdRef.current;
}, [clearAutosaveTimer]);

const handleDraftDiscard = useCallback(async () => {
if (!draftIdRef.current) return;
await discardDraft({ draftId: draftIdRef.current }).unwrap();
}, [discardDraft]);
Comment on lines 167 to 170

@coderabbitai coderabbitai Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP 'handleDraftDiscard|clearDraftRef|scheduleAutosave' -C3 src/components/compose-message

Repository: 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.


const clearDraftRef = useCallback(() => {
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = null;
}
clearAutosaveTimer();
draftIdRef.current = null;
draftReceivedAtRef.current = null;
resetDiscardDraft();
}, [resetDiscardDraft]);
}, [resetDiscardDraft, clearAutosaveTimer]);

useEffect(() => {
if (timerRef.current) clearTimeout(timerRef.current);
const scheduleAutosave = useCallback(() => {
clearAutosaveTimer();
timerRef.current = setTimeout(() => {
saveDraft().catch(() => {
// Autosave failures are silent; user can retry on close
});
}, AUTOSAVE_DELAY_MS);
}, [saveDraft, clearAutosaveTimer]);

useEffect(() => {
scheduleAutosave();

return () => {
if (timerRef.current) clearTimeout(timerRef.current);
};
}, [toRecipients, ccRecipients, bccRecipients, subject, editor, attachments, saveDraft]);
}, [toRecipients, ccRecipients, bccRecipients, subject, editor, attachments, scheduleAutosave]);

useEffect(() => {
if (!editor) return;
const onUpdate = () => scheduleAutosave();
editor.on('update', onUpdate);
return () => {
editor.off('update', onUpdate);
};
}, [editor, scheduleAutosave]);

return {
draftId: draftIdRef.current,
Expand All @@ -169,6 +209,7 @@ export const useDraftMessage = ({
isDiscarding,
handleDraftDiscard,
saveDraft,
resolveDraftId,
clearDraftRef,
};
};
Expand Down
4 changes: 2 additions & 2 deletions src/components/compose-message/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export const ComposeMessageDialog = () => {

const fileInputRef = useRef<HTMLInputElement>(null);

const { saveDraft, handleDraftDiscard, isDiscarding, draftId, draftSavedAt, clearDraftRef } = useDraftMessage({
const { saveDraft, resolveDraftId, handleDraftDiscard, isDiscarding, draftSavedAt, clearDraftRef } = useDraftMessage({
existentDraftId: initialDraftId,
draftReceivedAt: initialReceivedAtDraft,
attachments,
Expand Down Expand Up @@ -197,7 +197,7 @@ export const ComposeMessageDialog = () => {
subject: subjectValue,
toRecipients,
inReplyTo: inReplyItemId,
draftId: draftId ?? undefined,
resolveDraftId,
onSent,
markResolvingInherited,
markInheritedResolved,
Expand Down
Loading