From 5f5c84117c07272e5ce910428a789bf91508b414 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Wed, 8 Jul 2026 20:24:46 -0300 Subject: [PATCH 1/2] fix: update draft handling in compose message component and hooks - 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. --- .../hooks/useComposeSend.test.ts | 11 ++- .../compose-message/hooks/useComposeSend.ts | 8 +- .../hooks/useDraftMessage.test.ts | 86 ++++++++++++++++++- .../compose-message/hooks/useDraftMessage.ts | 79 +++++++++++++---- src/components/compose-message/index.tsx | 25 +++--- 5 files changed, 170 insertions(+), 39 deletions(-) diff --git a/src/components/compose-message/hooks/useComposeSend.test.ts b/src/components/compose-message/hooks/useComposeSend.test.ts index b4ce56d..e9e7f37 100644 --- a/src/components/compose-message/hooks/useComposeSend.test.ts +++ b/src/components/compose-message/hooks/useComposeSend.test.ts @@ -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 resolved draftId is forwarded to sendEmail so the server consumes the draft', 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 (no draft saved), then sendEmail is called without a draftId', async () => { + const resolveDraftId = vi.fn().mockResolvedValue(null); + const { result, onSent } = renderSend({ toRecipients: [recipient('bob@inxt.me')], resolveDraftId }); await act(async () => { await result.current.send(); diff --git a/src/components/compose-message/hooks/useComposeSend.ts b/src/components/compose-message/hooks/useComposeSend.ts index babcc64..33a1310 100644 --- a/src/components/compose-message/hooks/useComposeSend.ts +++ b/src/components/compose-message/hooks/useComposeSend.ts @@ -30,7 +30,7 @@ interface UseComposeSendParams { attachments: AttachmentTask[]; attachmentsSessionKey: Uint8Array; inReplyTo?: string; - draftId?: string; + resolveDraftId?: () => Promise; onSent: () => void; markResolvingInherited: (id: string) => void; markInheritedResolved: (id: string, blobId: string) => void; @@ -59,7 +59,7 @@ export const useComposeSend = ({ attachments, attachmentsSessionKey, inReplyTo, - draftId, + resolveDraftId, onSent, markResolvingInherited, markInheritedResolved, @@ -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, @@ -254,7 +256,7 @@ export const useComposeSend = ({ attachments, attachmentsSessionKey, inReplyTo, - draftId, + resolveDraftId, triggerLookup, sendEmail, onSent, diff --git a/src/components/compose-message/hooks/useDraftMessage.test.ts b/src/components/compose-message/hooks/useDraftMessage.test.ts index 9c55ddb..a800bb8 100644 --- a/src/components/compose-message/hooks/useDraftMessage.test.ts +++ b/src/components/compose-message/hooks/useDraftMessage.test.ts @@ -21,7 +21,7 @@ vi.mock('@/store/api/mail', () => ({ useGetMailAccountKeysQuery: () => ({ data: mocks.senderKeys }), })); -const editor = { getHTML: () => '

hi

', getText: () => 'hi' } as unknown as Editor; +const editor = { getHTML: () => '

hi

', getText: () => 'hi', on: vi.fn(), off: vi.fn() } as unknown as Editor; const recipient = (email: string): Recipient => ({ id: email, email }); const mockEncryptionBlock = { @@ -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: () => '

', getText: () => '' } as unknown as Editor; + const emptyEditor = { getHTML: () => '

', getText: () => '', on: vi.fn(), off: vi.fn() } as unknown as Editor; const { result } = renderDraft({ editor: emptyEditor }); @@ -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 flushing 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 flushedId: string | null = null; + await act(async () => { + const save = result.current.saveDraft(); + await vi.advanceTimersByTimeAsync(0); + const flush = result.current.flushPendingDraftSave(); + resolveCreate({ id: 'new-draft-1', receivedAt: '2026-06-22T13:42:30Z' }); + [, flushedId] = await Promise.all([save, flush]); + }); + + expect(flushedId).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: () => '

hi

', + 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); + }); }); diff --git a/src/components/compose-message/hooks/useDraftMessage.ts b/src/components/compose-message/hooks/useDraftMessage.ts index a057289..1293b84 100644 --- a/src/components/compose-message/hooks/useDraftMessage.ts +++ b/src/components/compose-message/hooks/useDraftMessage.ts @@ -44,6 +44,7 @@ interface UseDraftMessageResult { isDiscarding: boolean; handleDraftDiscard: () => Promise; saveDraft: () => Promise; + flushPendingDraftSave: () => Promise; clearDraftRef: () => void; } @@ -63,6 +64,8 @@ export const useDraftMessage = ({ // Using a ref instead an state to avoid losing data on re-renders const draftIdRef = useRef(existentDraftId ?? null); const draftReceivedAtRef = useRef(draftReceivedAt ?? null); + const pendingSaveRef = useRef | null>(null); + const activeSavesRef = useRef(0); useEffect(() => { if (existentDraftId && draftIdRef.current === null) { @@ -107,32 +110,58 @@ export const useDraftMessage = ({ }; }, [toRecipients, ccRecipients, bccRecipients, subject, editor, attachments, attachmentsSessionKey, senderKeys]); + 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 () => { if (timerRef.current) { clearTimeout(timerRef.current); timerRef.current = null; } + 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]); + + const flushPendingDraftSave = useCallback(async (): Promise => { + if (timerRef.current) { + clearTimeout(timerRef.current); + timerRef.current = null; + } + if (pendingSaveRef.current) { + try { + await pendingSaveRef.current; + } catch { + // Save failed; proceed with the last id that reached the server. no op + } + } + return draftIdRef.current; + }, []); const handleDraftDiscard = useCallback(async () => { if (!draftIdRef.current) return; @@ -149,18 +178,31 @@ export const useDraftMessage = ({ resetDiscardDraft(); }, [resetDiscardDraft]); - useEffect(() => { + const scheduleAutosave = useCallback(() => { if (timerRef.current) clearTimeout(timerRef.current); timerRef.current = setTimeout(() => { saveDraft().catch(() => { // Autosave failures are silent; user can retry on close }); }, AUTOSAVE_DELAY_MS); + }, [saveDraft]); + + 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, @@ -169,6 +211,7 @@ export const useDraftMessage = ({ isDiscarding, handleDraftDiscard, saveDraft, + flushPendingDraftSave, clearDraftRef, }; }; diff --git a/src/components/compose-message/index.tsx b/src/components/compose-message/index.tsx index f92b4d3..817781d 100644 --- a/src/components/compose-message/index.tsx +++ b/src/components/compose-message/index.tsx @@ -143,17 +143,18 @@ export const ComposeMessageDialog = () => { const fileInputRef = useRef(null); - const { saveDraft, handleDraftDiscard, isDiscarding, draftId, draftSavedAt, clearDraftRef } = useDraftMessage({ - existentDraftId: initialDraftId, - draftReceivedAt: initialReceivedAtDraft, - attachments, - toRecipients, - ccRecipients, - bccRecipients, - subject: subjectValue, - editor, - attachmentsSessionKey, - }); + const { saveDraft, flushPendingDraftSave, handleDraftDiscard, isDiscarding, draftSavedAt, clearDraftRef } = + useDraftMessage({ + existentDraftId: initialDraftId, + draftReceivedAt: initialReceivedAtDraft, + attachments, + toRecipients, + ccRecipients, + bccRecipients, + subject: subjectValue, + editor, + attachmentsSessionKey, + }); const closeDialog = useCallback(() => { clearAttachments(); @@ -197,7 +198,7 @@ export const ComposeMessageDialog = () => { subject: subjectValue, toRecipients, inReplyTo: inReplyItemId, - draftId: draftId ?? undefined, + resolveDraftId: flushPendingDraftSave, onSent, markResolvingInherited, markInheritedResolved, From d319beafaa7e807ebc42f0eb8d4a6431646e5161 Mon Sep 17 00:00:00 2001 From: jzunigax2 <125698953+jzunigax2@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:19:08 -0300 Subject: [PATCH 2/2] refactor: rename flushPendingDraftSave to resolveDraftId for improved 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. --- .../hooks/useComposeSend.test.ts | 4 +- .../hooks/useDraftMessage.test.ts | 10 ++--- .../compose-message/hooks/useDraftMessage.ts | 38 +++++++++---------- src/components/compose-message/index.tsx | 25 ++++++------ 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/components/compose-message/hooks/useComposeSend.test.ts b/src/components/compose-message/hooks/useComposeSend.test.ts index e9e7f37..6a22e3b 100644 --- a/src/components/compose-message/hooks/useComposeSend.test.ts +++ b/src/components/compose-message/hooks/useComposeSend.test.ts @@ -311,7 +311,7 @@ describe('useComposeSend', () => { vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock').mockResolvedValue(mockEncryptionBlock); }); - test('When sending an email opened from a draft, then the resolved 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')], @@ -327,7 +327,7 @@ describe('useComposeSend', () => { expect(onSent).toHaveBeenCalled(); }); - test('When sending a brand-new email (no draft saved), then sendEmail is called without a draftId', async () => { + 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 }); diff --git a/src/components/compose-message/hooks/useDraftMessage.test.ts b/src/components/compose-message/hooks/useDraftMessage.test.ts index a800bb8..ee85b3a 100644 --- a/src/components/compose-message/hooks/useDraftMessage.test.ts +++ b/src/components/compose-message/hooks/useDraftMessage.test.ts @@ -219,7 +219,7 @@ describe('Draft Message', () => { expect(mocks.updateDraft).toHaveBeenCalledWith(expect.objectContaining({ draftId: 'new-draft-1' })); }); - test('When flushing before send, then it waits for the in-flight save and returns the fresh draft id', async () => { + 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; @@ -232,16 +232,16 @@ describe('Draft Message', () => { const { result } = renderDraft({ subject: 'Send race', toRecipients: [recipient('bob@inxt.me')] }); - let flushedId: string | null = null; + let resolvedId: string | null = null; await act(async () => { const save = result.current.saveDraft(); await vi.advanceTimersByTimeAsync(0); - const flush = result.current.flushPendingDraftSave(); + const resolve = result.current.resolveDraftId(); resolveCreate({ id: 'new-draft-1', receivedAt: '2026-06-22T13:42:30Z' }); - [, flushedId] = await Promise.all([save, flush]); + [, resolvedId] = await Promise.all([save, resolve]); }); - expect(flushedId).toBe('new-draft-1'); + expect(resolvedId).toBe('new-draft-1'); }); test('When the body is edited, then autosave re-arms from the editor update event', async () => { diff --git a/src/components/compose-message/hooks/useDraftMessage.ts b/src/components/compose-message/hooks/useDraftMessage.ts index 1293b84..d9e4a26 100644 --- a/src/components/compose-message/hooks/useDraftMessage.ts +++ b/src/components/compose-message/hooks/useDraftMessage.ts @@ -44,7 +44,7 @@ interface UseDraftMessageResult { isDiscarding: boolean; handleDraftDiscard: () => Promise; saveDraft: () => Promise; - flushPendingDraftSave: () => Promise; + resolveDraftId: () => Promise; clearDraftRef: () => void; } @@ -110,6 +110,13 @@ export const useDraftMessage = ({ }; }, [toRecipients, ccRecipients, bccRecipients, subject, editor, attachments, attachmentsSessionKey, senderKeys]); + const clearAutosaveTimer = useCallback(() => { + if (timerRef.current) { + clearTimeout(timerRef.current); + timerRef.current = null; + } + }, []); + const performSave = useCallback(async () => { const payload = await buildPayload(); if (!payload) return; @@ -128,10 +135,7 @@ export const useDraftMessage = ({ }, [buildPayload, createDraft, updateDraft, editor]); const saveDraft = useCallback(async () => { - if (timerRef.current) { - clearTimeout(timerRef.current); - timerRef.current = null; - } + clearAutosaveTimer(); const previous = pendingSaveRef.current ?? Promise.resolve(); const current = previous.catch(() => undefined).then(performSave); @@ -146,13 +150,10 @@ export const useDraftMessage = ({ if (activeSavesRef.current === 0) setIsSaving(false); if (pendingSaveRef.current === current) pendingSaveRef.current = null; } - }, [performSave]); + }, [performSave, clearAutosaveTimer]); - const flushPendingDraftSave = useCallback(async (): Promise => { - if (timerRef.current) { - clearTimeout(timerRef.current); - timerRef.current = null; - } + const resolveDraftId = useCallback(async (): Promise => { + clearAutosaveTimer(); if (pendingSaveRef.current) { try { await pendingSaveRef.current; @@ -161,7 +162,7 @@ export const useDraftMessage = ({ } } return draftIdRef.current; - }, []); + }, [clearAutosaveTimer]); const handleDraftDiscard = useCallback(async () => { if (!draftIdRef.current) return; @@ -169,23 +170,20 @@ export const useDraftMessage = ({ }, [discardDraft]); const clearDraftRef = useCallback(() => { - if (timerRef.current) { - clearTimeout(timerRef.current); - timerRef.current = null; - } + clearAutosaveTimer(); draftIdRef.current = null; draftReceivedAtRef.current = null; resetDiscardDraft(); - }, [resetDiscardDraft]); + }, [resetDiscardDraft, clearAutosaveTimer]); const scheduleAutosave = useCallback(() => { - if (timerRef.current) clearTimeout(timerRef.current); + clearAutosaveTimer(); timerRef.current = setTimeout(() => { saveDraft().catch(() => { // Autosave failures are silent; user can retry on close }); }, AUTOSAVE_DELAY_MS); - }, [saveDraft]); + }, [saveDraft, clearAutosaveTimer]); useEffect(() => { scheduleAutosave(); @@ -211,7 +209,7 @@ export const useDraftMessage = ({ isDiscarding, handleDraftDiscard, saveDraft, - flushPendingDraftSave, + resolveDraftId, clearDraftRef, }; }; diff --git a/src/components/compose-message/index.tsx b/src/components/compose-message/index.tsx index 817781d..7c133f7 100644 --- a/src/components/compose-message/index.tsx +++ b/src/components/compose-message/index.tsx @@ -143,18 +143,17 @@ export const ComposeMessageDialog = () => { const fileInputRef = useRef(null); - const { saveDraft, flushPendingDraftSave, handleDraftDiscard, isDiscarding, draftSavedAt, clearDraftRef } = - useDraftMessage({ - existentDraftId: initialDraftId, - draftReceivedAt: initialReceivedAtDraft, - attachments, - toRecipients, - ccRecipients, - bccRecipients, - subject: subjectValue, - editor, - attachmentsSessionKey, - }); + const { saveDraft, resolveDraftId, handleDraftDiscard, isDiscarding, draftSavedAt, clearDraftRef } = useDraftMessage({ + existentDraftId: initialDraftId, + draftReceivedAt: initialReceivedAtDraft, + attachments, + toRecipients, + ccRecipients, + bccRecipients, + subject: subjectValue, + editor, + attachmentsSessionKey, + }); const closeDialog = useCallback(() => { clearAttachments(); @@ -198,7 +197,7 @@ export const ComposeMessageDialog = () => { subject: subjectValue, toRecipients, inReplyTo: inReplyItemId, - resolveDraftId: flushPendingDraftSave, + resolveDraftId, onSent, markResolvingInherited, markInheritedResolved,