feat: delete a filter rule at once with an undo in the filter store

src/api/__tests__/live-stalwart-mail.test.ts

@@ -45,8 +45,9 @@ import { pruneEmailSubmissions } from '../submissions';
 import { listPrincipals } from '../files';
 import { createContact, deleteContacts, getAddressBooks, getContact } from '../contacts';
 import {
-  createSieveScript, deleteSieveScript, getSieveScriptContent, getSieveScripts, validateSieveScript,
+  createSieveScript, deleteSieveScript, getSieveScriptContent, getSieveScripts, updateSieveScript, validateSieveScript,
 } from '../sieve';
+import { useFilterStore } from '../../stores/filter-store';
 import { inlineContactMediaForExport, writeContactWithNewPhoto } from '../../lib/contact-media';
 import { getBulwarkMetadata, resetBulwarkMetadata, supportsBulwarkMetadata } from '../bulwark-metadata';
 import { parseSharedTags, SHARED_TAGS_KEY } from '../../lib/shared-tags';
@@ -405,6 +406,47 @@ live('live Stalwart: mail, contacts, search (0.16 and 1.0)', () => {
       }
     }, 60_000);
 
+    it('deletes a filter rule at once and puts it back on Undo, in the script on the server', async () => {
+      // The account's filters as they are, to put back afterwards.
+      const before = await getSieveScripts();
+      const active = before.find((s) => s.isActive && s.name !== 'vacation');
+      const original = active ? await getSieveScriptContent(active.blobId) : null;
+      const activeContent = async () => {
+        const now = (await getSieveScripts()).find((s) => s.isActive && s.name !== 'vacation');
+        return now ? getSieveScriptContent(now.blobId) : '';
+      };
+      try {
+        await useFilterStore.getState().selectAccount(null);
+        expect(useFilterStore.getState().isOpaque, 'the test account has no hand-written filters').toBe(false);
+        const rule = (id: string) => ({
+          id, name: `live ${id} ${RUN}`, enabled: true, matchType: 'all' as const,
+          conditions: [{ field: 'subject' as const, comparator: 'contains' as const, value: `${id}-${RUN}` }],
+          actions: [{ type: 'mark_read' as const, value: '' }], stopProcessing: false,
+        });
+        useFilterStore.getState().addRule(rule('one'));
+        useFilterStore.getState().addRule(rule('two'));
+        await useFilterStore.getState().saveFilters();
+        expect(await activeContent()).toContain(`one-${RUN}`);
+
+        const undo = await useFilterStore.getState().deleteRuleNow('one');
+        let content = await activeContent();
+        expect(content).not.toContain(`one-${RUN}`);
+        expect(content).toContain(`two-${RUN}`);
+
+        expect(await undo!()).toBe(true);
+        content = await activeContent();
+        expect(content.indexOf(`one-${RUN}`)).toBeGreaterThan(-1);
+        expect(content.indexOf(`one-${RUN}`)).toBeLessThan(content.indexOf(`two-${RUN}`));
+      } finally {
+        const after = await getSieveScripts();
+        if (active && original !== null) await updateSieveScript(active.id, original, true);
+        for (const script of after) {
+          if (!before.some((b) => b.id === script.id)) await deleteSieveScript(script.id).catch(() => undefined);
+        }
+        useFilterStore.getState().clearState();
+      }
+    }, 60_000);
+
     it('changes and restores an identity\'s name and signature', async () => {
       const [identity] = await getIdentities();
       const before = { name: identity.name, textSignature: identity.textSignature ?? '' };

src/stores/__tests__/filter-store.test.ts

@@ -373,3 +373,46 @@ describe('readVacationFilters', () => {
     expect(content).toContain('    redirect :copy "backup@example.org";');
   });
 });
+
+describe('deleteRuleNow', () => {
+  const a = makeRule({ id: 'a', name: 'A' });
+  const b = makeRule({ id: 'b', name: 'B' });
+
+  it('deletes and saves at once, and Undo puts the rule back where it was and saves again', async () => {
+    serveScript([a, b]);
+    await useFilterStore.getState().selectAccount(null);
+    api.updateSieveScript.mockResolvedValue(undefined);
+
+    const undo = await useFilterStore.getState().deleteRuleNow('a');
+    expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['b']);
+    expect(api.updateSieveScript).toHaveBeenCalledTimes(1);
+    expect(api.updateSieveScript.mock.calls[0][1]).not.toContain('"A"');
+
+    expect(await undo!()).toBe(true);
+    expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['a', 'b']);
+    expect(api.updateSieveScript).toHaveBeenCalledTimes(2);
+    expect(api.updateSieveScript.mock.calls[1][1]).toContain('A');
+    // Once undone there is nothing left to undo.
+    expect(await undo!()).toBe(false);
+    expect(api.updateSieveScript).toHaveBeenCalledTimes(2);
+  });
+
+  it('keeps the rule when the save fails', async () => {
+    serveScript([a, b]);
+    await useFilterStore.getState().selectAccount(null);
+    api.updateSieveScript.mockRejectedValueOnce(new Error('down'));
+    await expect(useFilterStore.getState().deleteRuleNow('a')).rejects.toThrow('down');
+    expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['a', 'b']);
+  });
+
+  it("does not undo into another account's filters", async () => {
+    serveScript([a, b]);
+    await useFilterStore.getState().selectAccount(null);
+    api.updateSieveScript.mockResolvedValue(undefined);
+    const undo = await useFilterStore.getState().deleteRuleNow('a');
+    serveScript([b]);
+    await useFilterStore.getState().selectAccount('team');
+    expect(await undo!()).toBe(false);
+    expect(api.updateSieveScript).toHaveBeenCalledTimes(1);
+  });
+});

src/stores/filter-store.ts

@@ -57,6 +57,14 @@ interface FilterStore {
   addRule: (rule: FilterRule) => void;
   updateRule: (ruleId: string, updates: Partial<FilterRule>) => void;
   deleteRule: (ruleId: string) => void;
+  /**
+   * Delete a rule and save at once, as the webmail does with an Undo (R3).
+   * Resolves to the undo, or null when there is no such rule; throws, with
+   * the rules put back, when the save fails. The undo puts the rules back as
+   * they were and saves again; it resolves false when there is nothing to
+   * undo any more (the rule is back, or another account's filters show).
+   */
+  deleteRuleNow: (ruleId: string) => Promise<(() => Promise<boolean>) | null>;
   reorderRules: (ruleIds: string[]) => void;
   toggleRule: (ruleId: string) => void;
   setRawScript: (content: string) => void;
@@ -238,6 +246,31 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
     }));
   },
 
+  deleteRuleNow: async (ruleId) => {
+    const before = get().rules;
+    const account = get().selectedAccountId;
+    if (!before.some((r) => r.id === ruleId && r.origin !== 'external' && r.origin !== 'opaque')) return null;
+    get().deleteRule(ruleId);
+    try {
+      await get().saveFilters();
+    } catch (error) {
+      set({ rules: before });
+      throw error;
+    }
+    return async () => {
+      if (get().selectedAccountId !== account || get().rules.some((r) => r.id === ruleId)) return false;
+      const current = get().rules;
+      set({ rules: before });
+      try {
+        await get().saveFilters();
+      } catch (error) {
+        set({ rules: current });
+        throw error;
+      }
+      return true;
+    };
+  },
+
   reorderRules: (ruleIds) => {
     // Only reorder bulwark rules; external rules always stay at the end in
     // their original order.