Merge pull request #1467 from HOLOEYE-JZ/feat/plugin-hooks-account-and-composer

feat(plugins): star and after-send hooks name the mail's account

components/email/__tests__/composer-send-pipeline.test.tsx

@@ -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);

components/email/email-composer.tsx

@@ -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;
@@ -3679,6 +3681,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);

components/mail/__tests__/after-send-hook.test.tsx

@@ -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',
     });
   });
 

components/mail/mail-app.tsx

@@ -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;

lib/__tests__/jmap-send-sender-address.test.ts

@@ -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' });
+  });
+});

lib/__tests__/plugin-after-send.test.ts

@@ -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' }));
+  });
+});

lib/jmap/client.ts

@@ -5236,8 +5236,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 };
   }
 
   /**
@@ -11308,8 +11308,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 };
   }
 
   /**

lib/jmap/types.ts

@@ -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

lib/plugin-after-send.ts

@@ -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 });

lib/plugin-hook-catalog.ts

@@ -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'),

stores/__tests__/email-store-plugin-organize-hooks.test.ts

@@ -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);

stores/email-store.ts

@@ -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({