Merge pull request #102 from christianlouis/codex/import-detail-events
feat: add mail import detail events
This commit is contained in:
@@ -0,0 +1,27 @@
|
|||||||
|
"""add mail source import details
|
||||||
|
|
||||||
|
Revision ID: f6a7b8c9d0e1
|
||||||
|
Revises: e5f6a7b8c9d0
|
||||||
|
Create Date: 2026-05-22 20:05:00.000000
|
||||||
|
|
||||||
|
"""
|
||||||
|
from typing import Sequence, Union
|
||||||
|
|
||||||
|
import sqlalchemy as sa
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
# revision identifiers, used by Alembic.
|
||||||
|
revision: str = "f6a7b8c9d0e1"
|
||||||
|
down_revision: Union[str, Sequence[str], None] = "e5f6a7b8c9d0"
|
||||||
|
branch_labels: Union[str, Sequence[str], None] = None
|
||||||
|
depends_on: Union[str, Sequence[str], None] = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
"""Add a sanitized details payload to import history rows."""
|
||||||
|
op.add_column("mail_source_imports", sa.Column("details", sa.Text(), nullable=True))
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
"""Remove import details payloads."""
|
||||||
|
op.drop_column("mail_source_imports", "details")
|
||||||
@@ -121,6 +121,7 @@ class MailSourceImportResponse(BaseModel):
|
|||||||
error_count: int
|
error_count: int
|
||||||
new_domains: List[str]
|
new_domains: List[str]
|
||||||
errors: List[str]
|
errors: List[str]
|
||||||
|
details: List[Dict[str, str]]
|
||||||
started_at: datetime
|
started_at: datetime
|
||||||
finished_at: datetime
|
finished_at: datetime
|
||||||
created_at: datetime
|
created_at: datetime
|
||||||
@@ -183,6 +184,24 @@ def _decode_json_list(value: Optional[str]) -> List[str]:
|
|||||||
return [str(item) for item in decoded]
|
return [str(item) for item in decoded]
|
||||||
|
|
||||||
|
|
||||||
|
def _decode_json_details(value: Optional[str]) -> List[Dict[str, str]]:
|
||||||
|
"""Decode a sanitized JSON detail list stored on import history rows."""
|
||||||
|
if not value:
|
||||||
|
return []
|
||||||
|
try:
|
||||||
|
decoded = json.loads(value)
|
||||||
|
except (json.JSONDecodeError, TypeError):
|
||||||
|
return []
|
||||||
|
if not isinstance(decoded, list):
|
||||||
|
return []
|
||||||
|
|
||||||
|
details: List[Dict[str, str]] = []
|
||||||
|
for item in decoded:
|
||||||
|
if isinstance(item, dict):
|
||||||
|
details.append({str(key): str(val) for key, val in item.items()})
|
||||||
|
return details
|
||||||
|
|
||||||
|
|
||||||
def _import_to_response(row: MailSourceImport) -> MailSourceImportResponse:
|
def _import_to_response(row: MailSourceImport) -> MailSourceImportResponse:
|
||||||
"""Convert an import-history ORM row to an API response."""
|
"""Convert an import-history ORM row to an API response."""
|
||||||
return MailSourceImportResponse(
|
return MailSourceImportResponse(
|
||||||
@@ -196,6 +215,7 @@ def _import_to_response(row: MailSourceImport) -> MailSourceImportResponse:
|
|||||||
error_count=row.error_count,
|
error_count=row.error_count,
|
||||||
new_domains=_decode_json_list(row.new_domains),
|
new_domains=_decode_json_list(row.new_domains),
|
||||||
errors=_decode_json_list(row.errors),
|
errors=_decode_json_list(row.errors),
|
||||||
|
details=_decode_json_details(row.details),
|
||||||
started_at=row.started_at,
|
started_at=row.started_at,
|
||||||
finished_at=row.finished_at,
|
finished_at=row.finished_at,
|
||||||
created_at=row.created_at,
|
created_at=row.created_at,
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ class MailSourceImport(Base):
|
|||||||
|
|
||||||
new_domains = Column(Text, nullable=True)
|
new_domains = Column(Text, nullable=True)
|
||||||
errors = Column(Text, nullable=True)
|
errors = Column(Text, nullable=True)
|
||||||
|
details = Column(Text, nullable=True)
|
||||||
|
|
||||||
started_at = Column(DateTime, nullable=False, default=datetime.utcnow, index=True)
|
started_at = Column(DateTime, nullable=False, default=datetime.utcnow, index=True)
|
||||||
finished_at = Column(DateTime, nullable=False, default=datetime.utcnow, index=True)
|
finished_at = Column(DateTime, nullable=False, default=datetime.utcnow, index=True)
|
||||||
|
|||||||
@@ -214,6 +214,7 @@ class GmailClient:
|
|||||||
"new_domains": [],
|
"new_domains": [],
|
||||||
"errors": [],
|
"errors": [],
|
||||||
"new_ingested_ids": [],
|
"new_ingested_ids": [],
|
||||||
|
"details": [],
|
||||||
}
|
}
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -232,6 +233,12 @@ class GmailClient:
|
|||||||
|
|
||||||
for msg_id in message_ids:
|
for msg_id in message_ids:
|
||||||
if msg_id in self.already_ingested_ids:
|
if msg_id in self.already_ingested_ids:
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="skipped",
|
||||||
|
reason="already_ingested_message",
|
||||||
|
message_id=msg_id,
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
stats["processed"] += 1
|
stats["processed"] += 1
|
||||||
@@ -292,6 +299,11 @@ class GmailClient:
|
|||||||
|
|
||||||
return ids
|
return ids
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _append_detail(stats: dict, **detail: str) -> None:
|
||||||
|
"""Append a compact attachment/message outcome to the import stats."""
|
||||||
|
stats.setdefault("details", []).append({key: value for key, value in detail.items() if value})
|
||||||
|
|
||||||
def _process_message(self, service, msg_id: str, stats: dict) -> int:
|
def _process_message(self, service, msg_id: str, stats: dict) -> int:
|
||||||
"""
|
"""
|
||||||
Download a Gmail message and process any DMARC-report attachments.
|
Download a Gmail message and process any DMARC-report attachments.
|
||||||
@@ -305,11 +317,17 @@ class GmailClient:
|
|||||||
except HttpError as exc:
|
except HttpError as exc:
|
||||||
logger.error("Gmail API: failed to fetch message %s: %s", msg_id, exc)
|
logger.error("Gmail API: failed to fetch message %s: %s", msg_id, exc)
|
||||||
stats["errors"].append(f"Failed to fetch message {msg_id}")
|
stats["errors"].append(f"Failed to fetch message {msg_id}")
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="error",
|
||||||
|
reason="message_fetch_failed",
|
||||||
|
message_id=msg_id,
|
||||||
|
)
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
raw_bytes = base64.urlsafe_b64decode(msg_data.get("raw", ""))
|
raw_bytes = base64.urlsafe_b64decode(msg_data.get("raw", ""))
|
||||||
msg = email.message_from_bytes(raw_bytes)
|
msg = email.message_from_bytes(raw_bytes)
|
||||||
return self._process_attachments(msg, stats)
|
return self._process_attachments(msg, stats, message_id=msg_id)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _decode_part_filename(part: email.message.Message) -> str:
|
def _decode_part_filename(part: email.message.Message) -> str:
|
||||||
@@ -352,33 +370,81 @@ class GmailClient:
|
|||||||
self.report_store.add_report(report)
|
self.report_store.add_report(report)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
def _process_attachments(self, msg: email.message.Message, stats: dict) -> int:
|
def _process_attachments(
|
||||||
|
self,
|
||||||
|
msg: email.message.Message,
|
||||||
|
stats: dict,
|
||||||
|
message_id: Optional[str] = None,
|
||||||
|
) -> int:
|
||||||
"""Walk a parsed email message and extract DMARC report attachments."""
|
"""Walk a parsed email message and extract DMARC report attachments."""
|
||||||
reports_found = 0
|
reports_found = 0
|
||||||
|
|
||||||
for part in msg.walk():
|
for part in msg.walk():
|
||||||
filename = self._decode_part_filename(part)
|
filename = self._decode_part_filename(part)
|
||||||
if not filename or not self._is_dmarc_attachment(filename):
|
if not filename:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
disposition = part.get_content_disposition()
|
disposition = part.get_content_disposition()
|
||||||
if disposition not in ("attachment", None):
|
if disposition not in ("attachment", None):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
if not self._is_dmarc_attachment(filename):
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="skipped",
|
||||||
|
reason="unsupported_attachment",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
|
||||||
content = part.get_payload(decode=True)
|
content = part.get_payload(decode=True)
|
||||||
if not content:
|
if not content:
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="skipped",
|
||||||
|
reason="empty_attachment",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
try:
|
try:
|
||||||
report = DMARCParser.parse_file(content, filename)
|
report = DMARCParser.parse_file(content, filename)
|
||||||
|
domain = str(report.get("domain", "unknown"))
|
||||||
|
report_id = str(report.get("report_id", ""))
|
||||||
if self._store_report_if_new(report):
|
if self._store_report_if_new(report):
|
||||||
stats["reports_found"] += 1
|
stats["reports_found"] += 1
|
||||||
reports_found += 1
|
reports_found += 1
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="imported",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
domain=domain,
|
||||||
|
report_id=report_id,
|
||||||
|
)
|
||||||
else:
|
else:
|
||||||
stats["duplicate_reports"] = stats.get("duplicate_reports", 0) + 1
|
stats["duplicate_reports"] = stats.get("duplicate_reports", 0) + 1
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="duplicate",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
domain=domain,
|
||||||
|
report_id=report_id,
|
||||||
|
)
|
||||||
except Exception as exc: # pylint: disable=broad-exception-caught
|
except Exception as exc: # pylint: disable=broad-exception-caught
|
||||||
logger.error("Failed to parse DMARC attachment %s: %s", filename, exc)
|
logger.error("Failed to parse DMARC attachment %s: %s", filename, exc)
|
||||||
stats["errors"].append(f"Failed to parse {filename}: {exc}")
|
stats["errors"].append(f"Failed to parse {filename}: {exc}")
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="error",
|
||||||
|
reason="parse_failed",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
error=str(exc),
|
||||||
|
)
|
||||||
|
|
||||||
return reports_found
|
return reports_found
|
||||||
|
|
||||||
|
|||||||
@@ -140,17 +140,24 @@ class IMAPClient:
|
|||||||
|
|
||||||
def _process_single_email(self, mail, email_id: bytes, stats: dict) -> None:
|
def _process_single_email(self, mail, email_id: bytes, stats: dict) -> None:
|
||||||
"""Fetch, parse, and store DMARC attachments from one email message."""
|
"""Fetch, parse, and store DMARC attachments from one email message."""
|
||||||
|
message_id = email_id.decode("utf-8", errors="replace")
|
||||||
try:
|
try:
|
||||||
status, msg_data = mail.fetch(email_id, "(RFC822)")
|
status, msg_data = mail.fetch(email_id, "(RFC822)")
|
||||||
if status != "OK":
|
if status != "OK":
|
||||||
logger.error("Error fetching email ID %s", email_id)
|
logger.error("Error fetching email ID %s", email_id)
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="error",
|
||||||
|
reason="message_fetch_failed",
|
||||||
|
message_id=message_id,
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
raw_email = msg_data[0][1]
|
raw_email = msg_data[0][1]
|
||||||
msg = email.message_from_bytes(raw_email)
|
msg = email.message_from_bytes(raw_email)
|
||||||
|
|
||||||
if self._is_dmarc_report_email(msg):
|
if self._is_dmarc_report_email(msg):
|
||||||
reports_found = self._process_attachments(msg, stats)
|
reports_found = self._process_attachments(msg, stats, message_id=message_id)
|
||||||
stats["reports_found"] += reports_found
|
stats["reports_found"] += reports_found
|
||||||
|
|
||||||
# Mark email as read (and optionally delete)
|
# Mark email as read (and optionally delete)
|
||||||
@@ -163,6 +170,13 @@ class IMAPClient:
|
|||||||
error_msg = f"Error processing email ID {email_id}: {str(e)}"
|
error_msg = f"Error processing email ID {email_id}: {str(e)}"
|
||||||
logger.error(error_msg)
|
logger.error(error_msg)
|
||||||
stats["errors"].append(error_msg)
|
stats["errors"].append(error_msg)
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="error",
|
||||||
|
reason="message_processing_failed",
|
||||||
|
message_id=message_id,
|
||||||
|
error=str(e),
|
||||||
|
)
|
||||||
|
|
||||||
def fetch_reports(self, days: int = 7) -> Dict[str, Any]:
|
def fetch_reports(self, days: int = 7) -> Dict[str, Any]:
|
||||||
"""
|
"""
|
||||||
@@ -185,6 +199,7 @@ class IMAPClient:
|
|||||||
"duplicate_reports": 0,
|
"duplicate_reports": 0,
|
||||||
"new_domains": [],
|
"new_domains": [],
|
||||||
"errors": [],
|
"errors": [],
|
||||||
|
"details": [],
|
||||||
}
|
}
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -334,12 +349,7 @@ class IMAPClient:
|
|||||||
filename = self._decode_email_header(filename)
|
filename = self._decode_email_header(filename)
|
||||||
|
|
||||||
# Check file extension
|
# Check file extension
|
||||||
if (
|
if self._is_dmarc_filename(filename):
|
||||||
filename.lower().endswith(".xml")
|
|
||||||
or filename.lower().endswith(".zip")
|
|
||||||
or filename.lower().endswith(".gz")
|
|
||||||
or filename.lower().endswith(".gzip")
|
|
||||||
):
|
|
||||||
return True
|
return True
|
||||||
|
|
||||||
# Check content type
|
# Check content type
|
||||||
@@ -355,7 +365,115 @@ class IMAPClient:
|
|||||||
|
|
||||||
return False
|
return False
|
||||||
|
|
||||||
def _process_attachments(self, msg: email.message.Message, stats: dict | None = None) -> int:
|
@staticmethod
|
||||||
|
def _is_dmarc_filename(filename: str) -> bool:
|
||||||
|
lower = filename.lower()
|
||||||
|
return (
|
||||||
|
lower.endswith(".xml")
|
||||||
|
or lower.endswith(".zip")
|
||||||
|
or lower.endswith(".gz")
|
||||||
|
or lower.endswith(".gzip")
|
||||||
|
)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _append_detail(stats: dict | None, **detail: str) -> None:
|
||||||
|
"""Append a compact attachment/message outcome to the import stats."""
|
||||||
|
if stats is None:
|
||||||
|
return
|
||||||
|
stats.setdefault("details", []).append(
|
||||||
|
{key: value for key, value in detail.items() if value}
|
||||||
|
)
|
||||||
|
|
||||||
|
def _store_report_if_new(
|
||||||
|
self,
|
||||||
|
report: Dict[str, Any],
|
||||||
|
*,
|
||||||
|
filename: str,
|
||||||
|
stats: dict | None,
|
||||||
|
message_id: str | None,
|
||||||
|
) -> bool:
|
||||||
|
domain = report.get("domain", "unknown")
|
||||||
|
report_id = report.get("report_id", "")
|
||||||
|
if report_id and (
|
||||||
|
self.report_store.has_report(domain, report_id)
|
||||||
|
or (self.db is not None and report_exists(self.db, domain, report_id))
|
||||||
|
):
|
||||||
|
logger.info("Skipping duplicate DMARC report %s for %s", report_id, domain)
|
||||||
|
if stats is not None:
|
||||||
|
stats["duplicate_reports"] = stats.get("duplicate_reports", 0) + 1
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="duplicate",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
domain=str(domain),
|
||||||
|
report_id=str(report_id),
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
if self.db is not None:
|
||||||
|
save_parsed_report(self.db, report)
|
||||||
|
self.report_store.add_report(report)
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="imported",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
domain=str(domain),
|
||||||
|
report_id=str(report_id),
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
|
def _process_dmarc_attachment(
|
||||||
|
self,
|
||||||
|
part: email.message.Message,
|
||||||
|
*,
|
||||||
|
filename: str,
|
||||||
|
stats: dict | None,
|
||||||
|
message_id: str | None,
|
||||||
|
) -> bool:
|
||||||
|
try:
|
||||||
|
content = part.get_payload(decode=True)
|
||||||
|
if not content:
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="skipped",
|
||||||
|
reason="empty_attachment",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
report = DMARCParser.parse_file(content, filename)
|
||||||
|
stored = self._store_report_if_new(
|
||||||
|
report,
|
||||||
|
filename=filename,
|
||||||
|
stats=stats,
|
||||||
|
message_id=message_id,
|
||||||
|
)
|
||||||
|
if stored:
|
||||||
|
logger.info("Successfully processed DMARC report: %s", filename)
|
||||||
|
return stored
|
||||||
|
except Exception as exc: # pylint: disable=broad-exception-caught
|
||||||
|
logger.error("Error processing attachment %s: %s", filename, str(exc))
|
||||||
|
if stats is not None:
|
||||||
|
stats.setdefault("errors", []).append(f"Failed to parse {filename}: {exc}")
|
||||||
|
self._append_detail(
|
||||||
|
stats,
|
||||||
|
status="error",
|
||||||
|
reason="parse_failed",
|
||||||
|
message_id=message_id,
|
||||||
|
filename=filename,
|
||||||
|
error=str(exc),
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
def _process_attachments(
|
||||||
|
self,
|
||||||
|
msg: email.message.Message,
|
||||||
|
stats: dict | None = None,
|
||||||
|
message_id: str | None = None,
|
||||||
|
) -> int:
|
||||||
"""
|
"""
|
||||||
Process email attachments that might be DMARC reports
|
Process email attachments that might be DMARC reports
|
||||||
|
|
||||||
@@ -368,57 +486,30 @@ class IMAPClient:
|
|||||||
reports_found = 0
|
reports_found = 0
|
||||||
|
|
||||||
for part in msg.walk():
|
for part in msg.walk():
|
||||||
content_disposition = part.get_content_disposition()
|
if part.get_content_disposition() != "attachment":
|
||||||
|
continue
|
||||||
|
|
||||||
if content_disposition == "attachment":
|
|
||||||
filename = part.get_filename()
|
filename = part.get_filename()
|
||||||
if filename:
|
if not filename:
|
||||||
# Decode filename if needed
|
continue
|
||||||
|
|
||||||
filename = self._decode_email_header(filename)
|
filename = self._decode_email_header(filename)
|
||||||
|
if not self._is_dmarc_filename(filename):
|
||||||
# Check if it's a likely DMARC report file
|
self._append_detail(
|
||||||
if (
|
stats,
|
||||||
filename.lower().endswith(".xml")
|
status="skipped",
|
||||||
or filename.lower().endswith(".zip")
|
reason="unsupported_attachment",
|
||||||
or filename.lower().endswith(".gz")
|
message_id=message_id,
|
||||||
or filename.lower().endswith(".gzip")
|
filename=filename,
|
||||||
):
|
|
||||||
|
|
||||||
try:
|
|
||||||
# Get attachment content
|
|
||||||
content = part.get_payload(decode=True)
|
|
||||||
|
|
||||||
# Parse the DMARC report
|
|
||||||
report = DMARCParser.parse_file(content, filename)
|
|
||||||
|
|
||||||
domain = report.get("domain", "unknown")
|
|
||||||
report_id = report.get("report_id", "")
|
|
||||||
if report_id and (
|
|
||||||
self.report_store.has_report(domain, report_id)
|
|
||||||
or (
|
|
||||||
self.db is not None
|
|
||||||
and report_exists(self.db, domain, report_id)
|
|
||||||
)
|
|
||||||
):
|
|
||||||
logger.info(
|
|
||||||
"Skipping duplicate DMARC report %s for %s",
|
|
||||||
report_id,
|
|
||||||
domain,
|
|
||||||
)
|
|
||||||
if stats is not None:
|
|
||||||
stats["duplicate_reports"] = (
|
|
||||||
stats.get("duplicate_reports", 0) + 1
|
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Add the report to the store
|
if self._process_dmarc_attachment(
|
||||||
if self.db is not None:
|
part,
|
||||||
save_parsed_report(self.db, report)
|
filename=filename,
|
||||||
self.report_store.add_report(report)
|
stats=stats,
|
||||||
|
message_id=message_id,
|
||||||
|
):
|
||||||
reports_found += 1
|
reports_found += 1
|
||||||
logger.info("Successfully processed DMARC report: %s", filename)
|
|
||||||
except Exception as e: # pylint: disable=broad-exception-caught
|
|
||||||
logger.error("Error processing attachment %s: %s", filename, str(e))
|
|
||||||
|
|
||||||
return reports_found
|
return reports_found
|
||||||
|
|||||||
@@ -9,13 +9,24 @@ from app.models.mail_source_import import MailSourceImport
|
|||||||
|
|
||||||
MAX_STORED_ERRORS = 10
|
MAX_STORED_ERRORS = 10
|
||||||
MAX_ERROR_LENGTH = 500
|
MAX_ERROR_LENGTH = 500
|
||||||
|
MAX_STORED_DETAILS = 50
|
||||||
|
MAX_DETAIL_VALUE_LENGTH = 300
|
||||||
|
DETAIL_FIELDS = {
|
||||||
|
"status",
|
||||||
|
"reason",
|
||||||
|
"message_id",
|
||||||
|
"filename",
|
||||||
|
"domain",
|
||||||
|
"report_id",
|
||||||
|
"error",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def _sanitize_error(value: object) -> str:
|
def _sanitize_error(value: object) -> str:
|
||||||
"""Return a compact, log-safe error string for storage and UI display."""
|
"""Return a compact, log-safe error string for storage and UI display."""
|
||||||
text = str(value).replace("\r", "").replace("\n", " ").strip()
|
text = str(value).replace("\r", "").replace("\n", " ").strip()
|
||||||
if len(text) > MAX_ERROR_LENGTH:
|
if len(text) > MAX_ERROR_LENGTH:
|
||||||
return text[: MAX_ERROR_LENGTH - 1] + "..."
|
return text[: MAX_ERROR_LENGTH - 3] + "..."
|
||||||
return text
|
return text
|
||||||
|
|
||||||
|
|
||||||
@@ -23,6 +34,27 @@ def _json_list(values: Optional[Iterable[Any]]) -> str:
|
|||||||
return json.dumps([str(value) for value in values or []])
|
return json.dumps([str(value) for value in values or []])
|
||||||
|
|
||||||
|
|
||||||
|
def _sanitize_detail_value(value: object) -> str:
|
||||||
|
text = _sanitize_error(value)
|
||||||
|
if len(text) > MAX_DETAIL_VALUE_LENGTH:
|
||||||
|
return text[: MAX_DETAIL_VALUE_LENGTH - 3] + "..."
|
||||||
|
return text
|
||||||
|
|
||||||
|
|
||||||
|
def _json_details(values: Optional[Iterable[Any]]) -> str:
|
||||||
|
details = []
|
||||||
|
for value in list(values or [])[:MAX_STORED_DETAILS]:
|
||||||
|
if not isinstance(value, dict):
|
||||||
|
continue
|
||||||
|
entry = {}
|
||||||
|
for key in DETAIL_FIELDS:
|
||||||
|
if key in value and value[key] not in (None, ""):
|
||||||
|
entry[key] = _sanitize_detail_value(value[key])
|
||||||
|
if entry:
|
||||||
|
details.append(entry)
|
||||||
|
return json.dumps(details)
|
||||||
|
|
||||||
|
|
||||||
def record_import_attempt(
|
def record_import_attempt(
|
||||||
db: Session,
|
db: Session,
|
||||||
source: MailSource,
|
source: MailSource,
|
||||||
@@ -47,6 +79,7 @@ def record_import_attempt(
|
|||||||
error_count=len(result_errors),
|
error_count=len(result_errors),
|
||||||
new_domains=_json_list(results.get("new_domains", [])),
|
new_domains=_json_list(results.get("new_domains", [])),
|
||||||
errors=json.dumps(errors),
|
errors=json.dumps(errors),
|
||||||
|
details=_json_details(results.get("details", [])),
|
||||||
started_at=started_at,
|
started_at=started_at,
|
||||||
finished_at=datetime.utcnow(),
|
finished_at=datetime.utcnow(),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -178,6 +178,7 @@
|
|||||||
<th>Duplicates</th>
|
<th>Duplicates</th>
|
||||||
<th>New Domains</th>
|
<th>New Domains</th>
|
||||||
<th>Errors</th>
|
<th>Errors</th>
|
||||||
|
<th>Details</th>
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
<tbody>
|
<tbody>
|
||||||
@@ -207,6 +208,32 @@
|
|||||||
<span class="text-muted-foreground">None</span>
|
<span class="text-muted-foreground">None</span>
|
||||||
</template>
|
</template>
|
||||||
</td>
|
</td>
|
||||||
|
<td>
|
||||||
|
<template x-if="entry.details && entry.details.length">
|
||||||
|
<details class="max-w-sm">
|
||||||
|
<summary class="cursor-pointer" x-text="formatDetailsSummary(entry.details)"></summary>
|
||||||
|
<div class="mt-2 space-y-2 text-xs">
|
||||||
|
<template x-for="detail in entry.details" :key="`${detail.status}-${detail.message_id || ''}-${detail.filename || ''}-${detail.report_id || ''}`">
|
||||||
|
<div class="rounded border border-base-300 p-2">
|
||||||
|
<div class="flex flex-wrap items-center gap-2">
|
||||||
|
<span class="badge badge-sm" :class="detailBadgeClass(detail.status)" x-text="detail.status"></span>
|
||||||
|
<span class="font-mono" x-show="detail.report_id" x-text="detail.report_id"></span>
|
||||||
|
</div>
|
||||||
|
<div class="mt-1 text-muted-foreground">
|
||||||
|
<span x-show="detail.filename" x-text="detail.filename"></span>
|
||||||
|
<span x-show="detail.domain" x-text="` • ${detail.domain}`"></span>
|
||||||
|
<span x-show="detail.reason" x-text="` • ${(detail.reason || '').replaceAll('_', ' ')}`"></span>
|
||||||
|
<span x-show="detail.error" x-text="` • ${detail.error}`"></span>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</template>
|
||||||
|
</div>
|
||||||
|
</details>
|
||||||
|
</template>
|
||||||
|
<template x-if="!entry.details || entry.details.length === 0">
|
||||||
|
<span class="text-muted-foreground">None</span>
|
||||||
|
</template>
|
||||||
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
</template>
|
</template>
|
||||||
</tbody>
|
</tbody>
|
||||||
@@ -579,6 +606,17 @@ function mailSourcesApp() {
|
|||||||
return value && value.length ? value.join(', ') : '—';
|
return value && value.length ? value.join(', ') : '—';
|
||||||
},
|
},
|
||||||
|
|
||||||
|
formatDetailsSummary(details) {
|
||||||
|
const counts = details.reduce((acc, detail) => {
|
||||||
|
const key = detail.status || 'unknown';
|
||||||
|
acc[key] = (acc[key] || 0) + 1;
|
||||||
|
return acc;
|
||||||
|
}, {});
|
||||||
|
return Object.entries(counts)
|
||||||
|
.map(([key, count]) => `${count} ${key}`)
|
||||||
|
.join(', ');
|
||||||
|
},
|
||||||
|
|
||||||
statusBadgeClass(status) {
|
statusBadgeClass(status) {
|
||||||
if (status === 'success') return 'badge-success';
|
if (status === 'success') return 'badge-success';
|
||||||
if (status === 'warning') return 'badge-warning';
|
if (status === 'warning') return 'badge-warning';
|
||||||
@@ -586,6 +624,13 @@ function mailSourcesApp() {
|
|||||||
return 'badge-outline';
|
return 'badge-outline';
|
||||||
},
|
},
|
||||||
|
|
||||||
|
detailBadgeClass(status) {
|
||||||
|
if (status === 'imported') return 'badge-success';
|
||||||
|
if (status === 'duplicate' || status === 'skipped') return 'badge-warning';
|
||||||
|
if (status === 'error') return 'badge-error';
|
||||||
|
return 'badge-outline';
|
||||||
|
},
|
||||||
|
|
||||||
openAddForm() {
|
openAddForm() {
|
||||||
this.editingId = null;
|
this.editingId = null;
|
||||||
this.gmailConnected = false;
|
this.gmailConnected = false;
|
||||||
|
|||||||
@@ -439,6 +439,8 @@ class TestProcessMessage:
|
|||||||
assert count == 0
|
assert count == 0
|
||||||
assert len(stats["errors"]) == 1
|
assert len(stats["errors"]) == 1
|
||||||
assert "bad-id" in stats["errors"][0]
|
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"}])
|
raw = _make_raw_email([{"filename": "photo.png", "content": b"\x89PNG"}])
|
||||||
msg = email_mod.message_from_bytes(raw)
|
msg = email_mod.message_from_bytes(raw)
|
||||||
stats = {"reports_found": 0, "errors": []}
|
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 count == 0
|
||||||
assert stats["reports_found"] == 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):
|
def test_google_style_zip_attachment_is_parsed(self):
|
||||||
"""A Google DMARC ZIP attachment is parsed and counted."""
|
"""A Google DMARC ZIP attachment is parsed and counted."""
|
||||||
@@ -485,6 +514,9 @@ class TestProcessAttachments:
|
|||||||
|
|
||||||
assert count == 1
|
assert count == 1
|
||||||
assert stats["reports_found"] == 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()
|
assert "example.com" in client.report_store.get_domains()
|
||||||
|
|
||||||
def test_google_style_zip_attachment_is_persisted(self, db_session):
|
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, first_stats) == 1
|
||||||
assert client._process_attachments(msg, second_stats) == 0
|
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
|
assert client.report_store.get_domain_summary("example.com")["reports_processed"] == 1
|
||||||
|
|
||||||
def test_dmarc_attachment_with_empty_content_skipped(self):
|
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
|
# Empty payload → `get_payload(decode=True)` returns b"" which is
|
||||||
# falsy, so the attachment is skipped
|
# falsy, so the attachment is skipped
|
||||||
assert count == 0
|
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):
|
def test_parse_exception_adds_error_and_continues(self):
|
||||||
"""A parse error should be recorded in stats but not raise."""
|
"""A parse error should be recorded in stats but not raise."""
|
||||||
@@ -567,6 +603,9 @@ class TestProcessAttachments:
|
|||||||
|
|
||||||
assert len(stats["errors"]) == 1
|
assert len(stats["errors"]) == 1
|
||||||
assert "bad.xml" in stats["errors"][0]
|
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
|
assert count == 1 # second attachment still parsed
|
||||||
|
|
||||||
|
|
||||||
@@ -622,6 +661,9 @@ class TestFetchReports:
|
|||||||
call_args = mock_proc.call_args_list[0][0]
|
call_args = mock_proc.call_args_list[0][0]
|
||||||
assert call_args[1] == "id2"
|
assert call_args[1] == "id2"
|
||||||
assert result["processed"] == 1
|
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):
|
def test_tracks_new_ingested_ids(self):
|
||||||
client = _make_client()
|
client = _make_client()
|
||||||
|
|||||||
@@ -427,8 +427,12 @@ class TestProcessAttachments:
|
|||||||
msg = email.message_from_bytes(
|
msg = email.message_from_bytes(
|
||||||
_make_email_with_attachment("report.xml", MINIMAL_DMARC_XML, "application/xml")
|
_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 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):
|
def test_processes_zip_attachment(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
@@ -450,12 +454,73 @@ class TestProcessAttachments:
|
|||||||
assert count == 1
|
assert count == 1
|
||||||
assert db_session.query(DMARCReport).filter_by(report_id="abc-123").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):
|
def test_bad_attachment_does_not_raise(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
msg = email.message_from_bytes(_make_email_with_attachment("report.xml", b"not xml at all"))
|
msg = email.message_from_bytes(_make_email_with_attachment("report.xml", b"not xml at all"))
|
||||||
# Should not raise; just returns 0
|
# 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 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):
|
def test_no_attachments_returns_zero(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
@@ -497,6 +562,7 @@ class TestProcessSingleEmail:
|
|||||||
|
|
||||||
assert stats["processed"] == 1
|
assert stats["processed"] == 1
|
||||||
assert stats["reports_found"] == 1
|
assert stats["reports_found"] == 1
|
||||||
|
assert stats["details"][0]["status"] == "imported"
|
||||||
|
|
||||||
def test_fetch_error_skips_email(self):
|
def test_fetch_error_skips_email(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
@@ -507,6 +573,8 @@ class TestProcessSingleEmail:
|
|||||||
client._process_single_email(mock_mail, b"1", stats)
|
client._process_single_email(mock_mail, b"1", stats)
|
||||||
|
|
||||||
assert stats["processed"] == 0
|
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):
|
def test_exception_adds_to_errors(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
@@ -517,6 +585,7 @@ class TestProcessSingleEmail:
|
|||||||
client._process_single_email(mock_mail, b"1", stats)
|
client._process_single_email(mock_mail, b"1", stats)
|
||||||
|
|
||||||
assert len(stats["errors"]) == 1
|
assert len(stats["errors"]) == 1
|
||||||
|
assert stats["details"][0]["reason"] == "message_processing_failed"
|
||||||
|
|
||||||
def test_marks_deleted_when_flag_set(self):
|
def test_marks_deleted_when_flag_set(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
@@ -599,6 +668,7 @@ class TestFetchReports:
|
|||||||
|
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
assert result["reports_found"] >= 1
|
assert result["reports_found"] >= 1
|
||||||
|
assert result["details"][0]["status"] == "imported"
|
||||||
|
|
||||||
def test_connection_error_returns_failure(self):
|
def test_connection_error_returns_failure(self):
|
||||||
client = self._make_client()
|
client = self._make_client()
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ Tests for MailSource model and mail-sources API endpoints.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import json
|
||||||
|
from datetime import datetime
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
from urllib.parse import parse_qs, urlparse
|
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 MailSource
|
||||||
from app.models.mail_source_import import MailSourceImport
|
from app.models.mail_source_import import MailSourceImport
|
||||||
|
from app.services.import_history import record_import_attempt
|
||||||
|
|
||||||
|
|
||||||
class TestMailSourceModel:
|
class TestMailSourceModel:
|
||||||
@@ -95,6 +98,7 @@ class TestMailSourceImportModel:
|
|||||||
error_count=1,
|
error_count=1,
|
||||||
new_domains='["example.com"]',
|
new_domains='["example.com"]',
|
||||||
errors='["bad attachment"]',
|
errors='["bad attachment"]',
|
||||||
|
details='[{"status": "imported", "filename": "report.xml"}]',
|
||||||
)
|
)
|
||||||
db_session.add(row)
|
db_session.add(row)
|
||||||
db_session.commit()
|
db_session.commit()
|
||||||
@@ -103,8 +107,51 @@ class TestMailSourceImportModel:
|
|||||||
assert row.id is not None
|
assert row.id is not None
|
||||||
assert row.mail_source_id == source.id
|
assert row.mail_source_id == source.id
|
||||||
assert row.duplicate_reports == 1
|
assert row.duplicate_reports == 1
|
||||||
|
assert '"imported"' in row.details
|
||||||
assert row.mail_source.name == "History Source"
|
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:
|
class TestMailSourcesAPI:
|
||||||
"""Integration tests for /api/v1/mail-sources endpoints (no auth)."""
|
"""Integration tests for /api/v1/mail-sources endpoints (no auth)."""
|
||||||
@@ -267,6 +314,7 @@ class TestMailSourcesAPIAuthed:
|
|||||||
error_count=1,
|
error_count=1,
|
||||||
new_domains='["example.com"]',
|
new_domains='["example.com"]',
|
||||||
errors='["sanitized error"]',
|
errors='["sanitized error"]',
|
||||||
|
details='[{"status": "duplicate", "report_id": "abc-123"}]',
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
db_session.commit()
|
db_session.commit()
|
||||||
@@ -281,6 +329,7 @@ class TestMailSourcesAPIAuthed:
|
|||||||
assert data[0]["duplicate_reports"] == 1
|
assert data[0]["duplicate_reports"] == 1
|
||||||
assert data[0]["new_domains"] == ["example.com"]
|
assert data[0]["new_domains"] == ["example.com"]
|
||||||
assert data[0]["errors"] == ["sanitized error"]
|
assert data[0]["errors"] == ["sanitized error"]
|
||||||
|
assert data[0]["details"] == [{"status": "duplicate", "report_id": "abc-123"}]
|
||||||
|
|
||||||
def test_list_import_history_handles_malformed_json(
|
def test_list_import_history_handles_malformed_json(
|
||||||
self, authed_client: TestClient, db_session: Session
|
self, authed_client: TestClient, db_session: Session
|
||||||
@@ -297,6 +346,7 @@ class TestMailSourcesAPIAuthed:
|
|||||||
status="warning",
|
status="warning",
|
||||||
new_domains="not-json",
|
new_domains="not-json",
|
||||||
errors='{"not": "a list"}',
|
errors='{"not": "a list"}',
|
||||||
|
details='{"not": "a list"}',
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
db_session.commit()
|
db_session.commit()
|
||||||
@@ -306,6 +356,7 @@ class TestMailSourcesAPIAuthed:
|
|||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
assert resp.json()[0]["new_domains"] == []
|
assert resp.json()[0]["new_domains"] == []
|
||||||
assert resp.json()[0]["errors"] == []
|
assert resp.json()[0]["errors"] == []
|
||||||
|
assert resp.json()[0]["details"] == []
|
||||||
|
|
||||||
def test_list_import_history_unknown_source_returns_404(self, authed_client: TestClient):
|
def test_list_import_history_unknown_source_returns_404(self, authed_client: TestClient):
|
||||||
resp = authed_client.get("/api/v1/mail-sources/99999/imports")
|
resp = authed_client.get("/api/v1/mail-sources/99999/imports")
|
||||||
@@ -764,6 +815,7 @@ class TestManualSourceFetchEndpoint:
|
|||||||
"duplicate_reports": 1,
|
"duplicate_reports": 1,
|
||||||
"new_domains": ["example.com"],
|
"new_domains": ["example.com"],
|
||||||
"errors": ["bad attachment\nwith newline"],
|
"errors": ["bad attachment\nwith newline"],
|
||||||
|
"details": [{"status": "error", "filename": "bad.xml"}],
|
||||||
}
|
}
|
||||||
|
|
||||||
with (
|
with (
|
||||||
@@ -780,6 +832,10 @@ class TestManualSourceFetchEndpoint:
|
|||||||
assert data["duplicate_reports"] == 1
|
assert data["duplicate_reports"] == 1
|
||||||
assert data["error_count"] == 1
|
assert data["error_count"] == 1
|
||||||
assert "bad attachment with newline" in caplog.text
|
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)
|
mock_imap.fetch_reports.assert_called_once_with(days=30)
|
||||||
|
|
||||||
def test_fetch_gmail_source(self, authed_client: TestClient, db_session: Session):
|
def test_fetch_gmail_source(self, authed_client: TestClient, db_session: Session):
|
||||||
@@ -807,6 +863,7 @@ class TestManualSourceFetchEndpoint:
|
|||||||
"new_domains": [],
|
"new_domains": [],
|
||||||
"errors": [],
|
"errors": [],
|
||||||
"new_ingested_ids": ["id1"],
|
"new_ingested_ids": ["id1"],
|
||||||
|
"details": [{"status": "imported", "report_id": "abc-123"}],
|
||||||
}
|
}
|
||||||
mock_gmail.get_refreshed_tokens.return_value = None
|
mock_gmail.get_refreshed_tokens.return_value = None
|
||||||
|
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ Recently improved:
|
|||||||
- Report/domain API reads can hydrate the dashboard projection from persisted data after restart.
|
- Report/domain API reads can hydrate the dashboard projection from persisted data after restart.
|
||||||
- Mail source import history is visible in the Mail Sources UI.
|
- Mail source import history is visible in the Mail Sources UI.
|
||||||
- Individual mail sources can be manually imported from the Mail Sources UI.
|
- Individual mail sources can be manually imported from the Mail Sources UI.
|
||||||
|
- Import-history rows include sanitized per-attachment outcomes and imported report IDs.
|
||||||
- The current Alpine-based UI is allowed by CSP and renders dynamic tables in real browsers.
|
- The current Alpine-based UI is allowed by CSP and renders dynamic tables in real browsers.
|
||||||
|
|
||||||
Implementation note:
|
Implementation note:
|
||||||
@@ -38,9 +39,8 @@ Implementation note:
|
|||||||
Objective: make mailbox imports auditable and make report totals trustworthy.
|
Objective: make mailbox imports auditable and make report totals trustworthy.
|
||||||
|
|
||||||
Priority tasks:
|
Priority tasks:
|
||||||
- Expand import attempts with message ID, source, attachment filename, outcome, and sanitized error details.
|
- Add mailbox search controls for date-range backfills.
|
||||||
- Report duplicate skips separately from parse failures.
|
- Report duplicate skips separately from parse failures.
|
||||||
- Add backfill controls for Gmail and IMAP sources.
|
|
||||||
- Improve source rollups so a source IP tracks pass/fail counts over time.
|
- Improve source rollups so a source IP tracks pass/fail counts over time.
|
||||||
|
|
||||||
Quality bar:
|
Quality bar:
|
||||||
|
|||||||
+1
-1
@@ -63,10 +63,10 @@ Recently delivered:
|
|||||||
- Uploaded, Gmail-imported, and IMAP-imported reports are now persisted to report/record tables and can be reloaded into report/domain views.
|
- Uploaded, Gmail-imported, and IMAP-imported reports are now persisted to report/record tables and can be reloaded into report/domain views.
|
||||||
- Mail source import history is now visible from the Mail Sources UI.
|
- Mail source import history is now visible from the Mail Sources UI.
|
||||||
- A single mail source can be manually imported from the Mail Sources UI, with the result recorded in import history.
|
- A single mail source can be manually imported from the Mail Sources UI, with the result recorded in import history.
|
||||||
|
- Import history now includes per-attachment details for imported reports, duplicates, parse errors, unsupported attachments, and imported report IDs.
|
||||||
- The current Alpine-based UI can run under the configured CSP, so dynamic tables render in real browsers.
|
- The current Alpine-based UI can run under the configured CSP, so dynamic tables render in real browsers.
|
||||||
|
|
||||||
Next tasks:
|
Next tasks:
|
||||||
- Add per-import result details: skipped duplicates, parse failures, unsupported attachments, and imported report IDs.
|
|
||||||
- Add mailbox search controls for date range/backfill without requiring code changes.
|
- Add mailbox search controls for date range/backfill without requiring code changes.
|
||||||
- Improve source aggregation so each sender IP keeps pass/fail totals instead of only the latest result.
|
- Improve source aggregation so each sender IP keeps pass/fail totals instead of only the latest result.
|
||||||
|
|
||||||
|
|||||||
@@ -92,6 +92,7 @@ This file tracks the specific implementation tasks for each milestone of the DMA
|
|||||||
- [x] Count duplicate skips separately from parse failures
|
- [x] Count duplicate skips separately from parse failures
|
||||||
- [x] Show recent import history in the Mail Sources UI
|
- [x] Show recent import history in the Mail Sources UI
|
||||||
- [x] Add manual import trigger per mail source
|
- [x] Add manual import trigger per mail source
|
||||||
|
- [x] Add per-import result details for duplicates, parse failures, unsupported attachments, and imported report IDs
|
||||||
- [ ] Add retry/backfill controls per mail source
|
- [ ] Add retry/backfill controls per mail source
|
||||||
|
|
||||||
## Milestone 3: Database Integration
|
## Milestone 3: Database Integration
|
||||||
|
|||||||
Reference in New Issue
Block a user