From 5340499520dc70851a2cf693b39af3f2b979b0a4 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 02:00:51 +0100 Subject: [PATCH] fix: give quick reply the composer's placeholder, attachment and send confirmation checks --- components/mail/mail-app.tsx | 41 +++++++++++++- lib/outgoing/__tests__/quick-reply.test.ts | 66 +++++++++++++++++++++- lib/outgoing/quick-reply.ts | 43 ++++++++++++-- 3 files changed, 142 insertions(+), 8 deletions(-) diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 877c7d25c..38d0b9037 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -119,7 +119,8 @@ import { FilePreviewModal } from "@/components/files/file-preview-modal"; import { WopiEditor } from "@/components/files/wopi-editor"; import { useWopiStatus, canWopiOpen } from "@/hooks/use-wopi-status"; import { isFilePreviewable } from "@/lib/file-preview"; -import { prepareQuickReply } from "@/lib/outgoing/quick-reply"; +import { prepareQuickReply, type QuickReplyQuestion } from "@/lib/outgoing/quick-reply"; +import enMessages from "@/locales/en/common.json"; import { computeReplyThreadingHeaders } from "@/lib/email-threading"; import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import"; import { resolveComposeAccountEmail } from "@/lib/reply-identity"; @@ -4032,16 +4033,50 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const signatureIdentity = (sendingIdentity?.htmlSignature || sendingIdentity?.textSignature) ? sendingIdentity : primaryIdentity; - // The same checks and finalisers as the composer's send + // The same warnings, checks and finalisers as the composer's send // (lib/outgoing/send-pipeline.ts): a quick reply is no way around them. + const settings = useSettingsStore.getState(); + const askBeforeQuickReply = (question: QuickReplyQuestion) => { + switch (question.kind) { + case 'placeholders': + return confirmDialog({ + title: t('email_composer.placeholder_warning.title'), + message: `${t('email_composer.placeholder_warning.message')} ${t('email_composer.placeholder_warning.found')}: ${question.found.join(', ')}`, + confirmText: t('email_composer.placeholder_warning.send_anyway'), + cancelText: t('email_composer.placeholder_warning.back'), + }); + case 'attachment': + return confirmDialog({ + title: t('email_composer.forgot_attachment.title'), + message: t('email_composer.forgot_attachment.message', { keyword: question.keyword }), + confirmText: t('email_composer.forgot_attachment.send_anyway'), + cancelText: t('email_composer.forgot_attachment.back'), + }); + case 'send': + return confirmDialog({ + title: t('email_composer.send_confirm.title'), + message: t('email_composer.send_confirm.message', { count: recipients.length }), + confirmText: t('email_composer.send'), + }); + } + }; const prepared = await prepareQuickReply({ body, signatureIdentity, sendingIdentity, - separator: useSettingsStore.getState().signatureSeparatorEnabled, + separator: settings.signatureSeparatorEnabled, to: recipients, fromEmail: headerFromEmail, identityId: sendingIdentity?.id, + questions: { + // The fill-in words of the UI language and of English, as the composer. + placeholderWords: settings.placeholderWarningEnabled + ? [enMessages.email_composer.placeholder_words, t.raw('email_composer.placeholder_words')] + : null, + attachmentKeywords: settings.attachmentReminderEnabled ? settings.attachmentReminderKeywords : null, + confirmSend: settings.sendConfirmation, + ask: askBeforeQuickReply, + }, }); if (!prepared) return false; const { html: finalHtmlBody, text: finalBody } = prepared; diff --git a/lib/outgoing/__tests__/quick-reply.test.ts b/lib/outgoing/__tests__/quick-reply.test.ts index 834082a6b..8ee19a28e 100644 --- a/lib/outgoing/__tests__/quick-reply.test.ts +++ b/lib/outgoing/__tests__/quick-reply.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -import { prepareQuickReply } from '@/lib/outgoing/quick-reply'; +import { prepareQuickReply, type QuickReplyQuestion, type QuickReplyQuestions } from '@/lib/outgoing/quick-reply'; import { registerOutgoingFinalizer, registerPreSendCheck, resetSendPipeline } from '@/lib/outgoing/send-pipeline'; import type { Identity } from '@/lib/jmap/types'; @@ -43,3 +43,67 @@ describe('prepareQuickReply', () => { expect(await prepareQuickReply(input(text))).toBeNull(); }); }); + +// The composer warns about placeholders and forgotten attachments and asks +// "Send this message?" when that is on. Quick reply went out without any of +// them, an admin-enforced send confirmation included. +describe('prepareQuickReply warnings', () => { + const words = [{ before: ['your'], after: [], unfinished: ['TODO'] }]; + const questions = (answer: boolean, overrides: Partial = {}) => { + const asked: QuickReplyQuestion[] = []; + return { + asked, + questions: { + placeholderWords: words, + attachmentKeywords: ['attached', 'anbei'], + confirmSend: false, + ask: async (q: QuickReplyQuestion) => { asked.push(q); return answer; }, + ...overrides, + } satisfies QuickReplyQuestions, + }; + }; + const reply = (body: string, q: QuickReplyQuestions) => prepareQuickReply({ ...input(text), body, questions: q }); + + it('asks about unfilled placeholders and stops on "back"', async () => { + const { asked, questions: q } = questions(false); + expect(await reply('See you at [TIME]. TODO', q)).toBeNull(); + expect(asked).toEqual([{ kind: 'placeholders', found: ['[TIME]', 'TODO'] }]); + }); + + it('asks about a forgotten attachment and stops on "back"', async () => { + const { asked, questions: q } = questions(false); + expect(await reply('Die Datei ist anbei.', q)).toBeNull(); + expect(asked).toEqual([{ kind: 'attachment', keyword: 'anbei' }]); + }); + + it('asks before sending when send confirmation is on, after the pre-send checks', async () => { + const order: string[] = []; + registerPreSendCheck('external', 10, () => { order.push('check'); return true; }); + const { questions: q } = questions(false, { confirmSend: true }); + const ask = async (question: QuickReplyQuestion) => { order.push(question.kind); return false; }; + expect(await reply('Thanks!', { ...q, ask })).toBeNull(); + expect(order).toEqual(['check', 'send']); + }); + + it('sends when every question is answered with "send anyway"', async () => { + const { asked, questions: q } = questions(true, { confirmSend: true }); + const out = await reply('[TIME] works, file attached', q); + expect(out?.text).toContain('[TIME] works, file attached'); + expect(asked.map((a) => a.kind)).toEqual(['placeholders', 'attachment', 'send']); + }); + + it('asks nothing for a plain reply, or when the warnings are off', async () => { + const { asked, questions: q } = questions(false); + expect(await reply('Thanks, see you then.', q)).not.toBeNull(); + const off = questions(false, { placeholderWords: null, attachmentKeywords: null }); + expect(await reply('[TIME], attached', off.questions)).not.toBeNull(); + expect([...asked, ...off.asked]).toEqual([]); + }); + + it('leaves the signature out of the placeholder check', async () => { + const signed = { ...text, textSignature: 'Me\n[YOUR TITLE]' } as Identity; + const { asked, questions: q } = questions(false); + expect(await prepareQuickReply({ ...input(signed), body: 'Thanks!', questions: q })).not.toBeNull(); + expect(asked).toEqual([]); + }); +}); diff --git a/lib/outgoing/quick-reply.ts b/lib/outgoing/quick-reply.ts index 1d8f3e15a..e4fc39d19 100644 --- a/lib/outgoing/quick-reply.ts +++ b/lib/outgoing/quick-reply.ts @@ -1,15 +1,33 @@ import type { Identity } from '@/lib/jmap/types'; import { appendHtmlSignature, appendPlainTextSignature } from '@/lib/signature-utils'; import { currentSignatureIdentity } from '@/lib/org/signature-state'; +import { findUnfilledPlaceholders, type PlaceholderWords } from '@/lib/placeholder-check'; import { finalizeOutgoing, runPreSendChecks, type OutgoingBodies } from './send-pipeline'; // Registers the organisation signature check and the disclaimer. import '@/lib/org/outgoing'; +/** A question the composer asks before sending, asked by quick reply too. */ +export type QuickReplyQuestion = + | { kind: 'placeholders'; found: string[] } + | { kind: 'attachment'; keyword: string } + | { kind: 'send' }; + +export interface QuickReplyQuestions { + /** The fill-in words of the placeholder warning; null when it is off. */ + placeholderWords: PlaceholderWords[] | null; + /** The keywords of the attachment reminder; null when it is off. */ + attachmentKeywords: string[] | null; + /** "Confirm before sending" is on. */ + confirmSend: boolean; + /** Resolves true to send anyway. */ + ask: (question: QuickReplyQuestion) => Promise; +} + /** * The bodies a quick reply sends: the typed text with the signature, as - * HTML too when the signature is HTML, after the same pre-send checks and - * finalisers as the composer's send. Null when a check stopped the send. - * The organisation signature applies as in the composer + * HTML too when the signature is HTML, after the same warnings, pre-send + * checks and finalisers as the composer's send. Null when one stopped the + * send. The organisation signature applies as in the composer * (`signatureIdentity` is what the reply would sign with on its own, * `sendingIdentity` the identity it goes out from). */ @@ -21,8 +39,23 @@ export async function prepareQuickReply(input: { to: string[]; fromEmail?: string; identityId?: string; + questions?: QuickReplyQuestions; }): Promise { - const { body, separator } = input; + const { body, separator, questions } = input; + + // The composer's warnings, in its order. Only the typed text counts: the + // subject and the signature are not the user's words here. + if (questions?.placeholderWords) { + const found = findUnfilledPlaceholders('', body, questions.placeholderWords); + if (found.length > 0 && !(await questions.ask({ kind: 'placeholders', found }))) return null; + } + // A quick reply carries no attachments, so any keyword is a reminder. + if (questions?.attachmentKeywords) { + const lower = body.toLowerCase(); + const keyword = questions.attachmentKeywords.find(kw => kw.trim() && lower.includes(kw.toLowerCase())); + if (keyword && !(await questions.ask({ kind: 'attachment', keyword }))) return null; + } + const signatureIdentity = currentSignatureIdentity(input.signatureIdentity, input.sendingIdentity ?? undefined); const text = appendPlainTextSignature(body, signatureIdentity, { separator }); @@ -53,6 +86,8 @@ export async function prepareQuickReply(input: { plainText: !html, }; if (!(await runPreSendChecks(context))) return null; + // Asked last, as in the composer, so an answered warning doesn't ask twice. + if (questions?.confirmSend && !(await questions.ask({ kind: 'send' }))) return null; // Quick reply has no plugin step in between: both stages at once. return finalizeOutgoing(finalizeOutgoing({ html, text }, context), context, 'last'); }