From dadfb08f9ca9895deb6f49a6bff98e8d4e3aee99 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:09:19 +0200 Subject: [PATCH] fix: keep filter flags and folder targets when mail is moved The Sieve generator wrote actions in the order they were picked in the rule editor. `fileinto` stores the flags set at that moment, so "mark read", "star" or "add label" placed after a move never reached the moved message. Flag actions are now emitted first. Moves and copies also targeted the folder by path only. Renaming the folder silently sent the rule's mail to Inbox. Rules now store the folder id and emit `fileinto :mailboxid "" ""` when the server lists `mailboxid` (declaring `mailbox` too, which Stalwart requires), with the path as the fallback. Saving a rule refreshes the path from the id, so existing rules pick up the id on their next save. Forward actions get a "Keep a copy" option (`redirect :copy`), and the editor honours the server's `maxNumberRedirects` (1 on Stalwart): forward is disabled once the limit is used up, with a hint when existing rules already exceed it, because the server skips extra redirects without telling anyone. --- components/filters/filter-rule-modal.tsx | 122 ++++++++++++++++++----- components/settings/filter-settings.tsx | 5 + lib/jmap/sieve-types.ts | 9 ++ lib/sieve/__tests__/generator.test.ts | 45 +++++++++ lib/sieve/generator.ts | 52 ++++++++-- lib/sieve/parser.ts | 17 +++- locales/ar/common.json | 2 + locales/ca/common.json | 2 + locales/cs/common.json | 2 + locales/da/common.json | 2 + locales/de/common.json | 2 + locales/en/common.json | 2 + locales/es/common.json | 2 + locales/fa/common.json | 2 + locales/fr/common.json | 2 + locales/he/common.json | 2 + locales/hu/common.json | 2 + locales/it/common.json | 2 + locales/ja/common.json | 2 + locales/ko/common.json | 2 + locales/lv/common.json | 2 + locales/mn/common.json | 2 + locales/nb/common.json | 2 + locales/nl/common.json | 2 + locales/pl/common.json | 2 + locales/pt/common.json | 2 + locales/ro/common.json | 2 + locales/ru/common.json | 2 + locales/sk/common.json | 2 + locales/tr/common.json | 2 + locales/uk/common.json | 2 + locales/zh-TW/common.json | 2 + locales/zh/common.json | 2 + stores/filter-store.ts | 12 ++- 34 files changed, 273 insertions(+), 43 deletions(-) diff --git a/components/filters/filter-rule-modal.tsx b/components/filters/filter-rule-modal.tsx index 9f6fb2a7c..564f9e8b3 100644 --- a/components/filters/filter-rule-modal.tsx +++ b/components/filters/filter-rule-modal.tsx @@ -23,6 +23,10 @@ import { useKeywordFormat } from "@/hooks/use-keyword-format"; interface FilterRuleModalProps { rule?: FilterRule; mailboxes: Mailbox[]; + /** Server cap on redirects per message (Sieve `maxNumberRedirects`). */ + maxRedirects?: number | null; + /** Forward actions in the other enabled rules, which share that cap. */ + otherForwards?: number; onSave: (rule: FilterRule) => void; onClose: () => void; } @@ -84,6 +88,8 @@ function makeEmptyAction(): FilterAction { export function FilterRuleModal({ rule, mailboxes, + maxRedirects, + otherForwards = 0, onSave, onClose, }: FilterRuleModalProps) { @@ -121,6 +127,20 @@ export function FilterRuleModal({ return { hierarchicalMailboxes: flattenMailboxTree(tree), mailboxPathMap: pathMap }; }, [mailboxes]); + // Rules saved before folder ids were stored only carry the path; resolve it + // so the select shows the folder and the next save adds the id. + const mailboxIdFor = useCallback((action: FilterAction): string => { + if (action.mailboxId) return action.mailboxId; + for (const [id, path] of mailboxPathMap) { + if (path === action.value) return id; + } + return ""; + }, [mailboxPathMap]); + + const forwardCount = actions.filter((a) => a.type === "forward").length; + const forwardLimit = typeof maxRedirects === "number" && maxRedirects > 0 ? maxRedirects : null; + const forwardOverLimit = forwardLimit !== null && forwardCount + otherForwards > forwardLimit; + const handleSave = useCallback(() => { const trimmedName = name.trim(); if (!trimmedName) { @@ -150,9 +170,16 @@ export function FilterRuleModal({ return; } - const validActions = actions.filter( - (a) => !ACTIONS_WITH_VALUE.has(a.type) || a.value?.trim() - ); + const validActions = actions + .map((a) => { + if (!ACTIONS_WITH_MAILBOX.has(a.type)) return a; + // Store the folder id next to the path and refresh the path from it, + // so a renamed folder keeps receiving the rule's mail. + const mailboxId = mailboxIdFor(a); + const path = mailboxId ? mailboxPathMap.get(mailboxId) : undefined; + return mailboxId ? { ...a, mailboxId, value: path ?? a.value } : a; + }) + .filter((a) => !ACTIONS_WITH_VALUE.has(a.type) || a.value?.trim()); if (validActions.length === 0) { toast.error(t("validation_empty_actions")); return; @@ -167,7 +194,7 @@ export function FilterRuleModal({ actions: validActions, stopProcessing, }); - }, [name, matchType, conditions, actions, stopProcessing, rule, onSave, t]); + }, [name, matchType, conditions, actions, stopProcessing, rule, onSave, t, mailboxIdFor, mailboxPathMap]); const updateCondition = (index: number, updates: Partial) => { setConditions((prev) => @@ -213,9 +240,16 @@ export function FilterRuleModal({ 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 (updates.type && ACTIONS_WITH_MAILBOX.has(updates.type) && !updated.value) { const firstMb = hierarchicalMailboxes[0]; updated.value = firstMb ? (mailboxPathMap.get(firstMb.id) || firstMb.name) : ""; + if (firstMb) updated.mailboxId = firstMb.id; } return updated; }) @@ -429,36 +463,65 @@ export function FilterRuleModal({ aria-label={t("actions")} > {ALL_ACTION_TYPES.map((a) => ( - ))} - {ACTIONS_WITH_MAILBOX.has(action.type) && ( - - )} + {ACTIONS_WITH_MAILBOX.has(action.type) && (() => { + const selectedId = mailboxIdFor(action); + const missing = !!selectedId && !mailboxPathMap.has(selectedId); + return ( + + ); + })()} {action.type === "forward" && ( - updateAction(index, { value: e.target.value })} - placeholder={t("forward_placeholder")} - type="email" - className="flex-1 min-w-[180px]" - /> + <> + updateAction(index, { value: e.target.value })} + placeholder={t("forward_placeholder")} + type="email" + className="flex-1 min-w-[180px]" + /> + + )} {action.type === "reject" && ( @@ -496,6 +559,11 @@ export function FilterRuleModal({ ))} + {forwardOverLimit && ( +

+ {t("forward_limit", { count: forwardLimit })} +

+ )}