diff --git a/src/api/__tests__/live-stalwart-contacts.test.ts b/src/api/__tests__/live-stalwart-contacts.test.ts index 1f012675..83baa33a 100644 --- a/src/api/__tests__/live-stalwart-contacts.test.ts +++ b/src/api/__tests__/live-stalwart-contacts.test.ts @@ -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); }); }); diff --git a/src/components/settings/ContentSendersSettings.tsx b/src/components/settings/ContentSendersSettings.tsx index 6b340584..f8a14461 100644 --- a/src/components/settings/ContentSendersSettings.tsx +++ b/src/components/settings/ContentSendersSettings.tsx @@ -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() { )} + {/* The screen's toast host sits under this Modal; this one shows the + Undo after removing a sender above the list. */} + ); diff --git a/src/components/settings/FilterSettings.tsx b/src/components/settings/FilterSettings.tsx index 83d21b7d..983e0fbd 100644 --- a/src/components/settings/FilterSettings.tsx +++ b/src/components/settings/FilterSettings.tsx @@ -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 = {}) { )} + {!isOpaque && forwardLimit.ignored && ( + + {t('settings.filters.forward_unavailable', 'This server does not allow forwarding. Forward actions in filter rules have no effect.')} + + )} + + {!isOpaque && forwardLimit.tooMany && ( + + {t('settings.filters.forward_limit', 'Forward limit per message on this server: {count}. Extra forwards are skipped.', { count: sieveCapabilities?.maxNumberRedirects ?? 0 })} + + )} + {!isOpaque && rules.length > 0 && ( {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', diff --git a/src/components/settings/KeywordSettings.tsx b/src/components/settings/KeywordSettings.tsx index a9c399db..83d867fb 100644 --- a/src/components/settings/KeywordSettings.tsx +++ b/src/components/settings/KeywordSettings.tsx @@ -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() { setEditingId(kw.id)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.edit', "Edit tag")}> - deleteKeyword(kw.id, kw.label)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.delete', "Delete tag")}> + deleteKeyword(kw.id)} accessibilityRole="button" accessibilityLabel={t('settings.keywords.delete', "Delete tag")}> diff --git a/src/lib/__tests__/settings-undo.test.ts b/src/lib/__tests__/settings-undo.test.ts new file mode 100644 index 00000000..33245d82 --- /dev/null +++ b/src/lib/__tests__/settings-undo.test.ts @@ -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*]*>\s*\{t\('settings\.filters\.forward_unavailable'/); + expect(src).toMatch(/forwardLimit\.tooMany && \(\s*]*>\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(/\s*<\/Modal>/); + }); +}); diff --git a/src/lib/settings-undo.ts b/src/lib/settings-undo.ts new file mode 100644 index 00000000..0ed01b76 --- /dev/null +++ b/src/lib/settings-undo.ts @@ -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; + +/** 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 { + const failed = () => toast.error(t('notifications.filters_save_failed', 'Failed to save filters')); + let undo: (() => Promise) | 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; + addToBook: (email: string) => Promise; +} + +/** + * 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 { + 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')); + }); + } + }, + }, + }); +} diff --git a/src/stores/keywords-store.ts b/src/stores/keywords-store.ts index edb7595a..ab27669b 100644 --- a/src/stores/keywords-store.ts +++ b/src/stores/keywords-store.ts @@ -67,6 +67,8 @@ interface KeywordsState { add: (kw: KeywordDef) => void; update: (id: string, patch: Partial>) => 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((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);