feat: delete filter rules, tags and trusted senders with undo and warn about the forward limit (bulwarkmail/webmail#1208)

This commit is contained in:
Linus Rath
2026-10-31 05:21:32 +01:00
parent 4932764a66
commit 8ac1997149
7 changed files with 381 additions and 43 deletions
@@ -45,6 +45,8 @@ import {
import { updateAccountFilters } from '../sieve-rules';
import { useContactsStore } from '../../stores/contacts-store';
import { readVacationFilters, useFilterStore } from '../../stores/filter-store';
import { useToastStore } from '../../stores/toast-store';
import { deleteFilterRuleWithUndo } from '../../lib/settings-undo';
import { generateVCard, parseVCard } from '../../lib/vcard';
import { generateScript } from '../../lib/sieve/generator';
import { parseScript } from '../../lib/sieve/parser';
@@ -336,5 +338,33 @@ live('live Stalwart: contacts and filters (0.16 and 1.0)', () => {
expect({ file, ...(await validateSieveScript(content)) }).toEqual({ file, isValid: true });
}
}, 60_000);
// Settings deletes a rule at once and offers Undo (webmail R3).
it('deletes a rule on the server at once and the toast Undo saves it back', async () => {
const rule = (id: string, name: string) => ({
id, name, enabled: true, matchType: 'all' as const,
conditions: [{ field: 'from' as const, comparator: 'contains' as const, value: `${id}-${RUN}@example.com` }],
actions: [{ type: 'mark_read' as const, value: '' }],
stopProcessing: false,
});
await updateSieveScript(created!, generateScript([rule('keep', `Keep ${RUN}`), rule('gone', `Gone ${RUN}`)]), true);
await useFilterStore.getState().selectAccount(null);
const gone = useFilterStore.getState().rules.find((r) => r.name === `Gone ${RUN}`)!;
expect(gone).toBeTruthy();
useToastStore.getState().clearToasts();
const serverContent = async () =>
getSieveScriptContent((await getSieveScripts()).find((x) => x.id === created)!.blobId);
await deleteFilterRuleWithUndo(gone.id, t);
expect(await serverContent()).not.toContain(`gone-${RUN}@example.com`);
expect(await serverContent()).toContain(`keep-${RUN}@example.com`);
const toast = useToastStore.getState().toasts.at(-1)!;
expect(toast.title).toBe('Filter rule deleted');
toast.action!.onPress();
await expect.poll(async () => serverContent(), { timeout: 15_000 }).toContain(`gone-${RUN}@example.com`);
expect(useFilterStore.getState().rules.map((r) => r.name)).toEqual([`Keep ${RUN}`, `Gone ${RUN}`]);
expect(useToastStore.getState().toasts.some((x) => x.type === 'error')).toBe(false);
}, 60_000);
});
});
@@ -11,8 +11,10 @@ import { useSettingsStore, type ExternalContentPolicy } from '../../stores/setti
import { useContactsStore } from '../../stores/contacts-store';
import { useHasContacts } from '../../lib/capabilities';
import { isTrustedSendersSyncOn } from '../../lib/trusted-senders';
import { removeTrustedSenderWithUndo } from '../../lib/settings-undo';
import { useLocaleStore } from '../../stores/locale-store';
import { MODAL_ORIENTATIONS } from '../../lib/modal-orientations';
import { ToastHost } from '../ToastHost';
interface TrustedRow {
email: string;
@@ -107,20 +109,19 @@ export function ContentSendersSettings() {
}
};
// Taking a sender off the list is reversible: no question, a toast with
// Undo that adds the address back (webmail trusted-senders-modal R3).
const handleRemove = async (row: TrustedRow) => {
if (row.local) removeTrustedSender(row.email);
if (row.synced) {
setBusy(row.email);
try {
await removeFromTrustedSendersBook(row.email);
} catch (err) {
Alert.alert(
t('settings.email_behavior.trusted_senders.sync_remove_failed', 'Could not remove from address book'),
err instanceof Error ? err.message : t('identities.validation_errors.unknown_error', 'Unknown error'),
);
} finally {
setBusy(null);
}
if (row.synced) setBusy(row.email);
try {
await removeTrustedSenderWithUndo(row, {
removeLocal: removeTrustedSender,
addLocal: addTrustedSender,
removeFromBook: removeFromTrustedSendersBook,
addToBook: addToTrustedSendersBook,
}, t);
} finally {
if (row.synced) setBusy(null);
}
};
@@ -269,6 +270,9 @@ export function ContentSendersSettings() {
)}
</Pressable>
</Pressable>
{/* The screen's toast host sits under this Modal; this one shows the
Undo after removing a sender above the list. */}
<ToastHost />
</Modal>
</View>
);
+28 -17
View File
@@ -22,6 +22,8 @@ import { SieveEditorSheet } from '../filters/SieveEditorSheet';
import type { FilterRule } from '../../lib/sieve/types';
import { formatConditionValue, summarizeRule } from '../../lib/sieve/condition-value';
import { periodStatus, supportsPeriods } from '../../lib/sieve/period';
import { rulesForwardLimit } from '../../lib/sieve/forward-limit';
import { deleteFilterRuleWithUndo } from '../../lib/settings-undo';
import Badge from '../Badge';
type Translate = (key: string, fallback?: string) => string;
@@ -157,7 +159,8 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) {
const {
rules, isLoading, isSaving, error, isSupported, isOpaque, rawScript, vacationSettings, includeVacation,
selectAccount, saveFilters, addRule, updateRule, deleteRule, reorderRules, toggleRule,
vacationForward, sieveCapabilities,
selectAccount, saveFilters, addRule, updateRule, reorderRules, toggleRule,
setOpaqueScript, resetToVisualBuilder, validateScript,
} = useFilterStore();
const periodsSupported = useFilterStore((s) => supportsPeriods(s.sieveCapabilities?.sieveExtensions));
@@ -200,23 +203,18 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) {
void persist(() => toggleRule(ruleId));
}, [toggleRule, persist]);
// A rule can be put back as it was, so it goes at once with an Undo
// (webmail R3).
const handleDelete = useCallback((rule: FilterRule) => {
Alert.alert(
t('settings.filters.delete_rule', 'Delete Rule'),
t('settings.filters.delete_confirm', 'Are you sure you want to delete this rule?'),
[
{ text: t('settings.filters.cancel', 'Cancel'), style: 'cancel' },
{
text: t('settings.filters.confirm_delete', 'Delete'),
style: 'destructive',
onPress: () => {
deleteRule(rule.id);
void persist(() => addRule(rule));
},
},
],
);
}, [deleteRule, addRule, persist, t]);
void deleteFilterRuleWithUndo(rule.id, t);
}, [t]);
// A message can collect only so many forwards on this server; the rules
// run behind the out of office forwarding (#1155, #1208).
const forwardLimit = useMemo(
() => rulesForwardLimit(rules, vacationForward, sieveCapabilities?.maxNumberRedirects),
[rules, vacationForward, sieveCapabilities],
);
const handleSaveRule = useCallback((rule: FilterRule) => {
const previous = useFilterStore.getState().rules;
@@ -362,6 +360,18 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) {
</View>
)}
{!isOpaque && forwardLimit.ignored && (
<Text style={styles.forwardWarning}>
{t('settings.filters.forward_unavailable', 'This server does not allow forwarding. Forward actions in filter rules have no effect.')}
</Text>
)}
{!isOpaque && forwardLimit.tooMany && (
<Text style={styles.forwardWarning}>
{t('settings.filters.forward_limit', 'Forward limit per message on this server: {count}. Extra forwards are skipped.', { count: sieveCapabilities?.maxNumberRedirects ?? 0 })}
</Text>
)}
{!isOpaque && rules.length > 0 && (
<View style={{ gap: spacing.sm }}>
{rules.map((rule) => {
@@ -567,6 +577,7 @@ function makeStyles(c: ThemePalette) {
emptyState: { alignItems: 'center', paddingVertical: spacing.xxl, gap: spacing.md },
emptyText: { ...typography.body, color: c.mutedForeground },
forwardWarning: { ...typography.caption, color: c.warning },
ruleRow: {
flexDirection: 'row',
+6 -13
View File
@@ -12,6 +12,7 @@ import { useLocaleStore } from '../../stores/locale-store';
import { discoverKeywords } from '../../api/keyword-discovery';
import { findUnrecognizedKeywords, type UnrecognizedKeyword } from '../../lib/keyword-discovery';
import { jmapClient } from '../../api/jmap-client';
import { deleteKeywordWithUndo } from '../../lib/settings-undo';
type Keyword = KeywordDef;
@@ -26,7 +27,6 @@ export function KeywordSettings() {
const t = useLocaleStore((s) => s.t);
const addKeyword = useKeywordsStore((s) => s.add);
const updateKeyword = useKeywordsStore((s) => s.update);
const removeKeyword = useKeywordsStore((s) => s.remove);
const resetDefaults = useKeywordsStore((s) => s.resetDefaults);
const hydrated = useKeywordsStore((s) => s.hydrated);
const hydrate = useKeywordsStore((s) => s.hydrate);
@@ -47,17 +47,10 @@ export function KeywordSettings() {
}
};
// Deleting a tag takes it off every message's tag list on this device; ask
// first, as the reset next to it does.
const deleteKeyword = (id: string, label: string) => {
Alert.alert(
t('settings.keywords.delete_confirm_title', 'Delete tag "{label}"?', { label }),
undefined,
[
{ text: t('common.cancel', 'Cancel'), style: 'cancel' },
{ text: t('settings.keywords.delete', 'Delete tag'), style: 'destructive', onPress: () => removeKeyword(id) },
],
);
// A tag can be put back as it was, so it goes at once with an Undo
// (webmail R3).
const deleteKeyword = (id: string) => {
deleteKeywordWithUndo(id, t);
};
// One stray tap used to wipe a carefully built tag list (webmail removed
@@ -130,7 +123,7 @@ export function KeywordSettings() {
<Pressable style={styles.iconBtn} onPress={() => setEditingId(kw.id)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.edit', "Edit tag")}>
<Pencil size={14} color={c.mutedForeground} />
</Pressable>
<Pressable style={styles.iconBtn} onPress={() => deleteKeyword(kw.id, kw.label)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.delete', "Delete tag")}>
<Pressable style={styles.iconBtn} onPress={() => deleteKeyword(kw.id)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.delete', "Delete tag")}>
<Trash2 size={14} color={c.mutedForeground} />
</Pressable>
</View>
+190
View File
@@ -0,0 +1,190 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
vi.mock('../../api/sieve', () => ({
createSieveScript: vi.fn(),
getSieveAccountId: vi.fn(() => 'own'),
getSieveCapabilities: vi.fn(() => null),
getSieveScriptContent: vi.fn(),
getSieveScripts: vi.fn(),
isSieveSupported: vi.fn(() => true),
updateSieveScript: vi.fn(),
validateSieveScript: vi.fn(async () => ({ isValid: true })),
}));
import fs from 'node:fs';
import path from 'node:path';
import * as sieve from '../../api/sieve';
import { generateScript } from '../sieve/generator';
import type { FilterRule } from '../sieve/types';
import { useFilterStore } from '../../stores/filter-store';
import { useKeywordsStore } from '../../stores/keywords-store';
import { useToastStore } from '../../stores/toast-store';
import {
deleteFilterRuleWithUndo, deleteKeywordWithUndo, removeTrustedSenderWithUndo, UNDO_TOAST_DURATION,
} from '../settings-undo';
// Deleting a filter rule, a tag or a trusted sender in Settings goes at once
// with an Undo in the toast instead of a confirm (webmail R3).
const api = vi.mocked(sieve);
const t = (key: string, fallback?: string) => `${key}|${fallback ?? ''}`;
const toasts = () => useToastStore.getState().toasts;
const lastToast = () => toasts()[toasts().length - 1];
function rule(id: string, name: string): FilterRule {
return {
id, name, enabled: true, matchType: 'all',
conditions: [{ field: 'from', comparator: 'contains', value: `${id}@example.com` }],
actions: [{ type: 'move', value: 'Archive' }],
stopProcessing: false,
};
}
beforeEach(() => {
vi.clearAllMocks();
api.getSieveAccountId.mockReturnValue('own');
api.isSieveSupported.mockReturnValue(true);
api.getSieveCapabilities.mockReturnValue(null);
useFilterStore.getState().clearState();
useToastStore.getState().clearToasts();
});
describe('deleteFilterRuleWithUndo', () => {
async function load() {
api.getSieveScripts.mockResolvedValue([{ id: 's1', name: 'filters', blobId: 'b1', isActive: true }]);
api.getSieveScriptContent.mockResolvedValue(generateScript([rule('a', 'A'), rule('b', 'B')]));
await useFilterStore.getState().selectAccount(null);
}
it('deletes and saves without asking, and the toast Undo saves the rule back', async () => {
await load();
api.updateSieveScript.mockResolvedValue(undefined);
await deleteFilterRuleWithUndo('a', t);
expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['b']);
expect(api.updateSieveScript).toHaveBeenCalledTimes(1);
expect(lastToast()).toMatchObject({ type: 'success', title: 'notifications.filters_deleted|Filter rule deleted' });
expect(lastToast().action?.label).toBe('notifications.rule_undo|Undo');
lastToast().action!.onPress();
await vi.waitFor(() => expect(api.updateSieveScript).toHaveBeenCalledTimes(2));
expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['a', 'b']);
});
it('says the save failed and keeps the rule when the server refuses', async () => {
await load();
api.updateSieveScript.mockRejectedValueOnce(new Error('down'));
await deleteFilterRuleWithUndo('a', t);
expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['a', 'b']);
expect(toasts()).toHaveLength(1);
expect(lastToast()).toMatchObject({ type: 'error', title: 'notifications.filters_save_failed|Failed to save filters' });
});
it('says the save failed when the Undo cannot be saved', async () => {
await load();
api.updateSieveScript.mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error('down'));
await deleteFilterRuleWithUndo('a', t);
lastToast().action!.onPress();
await vi.waitFor(() => expect(lastToast().type).toBe('error'));
expect(useFilterStore.getState().rules.map((r) => r.id)).toEqual(['b']);
});
});
describe('deleteKeywordWithUndo', () => {
beforeEach(() => {
useKeywordsStore.setState({
keywords: [
{ id: 'red', label: 'Red', color: 'red' },
{ id: 'work', label: 'Work', color: 'blue' },
{ id: 'home', label: 'Home', color: 'green' },
],
} as never);
});
it('deletes the tag at once and Undo puts it back where it was', () => {
deleteKeywordWithUndo('work', t);
expect(useKeywordsStore.getState().keywords.map((k) => k.id)).toEqual(['red', 'home']);
expect(lastToast()).toMatchObject({ type: 'success', title: 'settings.keywords.deleted|Tag deleted' });
lastToast().action!.onPress();
expect(useKeywordsStore.getState().keywords.map((k) => k.id)).toEqual(['red', 'work', 'home']);
// A second tap does not add it twice.
lastToast().action!.onPress();
expect(useKeywordsStore.getState().keywords.map((k) => k.id)).toEqual(['red', 'work', 'home']);
});
});
describe('removeTrustedSenderWithUndo', () => {
function stores() {
return {
removeLocal: vi.fn(),
addLocal: vi.fn(),
removeFromBook: vi.fn(async () => undefined),
addToBook: vi.fn(async () => undefined),
};
}
it('removes the sender from the list and the book, and Undo adds it back to both', async () => {
const s = stores();
await removeTrustedSenderWithUndo({ email: 'ann@example.com', local: true, synced: true }, s, t);
expect(s.removeFromBook).toHaveBeenCalledWith('ann@example.com');
expect(s.removeLocal).toHaveBeenCalledWith('ann@example.com');
expect(lastToast()).toMatchObject({
type: 'success',
title: 'settings.email_behavior.trusted_senders.removed|Removed from trusted senders',
message: 'ann@example.com',
duration: UNDO_TOAST_DURATION,
});
lastToast().action!.onPress();
expect(s.addLocal).toHaveBeenCalledWith('ann@example.com');
expect(s.addToBook).toHaveBeenCalledWith('ann@example.com');
});
it('only touches the places the sender was in', async () => {
const s = stores();
await removeTrustedSenderWithUndo({ email: 'bo@example.com', local: true, synced: false }, s, t);
lastToast().action!.onPress();
expect(s.removeFromBook).not.toHaveBeenCalled();
expect(s.addToBook).not.toHaveBeenCalled();
expect(s.addLocal).toHaveBeenCalledWith('bo@example.com');
});
it('changes nothing and says so when the address book cannot be written', async () => {
const s = stores();
s.removeFromBook.mockRejectedValueOnce(new Error('down'));
await removeTrustedSenderWithUndo({ email: 'ann@example.com', local: true, synced: true }, s, t);
expect(s.removeLocal).not.toHaveBeenCalled();
expect(toasts()).toHaveLength(1);
expect(lastToast()).toMatchObject({ type: 'error', title: "settings.email_behavior.trusted_senders.remove_error|Couldn't remove the sender" });
});
});
// No React Native renderer in the test env: the panes are checked for the
// wiring.
describe('settings panes', () => {
const read = (file: string) => fs.readFileSync(path.resolve(__dirname, '../../components/settings', file), 'utf8');
it('filters: deletes with Undo and warns about the forward limit (#1155, #1208)', () => {
const src = read('FilterSettings.tsx');
expect(src).toMatch(/void deleteFilterRuleWithUndo\(rule\.id, t\)/);
expect(src).not.toMatch(/settings\.filters\.delete_confirm/);
expect(src).toMatch(/rulesForwardLimit\(rules, vacationForward, sieveCapabilities\?\.maxNumberRedirects\)/);
expect(src).toMatch(/forwardLimit\.ignored && \(\s*<Text[^>]*>\s*\{t\('settings\.filters\.forward_unavailable'/);
expect(src).toMatch(/forwardLimit\.tooMany && \(\s*<Text[^>]*>\s*\{t\('settings\.filters\.forward_limit'/);
});
it('tags: deletes with Undo', () => {
const src = read('KeywordSettings.tsx');
expect(src).toMatch(/deleteKeywordWithUndo\(id, t\)/);
expect(src).not.toMatch(/delete_confirm_title/);
});
it('trusted senders: removes with Undo, shown above the list modal', () => {
const src = read('ContentSendersSettings.tsx');
expect(src).toMatch(/await removeTrustedSenderWithUndo\(row, \{/);
expect(src).not.toMatch(/sync_remove_failed/);
expect(src).toMatch(/<ToastHost \/>\s*<\/Modal>/);
});
});
+101
View File
@@ -0,0 +1,101 @@
// Deleting things in Settings that can be put back as they were: they go at
// once, with an Undo in the toast, instead of behind a confirm (webmail R3:
// filter-settings.tsx handleDelete, keyword-settings.tsx handleDelete,
// trusted-senders-modal.tsx handleRemoveSender).
import { toast } from '../stores/toast-store';
import { useFilterStore } from '../stores/filter-store';
import { useKeywordsStore } from '../stores/keywords-store';
type Translate = (key: string, fallback?: string, params?: Record<string, string | number>) => string;
/** How long a toast offering Undo stays (webmail lib/email-action-toast.ts). */
export const UNDO_TOAST_DURATION = 8000;
/** Deletes a filter rule on the server at once; the toast's Undo saves it back. */
export async function deleteFilterRuleWithUndo(ruleId: string, t: Translate): Promise<void> {
const failed = () => toast.error(t('notifications.filters_save_failed', 'Failed to save filters'));
let undo: (() => Promise<boolean>) | null;
try {
undo = await useFilterStore.getState().deleteRuleNow(ruleId);
} catch {
failed();
return;
}
if (!undo) return;
const putBack = undo;
toast.success(t('notifications.filters_deleted', 'Filter rule deleted'), {
action: {
label: t('notifications.rule_undo', 'Undo'),
onPress: () => { void putBack().catch(failed); },
},
});
}
/** Deletes a tag at once; the toast's Undo puts it back where it was. */
export function deleteKeywordWithUndo(id: string, t: Translate): void {
const before = useKeywordsStore.getState().keywords;
const index = before.findIndex((k) => k.id === id);
if (index < 0) return;
const removed = before[index];
useKeywordsStore.getState().remove(id);
toast.success(t('settings.keywords.deleted', 'Tag deleted'), {
action: {
label: t('notifications.rule_undo', 'Undo'),
onPress: () => {
// Added again meanwhile (same id): nothing to put back.
if (useKeywordsStore.getState().keywords.some((k) => k.id === id)) return;
useKeywordsStore.getState().insert(removed, index);
},
},
});
}
export interface TrustedSenderRemoval {
email: string;
/** On the list kept in the settings. */
local: boolean;
/** In the synced "Trusted Senders" address book. */
synced: boolean;
}
export interface TrustedSenderStores {
removeLocal: (email: string) => void;
addLocal: (email: string) => void;
removeFromBook: (email: string) => Promise<void>;
addToBook: (email: string) => Promise<void>;
}
/**
* Takes a sender off the trusted list (the settings list and the synced
* address book, wherever it is); the toast's Undo adds it back to the same
* places. Nothing changes when the address book can't be written.
*/
export async function removeTrustedSenderWithUndo(
row: TrustedSenderRemoval,
stores: TrustedSenderStores,
t: Translate,
): Promise<void> {
try {
if (row.synced) await stores.removeFromBook(row.email);
} catch {
toast.error(t('settings.email_behavior.trusted_senders.remove_error', "Couldn't remove the sender"));
return;
}
if (row.local) stores.removeLocal(row.email);
toast.success(t('settings.email_behavior.trusted_senders.removed', 'Removed from trusted senders'), {
message: row.email,
duration: UNDO_TOAST_DURATION,
action: {
label: t('notifications.undo', 'Undo'),
onPress: () => {
if (row.local) stores.addLocal(row.email);
if (row.synced) {
void stores.addToBook(row.email).catch(() => {
toast.error(t('settings.email_behavior.trusted_senders.save_error', 'Failed to save - check the Contacts debug log for details'));
});
}
},
},
});
}
+9
View File
@@ -67,6 +67,8 @@ interface KeywordsState {
add: (kw: KeywordDef) => void;
update: (id: string, patch: Partial<Omit<KeywordDef, 'id'>>) => void;
remove: (id: string) => void;
/** Puts a tag back at `index` (the Undo after deleting it). */
insert: (kw: KeywordDef, index: number) => void;
resetDefaults: () => void;
}
@@ -167,6 +169,13 @@ export const useKeywordsStore = create<KeywordsState>((set, get) => ({
persist(next);
},
insert: (kw, index) => {
const next = [...get().keywords];
next.splice(Math.min(Math.max(index, 0), next.length), 0, kw);
set({ keywords: next });
persist(next);
},
resetDefaults: () => {
set({ keywords: DEFAULT_KEYWORDS });
persist(DEFAULT_KEYWORDS);