diff --git a/src/components/filters/FilterRuleModal.tsx b/src/components/filters/FilterRuleModal.tsx index 7402234c..acd8fc52 100644 --- a/src/components/filters/FilterRuleModal.tsx +++ b/src/components/filters/FilterRuleModal.tsx @@ -12,7 +12,6 @@ import Input from '../Input'; import Button from '../Button'; import { useLocaleStore } from '../../stores/locale-store'; import { useKeywordsStore } from '../../stores/keywords-store'; -import { buildMailboxTree, type MailboxNode } from '../../lib/mailbox-tree'; import { generateUUID } from '../../lib/uuid'; import { inputStringToValue, @@ -20,6 +19,15 @@ import { isHasAnyCondition, valueToInputString, } from '../../lib/sieve/condition-value'; +import { + ACTIONS_WITH_MAILBOX, + ACTIONS_WITH_VALUE, + buildMailboxTargets, + mailboxIdFor, + selectMailboxTarget, + updateFilterAction, + withMailboxTarget, +} from '../../lib/sieve/rule-actions'; import type { Mailbox } from '../../api/types'; import type { FilterRule, @@ -51,9 +59,6 @@ function seedActions(rule?: FilterRule): FilterAction[] { } const ALL_ACTION_TYPES: FilterActionType[] = ['move', 'copy', 'forward', 'mark_read', 'star', 'add_label', 'discard', 'reject', 'keep', 'stop']; -const ACTIONS_WITH_VALUE = new Set(['move', 'copy', 'forward', 'reject', 'add_label']); -const ACTIONS_WITH_MAILBOX = new Set(['move', 'copy']); - function makeEmptyCondition(): FilterCondition { return { field: 'from', comparator: 'contains', value: '' }; } @@ -69,23 +74,6 @@ interface FilterRuleModalProps { onClose: () => void; } -// Flatten the mailbox tree into pickable options, building the Sieve-canonical -// folder path for each (inbox -> "INBOX", not the localized display name). -function buildMailboxOptions(mailboxes: Mailbox[]): { value: string; label: string }[] { - const tree = buildMailboxTree(mailboxes); - const options: { value: string; label: string }[] = []; - const walk = (nodes: MailboxNode[], parentPath: string) => { - for (const node of nodes) { - const segment = node.role === 'inbox' ? 'INBOX' : node.name; - const fullPath = parentPath ? `${parentPath}/${segment}` : segment; - options.push({ value: fullPath, label: `${' '.repeat(node.depth * 3)}${node.name}` }); - if (node.children.length > 0) walk(node.children, fullPath); - } - }; - walk(tree, ''); - return options; -} - export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: FilterRuleModalProps) { const c = useColors(); const styles = useMemo(() => makeStyles(c), [c]); @@ -112,7 +100,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F setStopProcessing(rule?.stopProcessing ?? false); }, [visible, rule]); - const mailboxOptions = useMemo(() => buildMailboxOptions(mailboxes), [mailboxes]); + const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]); const fieldOptions = useMemo( () => ALL_FIELDS.map((f) => ({ value: f, label: t(`settings.filters.condition_fields.${f}`, f) })), @@ -165,20 +153,28 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F const updateAction = (index: number, updates: Partial) => { setActions((prev) => - prev.map((act, i) => { - if (i !== index) return act; - const updated = { ...act, ...updates }; - if (updates.type && !ACTIONS_WITH_VALUE.has(updates.type)) { - delete updated.value; - } - if (updates.type && ACTIONS_WITH_MAILBOX.has(updates.type) && !updated.value) { - updated.value = mailboxOptions[0]?.value ?? ''; - } - return updated; - }), + prev.map((act, i) => (i === index ? updateFilterAction(act, updates, mailboxTargets) : act)), ); }; + const selectFolder = (index: number, id: string) => { + setActions((prev) => + prev.map((act, i) => (i === index ? selectMailboxTarget(act, id, mailboxTargets) : act)), + ); + }; + + // Folder picker rows for an action. A folder the rule points at that isn't + // listed (deleted, or its account's folders not loaded) stays selectable + // under its path, so saving doesn't silently retarget the rule. + const folderOptions = (action: FilterAction) => { + const options = mailboxTargets.map((target) => ({ value: target.id, label: target.label })); + const selected = mailboxIdFor(action, mailboxTargets); + if (!options.some((o) => o.value === selected) && (selected || action.value)) { + options.unshift({ value: selected, label: action.value || selected }); + } + return options; + }; + const removeAction = (index: number) => { if (actions.length <= 1) return; setActions((prev) => prev.filter((_, i) => i !== index)); @@ -206,7 +202,9 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F Alert.alert(t('settings.filters.validation_empty_conditions', 'At least one condition with a value is required')); return; } - const validActions = actions.filter((a) => !ACTIONS_WITH_VALUE.has(a.type) || a.value?.trim()); + const validActions = actions + .map((a) => withMailboxTarget(a, mailboxTargets)) + .filter((a) => !ACTIONS_WITH_VALUE.has(a.type) || a.value?.trim()); if (validActions.length === 0) { Alert.alert(t('settings.filters.validation_empty_actions', 'At least one action is required')); return; @@ -220,7 +218,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F actions: validActions, stopProcessing, }); - }, [name, conditions, actions, matchType, stopProcessing, rule, onSave, t]); + }, [name, conditions, actions, matchType, stopProcessing, rule, onSave, t, mailboxTargets]); return ( @@ -364,30 +362,42 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F {ACTIONS_WITH_MAILBOX.has(action.type) && ( - mailboxOptions.length > 0 ? ( + mailboxTargets.length > 0 ? ( updateAction(index, { value: v })} + onChangeText={(v) => updateAction(index, { value: v, mailboxId: undefined })} placeholder={t('settings.filters.move_to_folder', 'Select folder')} /> ) )} {action.type === 'forward' && ( - updateAction(index, { value: v })} - placeholder={t('settings.filters.forward_placeholder', 'email@example.com')} - keyboardType="email-address" - autoCapitalize="none" - /> + <> + updateAction(index, { value: v })} + placeholder={t('settings.filters.forward_placeholder', 'email@example.com')} + keyboardType="email-address" + autoCapitalize="none" + /> + + + {t('settings.filters.forward_keep_copy', 'Keep a copy')} + + updateAction(index, { keepCopy: v || undefined })} + /> + + )} {action.type === 'reject' && ( @@ -505,6 +515,14 @@ function makeStyles(c: ThemePalette) { }, stopLabel: { ...typography.body, color: c.text, flex: 1 }, + optionRow: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'space-between', + gap: spacing.md, + }, + optionLabel: { ...typography.body, color: c.text, flex: 1 }, + footer: { flexDirection: 'row', justifyContent: 'flex-end', diff --git a/src/lib/sieve/__tests__/fixtures/webmail/folder-targets.sieve b/src/lib/sieve/__tests__/fixtures/webmail/folder-targets.sieve new file mode 100644 index 00000000..3cb6df4d --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/folder-targets.sieve @@ -0,0 +1,27 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"invoices","name":"Invoices","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"contains","value":["invoice","Rechnung"]}],"actions":[{"type":"move","value":"Finance/Invoices","mailboxId":"m12"},{"type":"star"},{"type":"add_label","value":"finance"}],"stopProcessing":false},{"id":"backup","name":"Backup","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"copy","value":"Archive","mailboxId":"m7"}],"stopProcessing":false},{"id":"legacy","name":"Legacy path","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"move","value":"Old/Path"}],"stopProcessing":false},{"id":"fwd","name":"Forward","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"forward","value":"me@elsewhere.example","keepCopy":true}],"stopProcessing":false}]} +@metadata:end */ + +require ["copy", "fileinto", "imap4flags", "mailbox", "mailboxid"]; + +# Rule: Invoices +if header :contains "Subject" ["invoice", "Rechnung"] { + addflag "\\Flagged"; + addflag "$label:finance"; + fileinto :mailboxid "m12" "Finance/Invoices"; +} + +# Rule: Backup +if header :contains "From" "news@example.com" { + fileinto :copy :mailboxid "m7" "Archive"; +} + +# Rule: Legacy path +if header :contains "From" "news@example.com" { + fileinto "Old/Path"; +} + +# Rule: Forward +if header :contains "From" "news@example.com" { + redirect :copy "me@elsewhere.example"; +} diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index 9bbb01c6..191318e3 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -260,6 +260,52 @@ describe('generateScript', () => { }); }); + describe('action order and targets', () => { + it('sets flags before the move so the moved message keeps them', () => { + const script = generateScript([makeRule({ + actions: [{ type: 'move', value: 'Archive' }, { type: 'star' }, { type: 'mark_read' }], + })]); + const flagged = script.indexOf('addflag "\\\\Flagged";'); + const seen = script.indexOf('addflag "\\\\Seen";'); + const move = script.indexOf('fileinto "Archive";'); + expect(flagged).toBeGreaterThan(-1); + expect(flagged).toBeLessThan(move); + expect(seen).toBeLessThan(move); + }); + + it('targets the folder id when the server supports mailboxid', () => { + const rules = [makeRule({ actions: [{ type: 'move', value: 'Work/Old', mailboxId: 'm42' }] })]; + const script = generateScript(rules, undefined, { extensions: ['fileinto', 'mailbox', 'mailboxid'] }); + expect(script).toContain('fileinto :mailboxid "m42" "Work/Old";'); + expect(script).toContain('require ["fileinto", "mailbox", "mailboxid"];'); + }); + + it('falls back to the path without mailboxid support', () => { + const rules = [makeRule({ actions: [{ type: 'copy', value: 'Work', mailboxId: 'm42' }] })]; + const script = generateScript(rules, undefined, { extensions: ['fileinto', 'copy'] }); + expect(script).toContain('fileinto :copy "Work";'); + expect(script).not.toContain('mailboxid'); + expect(parseScript(script).rules[0].actions[0]).toEqual({ type: 'copy', value: 'Work', mailboxId: 'm42' }); + }); + + it('keeps a copy of forwarded mail when asked', () => { + const script = generateScript([makeRule({ actions: [{ type: 'forward', value: 'a@b.c', keepCopy: true }] })]); + expect(script).toContain('redirect :copy "a@b.c";'); + expect(script).toContain('require ["copy"];'); + }); + + it('parses the new action forms from external scripts', () => { + const parsed = parseScript( + 'require ["fileinto", "copy", "mailbox", "mailboxid"];\n' + + 'if header :contains "From" "x" { fileinto :copy :mailboxid "m1" "A"; redirect :copy "a@b.c"; }', + ); + expect(parsed.rules[0].actions).toEqual([ + { type: 'copy', value: 'A', mailboxId: 'm1' }, + { type: 'forward', value: 'a@b.c', keepCopy: true }, + ]); + }); + }); + describe('vacation include', () => { it('includes the server vacation script before the rules', () => { const script = generateScript([makeRule()], undefined, { includeVacation: true }); diff --git a/src/lib/sieve/__tests__/rule-actions.test.ts b/src/lib/sieve/__tests__/rule-actions.test.ts new file mode 100644 index 00000000..9bbecf45 --- /dev/null +++ b/src/lib/sieve/__tests__/rule-actions.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect } from 'vitest'; +import type { Mailbox } from '../../../api/types'; +import { + buildMailboxTargets, + mailboxIdFor, + selectMailboxTarget, + updateFilterAction, + withMailboxTarget, +} from '../rule-actions'; + +const RIGHTS = { + mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, maySetKeywords: true, + mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true, +}; + +function mailbox(id: string, name: string, extra: Partial = {}): Mailbox { + return { + id, name, parentId: null, role: null, sortOrder: 0, + totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, + myRights: RIGHTS, ...extra, + }; +} + +const MAILBOXES = [ + mailbox('m1', 'Posteingang', { role: 'inbox' }), + mailbox('m2', 'Finance'), + mailbox('m3', 'Invoices 2026', { parentId: 'm2' }), +]; +const targets = buildMailboxTargets(MAILBOXES); + +describe('buildMailboxTargets', () => { + it('lists folders with their JMAP id and Sieve path', () => { + expect(targets.map(({ id, path }) => ({ id, path }))).toEqual([ + { id: 'm1', path: 'INBOX' }, + { id: 'm2', path: 'Finance' }, + { id: 'm3', path: 'Finance/Invoices 2026' }, + ]); + }); + + it('uses the raw JMAP id of a shared account\'s folders', () => { + const shared = buildMailboxTargets([ + mailbox('team:x1', 'Support', { originalId: 'x1', accountId: 'team' }), + ]); + expect(shared[0]).toMatchObject({ id: 'x1', path: 'Support' }); + }); +}); + +describe('folder targets of move and copy actions', () => { + it('resolves rules saved before folder ids were stored by their path', () => { + expect(mailboxIdFor({ type: 'move', value: 'Finance/Invoices 2026' }, targets)).toBe('m3'); + expect(mailboxIdFor({ type: 'move', value: 'Gone' }, targets)).toBe(''); + expect(mailboxIdFor({ type: 'move', value: 'Finance', mailboxId: 'm9' }, targets)).toBe('m9'); + }); + + it('stores the id on save and refreshes the path of a renamed folder', () => { + expect(withMailboxTarget({ type: 'move', value: 'Finance' }, targets)) + .toEqual({ type: 'move', value: 'Finance', mailboxId: 'm2' }); + expect(withMailboxTarget({ type: 'copy', value: 'Old name', mailboxId: 'm3' }, targets)) + .toEqual({ type: 'copy', value: 'Finance/Invoices 2026', mailboxId: 'm3' }); + }); + + it('keeps a target it cannot find as it is', () => { + const unknown = { type: 'move' as const, value: 'Elsewhere', mailboxId: 'm99' }; + expect(withMailboxTarget(unknown, targets)).toEqual(unknown); + expect(withMailboxTarget({ type: 'move', value: 'Nowhere' }, targets)).toEqual({ type: 'move', value: 'Nowhere' }); + expect(withMailboxTarget({ type: 'forward', value: 'a@b.c' }, targets)).toEqual({ type: 'forward', value: 'a@b.c' }); + }); + + it('picks a folder by id and keeps the path in step', () => { + const moved = selectMailboxTarget({ type: 'move', value: 'Finance', mailboxId: 'm2' }, 'm3', targets); + expect(moved).toEqual({ type: 'move', value: 'Finance/Invoices 2026', mailboxId: 'm3' }); + }); + + it('leaves an unlisted folder alone when it is picked again', () => { + const unknown = { type: 'move' as const, value: 'Elsewhere', mailboxId: 'm99' }; + expect(selectMailboxTarget(unknown, 'm99', targets)).toBe(unknown); + }); +}); + +describe('updateFilterAction', () => { + it('drops the folder id and the copy flag when the type changes', () => { + expect(updateFilterAction({ type: 'move', value: 'Finance', mailboxId: 'm2' }, { type: 'star' }, targets)) + .toEqual({ type: 'star' }); + expect(updateFilterAction({ type: 'forward', value: 'a@b.c', keepCopy: true }, { type: 'reject' }, targets)) + .toEqual({ type: 'reject', value: 'a@b.c' }); + }); + + it('keeps the folder when switching between move and copy', () => { + expect(updateFilterAction({ type: 'move', value: 'Finance', mailboxId: 'm2' }, { type: 'copy' }, targets)) + .toEqual({ type: 'copy', value: 'Finance', mailboxId: 'm2' }); + }); + + it('starts a new move on the first folder', () => { + expect(updateFilterAction({ type: 'star' }, { type: 'move' }, targets)) + .toEqual({ type: 'move', value: 'INBOX', mailboxId: 'm1' }); + }); + + it('drops the folder id when the path is typed by hand', () => { + expect(updateFilterAction({ type: 'move', value: 'Finance', mailboxId: 'm2' }, { value: 'Typed', mailboxId: undefined }, [])) + .toEqual({ type: 'move', value: 'Typed' }); + }); + + it('toggles keeping a copy of forwarded mail', () => { + const on = updateFilterAction({ type: 'forward', value: 'a@b.c' }, { keepCopy: true }, targets); + expect(on).toEqual({ type: 'forward', value: 'a@b.c', keepCopy: true }); + expect(updateFilterAction(on, { keepCopy: undefined }, targets)).toEqual({ type: 'forward', value: 'a@b.c' }); + }); +}); diff --git a/src/lib/sieve/__tests__/webmail-fixtures.test.ts b/src/lib/sieve/__tests__/webmail-fixtures.test.ts index fb01d165..730506d2 100644 --- a/src/lib/sieve/__tests__/webmail-fixtures.test.ts +++ b/src/lib/sieve/__tests__/webmail-fixtures.test.ts @@ -6,7 +6,7 @@ // // The fixtures in fixtures/webmail/ were written by the webmail's own // generator (jmap-webmail lib/sieve/generator.ts at e33ab899), run on the -// rules in each file's metadata. +// rules in each file's metadata for a server listing `extensions`. import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -15,10 +15,13 @@ import { parseScript } from '../parser'; interface WebmailFixture { file: string; + /** The server's sieveExtensions the webmail generated the script for. */ + extensions?: string[]; } const FIXTURES: WebmailFixture[] = [ { file: 'vacation-include.sieve' }, + { file: 'folder-targets.sieve', extensions: ['fileinto', 'copy', 'imap4flags', 'mailbox', 'mailboxid'] }, ]; function readFixture(file: string): string { @@ -32,7 +35,7 @@ function metadataRules(script: string): unknown[] { } describe('scripts written by the webmail', () => { - for (const { file } of FIXTURES) { + for (const { file, extensions } of FIXTURES) { describe(file, () => { const script = readFixture(file); const parsed = parseScript(script); @@ -47,6 +50,7 @@ describe('scripts written by the webmail', () => { const regenerated = generateScript(parsed.rules, parsed.vacation, { externalRequires: parsed.externalRequires, includeVacation: parsed.includeVacation, + extensions, }); expect(regenerated).toBe(script); }); diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index b22c910e..17625951 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -109,15 +109,31 @@ function generateCondition(condition: FilterCondition): string { } } -function generateActions(actions: FilterAction[]): string[] { - return actions.map(action => { +const FLAG_ACTIONS = new Set(['mark_read', 'star', 'add_label']); + +function fileintoTarget(action: FilterAction, useMailboxId: boolean): string { + const path = `"${escapeString(action.value || '')}"`; + // RFC 9042: the path is the fallback when the id no longer exists. + return useMailboxId && action.mailboxId + ? `:mailboxid "${escapeString(action.mailboxId)}" ${path}` + : path; +} + +function generateActions(actions: FilterAction[], useMailboxId: boolean): string[] { + // fileinto (and the implicit keep) store the flags set at that moment, so a + // flag action placed after a move in the UI would be lost. Set flags first. + const ordered = [ + ...actions.filter(a => FLAG_ACTIONS.has(a.type)), + ...actions.filter(a => !FLAG_ACTIONS.has(a.type)), + ]; + return ordered.map(action => { switch (action.type) { case 'move': - return `fileinto "${escapeString(action.value || '')}";`; + return `fileinto ${fileintoTarget(action, useMailboxId)};`; case 'copy': - return `fileinto :copy "${escapeString(action.value || '')}";`; + return `fileinto :copy ${fileintoTarget(action, useMailboxId)};`; case 'forward': - return `redirect "${escapeString(action.value || '')}";`; + return `redirect ${action.keepCopy ? ':copy ' : ''}"${escapeString(action.value || '')}";`; case 'mark_read': return 'addflag "\\\\Seen";'; case 'star': @@ -140,7 +156,7 @@ function generateActions(actions: FilterAction[]): string[] { }); } -function computeRequires(rules: FilterRule[], vacation?: VacationSieveConfig): string[] { +function computeRequires(rules: FilterRule[], vacation: VacationSieveConfig | undefined, useMailboxId: boolean): string[] { const extensions = new Set(); const enabledRules = rules.filter(r => r.enabled); @@ -156,12 +172,20 @@ function computeRequires(rules: FilterRule[], vacation?: VacationSieveConfig): s for (const action of rule.actions) { switch (action.type) { case 'move': + case 'copy': + extensions.add('fileinto'); + if (action.type === 'copy') extensions.add('copy'); + if (useMailboxId && action.mailboxId) { + // Stalwart rejects :mailboxid unless "mailbox" is declared too. + extensions.add('mailbox'); + extensions.add('mailboxid'); + } + break; case 'keep': extensions.add('fileinto'); break; - case 'copy': - extensions.add('fileinto'); - extensions.add('copy'); + case 'forward': + if (action.keepCopy) extensions.add('copy'); break; case 'mark_read': case 'star': @@ -203,6 +227,11 @@ export interface GenerateOptions { * while the filters stay active. */ includeVacation?: boolean; + /** + * The server's `sieveExtensions`. Folder moves use `:mailboxid` when it + * lists "mailboxid"; without it they target the folder path only. + */ + extensions?: string[]; } /** Name of the script a server builds for VacationResponse (RFC 9661). */ @@ -239,7 +268,8 @@ export function generateScript( lines.push('@metadata:end */'); lines.push(''); - const bulwarkRequires = computeRequires(bulwarkRules, vacation); + const useMailboxId = options.extensions?.includes('mailboxid') ?? false; + const bulwarkRequires = computeRequires(bulwarkRules, vacation, useMailboxId); if (options.includeVacation) bulwarkRequires.push('include'); const externalRequires = options.externalRequires ?? []; const allRequires = [...new Set([...bulwarkRequires, ...externalRequires])].sort(); @@ -290,7 +320,7 @@ export function generateScript( conditionStr = `${wrapper}(${conditions.join(', ')})`; } - const actionLines = generateActions(rule.actions); + const actionLines = generateActions(rule.actions, useMailboxId); if (rule.stopProcessing) { const lastAction = rule.actions[rule.actions.length - 1]; diff --git a/src/lib/sieve/parser.ts b/src/lib/sieve/parser.ts index c239afe2..57445a13 100644 --- a/src/lib/sieve/parser.ts +++ b/src/lib/sieve/parser.ts @@ -529,8 +529,21 @@ function parseAction(raw: string): FilterAction | null { m = /^fileinto\s+"((?:[^"\\]|\\.)*)"$/.exec(s); if (m) return { type: 'move', value: unescapeSieveString(m[1]) }; - m = /^redirect\s+"((?:[^"\\]|\\.)*)"$/.exec(s); - if (m) return { type: 'forward', value: unescapeSieveString(m[1]) }; + m = /^fileinto\s+(:copy\s+)?:mailboxid\s+"((?:[^"\\]|\\.)*)"\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) { + return { + type: m[1] ? 'copy' : 'move', + value: unescapeSieveString(m[3]), + mailboxId: unescapeSieveString(m[2]), + }; + } + + m = /^redirect\s+(:copy\s+)?"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) { + const action: FilterAction = { type: 'forward', value: unescapeSieveString(m[2]) }; + if (m[1]) action.keepCopy = true; + return action; + } m = /^addflag\s+"((?:[^"\\]|\\.)*)"$/.exec(s); if (m) { diff --git a/src/lib/sieve/rule-actions.ts b/src/lib/sieve/rule-actions.ts new file mode 100644 index 00000000..2215c8c5 --- /dev/null +++ b/src/lib/sieve/rule-actions.ts @@ -0,0 +1,98 @@ +import type { Mailbox } from '../../api/types'; +import { buildMailboxTree, type MailboxNode } from '../mailbox-tree'; +import type { FilterAction, FilterActionType } from './types'; + +// Editing helpers for the rule editor's action rows, ported from the +// webmail's components/filters/filter-rule-modal.tsx (dadfb08f). Kept out of +// the component so the folder-id bookkeeping can be tested on its own. + +export const ACTIONS_WITH_VALUE = new Set(['move', 'copy', 'forward', 'reject', 'add_label']); +export const ACTIONS_WITH_MAILBOX = new Set(['move', 'copy']); + +export interface MailboxTarget { + /** JMAP id of the folder, what `fileinto :mailboxid` targets. */ + id: string; + /** Sieve folder path, the fallback when the id no longer exists. */ + path: string; + label: string; +} + +/** + * Flatten the mailbox tree into pickable targets, building the Sieve-canonical + * folder path for each (inbox -> "INBOX", not the localized display name). + * A shared account's folders carry their JMAP id in `originalId`. + */ +export function buildMailboxTargets(mailboxes: Mailbox[]): MailboxTarget[] { + const targets: MailboxTarget[] = []; + const walk = (nodes: MailboxNode[], parentPath: string) => { + for (const node of nodes) { + const segment = node.role === 'inbox' ? 'INBOX' : node.name; + const path = parentPath ? `${parentPath}/${segment}` : segment; + targets.push({ + id: node.originalId ?? node.id, + path, + label: `${' '.repeat(node.depth * 3)}${node.name}`, + }); + if (node.children.length > 0) walk(node.children, path); + } + }; + walk(buildMailboxTree(mailboxes), ''); + return targets; +} + +/** + * Target folder id of a move/copy action. Rules saved before folder ids were + * stored only carry the path, so resolve it; '' when nothing matches. + */ +export function mailboxIdFor(action: FilterAction, targets: MailboxTarget[]): string { + if (action.mailboxId) return action.mailboxId; + return targets.find((t) => t.path === action.value)?.id ?? ''; +} + +/** + * Apply an edit to an action row. Fields the (new) type doesn't use are + * dropped, and a row switched to move/copy starts on the first folder. + */ +export function updateFilterAction( + action: FilterAction, + updates: Partial, + targets: MailboxTarget[], +): FilterAction { + const updated: FilterAction = { ...action, ...updates }; + if (updates.type && !ACTIONS_WITH_VALUE.has(updates.type)) delete updated.value; + if (updates.type && !ACTIONS_WITH_MAILBOX.has(updates.type)) delete updated.mailboxId; + if (updates.type && updates.type !== 'forward') delete updated.keepCopy; + if (!updated.mailboxId) delete updated.mailboxId; + if (!updated.keepCopy) delete updated.keepCopy; + if (updates.type && ACTIONS_WITH_MAILBOX.has(updates.type) && !updated.value) { + const first = targets[0]; + updated.value = first?.path ?? ''; + if (first) updated.mailboxId = first.id; + } + return updated; +} + +/** Point a move/copy row at the folder with this id (from the folder picker). */ +export function selectMailboxTarget( + action: FilterAction, + id: string, + targets: MailboxTarget[], +): FilterAction { + // Re-picking a folder that isn't listed (deleted, or not loaded) keeps it. + if (id === mailboxIdFor(action, targets)) return action; + const path = targets.find((t) => t.id === id)?.path ?? ''; + return updateFilterAction(action, { mailboxId: id || undefined, value: path }, targets); +} + +/** + * The action as saved: a move/copy stores the folder id next to the path, + * and the path is refreshed from the id, so a renamed folder keeps + * receiving the rule's mail. + */ +export function withMailboxTarget(action: FilterAction, targets: MailboxTarget[]): FilterAction { + if (!ACTIONS_WITH_MAILBOX.has(action.type)) return action; + const mailboxId = mailboxIdFor(action, targets); + if (!mailboxId) return action; + const path = targets.find((t) => t.id === mailboxId)?.path; + return { ...action, mailboxId, value: path ?? action.value }; +} diff --git a/src/lib/sieve/types.ts b/src/lib/sieve/types.ts index f96b7f47..1beaa788 100644 --- a/src/lib/sieve/types.ts +++ b/src/lib/sieve/types.ts @@ -16,6 +16,8 @@ export interface SieveCapabilities { sieveExtensions: string[]; notificationMethods: string[]; externalLists: string[]; + /** RFC 9661: redirects run per message; extra ones are skipped silently. */ + maxNumberRedirects?: number | null; } export type FilterConditionField = @@ -58,6 +60,13 @@ export interface FilterCondition { export interface FilterAction { type: FilterActionType; value?: string; + /** + * move/copy: JMAP id of the target folder. `value` keeps the path as the + * fallback; the id keeps the rule working after the folder is renamed. + */ + mailboxId?: string; + /** forward: also keep the message (`redirect :copy`). */ + keepCopy?: boolean; } export type FilterOrigin = 'bulwark' | 'external' | 'opaque'; diff --git a/src/stores/__tests__/filter-store.test.ts b/src/stores/__tests__/filter-store.test.ts index f809fa23..98fff1c0 100644 --- a/src/stores/__tests__/filter-store.test.ts +++ b/src/stores/__tests__/filter-store.test.ts @@ -214,3 +214,19 @@ describe('filter-store and the server vacation script', () => { }); }); }); + +describe('saving scripts the webmail wrote', () => { + it('keeps folder ids, forward copies and the flag order', async () => { + const script = webmailFixture('folder-targets.sieve'); + api.getSieveCapabilities.mockReturnValue({ + ...WITH_INCLUDE, + sieveExtensions: ['fileinto', 'copy', 'imap4flags', 'mailbox', 'mailboxid'], + }); + api.getSieveScripts.mockResolvedValue([{ id: 's1', name: 'filters', blobId: 'b1', isActive: true }]); + api.getSieveScriptContent.mockResolvedValue(script); + + await useFilterStore.getState().selectAccount(null); + await useFilterStore.getState().saveFilters(); + expect(api.updateSieveScript).toHaveBeenCalledWith('s1', script, true, 'own'); + }); +}); diff --git a/src/stores/filter-store.ts b/src/stores/filter-store.ts index 74219a8b..0418a577 100644 --- a/src/stores/filter-store.ts +++ b/src/stores/filter-store.ts @@ -152,13 +152,17 @@ export const useFilterStore = create()((set, get) => ({ try { const { isOpaque, rawScript, rules, activeScriptId, vacationSettings, externalRequires, includeVacation, - selectedAccountId, + selectedAccountId, sieveCapabilities, } = get(); const accountId = selectedAccountId ?? undefined; const content = isOpaque ? rawScript - : generateScript(rules, vacationSettings || undefined, { externalRequires, includeVacation }); + : generateScript(rules, vacationSettings || undefined, { + externalRequires, + includeVacation, + extensions: sieveCapabilities?.sieveExtensions, + }); if (activeScriptId) { await updateSieveScript(activeScriptId, content, true, accountId); @@ -292,7 +296,8 @@ export async function isVacationIncludedInFilters(accountId?: string): Promise { const sieveAccountId = accountId ?? getSieveAccountId(); - if (enabled && !supportsInclude(getSieveCapabilities(sieveAccountId))) return; + const capabilities = getSieveCapabilities(sieveAccountId); + if (enabled && !supportsInclude(capabilities)) return; const { vacationScript, target, parsed } = await loadManagedScript(sieveAccountId); if (!target || !parsed) return; @@ -307,6 +312,7 @@ export async function syncVacationWithFilters(enabled: boolean, accountId?: stri const content = generateScript(parsed.rules, parsed.vacation, { externalRequires: parsed.externalRequires, includeVacation: enabled, + extensions: capabilities?.sieveExtensions, }); await updateSieveScript(target.id, content, enabled || target.isActive, sieveAccountId);