Keep an edited filter rule where it was
Renaming or editing a Sieve rule moved it to the bottom of the list, and in Sieve the order is the order the rules run in, so mail started being filed by a different rule than before. Putting it back took a click per place moved. Two things did it. saveAndApply always appended the rule it was given — right for a rule created from a message, wrong for one being edited. And the "Also apply to existing messages" tick defaulted to on wherever it was offered, so every edit in Settings went down that path, including a plain rename. The rule now keeps its seat: a shared upsertRule replaces by id in place and only appends what is genuinely new. The tick defaults to on only in "Filter messages like this…", where applying it is the point, and the toast no longer calls an edited rule "created". Fixes #24
This commit is contained in:
@@ -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([]);
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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);
|
||||
} else setLocal(next);
|
||||
void saveAndApply(r, list, inbox.id);
|
||||
} else setLocal(upsertRule(list, r));
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
|
||||
@@ -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]);
|
||||
|
||||
Reference in New Issue
Block a user