fix(email-store): route shared-folder batch actions to the owner account

Batch actions (delete, move, archive, mark-as-read) performed while
viewing a shared/group mailbox directly from the "Shared" sidebar section
were dispatched to the user's OWN account instead of the shared owner
account. Emails in that view are undecorated (no sourceAccountId, that is
only set in unified/cross-account views) and are reached through the
active client, so they fell into the '__default__' bucket / non-unified
else-branch, which defaults the JMAP accountId to the active account.
batchArchive independently picked the archive folder from the merged
mailbox list, where the user's own archive is listed first.

Stalwart then applies Email/set to the wrong account: because the ids
belong to the shared account it returns them as `updated: null` with an
unchanged state (a silent no-op, not `notUpdated`), so the UI drops the
rows optimistically and they reappear on the next reload. It only appears
to work when the own and shared folder ids happen to collide.

Add resolveViewAccountId() — the owner accountId of the directly-viewed
shared folder (from the selected namespaced mailbox), undefined for a
normal own-account view, mirroring fetchEmails and the single-email path.
Route the four batch actions to that owner account (via the active
client); batchMoveToMailbox also resolves the destination to its bare
originalId, and batchArchive scopes the archive folder to that account.
Own-account and unified/cross-account views are unchanged.

Adds email-store-shared-folder-actions.test.ts covering all four batch
actions in the non-unified shared view plus an own-account regression.
This commit is contained in:
KazNIISA IT
2026-07-21 20:55:26 +02:00
committed by Linus Rath
parent 18e9cf6ee6
commit 88b07a1713
2 changed files with 216 additions and 5 deletions
@@ -0,0 +1,173 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { useEmailStore } from '../email-store';
import { useAuthStore } from '../auth-store';
import { useSettingsStore } from '@/stores/settings-store';
import type { Email, Mailbox } from '@/lib/jmap/types';
import type { IJMAPClient } from '@/lib/jmap/client-interface';
// Regression coverage for batch actions performed while viewing a shared/group
// mailbox DIRECTLY (the "Shared" sidebar section), not through the unified inbox.
//
// In this view the emails are undecorated (no `sourceAccountId`): they are
// fetched from the owner account through the active login client, and only their
// `mailboxIds` are namespaced. The batch actions grouped such emails under the
// `'__default__'` bucket and dispatched them to the *user's own* account with no
// JMAP accountId. Stalwart then returns the ids as `updated: null` with an
// unchanged state (a silent no-op, no `notUpdated`), so the UI drops the rows
// optimistically and they reappear on the next reload. The action must instead
// carry the viewed shared folder's owner accountId (reached via the active
// client), exactly like the single-email path and `fetchEmails` already do.
function makeMailbox(overrides: Partial<Mailbox> = {}): Mailbox {
return {
id: 'inbox',
name: 'Inbox',
sortOrder: 0,
totalEmails: 0,
unreadEmails: 0,
totalThreads: 0,
unreadThreads: 0,
myRights: {
mayReadItems: true,
mayAddItems: true,
mayRemoveItems: true,
maySetSeen: true,
maySetKeywords: true,
mayCreateChild: true,
mayRename: true,
mayDelete: true,
maySubmit: true,
},
isSubscribed: true,
isShared: false,
...overrides,
};
}
function makeEmail(overrides: Partial<Email> = {}): Email {
return {
id: 'email-1',
threadId: 'thread-1',
subject: 'Hi',
receivedAt: new Date().toISOString(),
keywords: {},
mailboxIds: {},
...overrides,
} as Email;
}
function makeClient() {
return {
markAsRead: vi.fn().mockResolvedValue(undefined),
toggleStar: vi.fn().mockResolvedValue(undefined),
moveEmail: vi.fn().mockResolvedValue(undefined),
batchMarkAsRead: vi.fn().mockResolvedValue(undefined),
batchDeleteEmails: vi.fn().mockResolvedValue(undefined),
batchMoveEmails: vi.fn().mockResolvedValue(undefined),
batchArchiveEmails: vi.fn().mockResolvedValue(undefined),
getEmails: vi.fn().mockResolvedValue({ emails: [], hasMore: false, total: 0 }),
getMailboxes: vi.fn().mockResolvedValue([]),
} as unknown as IJMAPClient;
}
describe('non-unified shared-folder batch action routing', () => {
let activeClient: IJMAPClient; // account-a, the logged-in user, also reaches owner-x
beforeEach(() => {
activeClient = makeClient();
// Only the active login exists; the shared owner 'owner-x' is reached
// THROUGH it (there is no separate login client for a group mailbox).
useAuthStore.setState({
activeAccountId: 'account-a',
getClientForAccount: (id: string) => (id === 'account-a' ? activeClient : undefined) as never,
} as never);
useSettingsStore.setState({ deleteAction: 'trash', permanentlyDeleteJunk: false } as never);
// Viewing the shared inbox directly: not unified, no viewingAccountId set (the
// "Shared" section selects the namespaced folder id through handleMailboxSelect).
// `mailboxes` is the merged own+shared list the sidebar renders, so it contains
// BOTH the user's own trash and the shared folders.
useEmailStore.setState({
isUnifiedView: false,
unifiedRole: null,
viewingAccountId: null,
selectedMailbox: 'owner-x:x-inbox',
mailboxes: [
// Own folders are listed first (as getMailboxes emits them), so an
// unscoped `find(role==='archive'|'trash')` would wrongly pick these.
makeMailbox({ id: 'a-inbox', role: 'inbox' }),
makeMailbox({ id: 'a-trash', name: 'Deleted Items', role: 'trash' }),
makeMailbox({ id: 'a-archive', name: 'Archive', role: 'archive' }),
makeMailbox({ id: 'owner-x:x-inbox', originalId: 'x-inbox', name: 'Shared Inbox', role: 'inbox', isShared: true, accountId: 'owner-x', unreadEmails: 2, totalEmails: 5 }),
makeMailbox({ id: 'owner-x:x-trash', originalId: 'x-trash', name: 'Deleted Items', role: 'trash', isShared: true, accountId: 'owner-x' }),
makeMailbox({ id: 'owner-x:x-archive', originalId: 'x-archive', name: 'Archive', role: 'archive', isShared: true, accountId: 'owner-x' }),
],
accountMailboxes: {
'owner-x': [
makeMailbox({ id: 'owner-x:x-inbox', originalId: 'x-inbox', role: 'inbox', isShared: true, accountId: 'owner-x' }),
makeMailbox({ id: 'owner-x:x-trash', originalId: 'x-trash', role: 'trash', isShared: true, accountId: 'owner-x' }),
makeMailbox({ id: 'owner-x:x-archive', originalId: 'x-archive', role: 'archive', isShared: true, accountId: 'owner-x' }),
],
},
processingReadStatus: new Set(),
selectedEmail: null,
emails: [
makeEmail({ id: 'e1', keywords: {}, mailboxIds: { 'owner-x:x-inbox': true } }),
makeEmail({ id: 'e2', keywords: {}, mailboxIds: { 'owner-x:x-inbox': true } }),
],
selectedEmailIds: new Set(['e1', 'e2']),
});
});
it('batchDelete moves shared emails to the SHARED trash on the owner account', async () => {
await useEmailStore.getState().batchDelete(activeClient, false);
// Owner account + owner's trash id, reached via the active client — NOT the
// user's own trash on their own account.
expect(activeClient.batchMoveEmails).toHaveBeenCalledWith(['e1', 'e2'], 'x-trash', 'owner-x', false);
});
it('batchMoveToMailbox moves shared emails within the owner account', async () => {
await useEmailStore.getState().batchMoveToMailbox(activeClient, 'owner-x:x-archive');
expect(activeClient.batchMoveEmails).toHaveBeenCalledWith(['e1', 'e2'], 'x-archive', 'owner-x');
});
it('batchMarkAsRead marks shared emails read on the owner account', async () => {
await useEmailStore.getState().batchMarkAsRead(activeClient, true);
expect(activeClient.batchMarkAsRead).toHaveBeenCalledWith(['e1', 'e2'], true, 'owner-x');
});
it('batchArchive archives shared emails into the SHARED archive on the owner account', async () => {
await useEmailStore.getState().batchArchive(activeClient);
// Owner archive id + owner accountId — not the user's own archive/account.
expect(activeClient.batchArchiveEmails).toHaveBeenCalledWith(
[{ id: 'e1', receivedAt: expect.any(String) }, { id: 'e2', receivedAt: expect.any(String) }],
'x-archive',
'single',
expect.anything(),
'owner-x',
);
});
it('leaves own-account batch delete untouched (no owner accountId)', async () => {
// Selecting the user's own inbox: undecorated emails must still route to the
// own account with no JMAP accountId (undefined), moving to the own trash.
useEmailStore.setState({
selectedMailbox: 'a-inbox',
emails: [
makeEmail({ id: 'o1', keywords: {}, mailboxIds: { 'a-inbox': true } }),
makeEmail({ id: 'o2', keywords: {}, mailboxIds: { 'a-inbox': true } }),
],
selectedEmailIds: new Set(['o1', 'o2']),
});
await useEmailStore.getState().batchDelete(activeClient, false);
expect(activeClient.batchMoveEmails).toHaveBeenCalledWith(['o1', 'o2'], 'a-trash', undefined, false);
});
});
+43 -5
View File
@@ -343,6 +343,22 @@ function resolveActionMailboxes(): Mailbox[] {
return state.mailboxes; return state.mailboxes;
} }
/**
* The owner JMAP accountId of the shared/group folder currently being viewed
* directly (the "Shared" sidebar section, non-unified), or `undefined` for a
* normal own-account view. Emails in this view are undecorated (no
* `sourceAccountId`) and are reached through the ACTIVE client, but their
* mutations must carry this accountId — otherwise JMAP `Email/set` is sent to
* the user's own account, where it silently no-ops (Stalwart returns the ids as
* `updated: null` with an unchanged state, not `notUpdated`) and the change is
* lost on the next reload. Mirrors `fetchEmails` and the single-email path.
*/
function resolveViewAccountId(): string | undefined {
const state = useEmailStore.getState();
const mb = resolveActionMailboxes().find(m => m.id === state.selectedMailbox);
return mb?.isShared ? mb.accountId : undefined;
}
/** /**
* Resolves the store-side mailbox ids that make up a personal account's * Resolves the store-side mailbox ids that make up a personal account's
* contribution to the unified cross views (All mail / Unread / Starred), honoring * contribution to the unified cross views (All mail / Unread / Starred), honoring
@@ -2100,7 +2116,9 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
}); });
await Promise.allSettled(promises); await Promise.allSettled(promises);
} else { } else {
await resolveActionClient(client).batchMarkAsRead(emailIdsArray, read); // Non-unified: route to the viewed shared/group account (if any), reached
// through the active client; undefined for a normal own-account view.
await resolveActionClient(client).batchMarkAsRead(emailIdsArray, read, resolveViewAccountId());
} }
// Update local state // Update local state
@@ -2180,14 +2198,21 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
bySource.get(key)!.ids.push(emailId); bySource.get(key)!.ids.push(emailId);
} }
// Undecorated emails viewed in a shared/group folder directly (non-unified)
// are reached through the active client but must carry the owner accountId,
// or the move/destroy silently no-ops on the user's own account (see
// resolveViewAccountId). undefined for a normal own-account view.
const viewAccountId = currentMailbox?.isShared ? currentMailbox.accountId : undefined;
const getClient = (sourceAccountId: string, clientAccountId?: string) => const getClient = (sourceAccountId: string, clientAccountId?: string) =>
sourceAccountId === '__default__' sourceAccountId === '__default__'
? resolveActionClient(client) ? resolveActionClient(client)
: (clientAccountId ? useAuthStore.getState().getClientForAccount(clientAccountId) : undefined); : (clientAccountId ? useAuthStore.getState().getClientForAccount(clientAccountId) : undefined);
const mailboxesFor = (sourceAccountId: string) => const mailboxesFor = (sourceAccountId: string) =>
sourceAccountId === '__default__' ? mailboxes : (accountMailboxes[sourceAccountId] ?? mailboxes); sourceAccountId === '__default__'
? (viewAccountId ? (accountMailboxes[viewAccountId] ?? mailboxes) : mailboxes)
: (accountMailboxes[sourceAccountId] ?? mailboxes);
const jmapIdFor = (sourceAccountId: string) => const jmapIdFor = (sourceAccountId: string) =>
sourceAccountId === '__default__' ? undefined : sourceAccountId; sourceAccountId === '__default__' ? viewAccountId : sourceAccountId;
if (forceDestroy) { if (forceDestroy) {
const promises = Array.from(bySource.entries()).map(async ([sourceAccountId, { clientAccountId, ids }]) => { const promises = Array.from(bySource.entries()).map(async ([sourceAccountId, { clientAccountId, ids }]) => {
@@ -2295,7 +2320,13 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
}); });
await Promise.allSettled(promises); await Promise.allSettled(promises);
} else { } else {
await resolveActionClient(client).batchMoveEmails(emailIdsArray, toMailboxId); // Non-unified: route to the viewed shared/group account (if any), reached
// through the active client. Resolve the destination to its bare owner id
// (`originalId`) since shared folders use namespaced store ids.
const viewAccountId = resolveViewAccountId();
const destMailbox = resolveActionMailboxes().find(mb => mb.id === toMailboxId);
const jmapDestId = destMailbox?.originalId || toMailboxId;
await resolveActionClient(client).batchMoveEmails(emailIdsArray, jmapDestId, viewAccountId);
} }
// Update local state - remove from current view since they moved // Update local state - remove from current view since they moved
@@ -2324,7 +2355,14 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
const mailboxes = resolveActionMailboxes(); const mailboxes = resolveActionMailboxes();
if (selectedEmailIds.size === 0) return; if (selectedEmailIds.size === 0) return;
const archiveMailbox = mailboxes.find(m => m.role === 'archive' || m.name.toLowerCase() === 'archive'); // Scope the archive folder to the viewed shared/group account (if any) so the
// move lands on the owner account, not the user's own archive (which appears
// first in the merged list); see resolveViewAccountId. Own view is unchanged.
const viewAccountId = resolveViewAccountId();
const isArchive = (m: Mailbox) => m.role === 'archive' || m.name.toLowerCase() === 'archive';
const archiveMailbox = mailboxes.find(m =>
isArchive(m) && (viewAccountId ? m.accountId === viewAccountId : !m.isShared)
);
if (!archiveMailbox) return; if (!archiveMailbox) return;
const mode = useSettingsStore.getState().archiveMode; const mode = useSettingsStore.getState().archiveMode;