fix: enhance mailbox handling with path mapping and add tests for sieve generator #91

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<string, string>();
+    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({
                     >
                       <option value="">{t("move_to_folder")}</option>
                       {hierarchicalMailboxes.map((mb) => (
-                        <option key={mb.id} value={mb.name}>
+                        <option key={mb.id} value={mailboxPathMap.get(mb.id) || mb.name}>
                           {"\u00A0".repeat(mb.depth * 3)}{mb.name}
                         </option>
                       ))}

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> = {}): 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<string, string> {
+  const pathMap = new Map<string, string>();
+  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);
+  });
+});

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> = {}): 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;');
+    });
+  });
+});