From c49f13db2868335745923b8088e54c34ceda9b2d Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Fri, 25 Sep 2026 19:41:16 +0200 Subject: [PATCH] fix: list a shared folder reachable through two logins only once in cross views When a shared folder was reachable through two logged-in accounts (owner + grantee, or two grantees), buildUnifiedAccountClients emitted one entry per reaching login. All mail / All unread then listed each message twice and the unread badge counted it twice. dedupeUnifiedAccounts keeps each (owning JMAP account, mailbox) once: personal entries win, shared entries keep only folders not already covered and are dropped when none remain. The owner's own mailbox list is no longer overwritten in accountMailboxes by a grantee's namespaced copy, so actions on the deduped messages resolve trash/archive in the right id space. --- .../tests/15-shared-folder-dedup.spec.ts | 110 ++++++++++++++++++ lib/__tests__/unified-mailbox-dedupe.test.ts | 57 +++++++++ lib/unified-mailbox.ts | 39 +++++++ stores/email-store.ts | 16 ++- 4 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 integration/tests/15-shared-folder-dedup.spec.ts create mode 100644 lib/__tests__/unified-mailbox-dedupe.test.ts 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 a9db07bf1..0cc812726 100644 --- a/lib/unified-mailbox.ts +++ b/lib/unified-mailbox.ts @@ -659,3 +659,42 @@ export function connectedAccountsGrew(previous: string | null, current: string): const before = new Set(previous ? previous.split(',').filter(Boolean) : []); return current.split(',').some((id) => id !== '' && !before.has(id)); } + +/** + * 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 88365d9a0..c69b8bd1e 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -10,7 +10,7 @@ import { emailHooks } from "@/lib/plugin-hooks"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils"; import type { ExternalSearchResult } from "@/lib/plugin-types"; -import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; +import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, dedupeUnifiedAccounts, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; import { useAuthStore } from "@/stores/auth-store"; import { useAccountStore } from "@/stores/account-store"; import { useMessageListTabsStore } from "@/stores/message-list-tabs-store"; @@ -915,6 +915,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`. */ export async function buildUnifiedAccountClients( opts: { includeGroup?: boolean; scopeToClientAccountId?: string } = {}, @@ -929,6 +932,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()); + } for (const a of authAccounts) { const c = allClients.get(a.id); if (!c) continue; @@ -970,7 +978,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. - fetchedMailboxes[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)) fetchedMailboxes[ownerId] = ownerMailboxes; } } } catch { @@ -982,7 +992,7 @@ export async function buildUnifiedAccountClients( accountMailboxes: { ...state.accountMailboxes, ...fetchedMailboxes }, })); } - return built; + return dedupeUnifiedAccounts(built); } /**