From df83cee10c5ac1dfc6d3e0361b9bc7f4bb801c51 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Wed, 15 Jul 2026 19:26:37 +0200 Subject: [PATCH] feat: mail phase 1 - HTML iframe display, bulk read/unread, IMAP flag sync - Fix FlagUpdatePayload field names to match backend schema (is_seen/is_flagged) - Replace dangerouslySetInnerHTML with sandboxed iframe (srcDoc, sandbox="") - Add bulk mark read/unread with checkbox selection in MailList - Add floating bulk action bar in Mail.tsx - Add imap_sync_mail_flags() to push Seen/Flagged flags to IMAP server - Call IMAP flag sync in PATCH /flags endpoint - Add i18n keys for bulk actions (de/en) --- app/plugins/builtins/mail/routes.py | 7 ++ app/plugins/builtins/mail/services.py | 108 ++++++++++++++++ frontend/src/api/mail.ts | 7 +- frontend/src/components/mail/MailDetail.tsx | 43 ++++--- frontend/src/components/mail/MailList.tsx | 129 ++++++++++++------- frontend/src/i18n/locales/de.json | 7 +- frontend/src/i18n/locales/en.json | 7 +- frontend/src/pages/Mail.tsx | 132 +++++++++++++++++++- test_report.md | 59 --------- 9 files changed, 364 insertions(+), 135 deletions(-) delete mode 100644 test_report.md diff --git a/app/plugins/builtins/mail/routes.py b/app/plugins/builtins/mail/routes.py index f16a93a..046af71 100644 --- a/app/plugins/builtins/mail/routes.py +++ b/app/plugins/builtins/mail/routes.py @@ -57,6 +57,7 @@ from app.plugins.builtins.mail.schemas import ( TemplateSubstituteRequest, VacationConfig, ) +import app.plugins.builtins.mail.services as mail_services from app.plugins.builtins.mail.services import ( MAX_ATTACHMENT_SIZE, _attachment_storage_path, @@ -1216,6 +1217,12 @@ async def update_flags( if data.is_forwarded is not None: mail.is_forwarded = data.is_forwarded await db.flush() + # Sync flags to IMAP server (non-critical — failures are logged but don't break the API) + try: + await mail_services.imap_sync_mail_flags(db, m_id, tenant_id) + except Exception as exc: + import logging + logging.getLogger(__name__).warning("IMAP flag sync failed for mail %s: %s", m_id, exc) return mail_to_response(mail) diff --git a/app/plugins/builtins/mail/services.py b/app/plugins/builtins/mail/services.py index a2b6318..b5540e8 100644 --- a/app/plugins/builtins/mail/services.py +++ b/app/plugins/builtins/mail/services.py @@ -1295,3 +1295,111 @@ def label_to_response(label: MailLabel) -> dict: "name": label.name, "color": label.color, } + + +# ─── IMAP Flag Sync ─── + + +async def imap_sync_mail_flags( + db: AsyncSession, + mail_id: uuid.UUID, + tenant_id: uuid.UUID, +) -> None: + """Sync is_seen/is_flagged flags from DB to IMAP server. + + Connects to the IMAP server, selects the mail's folder, + and uses UID STORE to set/remove \\Seen and \\Flagged flags. + Non-critical: logs warnings on failure but does not raise. + """ + import logging + + logger = logging.getLogger(__name__) + + # Load the mail with its folder and account + mail = ( + await db.execute( + select(Mail).where( + and_(Mail.id == mail_id, Mail.tenant_id == tenant_id) + ) + ) + ).scalar_one_or_none() + if not mail: + logger.warning("imap_sync_mail_flags: mail %s not found", mail_id) + return + + folder = ( + await db.execute( + select(MailFolder).where( + and_(MailFolder.id == mail.folder_id, MailFolder.tenant_id == tenant_id) + ) + ) + ).scalar_one_or_none() + if not folder: + logger.warning("imap_sync_mail_flags: folder %s not found for mail %s", mail.folder_id, mail_id) + return + + account = ( + await db.execute( + select(MailAccount).where( + and_(MailAccount.id == folder.account_id, MailAccount.tenant_id == tenant_id) + ) + ) + ).scalar_one_or_none() + if not account: + logger.warning("imap_sync_mail_flags: account not found for mail %s", mail_id) + return + + if not mail.message_id: + logger.warning("imap_sync_mail_flags: mail %s has no message_id, cannot sync", mail_id) + return + + password = await get_account_password(account) + client = None + + try: + client = aioimaplib.IMAP4_SSL(host=account.imap_host, port=account.imap_port) + await client.wait_hello_from_server() + await client.login(account.username, password) + + # Select the folder + select_resp = await client.select(folder.imap_name) + if select_resp.result != 'OK': + logger.warning("imap_sync_mail_flags: cannot select folder %s", folder.imap_name) + return + + # Find the UID by searching for the Message-ID header + search_resp = await client.uid_search(f'HEADER Message-ID "{mail.message_id}"') + uids_raw = search_resp[1][0] if search_resp[1] and search_resp[1][0] else b'' + if isinstance(uids_raw, (bytes, bytearray)): + uids = uids_raw.split() + else: + uids = [] + + if not uids: + logger.warning("imap_sync_mail_flags: no UID found for Message-ID %s", mail.message_id) + return + + uid_str = uids[0].decode() if isinstance(uids[0], bytes) else str(uids[0]) + + # Sync \Seen flag + if mail.is_seen: + await client.uid('store', uid_str, '+FLAGS (\\Seen)') + else: + await client.uid('store', uid_str, '-FLAGS (\\Seen)') + + # Sync \Flagged flag + if mail.is_flagged: + await client.uid('store', uid_str, '+FLAGS (\\Flagged)') + else: + await client.uid('store', uid_str, '-FLAGS (\\Flagged)') + + logger.info("imap_sync_mail_flags: synced flags for mail %s (UID %s)", mail_id, uid_str) + + except Exception as exc: + logger.warning("imap_sync_mail_flags: failed for mail %s: %s", mail_id, exc) + finally: + if client is not None: + try: + await client.logout() + except Exception: + pass diff --git a/frontend/src/api/mail.ts b/frontend/src/api/mail.ts index cdb4dd8..5d72017 100644 --- a/frontend/src/api/mail.ts +++ b/frontend/src/api/mail.ts @@ -246,8 +246,11 @@ export interface ForwardPayload { } export interface FlagUpdatePayload { - seen?: boolean; - flagged?: boolean; + is_seen?: boolean; + is_flagged?: boolean; + is_draft?: boolean; + is_answered?: boolean; + is_forwarded?: boolean; } export interface LinkMailPayload { diff --git a/frontend/src/components/mail/MailDetail.tsx b/frontend/src/components/mail/MailDetail.tsx index 6988738..85fa77e 100644 --- a/frontend/src/components/mail/MailDetail.tsx +++ b/frontend/src/components/mail/MailDetail.tsx @@ -3,7 +3,7 @@ * Shows mail headers, sanitized HTML body, attachments, reply/forward/create-event actions. */ -import React, { useMemo } from 'react'; +import React, { useMemo, useRef, useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import clsx from 'clsx'; import type { Mail, MailAttachment } from '@/api/mail'; @@ -38,14 +38,6 @@ function formatBytes(bytes: number): string { return `${(bytes / (1024 * 1024)).toFixed(1)} MB`; } -function isSanitizedHtmlSafe(html: string | null): boolean { - if (!html) return true; - const lower = html.toLowerCase(); - const dangerous = [' lower.includes(pattern)); - -} - export function MailDetail({ mail, loading, @@ -63,7 +55,21 @@ export function MailDetail({ return mail.sanitized_html || mail.body_html; }, [mail]); - const isSafe = useMemo(() => isSanitizedHtmlSafe(safeHtml), [safeHtml]); + const iframeRef = useRef(null); + + const handleIframeLoad = useCallback(() => { + const iframe = iframeRef.current; + if (!iframe) return; + try { + const doc = iframe.contentDocument || iframe.contentWindow?.document; + if (doc) { + const height = doc.documentElement.scrollHeight || doc.body.scrollHeight; + iframe.style.height = `${height}px`; + } + } catch { + // Cross-origin restrictions — leave default height + } + }, []); if (loading) { return ( @@ -172,17 +178,16 @@ export function MailDetail({ {/* Body */}
- {safeHtml && isSafe ? ( -
- ) : safeHtml && !isSafe ? ( -
-

{t('mail.htmlUnsafe')}

-
{mail.body_text}
-
) : (
{mail.body_text}
)} diff --git a/frontend/src/components/mail/MailList.tsx b/frontend/src/components/mail/MailList.tsx index 77c3b6c..4087e85 100644 --- a/frontend/src/components/mail/MailList.tsx +++ b/frontend/src/components/mail/MailList.tsx @@ -19,6 +19,9 @@ export interface MailListProps { total: number; pageSize: number; onPageChange: (page: number) => void; + selectedMailIds: Set; + onToggleSelect: (mailId: string) => void; + onSelectAll: () => void; } function formatDate(dateStr: string): string { @@ -39,9 +42,13 @@ export function MailList({ total, pageSize, onPageChange, + selectedMailIds, + onToggleSelect, + onSelectAll, }: MailListProps) { const { t } = useTranslation(); const totalPages = Math.ceil(total / pageSize); + const allSelected = mails.length > 0 && mails.every((m) => selectedMailIds.has(m.id)); if (loading) { return ( @@ -67,56 +74,84 @@ export function MailList({ return (
+ {/* Select-all header */} +
+ + {selectedMailIds.size > 0 && ( + + {t('mail.selectedCount', { count: selectedMailIds.size })} + + )} +
    {mails.map((mail) => ( -
  • - + +
))} diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index fe760a6..27372c4 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -535,6 +535,11 @@ "tabRules": "Regeln", "tabLabels": "Labels", "tabVacation": "Abwesenheit", - "tabPgp": "PGP-Verschlüsselung" + "tabPgp": "PGP-Verschlüsselung", + "markRead": "Als gelesen markieren", + "markUnread": "Als ungelesen markieren", + "selectAll": "Alle auswählen", + "bulkActions": "Sammelaktionen", + "selectedCount": "{{count}} ausgewählt" } } diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 8f51e30..8ab73ca 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -535,6 +535,11 @@ "tabRules": "Rules", "tabLabels": "Labels", "tabVacation": "Vacation", - "tabPgp": "PGP Encryption" + "tabPgp": "PGP Encryption", + "markRead": "Mark as Read", + "markUnread": "Mark as Unread", + "selectAll": "Select All", + "bulkActions": "Bulk Actions", + "selectedCount": "{{count}} selected" } } diff --git a/frontend/src/pages/Mail.tsx b/frontend/src/pages/Mail.tsx index c84874f..193958f 100644 --- a/frontend/src/pages/Mail.tsx +++ b/frontend/src/pages/Mail.tsx @@ -70,6 +70,7 @@ export function MailPage() { const [downloadingAttachmentId, setDownloadingAttachmentId] = useState(null); const [searchQuery, setSearchQuery] = useState(''); const [activeView, setActiveView] = useState<'folders' | 'list' | 'detail'>('folders'); + const [selectedMailIds, setSelectedMailIds] = useState>(new Set()); // Load accounts const loadAccounts = useCallback(async () => { @@ -169,6 +170,7 @@ export function MailPage() { setMailsPage(1); setSearchQuery(''); setSelectedMail(null); + setSelectedMailIds(new Set()); setActiveView('list'); }, [folders], @@ -184,7 +186,7 @@ export function MailPage() { setSelectedMail(detail); // Mark as seen if not seen if (!detail.is_seen) { - await updateFlags(mail.id, { seen: true }); + await updateFlags(mail.id, { is_seen: true }); setMails((prev) => prev.map((m) => (m.id === mail.id ? { ...m, is_seen: true } : m))); } } catch (err) { @@ -225,7 +227,7 @@ export function MailPage() { const handleToggleFlag = useCallback( async (mail: Mail) => { try { - await updateFlags(mail.id, { flagged: !mail.is_flagged }); + await updateFlags(mail.id, { is_flagged: !mail.is_flagged }); setSelectedMail((prev) => (prev ? { ...prev, is_flagged: !mail.is_flagged } : prev)); setMails((prev) => prev.map((m) => (m.id === mail.id ? { ...m, is_flagged: !mail.is_flagged } : m))); } catch (err) { @@ -235,6 +237,66 @@ export function MailPage() { [toast], ); + // Handle toggle select for bulk operations + const handleToggleSelect = useCallback((mailId: string) => { + setSelectedMailIds((prev) => { + const next = new Set(prev); + if (next.has(mailId)) { + next.delete(mailId); + } else { + next.add(mailId); + } + return next; + }); + }, []); + + // Handle select all mails on current page + const handleSelectAll = useCallback(() => { + setSelectedMailIds((prev) => { + const allSelected = mails.length > 0 && mails.every((m) => prev.has(m.id)); + if (allSelected) { + // Deselect all on current page + const next = new Set(prev); + for (const m of mails) { + next.delete(m.id); + } + return next; + } + // Select all on current page + const next = new Set(prev); + for (const m of mails) { + next.add(m.id); + } + return next; + }); + }, [mails]); + + // Handle bulk mark as read + const handleBulkMarkRead = useCallback(async () => { + const ids = Array.from(selectedMailIds); + try { + await Promise.all(ids.map((id) => updateFlags(id, { is_seen: true }))); + setMails((prev) => prev.map((m) => (selectedMailIds.has(m.id) ? { ...m, is_seen: true } : m))); + setSelectedMailIds(new Set()); + toast.success(t('mail.markRead')); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + }, [selectedMailIds, toast, t]); + + // Handle bulk mark as unread + const handleBulkMarkUnread = useCallback(async () => { + const ids = Array.from(selectedMailIds); + try { + await Promise.all(ids.map((id) => updateFlags(id, { is_seen: false }))); + setMails((prev) => prev.map((m) => (selectedMailIds.has(m.id) ? { ...m, is_seen: false } : m))); + setSelectedMailIds(new Set()); + toast.success(t('mail.markUnread')); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + }, [selectedMailIds, toast, t]); + // Handle create event from mail const handleCreateEvent = useCallback( async (mail: Mail) => { @@ -314,12 +376,13 @@ export function MailPage() { const selectedMailFlagged = selectedMail?.is_flagged; useEffect(() => { const hasMail = !!selectedMailId; - registerItems('mail', [ + const hasBulkSelection = selectedMailIds.size > 0; + const items = [ { id: 'search', plugin: 'mail', label: 'Suchen', - type: 'search', + type: 'search' as const, group: 'search', searchPlaceholder: 'E-Mails durchsuchen...', onSearch: handleSearch, @@ -402,9 +465,38 @@ export function MailPage() { ), onClick: () => { /* sync trigger */ }, }, - ]); + ]; + if (hasBulkSelection) { + items.push( + { + id: 'bulk-mark-read', + plugin: 'mail', + label: t('mail.markRead'), + group: 'bulk-actions', + icon: ( + + + + ), + onClick: handleBulkMarkRead, + }, + { + id: 'bulk-mark-unread', + plugin: 'mail', + label: t('mail.markUnread'), + group: 'bulk-actions', + icon: ( + + + + ), + onClick: handleBulkMarkUnread, + }, + ); + } + registerItems('mail', items); return () => unregisterPlugin('mail'); - }, [selectedMailId, selectedMailFlagged, handleSearch, handleCompose, handleReply, handleForward, handleToggleFlag, handleCreateEvent]); + }, [selectedMailId, selectedMailFlagged, selectedMailIds, handleSearch, handleCompose, handleReply, handleForward, handleToggleFlag, handleCreateEvent, handleBulkMarkRead, handleBulkMarkUnread, t]); if (loadingAccounts) { return ( @@ -476,6 +568,9 @@ export function MailPage() { total={mailsTotal} pageSize={PAGE_SIZE} onPageChange={setMailsPage} + selectedMailIds={selectedMailIds} + onToggleSelect={handleToggleSelect} + onSelectAll={handleSelectAll} /> @@ -540,6 +635,9 @@ export function MailPage() { total={mailsTotal} pageSize={PAGE_SIZE} onPageChange={setMailsPage} + selectedMailIds={selectedMailIds} + onToggleSelect={handleToggleSelect} + onSelectAll={handleSelectAll} />
)} @@ -574,6 +672,28 @@ export function MailPage() { )}
+ {/* Floating bulk action bar */} + {selectedMailIds.size > 0 && ( +
+ + {t('mail.selectedCount', { count: selectedMailIds.size })} + +
+ + + +
+ )} + &1 | grep -E 'mail|Mail|Compose|MailDetail' -→ No errors in mail files -``` -Pre-existing errors in SettingsPlugins.tsx and SettingsRoles.tsx (unrelated to this task). - -### Frontend Tests (vitest) - -| Test File | Result | -|-----------|--------| -| ComposeModal.test.tsx | ✅ 10/10 passed | -| MailSettings.test.tsx | ✅ 10/10 passed | -| MailPage.test.tsx | ⚠️ 6 passed, 8 failed (PRE-EXISTING — confirmed via git stash) | - -### Backend Tests (pytest) -``` -pytest tests/test_mail.py -→ ImportError: No module named 'redis' (pre-existing environment issue) -``` - -## Smoke Test Description - -1. **IMAP Sync**: Attachment content is now captured during IMAP sync (`part.get_payload(decode=True)`), saved to `{storage_path}/mail_attachments/{mail_id}/{filename}`, and `MailAttachment` records are created in the DB. -2. **Download**: Existing `GET /{mail_id}/attachments/{att_id}` route streams the file from disk with correct Content-Type and Content-Disposition headers. -3. **Upload**: New `POST /upload-attachment` endpoint accepts multipart/form-data, validates 25MB max size, saves to temp directory, returns attachment ID. -4. **Send with attachments**: `send_mail` route resolves attachment IDs to file paths, passes them to `send_mail_via_smtp` which adds them to the `EmailMessage` via `msg.add_attachment()`. -5. **Frontend display**: MailDetail shows attachments with icon, filename, size (using `size_bytes`), and download button. -6. **Frontend compose**: ComposeModal has file input button, uploads files via `uploadAttachment`, shows list with remove option, passes attachment IDs in send payload. - -## Security -- Filenames sanitized via `_sanitize_filename` (removes path components, replaces dangerous characters) -- Max 25MB per attachment enforced in both upload endpoint and frontend -- No path traversal possible (basename extraction + character replacement)