From 5f61d8f72ab00404d6a04853a3384973362de313 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Wed, 11 Nov 2026 14:08:57 +0100 Subject: [PATCH] fix(correspondence): protect pasted email recording and recovery --- .../verify-correspondence-recovery.mjs | 37 +++++++--- .../src/components/Correspondence.tsx | 55 ++++++++------ .../src/correspondence-recovery.test.tsx | 72 ++++++++++++++++++- 3 files changed, 131 insertions(+), 33 deletions(-) diff --git a/job-tracker-ui/scripts/verify-correspondence-recovery.mjs b/job-tracker-ui/scripts/verify-correspondence-recovery.mjs index 7f7e5c12..b1c834b1 100644 --- a/job-tracker-ui/scripts/verify-correspondence-recovery.mjs +++ b/job-tracker-ui/scripts/verify-correspondence-recovery.mjs @@ -8,7 +8,8 @@ import ts from 'typescript'; const compiled = ts.transpileModule(fs.readFileSync(new URL('../src/i18n/translations.ts', import.meta.url), 'utf8'), { compilerOptions: { module: ts.ModuleKind.ESNext } }).outputText; const { translations } = await import(`data:text/javascript;base64,${Buffer.from(compiled).toString('base64')}`); -const out = path.resolve(import.meta.dirname, '../../docs/ui-modernisation/evidence/correspondence-recovery'); +const pasted = process.env.A3_PASTED_EMAIL === '1'; +const out = path.resolve(import.meta.dirname, `../../docs/ui-modernisation/evidence/${pasted ? 'pasted-email-recovery' : 'correspondence-recovery'}`); fs.mkdirSync(out, { recursive: true }); const configurations = process.env.A3_PILOT ? [[390, 'nb', 'dark']] : [390, 1440].flatMap(width => ['en', 'nb'].flatMap(language => ['light', 'dark'].map(theme => [width, language, theme]))); const report = { journeys: [], captures: [], writes: [], errors: [], unavailableReads: [], blockedExternal: [] }; @@ -32,7 +33,7 @@ try { assert.equal(request.method(), 'POST'); assert.equal(url.pathname, '/api/correspondence'); posts += 1; const body = request.postDataJSON(); - assert.deepEqual(body, { jobApplicationId: 42, from: 'Me', content: 'Retain the synthetic note' }); + assert.deepEqual(body, pasted ? { jobApplicationId: 42, from: 'Me', channel: 'Email', subject: 'Synthetic interview', content: 'Synthetic email body', date: '2026-11-11T12:00:00.000Z', externalFrom: 'recruiter@example.test', externalTo: 'applicant@example.test' } : { jobApplicationId: 42, from: 'Me', content: 'Retain the synthetic note' }); report.writes.push({ name, attempt: posts, body, result: posts === 1 ? 'fixture500' : 'fixtureCommitted' }); if (posts === 1) return route.fulfill({ status: 500, json: {} }); assert.equal(posts, 2); await held; @@ -41,7 +42,7 @@ try { if (url.pathname === '/api/correspondence/42') { reads += 1; if (reads === 1 || reads === 3) return route.fulfill({ status: 500, json: {} }); - return route.fulfill({ json: reads > 3 ? [{ id: 91, jobApplicationId: 42, from: 'Me', content: 'Retain the synthetic note', date: '2026-11-11T12:00:00Z', channel: 'Note' }] : [] }); + return route.fulfill({ json: reads > 3 ? [{ id: 91, jobApplicationId: 42, from: 'Me', content: pasted ? 'Synthetic email body' : 'Retain the synthetic note', date: '2026-11-11T12:00:00Z', channel: pasted ? 'Email' : 'Note' }] : [] }); } const fixtures = { '/api/auth/config': { requireAuth: true }, @@ -66,7 +67,8 @@ try { await page.goto('http://localhost:4338/jobs/42?section=timeline&sub=correspondence'); const retry = page.getByRole('button', { name: copy.retry, exact: true }); await expect(retry).toBeVisible(); - const draft = page.getByRole('textbox', { name: copy.correspondenceLogNoteOrMessage, exact: true }); + const note = page.getByRole('textbox', { name: copy.correspondenceLogNoteOrMessage, exact: true }); + let draft = note; await draft.fill('Retain the synthetic note'); await capture('read-error'); const otherLanguage = language === 'nb' ? 'en' : 'nb'; @@ -79,20 +81,35 @@ try { await expect(retry).toBeVisible(); assert.equal(reads, 1); await retry.press('Enter'); await expect(retry).toBeHidden(); await expect(draft).toHaveValue('Retain the synthetic note'); assert.equal(reads, 2); assert.equal(posts, 0); - const add = page.getByRole('button', { name: copy.correspondenceAdd, exact: true }); + let add = page.getByRole('button', { name: copy.correspondenceAdd, exact: true }); + const importDialog = page.getByRole('dialog', { name: copy.correspondenceImportTitle, exact: true }); + const rawEmail = 'From: recruiter@example.test\nTo: applicant@example.test\nSubject: Synthetic interview\nDate: Wed, 11 Nov 2026 12:00:00 +0000\n\nSynthetic email body'; + if (pasted) { + await page.getByRole('button', { name: copy.correspondenceImportEmail, exact: true }).click(); + draft = importDialog.getByRole('textbox', { name: copy.correspondencePasteEmail, exact: true }); + await draft.fill(rawEmail); + add = importDialog.getByRole('button', { name: copy.correspondenceLogEmail, exact: true }); + } await add.click(); - await expect(page.getByText(copy.correspondenceAddFailed, { exact: true })).toBeVisible(); - await expect(draft).toHaveValue('Retain the synthetic note'); await expect(add).toBeEnabled(); + await expect(page.getByText(pasted ? copy.addJobModalImportFailed : copy.correspondenceAddFailed, { exact: true })).toBeVisible(); + await expect(draft).toHaveValue(pasted ? rawEmail : 'Retain the synthetic note'); await expect(add).toBeEnabled(); await capture('write-error'); - await page.keyboard.press('Escape'); + if (!pasted) await page.keyboard.press('Escape'); await add.click(); await expect(add).toBeDisabled(); await expect(draft).toBeDisabled(); await add.evaluate(button => { button.click(); button.click(); }); + if (pasted) { + await expect(importDialog.getByRole('button', { name: copy.correspondenceClose, exact: true })).toBeDisabled(); + await expect(importDialog.getByRole('tab', { name: copy.google, exact: true })).toBeDisabled(); + await page.keyboard.press('Escape'); await expect(importDialog).toBeVisible(); + } assert.equal(posts, 2); await capture('write-pending'); release(); - await expect(retry).toBeVisible(); await expect(draft).toBeEnabled(); await expect(draft).toHaveValue(''); + await expect(retry).toBeVisible(); + if (pasted) await expect(importDialog).toBeHidden(); + await expect(note).toBeEnabled(); await expect(note).toHaveValue(pasted ? 'Retain the synthetic note' : ''); await capture('committed-read-error'); await retry.press('Enter'); - await expect(page.getByText('Retain the synthetic note', { exact: true })).toBeVisible(); + await expect(page.getByText(pasted ? 'Synthetic email body' : 'Retain the synthetic note', { exact: true })).toBeVisible(); await expect(retry).toBeHidden(); assert.equal(reads, 4); assert.equal(posts, 2); await capture('recovered'); report.journeys.push({ name, readRetryPreservesDraft: true, rejectedWritePreservesDraft: true, pendingWriteExcludesDuplicate: true, committedReadFailureRetriesGetOnly: true, reads, posts }); diff --git a/job-tracker-ui/src/components/Correspondence.tsx b/job-tracker-ui/src/components/Correspondence.tsx index d13e48d2..394aad85 100644 --- a/job-tracker-ui/src/components/Correspondence.tsx +++ b/job-tracker-ui/src/components/Correspondence.tsx @@ -134,8 +134,8 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j const messageRequest = useRef(0); const [from, setFrom] = useState<"Me" | "Company">("Me"); const [text, setText] = useState(""); - const [noteSaving, setNoteSaving] = useState(false); - const noteSavingRef = useRef(false); + const [manualSaving, setManualSaving] = useState(false); + const manualSavingRef = useRef(false); const scrollRef = useRef(null); const [importOpen, setImportOpen] = useState(false); @@ -168,8 +168,8 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j const session = gmailOAuth.current; session.active = true; session.jobId = jobId; - noteSavingRef.current = false; - setNoteSaving(false); + manualSavingRef.current = false; + setManualSaving(false); return () => { session.active = false; session.generation += 1; session.starting = false; session.attempt = null; }; }, [jobId]); @@ -345,11 +345,11 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j }); const send = async () => { - if (!canSend || noteSavingRef.current) return; + if (!canSend || manualSavingRef.current) return; const session = gmailOAuth.current; const generation = session.generation; - noteSavingRef.current = true; - setNoteSaving(true); + manualSavingRef.current = true; + setManualSaving(true); try { await api.post("/correspondence", { jobApplicationId: jobId, from, content: text }); if (!session.active || generation !== session.generation) return; @@ -359,19 +359,24 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j if (session.active && generation === session.generation) toast(getApiErrorMessage(error, startupTranslation.current("correspondenceAddFailed")), "error"); } finally { if (session.active && generation === session.generation) { - noteSavingRef.current = false; - setNoteSaving(false); + manualSavingRef.current = false; + setManualSaving(false); } } }; const importEmail = async () => { + if (manualSavingRef.current) return; const parsed = parseRawEmail(rawEmail); if (!parsed.body && !parsed.subject && !rawEmail.trim()) { toast(t("addJobModalPasteUrlFirst"), "error"); return; } + const session = gmailOAuth.current; + const generation = session.generation; + manualSavingRef.current = true; + setManualSaving(true); try { await api.post("/correspondence", { jobApplicationId: jobId, @@ -383,12 +388,18 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j externalFrom: parsed.from ?? null, externalTo: parsed.to ?? null, }); + if (!session.active || generation !== session.generation) return; setImportOpen(false); setRawEmail(""); - await load(); - toast(t("correspondenceLogEmail"), "success"); + await load().catch(() => undefined); + toast(startupTranslation.current("correspondenceLogEmail"), "success"); } catch (error) { - toast(getApiErrorMessage(error, t("addJobModalImportFailed")), "error"); + if (session.active && generation === session.generation) toast(getApiErrorMessage(error, startupTranslation.current("addJobModalImportFailed")), "error"); + } finally { + if (session.active && generation === session.generation) { + manualSavingRef.current = false; + setManualSaving(false); + } } }; @@ -603,16 +614,16 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j - v && setFrom(v)} size="small"> + v && setFrom(v)} size="small"> {t("correspondenceMe")} {t("correspondenceCompany")} - + - setText(e.target.value)} multiline minRows={3} sx={{ flex: { xs: "1 1 100%", sm: "1 1 320px" }, minWidth: 0 }} slotProps={{ formHelperText: { disabled: false } }} helperText={t("correspondenceCharacters", { count: text.length })} /> + setText(e.target.value)} multiline minRows={3} sx={{ flex: { xs: "1 1 100%", sm: "1 1 320px" }, minWidth: 0 }} slotProps={{ formHelperText: { disabled: false } }} helperText={t("correspondenceCharacters", { count: text.length })} /> - + setManageThreadId(null)} fullWidth maxWidth="sm"> @@ -650,18 +661,18 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j - setImportOpen(false)} fullWidth maxWidth="md"> + { if (!manualSaving) setImportOpen(false); }} fullWidth maxWidth="md"> {t("correspondenceImportTitle")} setImportTab(v)} sx={{ mb: 2 }}> - - + + {importTab === 0 ? ( <> {t("correspondencePasteEmailHelp")} - setRawEmail(e.target.value)} placeholder={t("correspondencePasteEmailPlaceholder")} /> + setRawEmail(e.target.value)} placeholder={t("correspondencePasteEmailPlaceholder")} /> ) : ( @@ -799,8 +810,8 @@ export default function Correspondence({ jobId, jobContext }: { jobId: number; j )} - - {importTab === 0 ? : null} + + {importTab === 0 ? : null} diff --git a/job-tracker-ui/src/correspondence-recovery.test.tsx b/job-tracker-ui/src/correspondence-recovery.test.tsx index 4cbd9700..781b5cfc 100644 --- a/job-tracker-ui/src/correspondence-recovery.test.tsx +++ b/job-tracker-ui/src/correspondence-recovery.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import "@testing-library/jest-dom"; -import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import Correspondence from "./components/Correspondence"; import { I18nProvider } from "./i18n/I18nProvider"; import { ToastProvider } from "./toast"; @@ -189,3 +189,73 @@ test("a displayed read error follows UI language without another request or lost expect(screen.getByLabelText(translations.nb.correspondenceLogNoteOrMessage)).toHaveValue("Retain while translating error"); expect(client.get.mock.calls).toEqual(reads); expect(client.post).not.toHaveBeenCalled(); }); + +const rawEmail = "From: recruiter@example.test\nTo: applicant@example.test\nSubject: Synthetic interview\nDate: Wed, 11 Nov 2026 12:00:00 +0000\n\nSynthetic email body"; +async function openRawEmail() { + fireEvent.click(screen.getByRole("button", { name: translations.en.correspondenceImportEmail })); + const dialog = await screen.findByRole("dialog"); + const input = within(dialog).getByRole("textbox"); + fireEvent.change(input, { target: { value: rawEmail } }); + return { dialog, input, submit: within(dialog).getByRole("button", { name: translations.en.correspondenceLogEmail }) }; +} +function emptyReads() { + client.get.mockImplementation(url => Promise.resolve({ data: String(url).startsWith("/correspondence/") ? [] : { connected: false } } as any)); +} + +test("pasted-email submission locks its draft, tabs and close actions and excludes duplicate writes", async () => { + emptyReads(); const pending = deferred(); client.post.mockReturnValue(pending.promise); + render(view()); const { dialog, input, submit } = await openRawEmail(); + fireEvent.click(submit); fireEvent.click(submit); + expect(client.post).toHaveBeenCalledTimes(1); expect(input).toBeDisabled(); expect(submit).toBeDisabled(); + expect(within(dialog).getByRole("tab", { name: translations.en.google })).toBeDisabled(); + expect(within(dialog).getByRole("button", { name: translations.en.correspondenceClose })).toBeDisabled(); + fireEvent.keyDown(dialog, { key: "Escape" }); expect(dialog).toBeVisible(); + expect(client.post).toHaveBeenCalledWith("/correspondence", { jobApplicationId: 42, from: "Me", channel: "Email", subject: "Synthetic interview", content: "Synthetic email body", date: "2026-11-11T12:00:00.000Z", externalFrom: "recruiter@example.test", externalTo: "applicant@example.test" }); + await act(async () => pending.resolve({ data: {} })); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); +}); + +test("rejected pasted-email recording retains the complete source and supports a deliberate retry", async () => { + emptyReads(); client.post.mockRejectedValueOnce(new Error("offline")).mockResolvedValueOnce({ data: {} } as any); + render(view()); const { input, submit } = await openRawEmail(); fireEvent.click(submit); + expect(await screen.findByText(translations.en.addJobModalImportFailed)).toBeVisible(); + expect(input).toHaveValue(rawEmail); expect(submit).toBeEnabled(); + fireEvent.click(submit); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(client.post).toHaveBeenCalledTimes(2); +}); + +test("committed pasted email with failed refresh offers read retry without a failed-import alert", async () => { + let reads = 0; + client.get.mockImplementation(url => String(url).startsWith("/correspondence/") + ? ++reads === 2 ? Promise.reject(new Error("read offline")) : Promise.resolve({ data: reads > 2 ? [message(42, "Recorded email")] : [] } as any) + : Promise.resolve({ data: { connected: false } } as any)); + client.post.mockResolvedValue({ data: {} } as any); + render(view()); await waitFor(() => expect(reads).toBe(1)); + const { submit } = await openRawEmail(); fireEvent.click(submit); + const retry = await screen.findByRole("button", { name: translations.en.retry }); + expect(screen.queryByText(translations.en.addJobModalImportFailed)).not.toBeInTheDocument(); + fireEvent.click(retry); await screen.findByText("Recorded email"); + expect(client.post).toHaveBeenCalledTimes(1); +}); + +test.each(["success", "failure"])("old-job pasted-email %s cannot close or clear a new draft", async outcome => { + emptyReads(); const pending = deferred(); client.post.mockReturnValue(pending.promise); + const rendered = render(view()); const { submit } = await openRawEmail(); fireEvent.click(submit); + rendered.rerender(view(43)); + const next = within(screen.getByRole("dialog")).getByRole("textbox"); + fireEvent.change(next, { target: { value: "Next application email draft" } }); + const reads = [...client.get.mock.calls]; + await act(async () => { + if (outcome === "success") pending.resolve({ data: {} }); + else pending.reject(new Error("old import failed")); + }); + expect(screen.getByRole("dialog")).toBeVisible(); expect(next).toHaveValue("Next application email draft"); + expect(client.get.mock.calls).toEqual(reads); + expect(screen.queryByText(translations.en.addJobModalImportFailed)).not.toBeInTheDocument(); +}); + +test("pasted-email source has a visible accessible label", async () => { + emptyReads(); render(view()); const { dialog } = await openRawEmail(); + expect(within(dialog).getByRole("textbox", { name: translations.en.correspondencePasteEmail })).toBeVisible(); +});