From 7f382565e8cd18f81fd189f49c3ac3060bf47716 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 25 Aug 2026 12:19:38 -0700 Subject: [PATCH] Take the filing action, not the whole rule Deleting a folder removed every rule that filed into it, along with whatever else those rules did. A rule that filed into Work, marked read and stopped processing lost the marking and the stopping too, and deleting a folder says nothing about whether those were still wanted. Only the fileinto action goes now. A rule left with nothing to do is still removed, because it has nothing to do; a rule filing into two folders keeps the one that still exists. The toast says which happened. Verified against the running app with two rules aimed at the same folder, one filing only and one filing and marking read: the first was removed, the second kept its markread, and the script stored on the server agrees. --- web/src/lib/__tests__/sieveFolders.test.ts | 45 ++++++++++++++++------ web/src/lib/sieveFolders.ts | 39 ++++++++++++++----- web/src/store/mail.ts | 14 ++++--- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/web/src/lib/__tests__/sieveFolders.test.ts b/web/src/lib/__tests__/sieveFolders.test.ts index d2aa7fa..26d17d9 100644 --- a/web/src/lib/__tests__/sieveFolders.test.ts +++ b/web/src/lib/__tests__/sieveFolders.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { retargetRules, dropRulesForFolders } from "../sieveFolders"; +import { retargetRules, detachFolders } from "../sieveFolders"; import { newRule, type SieveRule } from "../sieve"; /** @@ -56,30 +56,53 @@ describe("retargetRules", () => { }); }); -describe("dropRulesForFolders", () => { - it("removes a rule whose destination is gone", () => { +describe("detachFolders", () => { + it("removes only the filing action, leaving the rest of the rule doing its job", () => { + const rules = [rule("news", fileinto("Newsletters", "mb1", [{ type: "markread" }, { type: "stop" }]))]; + const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]); + expect(out.removed).toEqual([]); + expect(out.edited).toHaveLength(1); + expect(out.rules[0]!.actions.map((a) => a.type)).toEqual(["markread", "stop"]); + }); + + it("removes the rule when filing was all it did", () => { const rules = [rule("news", fileinto("Newsletters", "mb1")), rule("keep", fileinto("Archive", "mb9"))]; - const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Newsletters" }]); + const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]); expect(out.removed.map((r) => r.name)).toEqual(["news"]); expect(out.rules.map((r) => r.name)).toEqual(["keep"]); }); - it("removes rules for a deleted folder's children too", () => { - const rules = [rule("a", fileinto("Work", "mb1")), rule("b", fileinto("Work/Invoices", "mb2"))]; - const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Work" }, { id: "mb2", path: "Work/Invoices" }]); - expect(out.rules).toEqual([]); - expect(out.removed).toHaveLength(2); + it("handles a deleted folder's children too", () => { + const rules = [ + rule("a", fileinto("Work", "mb1")), + rule("b", fileinto("Work/Invoices", "mb2", [{ type: "flag" }])), + ]; + const out = detachFolders(rules, [{ id: "mb1", path: "Work" }, { id: "mb2", path: "Work/Invoices" }]); + expect(out.removed.map((r) => r.name)).toEqual(["a"]); + expect(out.rules.map((r) => r.name)).toEqual(["b"]); + expect(out.rules[0]!.actions.map((a) => a.type)).toEqual(["flag"]); }); it("still finds the rule when only the path matches", () => { const rules = [rule("news", fileinto("Newsletters"))]; - expect(dropRulesForFolders(rules, [{ id: "mb1", path: "NEWSLETTERS" }]).removed).toHaveLength(1); + expect(detachFolders(rules, [{ id: "mb1", path: "NEWSLETTERS" }]).removed).toHaveLength(1); + }); + + it("keeps a second filing action aimed somewhere that still exists", () => { + const rules = [rule("both", [ + { type: "fileinto", mailbox: "Newsletters", mailboxId: "mb1" }, + { type: "fileinto", mailbox: "Archive", mailboxId: "mb9", copy: true }, + ])]; + const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]); + expect(out.removed).toEqual([]); + expect(out.rules[0]!.actions).toEqual([{ type: "fileinto", mailbox: "Archive", mailboxId: "mb9", copy: true }]); }); it("leaves the list untouched when nothing matches", () => { const rules = [rule("keep", fileinto("Archive", "mb9"))]; - const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Newsletters" }]); + const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]); expect(out.rules).toBe(rules); + expect(out.edited).toEqual([]); expect(out.removed).toEqual([]); }); }); diff --git a/web/src/lib/sieveFolders.ts b/web/src/lib/sieveFolders.ts index 5bf20a0..e18d801 100644 --- a/web/src/lib/sieveFolders.ts +++ b/web/src/lib/sieveFolders.ts @@ -47,16 +47,35 @@ export function retargetRules(rules: SieveRule[], moves: Array gone.some((ref) => filesInto(r, ref))); - if (!removed.length) return { rules, removed: [] }; - return { rules: rules.filter((r) => !removed.includes(r)), removed }; +export function detachFolders(rules: SieveRule[], gone: FolderRef[]): { rules: SieveRule[]; edited: SieveRule[]; removed: SieveRule[] } { + if (!gone.length) return { rules, edited: [], removed: [] }; + const targets = (a: SieveRule["actions"][number]) => + a.type === "fileinto" && gone.some((ref) => a.mailboxId === ref.id || a.mailbox.toLowerCase() === ref.path.toLowerCase()); + + const edited: SieveRule[] = []; + const removed: SieveRule[] = []; + const next: SieveRule[] = []; + for (const rule of rules) { + if (!rule.actions.some(targets)) { + next.push(rule); + continue; + } + const actions = rule.actions.filter((a) => !targets(a)); + if (!actions.length) { + removed.push(rule); + continue; + } + const trimmed = { ...rule, actions }; + edited.push(trimmed); + next.push(trimmed); + } + if (!edited.length && !removed.length) return { rules, edited: [], removed: [] }; + return { rules: next, edited, removed }; } diff --git a/web/src/store/mail.ts b/web/src/store/mail.ts index 1a1e523..002e852 100644 --- a/web/src/store/mail.ts +++ b/web/src/store/mail.ts @@ -1069,16 +1069,18 @@ async function followFolders(before: FolderRef[]): Promise { else gone.push(ref); } - const { retargetRules, dropRulesForFolders } = await import("@/lib/sieveFolders"); + const { retargetRules, detachFolders } = await import("@/lib/sieveFolders"); const retargeted = retargetRules(rules, moves); - const dropped = dropRulesForFolders(retargeted.rules, gone); - if (!retargeted.changed && !dropped.removed.length) return; + const detached = detachFolders(retargeted.rules, gone); + if (!retargeted.changed && !detached.edited.length && !detached.removed.length) return; - await useSieve.getState().saveRules(dropped.rules); + await useSieve.getState().saveRules(detached.rules); const { toast } = await import("@/ui/toast"); + const plural = (n: number) => (n === 1 ? "" : "s"); const said: string[] = []; - if (retargeted.changed) said.push(`${retargeted.changed} filter rule${retargeted.changed === 1 ? "" : "s"} updated`); - if (dropped.removed.length) said.push(`${dropped.removed.length} filter rule${dropped.removed.length === 1 ? "" : "s"} removed: ${dropped.removed.map((r) => `“${r.name}”`).join(", ")}`); + if (retargeted.changed) said.push(`${retargeted.changed} filter rule${plural(retargeted.changed)} updated`); + if (detached.edited.length) said.push(`${detached.edited.length} filter rule${plural(detached.edited.length)} no longer file${detached.edited.length === 1 ? "s" : ""} there`); + if (detached.removed.length) said.push(`${detached.removed.length} filter rule${plural(detached.removed.length)} removed, having nothing left to do: ${detached.removed.map((r) => `“${r.name}”`).join(", ")}`); toast.show(said.join(" · "), { duration: 8000 }); } catch (err) { const { toast } = await import("@/ui/toast");