diff --git a/src/documents/mail.py b/src/documents/mail.py index 6af48ca9b..21bf03c7a 100644 --- a/src/documents/mail.py +++ b/src/documents/mail.py @@ -1,25 +1,26 @@ from __future__ import annotations +from dataclasses import dataclass from email import message_from_bytes -from typing import TYPE_CHECKING +from pathlib import Path from django.conf import settings from django.core.mail import EmailMessage from filelock import FileLock -from documents.data_models import ConsumableDocument -if TYPE_CHECKING: - from documents.models import Document +@dataclass(frozen=True) +class EmailAttachment: + path: Path + mime_type: str + friendly_name: str def send_email( subject: str, body: str, to: list[str], - attachments: list[Document | ConsumableDocument], - *, - use_archive: bool, + attachments: list[EmailAttachment], ) -> int: """ Send an email with attachments. @@ -28,8 +29,7 @@ def send_email( subject: Email subject body: Email body text to: List of recipient email addresses - attachments: List of documents to attach (the list may be empty) - use_archive: Whether to attach archive versions when available + attachments: List of attachments Returns: Number of emails sent @@ -46,47 +46,41 @@ def send_email( # Something could be renaming the file concurrently so it can't be attached with FileLock(settings.MEDIA_LOCK): - for document in attachments: - if isinstance(document, ConsumableDocument): - attachment_path = document.original_file - friendly_filename = document.original_file.name - else: - attachment_path = ( - document.archive_path - if use_archive and document.has_archive_version - else document.source_path - ) - friendly_filename = _get_unique_filename( - document, - used_filenames, - archive=use_archive and document.has_archive_version, - ) - used_filenames.add(friendly_filename) + for attachment in attachments: + filename = _get_unique_filename( + attachment.friendly_name, + used_filenames, + ) + used_filenames.add(filename) - with attachment_path.open("rb") as f: + with attachment.path.open("rb") as f: content = f.read() - if document.mime_type == "message/rfc822": + if attachment.mime_type == "message/rfc822": # See https://forum.djangoproject.com/t/using-emailmessage-with-an-attached-email-file-crashes-due-to-non-ascii/37981 content = message_from_bytes(content) email.attach( - filename=friendly_filename, + filename=filename, content=content, - mimetype=document.mime_type, + mimetype=attachment.mime_type, ) return email.send() -def _get_unique_filename(doc: Document, used_names: set[str], *, archive: bool) -> str: +def _get_unique_filename(friendly_name: str, used_names: set[str]) -> str: """ - Constructs a unique friendly filename for the given document. + Constructs a unique friendly filename for the given document, append a counter if needed. + """ + if friendly_name not in used_names: + return friendly_name - The filename might not be unique enough, so a counter is appended if needed. - """ - counter = 0 + stem = Path(friendly_name).stem + suffix = "".join(Path(friendly_name).suffixes) + + counter = 1 while True: - filename = doc.get_public_filename(archive=archive, counter=counter) + filename = f"{stem}_{counter:02}{suffix}" if filename not in used_names: return filename counter += 1 diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 22abf625e..a7641ecf2 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -31,11 +31,10 @@ from guardian.shortcuts import remove_perm from documents import matching from documents.caching import clear_document_caches -from documents.data_models import ConsumableDocument -from documents.data_models import DocumentSource from documents.file_handling import create_source_path_directory from documents.file_handling import delete_empty_directories from documents.file_handling import generate_unique_filename +from documents.mail import EmailAttachment from documents.mail import send_email from documents.models import Correspondent from documents.models import CustomField @@ -57,6 +56,7 @@ from documents.templating.workflows import parse_w_workflow_placeholders if TYPE_CHECKING: from documents.classifier import DocumentClassifier + from documents.data_models import ConsumableDocument from documents.data_models import DocumentMetadataOverrides logger = logging.getLogger("paperless.handlers") @@ -1163,28 +1163,41 @@ def run_workflows( else "" ) try: - attachments = [] + attachments: list[EmailAttachment] = [] if action.email.include_document: + attachment: EmailAttachment | None = None if trigger_type in [ WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, WorkflowTrigger.WorkflowTriggerType.SCHEDULED, - ]: - # Updated and scheduled can pass the document directly - attachments = [document] + ] and isinstance(document, Document): + friendly_name = ( + Path(current_filename).name + if current_filename + else document.source_path.name + ) + attachment = EmailAttachment( + path=document.source_path, + mime_type=document.mime_type, + friendly_name=friendly_name, + ) elif original_file: - # For consumed and added document is not yet saved, so pass the original file - attachments = [ - ConsumableDocument( - source=DocumentSource.ApiUpload, - original_file=original_file, - ), - ] + friendly_name = ( + Path(current_filename).name + if current_filename + else original_file.name + ) + attachment = EmailAttachment( + path=original_file, + mime_type=document.mime_type, + friendly_name=friendly_name, + ) + if attachment: + attachments = [attachment] n_messages = send_email( subject=subject, body=body, to=action.email.to.split(","), attachments=attachments, - use_archive=False, ) logger.debug( f"Sent {n_messages} notification email(s) to {action.email.to}", diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index c25565ae6..a2a5aa9c3 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -8,8 +8,10 @@ from typing import TYPE_CHECKING from unittest import mock import pytest +from django.conf import settings from django.contrib.auth.models import Group from django.contrib.auth.models import User +from django.core import mail from django.test import override_settings from django.utils import timezone from guardian.shortcuts import assign_perm @@ -21,6 +23,8 @@ from pytest_httpx import HTTPXMock from rest_framework.test import APIClient from rest_framework.test import APITestCase +from documents.file_handling import create_source_path_directory +from documents.file_handling import generate_unique_filename from documents.signals.handlers import run_workflows from documents.signals.handlers import send_webhook @@ -2989,6 +2993,70 @@ class TestWorkflows( mock_email_send.assert_called_once() + @override_settings( + PAPERLESS_EMAIL_HOST="localhost", + EMAIL_ENABLED=True, + PAPERLESS_URL="http://localhost:8000", + EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend", + ) + def test_workflow_email_attachment_uses_storage_filename(self): + """ + GIVEN: + - Document updated workflow with include document action + - Document stored with formatted storage-path filename + WHEN: + - Workflow sends an email + THEN: + - Attachment filename matches the stored filename + """ + + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + ) + email_action = WorkflowActionEmail.objects.create( + subject="Test Notification: {doc_title}", + body="Test message: {doc_url}", + to="me@example.com", + include_document=True, + ) + action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.EMAIL, + email=email_action, + ) + workflow = Workflow.objects.create( + name="Workflow attachment filename", + order=0, + ) + workflow.triggers.add(trigger) + workflow.actions.add(action) + workflow.save() + + storage_path = StoragePath.objects.create( + name="Fancy Path", + path="formatted/{{ document.pk }}/{{ title }}", + ) + doc = Document.objects.create( + title="workflow doc", + correspondent=self.c, + checksum="workflow-email-attachment", + mime_type="application/pdf", + storage_path=storage_path, + original_filename="workflow-orig.pdf", + ) + + # eg what happens in update_filename_and_move_files + generated = generate_unique_filename(doc) + destination = (settings.ORIGINALS_DIR / generated).resolve() + create_source_path_directory(destination) + shutil.copy(self.SAMPLE_DIR / "simple.pdf", destination) + Document.objects.filter(pk=doc.pk).update(filename=generated.as_posix()) + + run_workflows(WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, doc) + + self.assertEqual(len(mail.outbox), 1) + attachment_names = [att[0] for att in mail.outbox[0].attachments] + self.assertEqual(attachment_names, [Path(generated).name]) + @override_settings( EMAIL_ENABLED=False, ) diff --git a/src/documents/views.py b/src/documents/views.py index 822647fdb..5c4228432 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -120,6 +120,7 @@ from documents.filters import PaperlessTaskFilterSet from documents.filters import ShareLinkFilterSet from documents.filters import StoragePathFilterSet from documents.filters import TagFilterSet +from documents.mail import EmailAttachment from documents.mail import send_email from documents.matching import match_correspondents from documents.matching import match_document_types @@ -1216,12 +1217,28 @@ class DocumentViewSet( return HttpResponseForbidden("Insufficient permissions") try: + attachments: list[EmailAttachment] = [] + for doc in documents: + attachment_path = ( + doc.archive_path + if use_archive_version and doc.has_archive_version + else doc.source_path + ) + attachments.append( + EmailAttachment( + path=attachment_path, + mime_type=doc.mime_type, + friendly_name=doc.get_public_filename( + archive=use_archive_version and doc.has_archive_version, + ), + ), + ) + send_email( subject=subject, body=message, to=addresses, - attachments=documents, - use_archive=use_archive_version, + attachments=attachments, ) logger.debug(