From 2e087731b92fe554e54955c97166d61fe644bafc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 01:19:32 +0000 Subject: [PATCH] fix(imap): address code review feedback on ingestion profiles - Improve warning log in _resolve_categories_for_profile() to include exception type name for better troubleshooting - Add SQLAlchemy IS NULL comment to imap_profiles.py filter - Pass default_categories from server to template to avoid hardcoded category list in JS (now uses {{ default_categories | tojson }}) - Simplify view profiles query (remove redundant unauthenticated path) - Update docs: ConfigurationGuide.md and EmailIngestion.md with full profiles documentation including category table and API reference Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/imap_profiles.py | 1 + app/tasks/imap_tasks.py | 7 ++++- app/views/imap_accounts.py | 14 ++++----- docs/ConfigurationGuide.md | 37 +++++++++++++++++++++++- docs/howto/EmailIngestion.md | 41 +++++++++++++++++++-------- frontend/templates/imap_accounts.html | 3 +- tests/test_imap_profiles.py | 1 - 7 files changed, 79 insertions(+), 25 deletions(-) diff --git a/app/api/imap_profiles.py b/app/api/imap_profiles.py index 8f267f63..90502686 100644 --- a/app/api/imap_profiles.py +++ b/app/api/imap_profiles.py @@ -154,6 +154,7 @@ def list_profiles(request: Request, db: DbSession, owner_id: CurrentOwner) -> li profiles = ( db.query(ImapIngestionProfile) .filter( + # SQLAlchemy requires `== None` for IS NULL comparison in ORM filters (ImapIngestionProfile.owner_id == None) | (ImapIngestionProfile.owner_id == owner_id) # noqa: E711 ) .order_by(ImapIngestionProfile.is_builtin.desc(), ImapIngestionProfile.id) diff --git a/app/tasks/imap_tasks.py b/app/tasks/imap_tasks.py index affd7a9d..d47ab6c2 100644 --- a/app/tasks/imap_tasks.py +++ b/app/tasks/imap_tasks.py @@ -73,7 +73,12 @@ def _resolve_categories_for_profile(profile_id: int | None) -> list[str]: finally: db.close() except Exception as exc: # noqa: BLE001 - logger.warning("Could not load IMAP ingestion profile %d: %s — using global default", profile_id, exc) + logger.warning( + "Could not load IMAP ingestion profile %d (%s: %s) — using global default", + profile_id, + type(exc).__name__, + exc, + ) # Fall back to global setting if settings.imap_attachment_filter == "all": diff --git a/app/views/imap_accounts.py b/app/views/imap_accounts.py index a8a2dcbd..c952a8cd 100644 --- a/app/views/imap_accounts.py +++ b/app/views/imap_accounts.py @@ -7,7 +7,7 @@ from fastapi import Request from sqlalchemy.orm import Session from app.models import ImapIngestionProfile, UserImapAccount -from app.utils.allowed_types import FILE_TYPE_CATEGORIES +from app.utils.allowed_types import DEFAULT_CATEGORIES, FILE_TYPE_CATEGORIES from app.utils.subscription import get_tier, get_user_tier_id from app.utils.user_scope import get_current_owner_id from app.views.base import APIRouter, Depends, get_db, require_login, templates @@ -68,19 +68,14 @@ async def imap_accounts_page(request: Request, db: Session = Depends(get_db)): can_add = max_mailboxes is None or (max_mailboxes > 0 and current_count < max_mailboxes) # Load ingestion profiles: system-global + user's own - profiles: list[ImapIngestionProfile] = ( + profiles = ( db.query(ImapIngestionProfile) .filter( - (ImapIngestionProfile.owner_id == None) # noqa: E711 - | (ImapIngestionProfile.owner_id == owner_id) + # SQLAlchemy requires `== None` for IS NULL comparison in ORM filters + (ImapIngestionProfile.owner_id == None) | (ImapIngestionProfile.owner_id == owner_id) # noqa: E711 ) .order_by(ImapIngestionProfile.is_builtin.desc(), ImapIngestionProfile.id) .all() - if owner_id - else db.query(ImapIngestionProfile) - .filter(ImapIngestionProfile.owner_id == None) # noqa: E711 - .order_by(ImapIngestionProfile.id) - .all() ) # Category definitions for the UI checkbox builder @@ -100,6 +95,7 @@ async def imap_accounts_page(request: Request, db: Session = Depends(get_db)): "accounts": accounts, "profiles": [_serialize_profile(p) for p in profiles], "categories": categories, + "default_categories": DEFAULT_CATEGORIES, "current_count": current_count, "max_mailboxes": max_mailboxes, "can_add": can_add, diff --git a/docs/ConfigurationGuide.md b/docs/ConfigurationGuide.md index 747db3ee..05445581 100644 --- a/docs/ConfigurationGuide.md +++ b/docs/ConfigurationGuide.md @@ -304,7 +304,42 @@ DocuElevate can automatically pull document attachments from IMAP mailboxes — | `IMAP1_SSL` | Use SSL (`true`/`false`). | `true` | | `IMAP1_POLL_INTERVAL_MINUTES` | Frequency in minutes to poll for new mail. | `5` | | `IMAP_READONLY_MODE` | When `true`, fetches and processes attachments but does **not** modify the mailbox (no starring, labeling, deleting, or flag changes). Use for pre-production instances sharing a mailbox with production. Default: `false`. | `false` | -| `IMAP_ATTACHMENT_FILTER` | Controls which attachment types are ingested from emails. `documents_only` (default) ingests PDFs and office files only — images are skipped. `all` ingests every supported file type including images. Individual per-user IMAP accounts can override this global default. | `documents_only` | +| `IMAP_ATTACHMENT_FILTER` | System-wide fallback for which attachment types are ingested when no ingestion profile is assigned to a mailbox. `documents_only` (default) ingests PDFs and office files only — images are skipped. `all` ingests every supported file type including images. Individual IMAP accounts can override this using ingestion profiles. | `documents_only` | + +#### IMAP Ingestion Profiles + +For fine-grained control, DocuElevate supports **Ingestion Profiles** — named configurations that let you choose exactly which file-type categories to accept from each mailbox. + +Each profile contains a list of enabled **categories**: + +| Category | Description | +|----------|-------------| +| `pdf` | PDF documents (`.pdf`) | +| `office` | Microsoft Office files (Word, Excel, PowerPoint — `.docx`, `.xlsx`, `.pptx`, …) | +| `opendocument` | LibreOffice/OpenOffice files (`.odt`, `.ods`, `.odp`, …) | +| `text` | Plain text, CSV and RTF files (`.txt`, `.csv`, `.rtf`) | +| `web` | HTML and Markdown files (`.html`, `.htm`, `.md`, `.markdown`) | +| `images` | Image files (`.jpg`, `.png`, `.gif`, `.bmp`, `.tiff`, `.webp`, `.svg`) | + +Two built-in system profiles are seeded automatically: + +| Profile | Categories | +|---------|------------| +| **Documents Only** | pdf, office, opendocument, text, web (no images) | +| **All Files** | All categories, including images | + +Users can create their own custom profiles via the **Email Ingestion** dashboard (`/imap-accounts`) by clicking the **Manage profiles** link or the **+** button next to the profile dropdown. Custom profiles are private to the creating user and can be freely edited or deleted. + +**API endpoints for ingestion profiles:** + +| Method | Endpoint | Description | +|--------|----------|-------------| +| `GET` | `/api/imap-profiles/` | List all visible profiles (system + user's own) | +| `POST` | `/api/imap-profiles/` | Create a new profile | +| `GET` | `/api/imap-profiles/categories` | List available file-type categories | +| `GET` | `/api/imap-profiles/{id}` | Get a single profile | +| `PUT` | `/api/imap-profiles/{id}` | Update a profile (not built-in) | +| `DELETE` | `/api/imap-profiles/{id}` | Delete a profile (not built-in) | #### Per-User IMAP Integrations diff --git a/docs/howto/EmailIngestion.md b/docs/howto/EmailIngestion.md index 12e76eae..bfb6b735 100644 --- a/docs/howto/EmailIngestion.md +++ b/docs/howto/EmailIngestion.md @@ -69,30 +69,47 @@ By default, DocuElevate only ingests **document** attachments (PDFs, Word, Excel #### Global Default (Admin Setting) -Set the `IMAP_ATTACHMENT_FILTER` environment variable to control the system-wide default: +Set the `IMAP_ATTACHMENT_FILTER` environment variable to control the system-wide fallback when no ingestion profile is assigned to a mailbox: | Value | Behaviour | |-------|-----------| -| `documents_only` | **(Default)** Only PDFs and office/document files. Images (JPEG, PNG, GIF, BMP, TIFF, WebP, SVG) are skipped. | +| `documents_only` | **(Default)** Only PDFs and office/document files. Images are skipped. | | `all` | All supported file types, including images. | ```env -# Only ingest document-type attachments (default behaviour) IMAP_ATTACHMENT_FILTER=documents_only - -# Ingest all supported file types, including images -IMAP_ATTACHMENT_FILTER=all ``` -#### Per-User Override +#### Ingestion Profiles (Fine-Grained Per-Mailbox Control) -Each user can override the global default for their personal IMAP accounts via the **Email Ingestion** dashboard (`/imap-accounts`). When creating or editing an account, select the desired setting from the **Attachment Types to Ingest** dropdown: +For precise control, you can create **Ingestion Profiles** that let you pick exactly which file-type categories to accept from each mailbox. This is more powerful than the binary global toggle and works independently per mailbox. -- **Use global default** — inherits the `IMAP_ATTACHMENT_FILTER` setting above. -- **Documents only** — PDFs and office files, no images. -- **All supported types (including images)** — overrides the global setting to allow images for this specific account. +**Available categories:** -This allows administrators to restrict image ingestion system-wide while individual users can opt-in to image ingestion on a per-mailbox basis. +| Category | File types included | +|----------|---------------------| +| PDF | `.pdf` | +| Microsoft Office | `.doc`, `.docx`, `.xls`, `.xlsx`, `.ppt`, `.pptx`, and macro-enabled variants | +| OpenDocument | `.odt`, `.ods`, `.odp`, `.odg`, `.odf` (LibreOffice / OpenOffice) | +| Text & Data | `.txt`, `.csv`, `.rtf` | +| Web & Markup | `.html`, `.htm`, `.md`, `.markdown` | +| Images | `.jpg`, `.png`, `.gif`, `.bmp`, `.tiff`, `.webp`, `.svg` | + +**Managing profiles:** + +1. Go to **Email Ingestion** (`/imap-accounts`) +2. Click **Manage profiles** (or the **+** icon next to the profile dropdown) +3. Create a new profile, give it a name, and tick the categories you want +4. When adding or editing a mailbox, select your profile from the dropdown + +Two built-in profiles are always available and cannot be deleted: + +- **Documents Only** — PDF, Office, OpenDocument, Text, Web (no images) +- **All Files** — all categories including images + +Users can also create unlimited **custom profiles** to mix and match exactly the categories they need per mailbox (e.g. a scanner mailbox that only accepts PDFs, or a finance mailbox that accepts Office and CSV but not images). + +Custom profiles are created via the UI or the `/api/imap-profiles/` API. --- diff --git a/frontend/templates/imap_accounts.html b/frontend/templates/imap_accounts.html index ecbe7eaa..226bb190 100644 --- a/frontend/templates/imap_accounts.html +++ b/frontend/templates/imap_accounts.html @@ -812,6 +812,7 @@ function imapAccountsApp() { accounts: {{ accounts | tojson }}, profiles: {{ profiles | tojson }}, categories: {{ categories | tojson }}, + defaultCategories: {{ default_categories | tojson }}, quota: { current_count: {{ current_count }}, max_mailboxes: {{ max_mailboxes | tojson }}, @@ -1083,7 +1084,7 @@ function imapAccountsApp() { allowed_categories: [...profile.allowed_categories], }; } else { - this.profileForm = { name: '', description: '', allowed_categories: ['pdf', 'office', 'opendocument', 'text', 'web'] }; + this.profileForm = { name: '', description: '', allowed_categories: [...this.defaultCategories] }; } this.profileFormError = null; this.profileModalOpen = true; diff --git a/tests/test_imap_profiles.py b/tests/test_imap_profiles.py index 45fd4645..bfc2fb2c 100644 --- a/tests/test_imap_profiles.py +++ b/tests/test_imap_profiles.py @@ -1,6 +1,5 @@ """Tests for app/api/imap_profiles.py and app/utils/allowed_types category helpers.""" -import json import pytest