diff --git a/src/api/__tests__/live-stalwart-mail.test.ts b/src/api/__tests__/live-stalwart-mail.test.ts index 9cfe4a25..2c7c805d 100644 --- a/src/api/__tests__/live-stalwart-mail.test.ts +++ b/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 ?? '' }; diff --git a/src/stores/__tests__/filter-store.test.ts b/src/stores/__tests__/filter-store.test.ts index 019ec910..dbfd1610 100644 --- a/src/stores/__tests__/filter-store.test.ts +++ b/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); + }); +}); diff --git a/src/stores/filter-store.ts b/src/stores/filter-store.ts index 57bd2f43..d153a7bb 100644 --- a/src/stores/filter-store.ts +++ b/src/stores/filter-store.ts @@ -57,6 +57,14 @@ interface FilterStore { addRule: (rule: FilterRule) => void; updateRule: (ruleId: string, updates: Partial) => 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) | null>; reorderRules: (ruleIds: string[]) => void; toggleRule: (ruleId: string) => void; setRawScript: (content: string) => void; @@ -238,6 +246,31 @@ export const useFilterStore = create()((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.