Merge pull request #39 from LINUXexpert-org/sieve-detach-only-fileinto

Take the filing action, not the whole rule

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([]);
   });
 });

web/src/lib/sieveFolders.ts

@@ -47,16 +47,35 @@ export function retargetRules(rules: SieveRule[], moves: Array<FolderRef & { new
 }
 
 /**
- * Drops rules that file into folders which no longer exist.
+ * Takes the deleted folders out of the rules that filed into them.
  *
- * The whole rule goes, not just its fileinto action: a rule whose destination
- * has been deleted has no destination, and leaving it behind to match mail and
- * do nothing is worse than removing it. Rules that merely mention the folder in
- * some other action are left alone.
+ * Only the `fileinto` action goes. A rule that also marks read, flags, or stops
+ * processing keeps doing those things — deleting a folder says nothing about
+ * whether the rest of the rule was still wanted. A rule left with no actions at
+ * all has nothing to do, so that one goes.
  */
-export function dropRulesForFolders(rules: SieveRule[], gone: FolderRef[]): { rules: SieveRule[]; removed: SieveRule[] } {
-  if (!gone.length) return { rules, removed: [] };
-  const removed = rules.filter((r) => 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 };
 }

web/src/store/mail.ts

@@ -1069,16 +1069,18 @@ async function followFolders(before: FolderRef[]): Promise<void> {
       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");