Merge pull request #27 from LINUXexpert-org/sieve-rule-order-on-edit

Keep an edited filter rule where it was

web/src/lib/__tests__/sieve.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { newRule, rulesToSieve, sieveToRules, testToSieve, sieveString } from "../sieve";
+import { newRule, rulesToSieve, sieveToRules, testToSieve, sieveString, upsertRule } from "../sieve";
 
 describe("sieve codec", () => {
   it("escapes strings", () => {
@@ -24,6 +24,14 @@ describe("sieve codec", () => {
     expect(script).toContain("# (disabled) Big");
     expect(sieveToRules(script)).toEqual(rules);
   });
+  it("keeps an edited rule in its place and appends a new one", () => {
+    const rules = ["r1", "r2", "r3"].map((id) => newRule({ id, name: id }));
+    const renamed = { ...rules[1]!, name: "Renamed" };
+    expect(upsertRule(rules, renamed).map((r) => r.id)).toEqual(["r1", "r2", "r3"]);
+    expect(upsertRule(rules, renamed)[1]!.name).toBe("Renamed");
+    expect(upsertRule(rules, newRule({ id: "r4" })).map((r) => r.id)).toEqual(["r1", "r2", "r3", "r4"]);
+    expect(rules.map((r) => r.name)).toEqual(["r1", "r2", "r3"]);
+  });
   it("reports hand-written scripts as raw", () => {
     expect(sieveToRules('require ["fileinto"];\nif true { keep; }')).toBeNull();
     expect(sieveToRules("")).toEqual([]);

web/src/lib/sieve.ts

@@ -202,6 +202,14 @@ export function newRule(partial: Partial<SieveRule> = {}): SieveRule {
   };
 }
 
+/**
+ * Replaces a rule with the same id in place, or appends it when it is new.
+ * Order is evaluation order in Sieve, so an edited rule has to keep its seat.
+ */
+export function upsertRule(rules: SieveRule[], rule: SieveRule): SieveRule[] {
+  return rules.some((x) => x.id === rule.id) ? rules.map((x) => (x.id === rule.id ? rule : x)) : [...rules, rule];
+}
+
 export function describeRule(r: SieveRule): string {
   const tests = r.tests
     .map((t) => {

web/src/views/mail/FilterFromMessage.tsx

@@ -3,7 +3,7 @@ import type { Email, Id } from "@/jmap/types";
 import { useSieve } from "@/store/sieve";
 import { useMail } from "@/store/mail";
 import { ruleFromEmail, applyRuleToMailbox } from "@/lib/sieveApply";
-import type { SieveRule } from "@/lib/sieve";
+import { upsertRule, type SieveRule } from "@/lib/sieve";
 import { RuleDialog } from "../settings/RuleDialog";
 import { toast } from "@/ui/toast";
 import { Spinner } from "@/ui/misc";
@@ -48,6 +48,7 @@ export function FilterFromMessageDialog({ email, mailboxId, onClose }: { email:
       title="Filter messages like this"
       saveLabel="Create filter"
       applyMailbox={mailbox ? { id: mailbox.id, name: mailbox.name } : null}
+      applyByDefault
       onClose={onClose}
       onSave={(r, applyNow) => {
         onClose();
@@ -57,23 +58,30 @@ export function FilterFromMessageDialog({ email, mailboxId, onClose }: { email:
   );
 }
 
+/**
+ * Saves `r` into `existing` — replacing it in place when it is already there,
+ * appending it when it is new — and optionally runs it over a folder.
+ * `existing` is the rule list as it stands *before* the edit.
+ */
 export async function saveAndApply(r: SieveRule, existing: SieveRule[], applyMailboxId: Id | null) {
   const sieve = useSieve.getState();
+  const created = !existing.some((x) => x.id === r.id);
+  const verb = created ? "created" : "saved";
   try {
-    await sieve.saveRules([...existing.filter((x) => x.id !== r.id), r]);
+    await sieve.saveRules(upsertRule(existing, r));
   } catch (err) {
     toast.error(`Could not save filter: ${(err as Error).message}`);
     return;
   }
   if (!applyMailboxId) {
-    toast.success("Filter created — it will run on new mail");
+    toast.success(`Filter ${verb} — it will run on new mail`);
     return;
   }
   const tid = toast.show("Applying filter to existing messages…", { duration: 0 });
   try {
     const res = await applyRuleToMailbox(r, applyMailboxId);
     toast.dismiss(tid);
-    toast.success(`Filter created · applied to ${res.matched} of ${res.scanned} message${res.scanned === 1 ? "" : "s"}${res.skippedActions.length ? ` (skipped: ${res.skippedActions.join("; ")})` : ""}`, { duration: 8000 });
+    toast.success(`Filter ${verb} · applied to ${res.matched} of ${res.scanned} message${res.scanned === 1 ? "" : "s"}${res.skippedActions.length ? ` (skipped: ${res.skippedActions.join("; ")})` : ""}`, { duration: 8000 });
   } catch (err) {
     toast.dismiss(tid);
     toast.error(`Filter saved, but applying it failed: ${(err as Error).message}`);

web/src/views/settings/FiltersSettings.tsx

@@ -2,7 +2,7 @@ import { useEffect, useState } from "react";
 import { ArrowDown, ArrowUp, Code, Plus, Trash2, Wand2, Play, AlertTriangle, Power } from "lucide-react";
 import { useSieve } from "@/store/sieve";
 import { useMail } from "@/store/mail";
-import { describeRule, newRule, rulesToSieve, type SieveRule } from "@/lib/sieve";
+import { describeRule, newRule, rulesToSieve, upsertRule, type SieveRule } from "@/lib/sieve";
 import { RuleDialog } from "./RuleDialog";
 import { saveAndApply } from "../mail/FilterFromMessage";
 import { confirmDialog, promptDialog } from "@/ui/dialog";
@@ -110,14 +110,13 @@ function RulesEditor() {
           onClose={() => setEditing(null)}
           applyMailbox={inbox ? { id: inbox.id, name: inbox.name } : null}
           onSave={(r, applyNow) => {
-            const exists = list.some((x) => x.id === r.id);
-            const next = exists ? list.map((x) => (x.id === r.id ? r : x)) : [...list, r];
             setEditing(null);
             if (applyNow && inbox) {
               // Save immediately so the rule is live, then apply it to the Inbox.
+              // saveAndApply takes the list as it stands now: an edited rule keeps its place.
               setLocal(null);
-              void saveAndApply(r, next.filter((x) => x.id !== r.id), inbox.id);
+              void saveAndApply(r, list, inbox.id);
-            } else setLocal(next);
+            } else setLocal(upsertRule(list, r));
           }}
         />
       )}

web/src/views/settings/RuleDialog.tsx

@@ -13,13 +13,15 @@ export interface RuleDialogProps {
   onSave: (r: SieveRule, applyNow: boolean) => void;
   /** When set, offers "Also apply to existing messages in <folder>". */
   applyMailbox?: { id: Id; name: string } | null;
+  /** Ticks that offer by default. Off unless applying is the point of the dialog. */
+  applyByDefault?: boolean;
   title?: string;
   saveLabel?: string;
 }
 
-export function RuleDialog({ rule, onClose, onSave, applyMailbox, title, saveLabel }: RuleDialogProps) {
+export function RuleDialog({ rule, onClose, onSave, applyMailbox, applyByDefault, title, saveLabel }: RuleDialogProps) {
   const [r, setR] = useState<SieveRule>(rule);
-  const [applyNow, setApplyNow] = useState(Boolean(applyMailbox));
+  const [applyNow, setApplyNow] = useState(Boolean(applyMailbox && applyByDefault));
   const mailboxes = useMail((s) => s.mailboxes);
   const mailboxPath = useMail((s) => s.mailboxPath);
   const folders = useMemo(() => Object.values(mailboxes).map((m) => ({ id: m.id, path: mailboxPath(m.id) })).sort((a, b) => a.path.localeCompare(b.path)), [mailboxes, mailboxPath]);