Merge pull request #1108 from hildebrandttk/fix/cross-view-shared-folder-dedup

fix: list a shared folder reachable through two logins only once in c…

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<void> {
+  await seedSettings(page, {
+    enableUnifiedMailbox: true,
+    enableCrossAllView: true,
+    enableCrossUnreadView: true,
+    includeGroupInUnified: true,
+    unifiedCrossAccount: true,
+  });
+}
+
+async function expectSingleInView(page: Page, view: string, subject: string): Promise<void> {
+  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<void> {
+    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);
+  });
+});

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<UnifiedAccountClient> & { 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]);
+  });
+});

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<string>();
+  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;
+}

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<string, Mailbox[]> = {};
+  const ownLoginJmapIds = new Set<string>();
+  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);
 }
 
 /**