From 2baf5e84a58d4a291724112aa042889414f50525 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:04:07 +0100 Subject: [PATCH] fix: enhance mailbox handling with path mapping and add tests for sieve generator #91 --- components/filters/filter-rule-modal.tsx | 24 +++-- lib/__tests__/mailbox-path.test.ts | 121 +++++++++++++++++++++++ lib/__tests__/sieve-generator.test.ts | 117 ++++++++++++++++++++++ 3 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 lib/__tests__/mailbox-path.test.ts create mode 100644 lib/__tests__/sieve-generator.test.ts diff --git a/components/filters/filter-rule-modal.tsx b/components/filters/filter-rule-modal.tsx index f7acacc0..081f536b 100644 --- a/components/filters/filter-rule-modal.tsx +++ b/components/filters/filter-rule-modal.tsx @@ -16,7 +16,7 @@ import type { FilterActionType, } from "@/lib/jmap/sieve-types"; import type { Mailbox } from "@/lib/jmap/types"; -import { buildMailboxTree, flattenMailboxTree } from "@/lib/utils"; +import { buildMailboxTree, flattenMailboxTree, type MailboxNode } from "@/lib/utils"; interface FilterRuleModalProps { rule?: FilterRule; @@ -71,10 +71,19 @@ export function FilterRuleModal({ const modalRef = useFocusTrap({ isActive: true, onEscape: onClose }); - const hierarchicalMailboxes = useMemo( - () => flattenMailboxTree(buildMailboxTree(mailboxes.filter((mb) => !mb.isShared))), - [mailboxes] - ); + const { hierarchicalMailboxes, mailboxPathMap } = useMemo(() => { + const tree = buildMailboxTree(mailboxes.filter((mb) => !mb.isShared)); + const pathMap = new Map(); + const buildPaths = (nodes: MailboxNode[], parentPath = "") => { + for (const node of nodes) { + const fullPath = parentPath ? `${parentPath}/${node.name}` : node.name; + pathMap.set(node.id, fullPath); + if (node.children.length > 0) buildPaths(node.children, fullPath); + } + }; + buildPaths(tree); + return { hierarchicalMailboxes: flattenMailboxTree(tree), mailboxPathMap: pathMap }; + }, [mailboxes]); const handleSave = useCallback(() => { const trimmedName = name.trim(); @@ -143,7 +152,8 @@ export function FilterRuleModal({ delete updated.value; } if (updates.type && ACTIONS_WITH_MAILBOX.has(updates.type) && !updated.value) { - updated.value = mailboxes[0]?.name || ""; + const firstMb = hierarchicalMailboxes[0]; + updated.value = firstMb ? (mailboxPathMap.get(firstMb.id) || firstMb.name) : ""; } return updated; }) @@ -338,7 +348,7 @@ export function FilterRuleModal({ > {hierarchicalMailboxes.map((mb) => ( - ))} diff --git a/lib/__tests__/mailbox-path.test.ts b/lib/__tests__/mailbox-path.test.ts new file mode 100644 index 00000000..a0cfd24a --- /dev/null +++ b/lib/__tests__/mailbox-path.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect } from 'vitest'; +import { buildMailboxTree, flattenMailboxTree, type MailboxNode } from '@/lib/utils'; +import type { Mailbox } from '@/lib/jmap/types'; + +const makeMailbox = (overrides: Partial = {}): Mailbox => ({ + id: 'mb-1', + 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, + ...overrides, +}); + +/** + * Builds a full-path map from a mailbox tree, matching the logic + * used in FilterRuleModal for sieve fileinto folder paths. + */ +function buildMailboxPathMap(tree: MailboxNode[]): Map { + const pathMap = new Map(); + const walk = (nodes: MailboxNode[], parentPath = '') => { + for (const node of nodes) { + const fullPath = parentPath ? `${parentPath}/${node.name}` : node.name; + pathMap.set(node.id, fullPath); + if (node.children.length > 0) walk(node.children, fullPath); + } + }; + walk(tree); + return pathMap; +} + +describe('mailbox path building for sieve fileinto', () => { + it('should produce correct path for a root mailbox', () => { + const mailboxes = [makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' })]; + const tree = buildMailboxTree(mailboxes); + const paths = buildMailboxPathMap(tree); + expect(paths.get('inbox')).toBe('Inbox'); + }); + + it('should produce correct path for a single-level subfolder', () => { + const mailboxes = [ + makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' }), + makeMailbox({ id: 'sub1', name: 'Projects', parentId: 'inbox' }), + ]; + const tree = buildMailboxTree(mailboxes); + const paths = buildMailboxPathMap(tree); + expect(paths.get('sub1')).toBe('Inbox/Projects'); + }); + + it('should produce correct path for deeply nested subfolders', () => { + const mailboxes = [ + makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' }), + makeMailbox({ id: 'sub1', name: 'Test', parentId: 'inbox' }), + makeMailbox({ id: 'sub2', name: 'Test2', parentId: 'sub1' }), + ]; + const tree = buildMailboxTree(mailboxes); + const paths = buildMailboxPathMap(tree); + expect(paths.get('sub2')).toBe('Inbox/Test/Test2'); + }); + + it('should handle multiple root-level folders', () => { + const mailboxes = [ + makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' }), + makeMailbox({ id: 'archive', name: 'Archive', role: 'archive' }), + makeMailbox({ id: 'sub1', name: 'Work', parentId: 'archive' }), + ]; + const tree = buildMailboxTree(mailboxes); + const paths = buildMailboxPathMap(tree); + expect(paths.get('inbox')).toBe('Inbox'); + expect(paths.get('archive')).toBe('Archive'); + expect(paths.get('sub1')).toBe('Archive/Work'); + }); + + it('should produce paths for all nodes in a flattened tree', () => { + const mailboxes = [ + makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' }), + makeMailbox({ id: 'sub1', name: 'Projects', parentId: 'inbox' }), + makeMailbox({ id: 'sub2', name: 'Active', parentId: 'sub1' }), + ]; + const tree = buildMailboxTree(mailboxes); + const flat = flattenMailboxTree(tree); + const paths = buildMailboxPathMap(tree); + + // Every node in the flat list should have a path entry + for (const node of flat) { + expect(paths.has(node.id)).toBe(true); + } + + expect(paths.get('inbox')).toBe('Inbox'); + expect(paths.get('sub1')).toBe('Inbox/Projects'); + expect(paths.get('sub2')).toBe('Inbox/Projects/Active'); + }); + + it('should preserve depth info in flattened tree', () => { + const mailboxes = [ + makeMailbox({ id: 'inbox', name: 'Inbox', role: 'inbox' }), + makeMailbox({ id: 'sub1', name: 'Projects', parentId: 'inbox' }), + makeMailbox({ id: 'sub2', name: 'Alpha', parentId: 'sub1' }), + ]; + const tree = buildMailboxTree(mailboxes); + const flat = flattenMailboxTree(tree); + const byId = Object.fromEntries(flat.map((n) => [n.id, n])); + + expect(byId['inbox'].depth).toBe(0); + expect(byId['sub1'].depth).toBe(1); + expect(byId['sub2'].depth).toBe(2); + }); +}); diff --git a/lib/__tests__/sieve-generator.test.ts b/lib/__tests__/sieve-generator.test.ts new file mode 100644 index 00000000..17146a53 --- /dev/null +++ b/lib/__tests__/sieve-generator.test.ts @@ -0,0 +1,117 @@ +import { describe, it, expect } from 'vitest'; +import { generateScript } from '@/lib/sieve/generator'; +import { parseScript } from '@/lib/sieve/parser'; +import type { FilterRule } from '@/lib/jmap/sieve-types'; + +const makeRule = (overrides: Partial = {}): FilterRule => ({ + id: 'rule-1', + name: 'Test Rule', + enabled: true, + matchType: 'all', + conditions: [{ field: 'subject', comparator: 'contains', value: 'Test' }], + actions: [{ type: 'move', value: 'Archive' }], + stopProcessing: false, + ...overrides, +}); + +describe('sieve generator', () => { + describe('fileinto with subfolder paths', () => { + it('should generate correct fileinto for a root folder', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Archive' }] })]; + const script = generateScript(rules); + expect(script).toContain('fileinto "Archive";'); + }); + + it('should generate correct fileinto for a subfolder path', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Inbox/Test/Test2' }] })]; + const script = generateScript(rules); + expect(script).toContain('fileinto "Inbox/Test/Test2";'); + }); + + it('should preserve subfolder path in metadata round-trip', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Inbox/Projects/Work' }] })]; + const script = generateScript(rules); + const parsed = parseScript(script); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0].actions[0].value).toBe('Inbox/Projects/Work'); + }); + + it('should generate correct fileinto :copy for a subfolder path', () => { + const rules = [makeRule({ actions: [{ type: 'copy', value: 'Inbox/Backup/Important' }] })]; + const script = generateScript(rules); + expect(script).toContain('fileinto :copy "Inbox/Backup/Important";'); + }); + + it('should handle deeply nested subfolder paths', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Inbox/A/B/C/D' }] })]; + const script = generateScript(rules); + expect(script).toContain('fileinto "Inbox/A/B/C/D";'); + }); + }); + + describe('require extensions', () => { + it('should require fileinto for move actions', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Test' }] })]; + const script = generateScript(rules); + expect(script).toContain('require ["fileinto"]'); + }); + + it('should require fileinto and copy for copy actions', () => { + const rules = [makeRule({ actions: [{ type: 'copy', value: 'Test' }] })]; + const script = generateScript(rules); + expect(script).toContain('"copy"'); + expect(script).toContain('"fileinto"'); + }); + }); + + describe('conditions', () => { + it('should generate header :contains for subject contains', () => { + const rules = [makeRule({ + conditions: [{ field: 'subject', comparator: 'contains', value: 'hello' }], + })]; + const script = generateScript(rules); + expect(script).toContain('header :contains "Subject" "hello"'); + }); + + it('should combine multiple conditions with allof', () => { + const rules = [makeRule({ + matchType: 'all', + conditions: [ + { field: 'from', comparator: 'contains', value: 'alice' }, + { field: 'subject', comparator: 'contains', value: 'urgent' }, + ], + })]; + const script = generateScript(rules); + expect(script).toContain('allof('); + }); + + it('should combine multiple conditions with anyof', () => { + const rules = [makeRule({ + matchType: 'any', + conditions: [ + { field: 'from', comparator: 'contains', value: 'alice' }, + { field: 'subject', comparator: 'contains', value: 'urgent' }, + ], + })]; + const script = generateScript(rules); + expect(script).toContain('anyof('); + }); + }); + + describe('disabled rules', () => { + it('should not generate code for disabled rules', () => { + const rules = [makeRule({ enabled: false })]; + const script = generateScript(rules); + expect(script).not.toContain('fileinto'); + expect(script).not.toContain('if '); + }); + }); + + describe('stop processing', () => { + it('should append stop when stopProcessing is true', () => { + const rules = [makeRule({ stopProcessing: true })]; + const script = generateScript(rules); + expect(script).toContain('stop;'); + }); + }); +});