mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-02 16:15:39 +00:00
feat(plugins): star and after-send hooks name the mail's account
Message ids repeat across accounts, so a hook payload naming only an emailId could not tell a plugin which mailbox the message lies in when it is a shared or group account's. onBeforeEmailStarToggle and onEmailStarToggle now carry accountId when the starred message is not in the user's own account, as the message-row-actions click context does. Every star (the row's button, the reader's star, the key, the menu) goes through toggleStar, one message at a time, so one accountId names it; the account is resolved before the plugins are asked. An own message's payload is unchanged. onAfterEmailSend now carries accountId, the JMAP account the sent message lies in (a mail sent from a shared or group identity lies in that account), so api.email.get finds it. sendEmail returns that account as emailAccountId, the S/MIME send too. The hook also carries composerId, the id the sending composer's slots and actions gave plugins, so a plugin can act on what the user chose in that composer.
This commit is contained in:
@@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import React from 'react';
|
||||
import { EmailComposer } from '../email-composer';
|
||||
import { registerOutgoingFinalizer, registerPreSendCheck, resetSendPipeline } from '@/lib/outgoing/send-pipeline';
|
||||
import { composerHandle } from '@/lib/plugin-sandbox/composer-registry';
|
||||
|
||||
// ─── Heavy component mocks (mirrors composer-reopened-draft-signature.test.tsx) ─
|
||||
|
||||
@@ -231,6 +232,16 @@ describe('composer send pipeline', () => {
|
||||
})]);
|
||||
});
|
||||
|
||||
it('names the composer plugins know it by, for onAfterEmailSend', async () => {
|
||||
let open = false;
|
||||
const onSend = vi.fn((data: { composerId?: string }) => { open = !!composerHandle(data.composerId); });
|
||||
render(<EmailComposer initialData={DRAFT} onSend={onSend} />);
|
||||
clickSend();
|
||||
await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1));
|
||||
expect(onSend.mock.calls[0][0]).toEqual(expect.objectContaining({ composerId: expect.any(String) }));
|
||||
expect(open).toBe(true);
|
||||
});
|
||||
|
||||
it('sends nothing when a check says no, and sends after it says yes', async () => {
|
||||
let allow = false;
|
||||
const check = vi.fn(() => allow);
|
||||
|
||||
@@ -296,6 +296,8 @@ interface EmailComposerProps {
|
||||
requireTls?: boolean;
|
||||
/** Importance / X-Priority; absent for normal (#970). */
|
||||
priority?: MessagePriority;
|
||||
/** The id plugins know this composer by, for onAfterEmailSend. */
|
||||
composerId?: string;
|
||||
}) => void | false | Promise<void | false>;
|
||||
onScheduledSendCreated?: () => void | Promise<void>;
|
||||
onClose?: () => void;
|
||||
@@ -3667,6 +3669,7 @@ export function EmailComposer({
|
||||
requireTls: requireTls || undefined,
|
||||
priority: priority !== 'normal' ? priority : undefined,
|
||||
delayedUntil: effectiveDelayedUntil,
|
||||
composerId: pluginComposerIdRef.current ?? undefined,
|
||||
});
|
||||
// Not when nothing went: no host, or one that sent nothing.
|
||||
if (draftElsewhere && onSend && outcome !== false) await removeDraftVersion(finalDraftId!, draftLogin, draftAccountAtSend);
|
||||
|
||||
@@ -25,7 +25,7 @@ const sendData = vi.hoisted(() => ({ delayedUntil: undefined as string | undefin
|
||||
vi.mock('@/components/email/email-composer', () => ({
|
||||
EmailComposer: ({ onSend }: { onSend?: (data: unknown) => Promise<void> }) => (
|
||||
<button onClick={() => {
|
||||
onSend?.({ to: ['bob@example.com'], cc: ['carol@example.com'], bcc: [], subject: 'Hi', body: 'Hello', delayedUntil: sendData.delayedUntil })
|
||||
onSend?.({ to: ['bob@example.com'], cc: ['carol@example.com'], bcc: [], subject: 'Hi', body: 'Hello', delayedUntil: sendData.delayedUntil, composerId: 'composer-1' })
|
||||
.catch((e) => { sendData.error = e; });
|
||||
}}>
|
||||
stub send
|
||||
@@ -121,14 +121,14 @@ describe('onAfterEmailSend', () => {
|
||||
useSettingsStore.setState({ sendDelaySeconds: 0 } as never);
|
||||
const sent = vi.fn();
|
||||
emailHooks.onAfterEmailSend.register('test', sent);
|
||||
sendEmail.mockResolvedValue({ scheduled: false, emailId: 'e1', emailSubmissionId: 's1' });
|
||||
sendEmail.mockResolvedValue({ scheduled: false, emailId: 'e1', emailAccountId: 'acc', emailSubmissionId: 's1' });
|
||||
|
||||
await sendFromComposer();
|
||||
|
||||
await waitFor(() => expect(sent).toHaveBeenCalledTimes(1));
|
||||
expect(sent).toHaveBeenCalledWith({
|
||||
emailId: 'e1', submissionId: 's1', to: ['bob@example.com'], cc: ['carol@example.com'], bcc: [],
|
||||
subject: 'Hi', scheduledFor: null,
|
||||
emailId: 'e1', accountId: 'acc', submissionId: 's1', to: ['bob@example.com'], cc: ['carol@example.com'], bcc: [],
|
||||
subject: 'Hi', scheduledFor: null, composerId: 'composer-1',
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2150,6 +2150,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
|
||||
requestDsn?: boolean;
|
||||
requireTls?: boolean;
|
||||
priority?: MessagePriority;
|
||||
composerId?: string;
|
||||
}) => {
|
||||
// Nothing is sent: the composer keeps what only goes once the mail is out.
|
||||
if (!client) return false;
|
||||
|
||||
@@ -77,3 +77,18 @@ describe('sendEmail without a From address from the composer (#1377)', () => {
|
||||
expect(created[0].from).toEqual([{ name: 'Sales', email: 'sales@kisaanu.org' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('sendEmail names the account the sent message lies in', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('the own account, or the group account a group identity sends from', async () => {
|
||||
const client = createClient('me@example.com');
|
||||
mockServer([{ id: 'identity-1', email: 'me@example.com' }]);
|
||||
expect(await send(client)).toMatchObject({ emailId: 'email-9', emailAccountId: 'account-1' });
|
||||
|
||||
vi.spyOn(client, 'resolveSendAccountId').mockResolvedValue('group-1');
|
||||
expect(await send(client)).toMatchObject({ emailId: 'email-9', emailAccountId: 'group-1' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -22,7 +22,7 @@ describe('announceSent for a send held back for undo', () => {
|
||||
vi.advanceTimersByTime(9_000);
|
||||
expect(sent).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(1_000);
|
||||
expect(sent).toHaveBeenCalledWith({ emailId: 'e1', submissionId: 's1', ...message, scheduledFor: null });
|
||||
expect(sent).toHaveBeenCalledWith({ emailId: 'e1', accountId: null, submissionId: 's1', ...message, scheduledFor: null, composerId: null });
|
||||
});
|
||||
|
||||
it('reports at once on "Send now"', () => {
|
||||
@@ -44,3 +44,15 @@ describe('announceSent for a send held back for undo', () => {
|
||||
expect(sent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('announceSent names the message and the composer', () => {
|
||||
it("gives the account the message lies in and the sending composer's id", () => {
|
||||
const sent = vi.fn();
|
||||
emailHooks.onAfterEmailSend.register('t', sent);
|
||||
announceSent(
|
||||
{ scheduled: false, emailId: 'e9', emailAccountId: 'group', emailSubmissionId: 's9' },
|
||||
{ ...message, composerId: 'composer-1' },
|
||||
);
|
||||
expect(sent).toHaveBeenCalledWith(expect.objectContaining({ emailId: 'e9', accountId: 'group', composerId: 'composer-1' }));
|
||||
});
|
||||
});
|
||||
|
||||
+4
-4
@@ -5228,8 +5228,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
// who doesn't.
|
||||
const rejectedRecipientsResult = rejected.length ? rejected : undefined;
|
||||
return delayedUntil
|
||||
? { scheduled: true, emailId: createdEmailId, emailSubmissionId, sendAt: serverSendAt, submissionAccountId, filingError, rejectedRecipients: rejectedRecipientsResult }
|
||||
: { scheduled: false, emailId: createdEmailId, emailSubmissionId, submissionAccountId, filingError, rejectedRecipients: rejectedRecipientsResult };
|
||||
? { scheduled: true, emailId: createdEmailId, emailAccountId: targetAccountId, emailSubmissionId, sendAt: serverSendAt, submissionAccountId, filingError, rejectedRecipients: rejectedRecipientsResult }
|
||||
: { scheduled: false, emailId: createdEmailId, emailAccountId: targetAccountId, emailSubmissionId, submissionAccountId, filingError, rejectedRecipients: rejectedRecipientsResult };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -11254,8 +11254,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
const rejectedRecipientsResult = rejected.length ? rejected : undefined;
|
||||
return delayedUntil
|
||||
? { scheduled: true, emailId, emailSubmissionId, sendAt: serverSendAt, submissionAccountId, isSmime: true, rejectedRecipients: rejectedRecipientsResult }
|
||||
: { scheduled: false, emailId, emailSubmissionId, submissionAccountId, isSmime: true, rejectedRecipients: rejectedRecipientsResult };
|
||||
? { scheduled: true, emailId, emailAccountId: this.accountId, emailSubmissionId, sendAt: serverSendAt, submissionAccountId, isSmime: true, rejectedRecipients: rejectedRecipientsResult }
|
||||
: { scheduled: false, emailId, emailAccountId: this.accountId, emailSubmissionId, submissionAccountId, isSmime: true, rejectedRecipients: rejectedRecipientsResult };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -124,6 +124,12 @@ export interface SendEmailResult {
|
||||
* search for it.
|
||||
*/
|
||||
submissionAccountId?: string;
|
||||
/**
|
||||
* JMAP account the sent message (`emailId`) lies in: a shared or group
|
||||
* account when sent from its identity. Ids repeat across accounts, so the
|
||||
* id alone does not name the message.
|
||||
*/
|
||||
emailAccountId?: string;
|
||||
/**
|
||||
* Set when the submission succeeded but a post-send step was rejected
|
||||
* (the implicit onSuccessUpdateEmail filing patch, or destroying the
|
||||
|
||||
@@ -7,6 +7,11 @@
|
||||
// hold runs out or the user picks "Send now". Undo drops it: that message
|
||||
// never leaves.
|
||||
// A failed send never gets here; the callers only report a resolved send.
|
||||
//
|
||||
// `accountId` names the account the sent message lies in (ids repeat across
|
||||
// accounts; a group identity's send lies in the group's), `composerId` the
|
||||
// composer it came from, as its slots and actions know it, so a plugin can
|
||||
// act on what the user chose there.
|
||||
|
||||
import type { SendEmailResult } from './jmap/types';
|
||||
import { emailHooks } from './plugin-hooks';
|
||||
@@ -18,16 +23,21 @@ export interface SentMessage {
|
||||
subject: string;
|
||||
/** Set when the user scheduled the message; absent for an undo-send hold. */
|
||||
delayedUntil?: string;
|
||||
/** The id plugins know the sending composer by (composer-registry.ts). */
|
||||
composerId?: string;
|
||||
}
|
||||
|
||||
interface AfterSendPayload {
|
||||
emailId: string | null;
|
||||
/** The JMAP account `emailId` lies in. */
|
||||
accountId: string | null;
|
||||
submissionId: string | null;
|
||||
to: string[];
|
||||
cc: string[];
|
||||
bcc: string[];
|
||||
subject: string;
|
||||
scheduledFor: string | null;
|
||||
composerId: string | null;
|
||||
}
|
||||
|
||||
const held = new Map<string, { timer: ReturnType<typeof setTimeout>; payload: AfterSendPayload }>();
|
||||
@@ -41,12 +51,14 @@ function emit(payload: AfterSendPayload): void {
|
||||
export function announceSent(result: SendEmailResult, message: SentMessage): void {
|
||||
const payload: AfterSendPayload = {
|
||||
emailId: result.emailId ?? null,
|
||||
accountId: result.emailAccountId ?? null,
|
||||
submissionId: result.emailSubmissionId ?? null,
|
||||
to: [...message.to],
|
||||
cc: [...message.cc],
|
||||
bcc: [...message.bcc],
|
||||
subject: message.subject,
|
||||
scheduledFor: null,
|
||||
composerId: message.composerId ?? null,
|
||||
};
|
||||
if (message.delayedUntil) {
|
||||
emit({ ...payload, scheduledFor: result.sendAt ?? message.delayedUntil });
|
||||
|
||||
@@ -80,7 +80,7 @@ export const HOOK_CATALOG = {
|
||||
onBeforeEmailSend: H('mail-send', 'intercept', 'email:send', 'OutgoingEmail', 'when Send is pressed; a handler that fails or times out stops the send'),
|
||||
onTransformOutgoingEmail: H('mail-send', 'transform', 'email:send', 'OutgoingEmail', 'right before the message is submitted'),
|
||||
onComposeSend: H('mail-send', 'takeover', 'email:send', 'ComposeSendRequest', 'at the start of sending; may send the message itself'),
|
||||
onAfterEmailSend: H('mail-send', 'observer', 'email:read', '{ emailId, submissionId, to, cc, bcc, subject, scheduledFor }', 'after the server accepted the message'),
|
||||
onAfterEmailSend: H('mail-send', 'observer', 'email:read', '{ emailId, accountId, submissionId, to, cc, bcc, subject, scheduledFor, composerId }', 'after the server accepted the message'),
|
||||
|
||||
// ─── Mail: organizing ──────────────────────────────────────
|
||||
onBeforeEmailDelete: H('mail-organize', 'intercept', 'email:write', '{ emailIds, permanent }', 'before messages are deleted or moved to Trash'),
|
||||
@@ -94,8 +94,8 @@ export const HOOK_CATALOG = {
|
||||
onEmailSpamToggle: H('mail-organize', 'observer', 'email:read', '{ emailIds, spam }', 'after messages were marked as spam or not spam'),
|
||||
onBeforeEmailReadStateChange: H('mail-organize', 'intercept', 'email:write', '{ emailIds, read }', 'before messages are marked read or unread'),
|
||||
onEmailReadStateChange: H('mail-organize', 'observer', 'email:read', '{ emailIds, read }', 'after messages were marked read or unread'),
|
||||
onBeforeEmailStarToggle: H('mail-organize', 'intercept', 'email:write', '{ emailIds, starred }', 'before messages are starred or unstarred'),
|
||||
onEmailStarToggle: H('mail-organize', 'observer', 'email:read', '{ emailIds, starred }', 'after messages were starred or unstarred'),
|
||||
onBeforeEmailStarToggle: H('mail-organize', 'intercept', 'email:write', '{ emailIds, starred, accountId }', 'before messages are starred or unstarred; `accountId` names the messages\' account when it is not the user\'s own (a team or shared mailbox)'),
|
||||
onEmailStarToggle: H('mail-organize', 'observer', 'email:read', '{ emailIds, starred, accountId }', 'after messages were starred or unstarred; `accountId` as on onBeforeEmailStarToggle'),
|
||||
onBeforeEmailKeywordChange: H('mail-organize', 'intercept', 'email:write', '{ emailIds, added: string[], removed: string[] }', 'before tags or other keywords are changed'),
|
||||
onEmailKeywordChange: H('mail-organize', 'observer', 'email:read', '{ emailIds, added: string[], removed: string[] }', 'after tags or other keywords changed'),
|
||||
onBeforeEmailCategorize: H('mail-organize', 'intercept', 'email:read', 'EmailCategorizeContext', 'before messages are moved to another inbox tab'),
|
||||
|
||||
@@ -373,6 +373,45 @@ describe('read, star and keyword hooks', () => {
|
||||
expect(after).toHaveBeenCalledWith({ emailIds: ['e1'], starred: true });
|
||||
});
|
||||
|
||||
it('names a group mailbox\'s account on a star toggle', async () => {
|
||||
const groupInbox = makeMailbox({ id: 'vertrieb:inbox', originalId: 'inbox', role: 'inbox', isShared: true, accountId: 'vertrieb' });
|
||||
openFolder('vertrieb:inbox', [makeEmail('e1', { mailboxIds: { 'vertrieb:inbox': true } })]);
|
||||
useEmailStore.setState({ mailboxes: [...mailboxes, groupInbox] });
|
||||
const before = vi.fn(() => true);
|
||||
const after = vi.fn();
|
||||
emailHooks.onBeforeEmailStarToggle.register('t', before);
|
||||
emailHooks.onEmailStarToggle.register('t', after);
|
||||
|
||||
expect(await useEmailStore.getState().toggleStar(client, 'e1')).toBe(true);
|
||||
|
||||
expect(client.toggleStar).toHaveBeenCalledWith('e1', true, 'vertrieb');
|
||||
expect(before).toHaveBeenCalledWith({ emailIds: ['e1'], starred: true, accountId: 'vertrieb' });
|
||||
expect(after).toHaveBeenCalledWith({ emailIds: ['e1'], starred: true, accountId: 'vertrieb' });
|
||||
});
|
||||
|
||||
it('names a stamped row\'s account only when it is not the own one', async () => {
|
||||
openFolder('inbox', [
|
||||
makeEmail('own', { sourceClientAccountId: 'acct', sourceAccountId: 'me' }),
|
||||
makeEmail('shared', { sourceClientAccountId: 'acct', sourceAccountId: 'vertrieb' }),
|
||||
]);
|
||||
const before = vi.fn(() => true);
|
||||
const after = vi.fn();
|
||||
emailHooks.onBeforeEmailStarToggle.register('t', before);
|
||||
emailHooks.onEmailStarToggle.register('t', after);
|
||||
|
||||
await useEmailStore.getState().toggleStar(client, 'own');
|
||||
await useEmailStore.getState().toggleStar(client, 'shared');
|
||||
|
||||
expect(before.mock.calls).toEqual([
|
||||
[{ emailIds: ['own'], starred: true }],
|
||||
[{ emailIds: ['shared'], starred: true, accountId: 'vertrieb' }],
|
||||
]);
|
||||
expect(after.mock.calls).toEqual([
|
||||
[{ emailIds: ['own'], starred: true }],
|
||||
[{ emailIds: ['shared'], starred: true, accountId: 'vertrieb' }],
|
||||
]);
|
||||
});
|
||||
|
||||
it('stops a star toggle a plugin refuses', async () => {
|
||||
openFolder('inbox', [makeEmail('e1')]);
|
||||
emailHooks.onBeforeEmailStarToggle.register('t', () => false);
|
||||
|
||||
@@ -5509,13 +5509,17 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
if (!email) return false;
|
||||
|
||||
const isFlagged = email.keywords.$flagged || false;
|
||||
if (emailHooks.onBeforeEmailStarToggle.size > 0
|
||||
&& !(await emailHooks.onBeforeEmailStarToggle.intercept({ emailIds: [emailId], starred: !isFlagged }))) {
|
||||
return false;
|
||||
}
|
||||
// In unified view route to the email's own account client + owner accountId
|
||||
// (the reaching client's primary is not the owner for shared sources). (#281)
|
||||
const { client: actionClient, accountId } = resolveEmailActionContext(email, client);
|
||||
// The hooks name the account as message-row-actions' click does: only
|
||||
// when it is not the user's own, since ids repeat across accounts.
|
||||
const hookAccountId = accountId && accountId !== actionClient.getAccountId?.() ? accountId : undefined;
|
||||
const hookPayload = { emailIds: [emailId], starred: !isFlagged, ...(hookAccountId ? { accountId: hookAccountId } : {}) };
|
||||
if (emailHooks.onBeforeEmailStarToggle.size > 0
|
||||
&& !(await emailHooks.onBeforeEmailStarToggle.intercept(hookPayload))) {
|
||||
return false;
|
||||
}
|
||||
await actionClient.toggleStar(emailId, !isFlagged, accountId);
|
||||
|
||||
// Update local state
|
||||
@@ -5535,7 +5539,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
emailId,
|
||||
),
|
||||
}));
|
||||
void emailHooks.onEmailStarToggle.emit({ emailIds: [emailId], starred: !isFlagged });
|
||||
void emailHooks.onEmailStarToggle.emit(hookPayload);
|
||||
refillAfterKeywordChange(get, client, ['$flagged']);
|
||||
} catch (error) {
|
||||
set({
|
||||
|
||||
Reference in New Issue
Block a user