diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index 77af5192..038e45f8 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -1172,18 +1172,22 @@ export default function Home() { return; } - // Mark the original email with $answered or $forwarded keyword - if (originalEmailId && (effectiveMode === 'reply' || effectiveMode === 'replyAll')) { + // Mark the original email with $answered or $forwarded keyword. Route the + // write to the email's own account so the flag lands on shared/group-mailbox + // messages instead of being dropped against the reaching account. (#281) + if (originalEmailId && (effectiveMode === 'reply' || effectiveMode === 'replyAll' || effectiveMode === 'forward')) { + const s = useEmailStore.getState(); + const orig = s.emails.find(e => e.id === originalEmailId); + const kwClientId = s.isUnifiedView ? orig?.sourceClientAccountId : undefined; + const kwAccountId = s.isUnifiedView ? orig?.sourceAccountId : undefined; + const kwClient = kwClientId + ? (useAuthStore.getState().getClientForAccount(kwClientId) ?? client) + : client; + const keyword = effectiveMode === 'forward' ? '$forwarded' : '$answered'; try { - await client.setKeyword(originalEmailId, '$answered'); + await kwClient.setKeyword(originalEmailId, keyword, kwAccountId); } catch (e) { - debug.error('Failed to set $answered keyword:', e); - } - } else if (originalEmailId && effectiveMode === 'forward') { - try { - await client.setKeyword(originalEmailId, '$forwarded'); - } catch (e) { - debug.error('Failed to set $forwarded keyword:', e); + debug.error(`Failed to set ${keyword} keyword:`, e); } } @@ -1663,8 +1667,20 @@ export default function Home() { } } + // In unified view route the write to the email's own account, reached + // through the login it is reachable via (`sourceClientAccountId`) and + // applied to its owning JMAP account (`sourceAccountId`). For personal + // sources these resolve to the account itself, so behavior is unchanged. + // Without this, tags on shared/group-mailbox messages are written to the + // reaching account and silently dropped by the server. (#281) + const tagClientId = isUnifiedView ? email.sourceClientAccountId : undefined; + const tagAccountId = isUnifiedView ? email.sourceAccountId : undefined; + const tagClient = tagClientId + ? (useAuthStore.getState().getClientForAccount(tagClientId) ?? client) + : client; + // Update email keywords via JMAP - await client.updateEmailKeywords(emailId, keywords); + await tagClient.updateEmailKeywords(emailId, keywords, tagAccountId); // Patch the email in place so the list keeps its scroll/pagination state // instead of being reset to the first page by a full refetch. @@ -2271,11 +2287,22 @@ export default function Home() { return; } - // Mark the original email as answered - try { - await client.setKeyword(originalEmailId, '$answered'); - } catch (e) { - debug.error('Failed to set $answered keyword:', e); + // Mark the original email as answered. Route the write to the email's own + // account so the flag lands on shared/group-mailbox messages instead of + // being dropped against the reaching account. (#281) + { + const s = useEmailStore.getState(); + const orig = s.emails.find(e => e.id === originalEmailId); + const kwClientId = s.isUnifiedView ? orig?.sourceClientAccountId : undefined; + const kwAccountId = s.isUnifiedView ? orig?.sourceAccountId : undefined; + const kwClient = kwClientId + ? (useAuthStore.getState().getClientForAccount(kwClientId) ?? client) + : client; + try { + await kwClient.setKeyword(originalEmailId, '$answered', kwAccountId); + } catch (e) { + debug.error('Failed to set $answered keyword:', e); + } } // Refresh emails to show the sent reply diff --git a/hooks/use-tag-drop.ts b/hooks/use-tag-drop.ts index 61e7a391..2c0d2803 100644 --- a/hooks/use-tag-drop.ts +++ b/hooks/use-tag-drop.ts @@ -74,14 +74,26 @@ export function useTagDrop({ tagId, onSuccess, onError }: UseTagDropOptions): Us for (const emailId of emailIds) { // Read fresh state to avoid stale closures - const currentEmails = useEmailStore.getState().emails; - const email = currentEmails.find(em => em.id === emailId); + const emailState = useEmailStore.getState(); + const email = emailState.emails.find(em => em.id === emailId); const keywords = { ...(email?.keywords || {}) }; // Add the tag without removing existing ones keywords[`$label:${tagId}`] = true; - await client.updateEmailKeywords(emailId, keywords); + // In unified view route the write to the email's own account, reached + // through the login it is reachable via (`sourceClientAccountId`) and + // applied to its owning JMAP account (`sourceAccountId`). For personal + // sources these resolve to the account itself, so behavior is unchanged. + // Without this, tags on shared/group-mailbox messages are written to the + // reaching account and silently dropped by the server. (#281) + const tagClientId = emailState.isUnifiedView ? email?.sourceClientAccountId : undefined; + const tagAccountId = emailState.isUnifiedView ? email?.sourceAccountId : undefined; + const tagClient = tagClientId + ? (useAuthStore.getState().getClientForAccount(tagClientId) ?? client) + : client; + + await tagClient.updateEmailKeywords(emailId, keywords, tagAccountId); } // Refresh the email list diff --git a/lib/__tests__/jmap-keyword-account.test.ts b/lib/__tests__/jmap-keyword-account.test.ts new file mode 100644 index 00000000..7c4585e3 --- /dev/null +++ b/lib/__tests__/jmap-keyword-account.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; + +// Regression coverage for the shared-account keyword write path (#281): keyword +// mutations (tags, $answered/$forwarded) on a unified-inbox message must target +// the email's owning account, not the reaching client's primary. Writing to the +// primary account silently no-ops server-side (JMAP returns notUpdated without +// throwing), so the keyword is lost on the next reload. toggleStar already +// threaded accountId through; updateEmailKeywords/setKeyword did not. + +function createClient(): JMAPClient { + const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); + Object.assign(client, { + apiUrl: 'https://jmap.example.com/api', + accountId: 'primary-account', + username: 'user@example.com', + }); + return client; +} + +interface JMAPMethodCall { + 0: string; + 1: Record; + 2: string; +} + +function mockEmailSet() { + const captured: JMAPMethodCall[] = []; + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + fetchSpy.mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + captured.push(...body.methodCalls); + return new Response(JSON.stringify({ methodResponses: [['Email/set', { updated: {} }, '0']] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + return { captured, fetchSpy }; +} + +describe('JMAP keyword writes route to the email account (#281)', () => { + beforeEach(() => vi.restoreAllMocks()); + afterEach(() => vi.restoreAllMocks()); + + it('updateEmailKeywords sends the explicit accountId', async () => { + const client = createClient(); + const { captured } = mockEmailSet(); + await client.updateEmailKeywords('email-x', { '$label:work': true }, 'shared-account'); + expect(captured[0][0]).toBe('Email/set'); + expect(captured[0][1].accountId).toBe('shared-account'); + }); + + it('updateEmailKeywords falls back to the primary account when none is given', async () => { + const client = createClient(); + const { captured } = mockEmailSet(); + await client.updateEmailKeywords('email-x', { '$label:work': true }); + expect(captured[0][1].accountId).toBe('primary-account'); + }); + + it('setKeyword sends the explicit accountId', async () => { + const client = createClient(); + const { captured } = mockEmailSet(); + await client.setKeyword('email-x', '$answered', 'shared-account'); + expect(captured[0][0]).toBe('Email/set'); + expect(captured[0][1].accountId).toBe('shared-account'); + }); + + it('setKeyword falls back to the primary account when none is given', async () => { + const client = createClient(); + const { captured } = mockEmailSet(); + await client.setKeyword('email-x', '$answered'); + expect(captured[0][1].accountId).toBe('primary-account'); + }); +}); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 1cbbe0dc..5b332cfa 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -94,8 +94,8 @@ export interface IJMAPClient { markAsRead(emailId: string, read?: boolean, accountId?: string): Promise; batchMarkAsRead(emailIds: string[], read?: boolean, accountId?: string): Promise; toggleStar(emailId: string, starred: boolean, accountId?: string): Promise; - updateEmailKeywords(emailId: string, keywords: Record): Promise; - setKeyword(emailId: string, keyword: string): Promise; + updateEmailKeywords(emailId: string, keywords: Record, accountId?: string): Promise; + setKeyword(emailId: string, keyword: string, accountId?: string): Promise; migrateKeyword(oldKeyword: string, newKeyword: string): Promise; deleteEmail(emailId: string, accountId?: string): Promise; moveToTrash(emailId: string, trashMailboxId: string, accountId?: string, markAsRead?: boolean): Promise; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 184c5883..7c04ec45 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1301,10 +1301,10 @@ export class JMAPClient implements IJMAPClient { ]); } - async updateEmailKeywords(emailId: string, keywords: Record): Promise { + async updateEmailKeywords(emailId: string, keywords: Record, accountId?: string): Promise { await this.request([ ["Email/set", { - accountId: this.accountId, + accountId: accountId || this.accountId, update: { [emailId]: { keywords, @@ -1314,10 +1314,10 @@ export class JMAPClient implements IJMAPClient { ]); } - async setKeyword(emailId: string, keyword: string): Promise { + async setKeyword(emailId: string, keyword: string, accountId?: string): Promise { await this.request([ ["Email/set", { - accountId: this.accountId, + accountId: accountId || this.accountId, update: { [emailId]: { [`keywords/${keyword}`]: true,