diff --git a/integration/tests/15-shared-folder-dedup.spec.ts b/integration/tests/15-shared-folder-dedup.spec.ts new file mode 100644 index 000000000..988e6b191 --- /dev/null +++ b/integration/tests/15-shared-folder-dedup.spec.ts @@ -0,0 +1,110 @@ +import { test, expect, type Page } from '@playwright/test'; +import { ACCOUNTS } from './helpers/config'; +import { sendMail } from './helpers/smtp'; +import { JmapClient } from './helpers/jmap'; +import { + login, + addAccount, + seedSettings, + folderRow, + openFolder, + expectFolderCountsSynced, + expectEmailVisible, + emailItem, + forceSync, +} from './helpers/app'; + +/** + * A shared folder reachable through TWO logged-in accounts must show up only + * once in the cross-account views (All mail / Unread) — both the list rows and + * the unread badge. + * + * Scenarios: + * - owner + grantee: alice shares a folder with carol; alice and carol are both + * logged in, so the folder is reachable as alice's own folder AND as a + * shared folder via carol's session. + * - two grantees: alice shares a folder with bob and carol; bob and carol are + * logged in (alice is not), so the folder is reachable via both sessions. + */ +const { alice, bob, carol } = ACCOUNTS; +const SHARED = 'DupShared'; +const ALL_MAIL = '__cross_all__'; +const UNREAD = '__cross_unread__'; + +let seq = 0; +const subj = (l: string) => `IT ${l} ${Date.now()}-${seq++}`; + +async function seedCrossViews(page: Page): Promise { + await seedSettings(page, { + enableUnifiedMailbox: true, + enableCrossAllView: true, + enableCrossUnreadView: true, + includeGroupInUnified: true, + unifiedCrossAccount: true, + }); +} + +async function expectSingleInView(page: Page, view: string, subject: string): Promise { + await openFolder(page, { name: view }); + await forceSync(page); + await expectEmailVisible(page, subject); + // Give a second (duplicate) row time to render before counting. + await page.waitForTimeout(1500); + expect.soft(await emailItem(page, subject).count(), `rows for "${subject}" in ${view}`).toBe(1); +} + +test.describe('Shared folder reachable via two logged-in accounts', () => { + let ja: JmapClient; + let sharedId: string; + + test.beforeEach(async () => { + ja = await JmapClient.connect(alice.email, alice.password); + for (const a of [alice, bob, carol]) await (await JmapClient.connect(a.email, a.password)).reset(); + }); + + test.afterEach(async () => { + // Drop the shared folder so the grants don't leak into other specs. + await ja.reset(); + }); + + async function seedIntoShared(subject: string): Promise { + await sendMail({ from: alice.email, authPass: alice.password, to: alice.email, subject, body: 'x' }); + const m = await ja.waitForEmail(subject); + await ja.moveEmail(m.id, sharedId); + } + + test('owner + grantee logged in: shared message listed and counted once', async ({ page }) => { + sharedId = await ja.createSharedFolder(SHARED, carol.email); + const s = subj('dup-owner'); + await seedIntoShared(s); + + await seedCrossViews(page); + await login(page, alice); + await addAccount(page, carol); + + await expect(folderRow(page, { name: ALL_MAIL }).first()).toBeVisible(); + await expectSingleInView(page, ALL_MAIL, s); + await expectSingleInView(page, UNREAD, s); + + await expectFolderCountsSynced(page, { name: UNREAD }, { unread: 1 }, 15000); + await expectFolderCountsSynced(page, { name: ALL_MAIL }, { unread: 1 }, 15000); + }); + + test('two grantees logged in: shared message listed and counted once', async ({ page }) => { + sharedId = await ja.createSharedFolder(SHARED, carol.email); + await ja.shareMailbox(sharedId, bob.email); + const s = subj('dup-grantees'); + await seedIntoShared(s); + + await seedCrossViews(page); + await login(page, bob); + await addAccount(page, carol); + + await expect(folderRow(page, { name: ALL_MAIL }).first()).toBeVisible(); + await expectSingleInView(page, ALL_MAIL, s); + await expectSingleInView(page, UNREAD, s); + + await expectFolderCountsSynced(page, { name: UNREAD }, { unread: 1 }, 15000); + await expectFolderCountsSynced(page, { name: ALL_MAIL }, { unread: 1 }, 15000); + }); +}); diff --git a/lib/__tests__/unified-mailbox-dedupe.test.ts b/lib/__tests__/unified-mailbox-dedupe.test.ts new file mode 100644 index 000000000..4499a0b96 --- /dev/null +++ b/lib/__tests__/unified-mailbox-dedupe.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import type { Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import { dedupeUnifiedAccounts, getCrossUnreadTotal, type UnifiedAccountClient } from '@/lib/unified-mailbox'; + +const mb = (id: string, role: string | undefined, unread = 0, originalId?: string): Mailbox => + ({ id, name: id, role, unreadEmails: unread, totalEmails: 0, originalId } as unknown as Mailbox); + +const makeAccount = (over: Partial & { accountId: string }): UnifiedAccountClient => ({ + accountLabel: over.accountId, + mailboxes: [], + client: {} as unknown as IJMAPClient, + clientAccountId: over.accountId, + jmapAccountId: over.accountId, + ...over, +}); + +// A folder of JMAP account `owner`, as seen through the login `via`. +const sharedEntry = (owner: string, via: string, mailboxes: Mailbox[]) => + makeAccount({ accountId: owner, jmapAccountId: owner, clientAccountId: via, isShared: true, mailboxes }); + +describe('dedupeUnifiedAccounts', () => { + it('drops a shared entry whose folders the owner login already covers', () => { + const alice = makeAccount({ accountId: 'login-a', jmapAccountId: 'A', mailboxes: [mb('inbox', 'inbox'), mb('f1', undefined, 1)] }); + const carolSeesAlice = sharedEntry('A', 'login-c', [mb('A:f1', undefined, 1, 'f1')]); + const carol = makeAccount({ accountId: 'login-c', jmapAccountId: 'C', mailboxes: [mb('inbox', 'inbox')] }); + + // The grantee's login may come first - the owner's own entry still wins. + const out = dedupeUnifiedAccounts([carol, carolSeesAlice, alice]); + expect(out).toEqual([carol, alice]); + expect(getCrossUnreadTotal(out)).toBe(1); + }); + + it('keeps one copy when two grantee logins reach the same shared folder', () => { + const viaBob = sharedEntry('A', 'login-b', [mb('A:f1', undefined, 1, 'f1')]); + const viaCarol = sharedEntry('A', 'login-c', [mb('A:f1', undefined, 1, 'f1')]); + const out = dedupeUnifiedAccounts([viaBob, viaCarol]); + expect(out).toEqual([viaBob]); + expect(getCrossUnreadTotal(out)).toBe(1); + }); + + it('keeps folders only one of the grantees can reach', () => { + const viaBob = sharedEntry('A', 'login-b', [mb('A:f1', undefined, 1, 'f1')]); + const viaCarol = sharedEntry('A', 'login-c', [mb('A:f1', undefined, 1, 'f1'), mb('A:f2', undefined, 2, 'f2')]); + const out = dedupeUnifiedAccounts([viaBob, viaCarol]); + expect(out.map((a) => a.mailboxes.map((m) => m.id))).toEqual([['A:f1'], ['A:f2']]); + expect(out[1].clientAccountId).toBe('login-c'); + expect(getCrossUnreadTotal(out)).toBe(3); + }); + + it('does not merge distinct owners or personal accounts with equal mailbox ids', () => { + const a = makeAccount({ accountId: 'login-a', jmapAccountId: 'A', mailboxes: [mb('inbox', 'inbox', 1)] }); + const b = makeAccount({ accountId: 'login-b', jmapAccountId: 'B', mailboxes: [mb('inbox', 'inbox', 1)] }); + const group = sharedEntry('G', 'login-a', [mb('G:inbox', 'inbox', 1, 'inbox')]); + expect(dedupeUnifiedAccounts([a, b, group])).toEqual([a, b, group]); + }); +}); diff --git a/lib/unified-mailbox.ts b/lib/unified-mailbox.ts index 4e8815db1..2be114442 100644 --- a/lib/unified-mailbox.ts +++ b/lib/unified-mailbox.ts @@ -786,3 +786,42 @@ export function unifiedLoadProgress(input: { const loaded = new Set(input.scope.filter((e) => !e.isShared).map((e) => e.clientAccountId)).size; return loaded < total ? { loaded, total } : null; } + +/** + * Drops mailboxes that are reachable through more than one logged-in account, + * so each folder contributes to the aggregate views exactly once. + * + * A folder shared with a second login (owner + grantee both logged in), or + * shared with two logins at once, otherwise yields one entry per reaching + * login: its messages list twice and its unread count is summed twice. + * + * A mailbox is identified by its owning JMAP account + server-side id (a shared + * entry's `originalId`). Personal entries always win and are never trimmed, so + * an owner's folder is read through the owner's own login and honors their + * folder selection. Shared entries keep only mailboxes not already covered and + * are dropped when none remain. Order is preserved. + */ +export function dedupeUnifiedAccounts(accounts: UnifiedAccountClient[]): UnifiedAccountClient[] { + const seen = new Set(); + const key = (jmapAccountId: string, mailboxId: string) => `${jmapAccountId}\u0000${mailboxId}`; + for (const account of accounts) { + if (account.isShared) continue; + for (const m of account.mailboxes) seen.add(key(account.jmapAccountId, m.id)); + } + const out: UnifiedAccountClient[] = []; + for (const account of accounts) { + if (!account.isShared) { + out.push(account); + continue; + } + const mailboxes = account.mailboxes.filter((m) => { + const k = key(account.jmapAccountId, m.originalId ?? m.id); + if (seen.has(k)) return false; + seen.add(k); + return true; + }); + if (mailboxes.length === 0) continue; + out.push(mailboxes.length === account.mailboxes.length ? account : { ...account, mailboxes }); + } + return out; +} diff --git a/stores/email-store.ts b/stores/email-store.ts index afd049c22..c5ec41781 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -18,7 +18,7 @@ import { emailSummary, emailToReadView } from "@/lib/plugin-projection"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { emailKeyFor, threadKeyFor, threadIdFromKey, KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils"; import type { ExternalSearchResult } from "@/lib/plugin-types"; -import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, trashAndJunkIds, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; +import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, trashAndJunkIds, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, dedupeUnifiedAccounts, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; import { defaultSearchScopeFor, isAllFoldersSearchScope, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders"; import { useAuthStore } from "@/stores/auth-store"; import { pathNamesMailFolder } from "@/lib/deep-links"; @@ -1395,6 +1395,9 @@ function resolveDestLocalAccountId(mailbox: Mailbox): string | null { * `allMailFolderIds` folder selection, restricting the All mail / Unread / * Starred cross views to the chosen own folders (shared entries are left * unrestricted so all their folders are included). + * + * A folder reachable through several logins (owner + grantee, or two grantees) + * is kept only once - see `dedupeUnifiedAccounts`. */ /** * Folder lists of the logins in the unified scope, shared between builds. @@ -1486,6 +1489,11 @@ export async function buildUnifiedAccountClients( // after the fan-out so single-email actions can resolve role-based // destinations (trash/archive) in the email's own account (issue #281). const fetchedMailboxes: Record = {}; + const ownLoginJmapIds = new Set(); + for (const a of authAccounts) { + const c = allClients.get(a.id); + if (c) ownLoginJmapIds.add(c.getAccountId()); + } // Every login at once rather than one after the other: the slowest login, // not the sum of all of them, bounds how long the scope takes. const fetched = await Promise.allSettled(authAccounts.map(async (a) => { @@ -1538,7 +1546,9 @@ export async function buildUnifiedAccountClients( // Cache the owner's mailbox list keyed by its JMAP id so single-email // and batch actions can resolve role-based destinations (trash/archive) // in the owner account instead of falling back to the active account. - publish(ownerId, ownerMailboxes); + // When the owner is logged in too, its own (un-namespaced) list wins: + // dedupeUnifiedAccounts routes that folder through the owner's login. + if (!ownLoginJmapIds.has(ownerId)) publish(ownerId, ownerMailboxes); } } } catch { @@ -1550,7 +1560,7 @@ export async function buildUnifiedAccountClients( accountMailboxes: { ...state.accountMailboxes, ...fetchedMailboxes }, })); } - return built; + return dedupeUnifiedAccounts(built); } /**