fix: let search results from other folders move into the open folder (bulwarkmail/webmail#1452)

This commit is contained in:
Linus Rath
2026-10-31 04:13:42 +01:00
parent 4849cf233e
commit 9a4515f905
3 changed files with 71 additions and 3 deletions
+33
View File
@@ -0,0 +1,33 @@
import { describe, expect, it } from 'vitest';
import fs from 'node:fs';
import path from 'node:path';
import { moveSheetCurrentId } from '../move-targets';
import type { Mailbox } from '../../api/types';
// A search across folders keeps the open folder selected, and the move
// sheet greyed it out, so a hit from another folder couldn't be moved into
// it (webmail dafdda534, #1452).
const box = (id: string, extra: Partial<Mailbox> = {}): Mailbox => ({ id, name: id, totalEmails: 0, unreadEmails: 0, ...extra } as Mailbox);
const mailboxes = [box('inbox'), box('work'), box('g:raw', { originalId: 'raw', accountId: 'g' })];
describe('move sheet current folder (bulwarkmail/webmail#1452)', () => {
it('keeps the open folder out only when every row is already in it', () => {
expect(moveSheetCurrentId(mailboxes, 'inbox', [{ mailboxIds: { inbox: true } }])).toBe('inbox');
expect(moveSheetCurrentId(mailboxes, 'inbox', [{ mailboxIds: { work: true }, jmapAccountId: 'me' }])).toBeNull();
expect(moveSheetCurrentId(mailboxes, 'inbox', [{ mailboxIds: { inbox: true } }, { mailboxIds: { work: true } }])).toBeNull();
expect(moveSheetCurrentId(mailboxes, null, [{ mailboxIds: { work: true } }])).toBeNull();
});
it("matches a shared folder by its server id and account", () => {
expect(moveSheetCurrentId(mailboxes, 'g:raw', [{ mailboxIds: { raw: true }, jmapAccountId: 'g' }])).toBe('g:raw');
expect(moveSheetCurrentId(mailboxes, 'g:raw', [{ mailboxIds: { raw: true }, jmapAccountId: 'other' }])).toBeNull();
});
it('is what both move sheets of the list use', () => {
const list = fs.readFileSync(path.resolve(__dirname, '../../screens/EmailListScreen.tsx'), 'utf8');
expect(list).toMatch(/currentMailboxId=\{pendingMoveCurrentId\}/);
expect(list).toMatch(/toId !== pendingMoveCurrentId/);
expect(list).toMatch(/currentMailboxId=\{moveSheetCurrentId\(mailboxes, currentMailboxId, selectedEmails\)\}/);
});
});
+26
View File
@@ -0,0 +1,26 @@
import type { Email, Mailbox } from '../api/types';
/** Whether a list row sits in `mailbox` (a shared folder's id carries its account). */
function rowInMailbox(row: Pick<Email, 'mailboxIds' | 'jmapAccountId'>, mailbox: Mailbox): boolean {
if (row.jmapAccountId && mailbox.accountId && row.jmapAccountId !== mailbox.accountId) return false;
const raw = mailbox.originalId ?? mailbox.id;
return !!row.mailboxIds?.[raw] || !!row.mailboxIds?.[mailbox.id];
}
/**
* The folder the move sheet marks as current (checked, not pickable). The
* open folder only when every message being moved is already in it: a search
* across folders lists messages from other folders while the open one stays
* selected, and those can be moved into it (webmail lib/move-targets.ts,
* #1452). Null when it can be picked.
*/
export function moveSheetCurrentId(
mailboxes: readonly Mailbox[],
currentMailboxId: string | null,
rows: ReadonlyArray<Pick<Email, 'mailboxIds' | 'jmapAccountId'>>,
): string | null {
if (!currentMailboxId) return null;
const current = mailboxes.find((m) => m.id === currentMailboxId);
if (!current || rows.length === 0) return currentMailboxId;
return rows.every((row) => rowInMailbox(row, current)) ? currentMailboxId : null;
}
+12 -3
View File
@@ -57,6 +57,7 @@ import {
} from '../lib/thread-utils';
import { isPermanentDelete, confirmPermanentDelete } from '../lib/delete-confirm';
import { confirmMailRemoval } from '../lib/mail-removal-confirm';
import { moveSheetCurrentId } from '../lib/move-targets';
import { draftContextFromEmail, isDraftEmail } from '../lib/draft-context';
import { getFullEmail, emptyMailbox as apiEmptyMailbox } from '../api/email';
import type { RootStackParamList } from '../navigation/types';
@@ -868,6 +869,14 @@ export default function EmailListScreen({
() => expandThreadSelection(selectedIds, emails, disableThreading),
[selectedIds, emails, disableThreading],
);
// The open folder can't be picked in the move sheets only when the rows
// being moved are all in it: search hits from other folders can move into
// it (#1452).
const pendingMoveCurrentId = React.useMemo(() => {
if (!pendingMoveId) return currentMailboxId;
const wanted = new Set(expandThreadSelection([pendingMoveId], emails, disableThreading));
return moveSheetCurrentId(mailboxes, currentMailboxId, emails.filter((e) => wanted.has(rowKeyOf(e))));
}, [pendingMoveId, emails, disableThreading, mailboxes, currentMailboxId]);
const selectedEmails = React.useMemo(() => {
const wanted = new Set(selectedMessageIds);
return emails.filter((e) => wanted.has(rowKeyOf(e)));
@@ -1921,11 +1930,11 @@ export default function EmailListScreen({
visible={pendingMoveId !== null}
onClose={() => setPendingMoveId(null)}
mailboxes={mailboxes}
currentMailboxId={currentMailboxId}
currentMailboxId={pendingMoveCurrentId}
onPick={(toId) => {
const id = pendingMoveId;
setPendingMoveId(null);
if (id && currentMailboxId && toId !== currentMailboxId) {
if (id && currentMailboxId && toId !== pendingMoveCurrentId) {
const ids = idsForRow(id);
if (ids.length > 1) reportFailure(moveEmailsToMailbox(ids, toId));
else reportFailure(moveToMailboxAction(id, currentMailboxId, toId));
@@ -1937,7 +1946,7 @@ export default function EmailListScreen({
visible={batchMoveOpen}
onClose={() => setBatchMoveOpen(false)}
mailboxes={mailboxes}
currentMailboxId={currentMailboxId}
currentMailboxId={moveSheetCurrentId(mailboxes, currentMailboxId, selectedEmails)}
onPick={handleBatchMovePick}
/>