feat: add mail import detail events

2026-05-22 20:06:51 +02:00
parent 5f416d042f
commit c1b81d3e06
13 changed files with 520 additions and 67 deletions
@@ -439,6 +439,8 @@ class TestProcessMessage:
        assert count == 0
        assert len(stats["errors"]) == 1
        assert "bad-id" in stats["errors"][0]
+        assert stats["details"][0]["status"] == "error"
+        assert stats["details"][0]["message_id"] == "bad-id"


 # ===========================================================================
@@ -463,9 +465,36 @@ class TestProcessAttachments:
        raw = _make_raw_email([{"filename": "photo.png", "content": b"\x89PNG"}])
        msg = email_mod.message_from_bytes(raw)
        stats = {"reports_found": 0, "errors": []}
-        count = client._process_attachments(msg, stats)
+        count = client._process_attachments(msg, stats, message_id="msg-1")
        assert count == 0
        assert stats["reports_found"] == 0
+        assert stats["details"] == [
+            {
+                "status": "skipped",
+                "reason": "unsupported_attachment",
+                "message_id": "msg-1",
+                "filename": "photo.png",
+            }
+        ]
+
+    def test_inline_dmarc_attachment_is_skipped(self):
+        client = _make_client()
+        raw = _make_raw_email(
+            [
+                {
+                    "filename": "report.xml",
+                    "content": SAMPLE_XML.encode(),
+                    "disposition": "inline",
+                }
+            ]
+        )
+        msg = email_mod.message_from_bytes(raw)
+        stats = {"reports_found": 0, "errors": []}
+
+        count = client._process_attachments(msg, stats)
+
+        assert count == 0
+        assert stats.get("details") is None

    def test_google_style_zip_attachment_is_parsed(self):
        """A Google DMARC ZIP attachment is parsed and counted."""
@@ -485,6 +514,9 @@ class TestProcessAttachments:

        assert count == 1
        assert stats["reports_found"] == 1
+        assert stats["details"][0]["status"] == "imported"
+        assert stats["details"][0]["filename"].endswith(".zip")
+        assert stats["details"][0]["report_id"] == "123456789"
        assert "example.com" in client.report_store.get_domains()

    def test_google_style_zip_attachment_is_persisted(self, db_session):
@@ -524,6 +556,8 @@ class TestProcessAttachments:

        assert client._process_attachments(msg, first_stats) == 1
        assert client._process_attachments(msg, second_stats) == 0
+        assert second_stats["details"][0]["status"] == "duplicate"
+        assert second_stats["details"][0]["report_id"] == "123456789"
        assert client.report_store.get_domain_summary("example.com")["reports_processed"] == 1

    def test_dmarc_attachment_with_empty_content_skipped(self):
@@ -537,6 +571,8 @@ class TestProcessAttachments:
        # Empty payload → `get_payload(decode=True)` returns b"" which is
        # falsy, so the attachment is skipped
        assert count == 0
+        assert stats["details"][0]["status"] == "skipped"
+        assert stats["details"][0]["reason"] == "empty_attachment"

    def test_parse_exception_adds_error_and_continues(self):
        """A parse error should be recorded in stats but not raise."""
@@ -567,6 +603,9 @@ class TestProcessAttachments:

        assert len(stats["errors"]) == 1
        assert "bad.xml" in stats["errors"][0]
+        assert stats["details"][0]["status"] == "error"
+        assert stats["details"][0]["filename"] == "bad.xml"
+        assert stats["details"][1]["status"] == "imported"
        assert count == 1  # second attachment still parsed


@@ -622,6 +661,9 @@ class TestFetchReports:
        call_args = mock_proc.call_args_list[0][0]
        assert call_args[1] == "id2"
        assert result["processed"] == 1
+        assert result["details"][0]["status"] == "skipped"
+        assert result["details"][0]["reason"] == "already_ingested_message"
+        assert result["details"][0]["message_id"] == "id1"

    def test_tracks_new_ingested_ids(self):
        client = _make_client()
@@ -427,8 +427,12 @@ class TestProcessAttachments:
        msg = email.message_from_bytes(
            _make_email_with_attachment("report.xml", MINIMAL_DMARC_XML, "application/xml")
        )
-        count = client._process_attachments(msg)
+        stats = {"processed": 0, "reports_found": 0, "errors": []}
+        count = client._process_attachments(msg, stats, message_id="1")
        assert count == 1
+        assert stats["details"][0]["status"] == "imported"
+        assert stats["details"][0]["message_id"] == "1"
+        assert stats["details"][0]["report_id"] == "abc-123"

    def test_processes_zip_attachment(self):
        client = self._make_client()
@@ -450,12 +454,73 @@ class TestProcessAttachments:
        assert count == 1
        assert db_session.query(DMARCReport).filter_by(report_id="abc-123").count() == 1

+    def test_duplicate_report_adds_detail(self):
+        client = self._make_client()
+        msg = email.message_from_bytes(
+            _make_email_with_attachment("report.xml", MINIMAL_DMARC_XML, "application/xml")
+        )
+        first_stats = {"processed": 0, "reports_found": 0, "errors": []}
+        second_stats = {"processed": 0, "reports_found": 0, "errors": []}
+
+        assert client._process_attachments(msg, first_stats) == 1
+        assert client._process_attachments(msg, second_stats) == 0
+        assert second_stats["details"][0]["status"] == "duplicate"
+        assert second_stats["details"][0]["report_id"] == "abc-123"
+
    def test_bad_attachment_does_not_raise(self):
        client = self._make_client()
        msg = email.message_from_bytes(_make_email_with_attachment("report.xml", b"not xml at all"))
        # Should not raise; just returns 0
-        count = client._process_attachments(msg)
+        stats = {"processed": 0, "reports_found": 0, "errors": []}
+        count = client._process_attachments(msg, stats)
        assert count == 0
+        assert stats["errors"]
+        assert stats["details"][0]["status"] == "error"
+        assert stats["details"][0]["filename"] == "report.xml"
+
+    def test_empty_attachment_adds_detail(self):
+        client = self._make_client()
+        msg = email.message_from_bytes(_make_email_with_attachment("report.xml", b""))
+        stats = {"processed": 0, "reports_found": 0, "errors": []}
+
+        count = client._process_attachments(msg, stats)
+
+        assert count == 0
+        assert stats["details"][0]["status"] == "skipped"
+        assert stats["details"][0]["reason"] == "empty_attachment"
+
+    def test_unsupported_attachment_adds_detail(self):
+        client = self._make_client()
+        msg = email.message_from_bytes(
+            _make_email_with_attachment("notes.txt", b"not xml", "text/plain")
+        )
+        stats = {"processed": 0, "reports_found": 0, "errors": []}
+
+        count = client._process_attachments(msg, stats, message_id="2")
+
+        assert count == 0
+        assert stats["details"] == [
+            {
+                "status": "skipped",
+                "reason": "unsupported_attachment",
+                "message_id": "2",
+                "filename": "notes.txt",
+            }
+        ]
+
+    def test_attachment_without_filename_is_skipped(self):
+        client = self._make_client()
+        msg = MIMEMultipart()
+        msg.attach(MIMEText("body"))
+        part = MIMEApplication(MINIMAL_DMARC_XML)
+        part["Content-Disposition"] = "attachment"
+        msg.attach(part)
+        stats = {"processed": 0, "reports_found": 0, "errors": []}
+
+        count = client._process_attachments(msg, stats)
+
+        assert count == 0
+        assert stats.get("details") is None

    def test_no_attachments_returns_zero(self):
        client = self._make_client()
@@ -497,6 +562,7 @@ class TestProcessSingleEmail:

        assert stats["processed"] == 1
        assert stats["reports_found"] == 1
+        assert stats["details"][0]["status"] == "imported"

    def test_fetch_error_skips_email(self):
        client = self._make_client()
@@ -507,6 +573,8 @@ class TestProcessSingleEmail:
        client._process_single_email(mock_mail, b"1", stats)

        assert stats["processed"] == 0
+        assert stats["details"][0]["status"] == "error"
+        assert stats["details"][0]["reason"] == "message_fetch_failed"

    def test_exception_adds_to_errors(self):
        client = self._make_client()
@@ -517,6 +585,7 @@ class TestProcessSingleEmail:
        client._process_single_email(mock_mail, b"1", stats)

        assert len(stats["errors"]) == 1
+        assert stats["details"][0]["reason"] == "message_processing_failed"

    def test_marks_deleted_when_flag_set(self):
        client = self._make_client()
@@ -599,6 +668,7 @@ class TestFetchReports:

        assert result["success"] is True
        assert result["reports_found"] >= 1
+        assert result["details"][0]["status"] == "imported"

    def test_connection_error_returns_failure(self):
        client = self._make_client()
@@ -3,6 +3,8 @@ Tests for MailSource model and mail-sources API endpoints.
 """

 import asyncio
+import json
+from datetime import datetime
 from unittest.mock import MagicMock, patch
 from urllib.parse import parse_qs, urlparse

@@ -12,6 +14,7 @@ from sqlalchemy.orm import Session

 from app.models.mail_source import MailSource
 from app.models.mail_source_import import MailSourceImport
+from app.services.import_history import record_import_attempt


 class TestMailSourceModel:
@@ -95,6 +98,7 @@ class TestMailSourceImportModel:
            error_count=1,
            new_domains='["example.com"]',
            errors='["bad attachment"]',
+            details='[{"status": "imported", "filename": "report.xml"}]',
        )
        db_session.add(row)
        db_session.commit()
@@ -103,8 +107,51 @@ class TestMailSourceImportModel:
        assert row.id is not None
        assert row.mail_source_id == source.id
        assert row.duplicate_reports == 1
+        assert '"imported"' in row.details
        assert row.mail_source.name == "History Source"

+    def test_record_import_attempt_sanitizes_details(self, db_session: Session):
+        source = MailSource(name="Detail Sanitizer", method="IMAP")
+        db_session.add(source)
+        db_session.commit()
+        db_session.refresh(source)
+
+        attempt = record_import_attempt(
+            db_session,
+            source,
+            {
+                "success": True,
+                "errors": ["x" * 600],
+                "details": [
+                    "skip-me",
+                    {"status": "imported", "filename": "a" * 400, "ignored": "secret"},
+                ],
+            },
+            started_at=datetime.utcnow(),
+            trigger="manual",
+        )
+
+        details = json.loads(attempt.details)
+        errors = json.loads(attempt.errors)
+        assert len(errors[0]) == 500
+        assert details[0]["status"] == "imported"
+        assert len(details[0]["filename"]) == 300
+        assert "ignored" not in details[0]
+
+
+class TestImportHistoryDecoding:
+    """Unit tests for import-history JSON decoding helpers."""
+
+    def test_decode_details_handles_empty_and_malformed_values(self):
+        from app.api.api_v1.endpoints.mail_sources import _decode_json_details
+
+        assert _decode_json_details(None) == []
+        assert _decode_json_details("not-json") == []
+        assert _decode_json_details('{"not": "a list"}') == []
+        assert _decode_json_details('["skip", {"status": "imported", "report_id": 123}]') == [
+            {"status": "imported", "report_id": "123"}
+        ]
+

 class TestMailSourcesAPI:
    """Integration tests for /api/v1/mail-sources endpoints (no auth)."""
@@ -267,6 +314,7 @@ class TestMailSourcesAPIAuthed:
                error_count=1,
                new_domains='["example.com"]',
                errors='["sanitized error"]',
+                details='[{"status": "duplicate", "report_id": "abc-123"}]',
            )
        )
        db_session.commit()
@@ -281,6 +329,7 @@ class TestMailSourcesAPIAuthed:
        assert data[0]["duplicate_reports"] == 1
        assert data[0]["new_domains"] == ["example.com"]
        assert data[0]["errors"] == ["sanitized error"]
+        assert data[0]["details"] == [{"status": "duplicate", "report_id": "abc-123"}]

    def test_list_import_history_handles_malformed_json(
        self, authed_client: TestClient, db_session: Session
@@ -297,6 +346,7 @@ class TestMailSourcesAPIAuthed:
                status="warning",
                new_domains="not-json",
                errors='{"not": "a list"}',
+                details='{"not": "a list"}',
            )
        )
        db_session.commit()
@@ -306,6 +356,7 @@ class TestMailSourcesAPIAuthed:
        assert resp.status_code == 200
        assert resp.json()[0]["new_domains"] == []
        assert resp.json()[0]["errors"] == []
+        assert resp.json()[0]["details"] == []

    def test_list_import_history_unknown_source_returns_404(self, authed_client: TestClient):
        resp = authed_client.get("/api/v1/mail-sources/99999/imports")
@@ -764,6 +815,7 @@ class TestManualSourceFetchEndpoint:
            "duplicate_reports": 1,
            "new_domains": ["example.com"],
            "errors": ["bad attachment\nwith newline"],
+            "details": [{"status": "error", "filename": "bad.xml"}],
        }

        with (
@@ -780,6 +832,10 @@ class TestManualSourceFetchEndpoint:
        assert data["duplicate_reports"] == 1
        assert data["error_count"] == 1
        assert "bad attachment with newline" in caplog.text
+        history_resp = authed_client.get(f"/api/v1/mail-sources/{source_id}/imports")
+        assert history_resp.json()[0]["details"] == [
+            {"status": "error", "filename": "bad.xml"}
+        ]
        mock_imap.fetch_reports.assert_called_once_with(days=30)

    def test_fetch_gmail_source(self, authed_client: TestClient, db_session: Session):
@@ -807,6 +863,7 @@ class TestManualSourceFetchEndpoint:
            "new_domains": [],
            "errors": [],
            "new_ingested_ids": ["id1"],
+            "details": [{"status": "imported", "report_id": "abc-123"}],
        }
        mock_gmail.get_refreshed_tokens.return_value = None