Improve testing coverage, template multi-assignment merges

docs/usage.md

@@ -260,7 +260,8 @@ Introduced in v2.0, consumption templates allow for finer control over what meta
 and permissions (owner, privileges) are assigned to documents during consumption. In general, templates
 are applied sequentially (by sort order) but subsequent templates will never override an assignment from
 a preceding template. The same is true for mail rules, e.g. if you set the correspondent in a mail rule
-any subsequent consumption templates that are applied _will not_ overwrite this.
+any subsequent consumption templates that are applied _will not_ overwrite this. The exception to this
+is assignments that can be multiple e.g. tags and permissions which will be merged.
 
 Consumption templates allow you to filter by:
 

src-ui/src/app/components/common/edit-dialog/consumption-template-edit-dialog/consumption-template-edit-dialog.component.spec.ts

@@ -14,6 +14,10 @@ import { TagsComponent } from '../../input/tags/tags.component'
 import { TextComponent } from '../../input/text/text.component'
 import { ConsumptionTemplateEditDialogComponent } from './consumption-template-edit-dialog.component'
 import { EditDialogMode } from '../edit-dialog.component'
+import { of } from 'rxjs'
+import { CorrespondentService } from 'src/app/services/rest/correspondent.service'
+import { DocumentTypeService } from 'src/app/services/rest/document-type.service'
+import { StoragePathService } from 'src/app/services/rest/storage-path.service'
 
 describe('ConsumptionTemplateEditDialogComponent', () => {
   let component: ConsumptionTemplateEditDialogComponent
@@ -33,7 +37,51 @@ describe('ConsumptionTemplateEditDialogComponent', () => {
         PermissionsGroupComponent,
         SafeHtmlPipe,
       ],
-      providers: [NgbActiveModal],
+      providers: [
+        NgbActiveModal,
+        {
+          provide: CorrespondentService,
+          useValue: {
+            listAll: () =>
+              of({
+                results: [
+                  {
+                    id: 1,
+                    username: 'c1',
+                  },
+                ],
+              }),
+          },
+        },
+        {
+          provide: DocumentTypeService,
+          useValue: {
+            listAll: () =>
+              of({
+                results: [
+                  {
+                    id: 1,
+                    username: 'dt1',
+                  },
+                ],
+              }),
+          },
+        },
+        {
+          provide: StoragePathService,
+          useValue: {
+            listAll: () =>
+              of({
+                results: [
+                  {
+                    id: 1,
+                    username: 'sp1',
+                  },
+                ],
+              }),
+          },
+        },
+      ],
       imports: [
         HttpClientTestingModule,
         FormsModule,

src-ui/src/app/services/rest/consumption-template.service.spec.ts

@@ -1,7 +1,64 @@
+import { HttpTestingController } from '@angular/common/http/testing'
+import { TestBed } from '@angular/core/testing'
+import { Subscription } from 'rxjs'
+import { environment } from 'src/environments/environment'
 import { commonAbstractPaperlessServiceTests } from './abstract-paperless-service.spec'
 import { ConsumptionTemplateService } from './consumption-template.service'
+import {
+  DocumentSource,
+  PaperlessConsumptionTemplate,
+} from 'src/app/data/paperless-consumption-template'
 
+let httpTestingController: HttpTestingController
+let service: ConsumptionTemplateService
+const endpoint = 'consumption_templates'
+const templates: PaperlessConsumptionTemplate[] = [
+  {
+    name: 'Template 1',
+    id: 1,
+    order: 1,
+    filter_filename: '*test*',
+    filter_path: null,
+    sources: [DocumentSource.ApiUpload],
+    assign_correspondent: 2,
+  },
+  {
+    name: 'Template 2',
+    id: 2,
+    order: 2,
+    filter_filename: null,
+    filter_path: '/test/',
+    sources: [DocumentSource.ConsumeFolder, DocumentSource.ApiUpload],
+    assign_document_type: 1,
+  },
+]
+
+// run common tests
 commonAbstractPaperlessServiceTests(
   'consumption_templates',
   ConsumptionTemplateService
 )
+
+describe(`Additional service tests for ConsumptionTemplateService`, () => {
+  it('should reload', () => {
+    service.reload()
+    const req = httpTestingController.expectOne(
+      `${environment.apiBaseUrl}${endpoint}/?page=1&page_size=100000`
+    )
+    req.flush({
+      results: templates,
+    })
+    expect(service.allTemplates).toEqual(templates)
+  })
+
+  beforeEach(() => {
+    // Dont need to setup again
+
+    httpTestingController = TestBed.inject(HttpTestingController)
+    service = TestBed.inject(ConsumptionTemplateService)
+  })
+
+  afterEach(() => {
+    httpTestingController.verify()
+  })
+})

src-ui/src/app/services/rest/consumption-template.service.ts

@@ -14,7 +14,7 @@ export class ConsumptionTemplateService extends AbstractPaperlessService<Paperle
     super(http, 'consumption_templates')
   }
 
-  private reload() {
+  public reload() {
     this.loading = true
     this.listAll().subscribe((r) => {
       this.templates = r.results

src/documents/consumer.py

@@ -608,14 +608,16 @@ class Consumer(LoggingMixin):
 
             if int(input_doc.source) in [int(x) for x in list(template.sources)] and (
                 (
-                    len(template.filter_filename) == 0
+                    template.filter_filename is None
+                    or len(template.filter_filename) == 0
                     or fnmatch(
                         input_doc.original_file.name.lower(),
                         template.filter_filename.lower(),
                     )
                 )
                 and (
-                    len(template.filter_path) == 0
+                    template.filter_path is None
+                    or len(template.filter_path) == 0
                     or input_doc.original_file.match(template.filter_path)
                 )
             ):
@@ -637,7 +639,7 @@ class Consumer(LoggingMixin):
                 if template.assign_storage_path is not None:
                     template_overrides.storage_path_id = template.assign_storage_path.pk
                 if template.assign_owner is not None:
-                    template_overrides.owner_id = template.assign_owner
+                    template_overrides.owner_id = template.assign_owner.pk
                 if template.assign_view_users is not None:
                     template_overrides.view_users = [
                         user.pk for user in template.assign_view_users.all()
@@ -786,12 +788,12 @@ class Consumer(LoggingMixin):
         ):
             permissions = {
                 "view": {
-                    "users": self.override_view_users,
+                    "users": self.override_view_users or [],
-                    "groups": self.override_view_groups,
+                    "groups": self.override_view_groups or [],
                 },
                 "change": {
-                    "users": self.override_change_users,
+                    "users": self.override_change_users or [],
-                    "groups": self.override_change_groups,
+                    "groups": self.override_change_groups or [],
                 },
             }
             set_permissions_for_object(permissions=permissions, object=document)
@@ -848,12 +850,12 @@ def merge_overrides(
 ) -> DocumentMetadataOverrides:
     """
     Merges two DocumentMetadataOverrides objects such that object B's overrides
-    are only applied if the property is empty in object A
+    are only applied if the property is empty in object A or merged if multiple
+    are accepted
     """
+    # only if empty
     if overridesA.title is None:
         overridesA.title = overridesB.title
-    if overridesA.tag_ids is None:
-        overridesA.tag_ids = overridesB.tag_ids
     if overridesA.correspondent_id is None:
         overridesA.correspondent_id = overridesB.correspondent_id
     if overridesA.document_type_id is None:
@@ -862,12 +864,28 @@ def merge_overrides(
         overridesA.storage_path_id = overridesB.storage_path_id
     if overridesA.owner_id is None:
         overridesA.owner_id = overridesB.owner_id
+    # merge
+    if overridesA.tag_ids is None:
+        overridesA.tag_ids = overridesB.tag_ids
+    else:
+        overridesA.tag_ids = [*overridesA.tag_ids, *overridesB.tag_ids]
     if overridesA.view_users is None:
         overridesA.view_users = overridesB.view_users
+    else:
+        overridesA.view_users = [*overridesA.view_users, *overridesB.view_users]
     if overridesA.view_groups is None:
         overridesA.view_groups = overridesB.view_groups
+    else:
+        overridesA.view_groups = [*overridesA.view_groups, *overridesB.view_groups]
     if overridesA.change_users is None:
         overridesA.change_users = overridesB.change_users
+    else:
+        overridesA.change_users = [*overridesA.change_users, *overridesB.change_users]
     if overridesA.change_groups is None:
         overridesA.change_groups = overridesB.change_groups
+    else:
+        overridesA.change_groups = [
+            *overridesA.change_groups,
+            *overridesB.change_groups,
+        ]
     return overridesA

src/documents/serialisers.py

@@ -1082,12 +1082,10 @@ class ConsumptionTemplateSerializer(OwnedObjectSerializer):
             "set_permissions",
         ]
 
-    def update(self, instance, validated_data):
-        super().update(instance, validated_data)
-        return instance
-
     def validate(self, attrs):
-        if len(attrs["filter_filename"]) == 0 and len(attrs["filter_path"]) == 0:
+        if ("filter_filename" not in attrs or len(attrs["filter_filename"]) == 0) and (
+            "filter_path" not in attrs or len(attrs["filter_path"]) == 0
+        ):
             raise serializers.ValidationError("File name or path filter are required")
 
         return attrs

src/documents/tests/test_api.py

@@ -32,6 +32,8 @@ from whoosh.writing import AsyncWriter
 
 from documents import bulk_edit
 from documents import index
+from documents.data_models import DocumentSource
+from documents.models import ConsumptionTemplate
 from documents.models import Correspondent
 from documents.models import Document
 from documents.models import DocumentType
@@ -5313,3 +5315,116 @@ class TestBulkEditObjectPermissions(APITestCase):
         )
 
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+
+class TestApiConsumptionTemplates(DirectoriesMixin, APITestCase):
+    ENDPOINT = "/api/consumption_templates/"
+
+    def setUp(self) -> None:
+        super().setUp()
+
+        user = User.objects.create_superuser(username="temp_admin")
+        self.client.force_authenticate(user=user)
+        self.user2 = User.objects.create(username="user2")
+        self.user3 = User.objects.create(username="user3")
+        self.group1 = Group.objects.create(name="group1")
+
+        self.c = Correspondent.objects.create(name="Correspondent Name")
+        self.c2 = Correspondent.objects.create(name="Correspondent Name 2")
+        self.dt = DocumentType.objects.create(name="DocType Name")
+        self.t1 = Tag.objects.create(name="t1")
+        self.t2 = Tag.objects.create(name="t2")
+        self.t3 = Tag.objects.create(name="t3")
+        self.sp = StoragePath.objects.create(path="/test/")
+
+        self.ct = ConsumptionTemplate.objects.create(
+            name="Template 1",
+            order=0,
+            sources=f"{int(DocumentSource.ApiUpload)},{int(DocumentSource.ConsumeFolder)},{int(DocumentSource.MailFetch)}",
+            filter_filename="*simple*",
+            filter_path="*/samples/*",
+            assign_title="Doc from {correspondent}",
+            assign_correspondent=self.c,
+            assign_document_type=self.dt,
+            assign_storage_path=self.sp,
+            assign_owner=self.user2,
+        )
+        self.ct.assign_tags.add(self.t1)
+        self.ct.assign_tags.add(self.t2)
+        self.ct.assign_tags.add(self.t3)
+        self.ct.assign_view_users.add(self.user3.pk)
+        self.ct.assign_view_groups.add(self.group1.pk)
+        self.ct.assign_change_users.add(self.user3.pk)
+        self.ct.assign_change_groups.add(self.group1.pk)
+        self.ct.save()
+
+    def test_api_get_consumption_template(self):
+        """
+        GIVEN:
+            - API request to get all consumption template
+        WHEN:
+            - API is called
+        THEN:
+            - Existing consumption templates are returned
+        """
+        response = self.client.get(self.ENDPOINT, format="json")
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data["count"], 1)
+
+        resp_consumption_template = response.data["results"][0]
+        self.assertEqual(resp_consumption_template["id"], self.ct.id)
+        self.assertEqual(
+            resp_consumption_template["assign_correspondent"],
+            self.ct.assign_correspondent.pk,
+        )
+
+    def test_api_create_consumption_template(self):
+        """
+        GIVEN:
+            - API request to create a consumption template
+        WHEN:
+            - API is called
+        THEN:
+            - Correct HTTP response
+            - New template is created
+        """
+        response = self.client.post(
+            self.ENDPOINT,
+            json.dumps(
+                {
+                    "name": "Template 2",
+                    "order": 1,
+                    "sources": [DocumentSource.ApiUpload],
+                    "filter_filename": "*test*",
+                },
+            ),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(ConsumptionTemplate.objects.count(), 2)
+
+    def test_api_create_invalid_consumption_template(self):
+        """
+        GIVEN:
+            - API request to create a consumption template
+            - Neither file name nor path filter are specified
+        WHEN:
+            - API is called
+        THEN:
+            - Correct HTTP 400 response
+            - No template is created
+        """
+        response = self.client.post(
+            self.ENDPOINT,
+            json.dumps(
+                {
+                    "name": "Template 2",
+                    "order": 1,
+                    "sources": [DocumentSource.ApiUpload],
+                },
+            ),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(StoragePath.objects.count(), 1)

src/documents/tests/test_consumer.py

@@ -11,9 +11,12 @@ from unittest.mock import MagicMock
 
 from dateutil import tz
 from django.conf import settings
+from django.contrib.auth.models import Group
+from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test import override_settings
 from django.utils import timezone
+from guardian.core import ObjectPermissionChecker
 
 from documents.consumer import Consumer
 from documents.consumer import ConsumerError
@@ -456,6 +459,14 @@ class TestConsumer(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         self.assertIn(t3, document.tags.all())
         self._assert_first_last_send_progress()
 
+    def testOverrideAsn(self):
+        document = self.consumer.try_consume_file(
+            self.get_test_file(),
+            override_asn=123,
+        )
+        self.assertEqual(document.archive_serial_number, 123)
+        self._assert_first_last_send_progress()
+
     def testOverrideTitlePlaceholders(self):
         c = Correspondent.objects.create(name="Correspondent Name")
         dt = DocumentType.objects.create(name="DocType Name")
@@ -470,6 +481,29 @@ class TestConsumer(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         self.assertEqual(document.title, f"{c.name}{dt.name} {now.strftime('%m-%y')}")
         self._assert_first_last_send_progress()
 
+    def testOverrideOwner(self):
+        testuser = User.objects.create(username="testuser")
+        document = self.consumer.try_consume_file(
+            self.get_test_file(),
+            override_owner_id=testuser.pk,
+        )
+        self.assertEqual(document.owner, testuser)
+        self._assert_first_last_send_progress()
+
+    def testOverridePermissions(self):
+        testuser = User.objects.create(username="testuser")
+        testgroup = Group.objects.create(name="testgroup")
+        document = self.consumer.try_consume_file(
+            self.get_test_file(),
+            override_view_users=[testuser.pk],
+            override_view_groups=[testgroup.pk],
+        )
+        user_checker = ObjectPermissionChecker(testuser)
+        self.assertTrue(user_checker.has_perm("view_document", document))
+        group_checker = ObjectPermissionChecker(testgroup)
+        self.assertTrue(group_checker.has_perm("view_document", document))
+        self._assert_first_last_send_progress()
+
     def testNotAFile(self):
         self.assertRaisesMessage(
             ConsumerError,

src/documents/tests/test_consumption_templates.py

@@ -0,0 +1,207 @@
+from pathlib import Path
+from unittest import TestCase
+from unittest import mock
+
+import pytest
+from django.contrib.auth.models import Group
+from django.contrib.auth.models import User
+
+from documents import tasks
+from documents.data_models import ConsumableDocument
+from documents.data_models import DocumentSource
+from documents.models import ConsumptionTemplate
+from documents.models import Correspondent
+from documents.models import DocumentType
+from documents.models import StoragePath
+from documents.models import Tag
+from documents.tests.utils import DirectoriesMixin
+from documents.tests.utils import FileSystemAssertsMixin
+
+
+@pytest.mark.django_db
+class TestConsumptionTemplates(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
+    SAMPLE_DIR = Path(__file__).parent / "samples"
+
+    def setUp(self) -> None:
+        self.c = Correspondent.objects.create(name="Correspondent Name")
+        self.c2 = Correspondent.objects.create(name="Correspondent Name 2")
+        self.dt = DocumentType.objects.create(name="DocType Name")
+        self.t1 = Tag.objects.create(name="t1")
+        self.t2 = Tag.objects.create(name="t2")
+        self.t3 = Tag.objects.create(name="t3")
+        self.sp = StoragePath.objects.create(path="/test/")
+
+        self.user2 = User.objects.create(username="user2")
+        self.user3 = User.objects.create(username="user3")
+        self.group1 = Group.objects.create(name="group1")
+
+        return super().setUp()
+
+    @mock.patch("documents.consumer.Consumer.try_consume_file")
+    def test_consumption_template_match(self, m):
+        """
+        GIVEN:
+            - Existing consumption template
+        WHEN:
+            - File that matches is consumed
+        THEN:
+            - Template overrides are applied
+        """
+        ct = ConsumptionTemplate.objects.create(
+            name="Template 1",
+            order=0,
+            sources=f"{int(DocumentSource.ApiUpload)},{int(DocumentSource.ConsumeFolder)},{int(DocumentSource.MailFetch)}",
+            filter_filename="*simple*",
+            filter_path="*/samples/*",
+            assign_title="Doc from {correspondent}",
+            assign_correspondent=self.c,
+            assign_document_type=self.dt,
+            assign_storage_path=self.sp,
+            assign_owner=self.user2,
+        )
+        ct.assign_tags.add(self.t1)
+        ct.assign_tags.add(self.t2)
+        ct.assign_tags.add(self.t3)
+        ct.assign_view_users.add(self.user3.pk)
+        ct.assign_view_groups.add(self.group1.pk)
+        ct.assign_change_users.add(self.user3.pk)
+        ct.assign_change_groups.add(self.group1.pk)
+        ct.save()
+
+        self.assertEqual(ct.__str__(), "Template 1")
+
+        test_file = self.SAMPLE_DIR / "simple.pdf"
+
+        with mock.patch("documents.tasks.async_to_sync"):
+            tasks.consume_file(
+                ConsumableDocument(
+                    source=DocumentSource.ConsumeFolder,
+                    original_file=test_file,
+                ),
+                None,
+            )
+            m.assert_called_once()
+            _, overrides = m.call_args
+            self.assertEqual(overrides["override_correspondent_id"], self.c.pk)
+            self.assertEqual(overrides["override_document_type_id"], self.dt.pk)
+            self.assertEqual(
+                overrides["override_tag_ids"],
+                [self.t1.pk, self.t2.pk, self.t3.pk],
+            )
+            self.assertEqual(overrides["override_storage_path_id"], self.sp.pk)
+            self.assertEqual(overrides["override_owner_id"], self.user2.pk)
+            self.assertEqual(overrides["override_view_users"], [self.user3.pk])
+            self.assertEqual(overrides["override_view_groups"], [self.group1.pk])
+            self.assertEqual(overrides["override_change_users"], [self.user3.pk])
+            self.assertEqual(overrides["override_change_groups"], [self.group1.pk])
+            self.assertEqual(overrides["override_title"], "Doc from {correspondent}")
+
+    @mock.patch("documents.consumer.Consumer.try_consume_file")
+    def test_consumption_template_match_multiple(self, m):
+        """
+        GIVEN:
+            - Multiple existing consumption template
+        WHEN:
+            - File that matches is consumed
+        THEN:
+            - Template overrides are applied with subsequent templates only overwriting empty values
+            or merging if multiple
+        """
+        ct1 = ConsumptionTemplate.objects.create(
+            name="Template 1",
+            order=0,
+            sources=f"{int(DocumentSource.ApiUpload)},{int(DocumentSource.ConsumeFolder)},{int(DocumentSource.MailFetch)}",
+            filter_path="*/samples/*",
+            assign_title="Doc from {correspondent}",
+            assign_correspondent=self.c,
+            assign_document_type=self.dt,
+        )
+        ct1.assign_tags.add(self.t1)
+        ct1.assign_tags.add(self.t2)
+        ct1.assign_view_users.add(self.user2)
+        ct1.save()
+        ct2 = ConsumptionTemplate.objects.create(
+            name="Template 2",
+            order=0,
+            sources=f"{int(DocumentSource.ApiUpload)},{int(DocumentSource.ConsumeFolder)},{int(DocumentSource.MailFetch)}",
+            filter_filename="*simple*",
+            assign_title="Doc from {correspondent}",
+            assign_correspondent=self.c2,
+            assign_storage_path=self.sp,
+        )
+        ct2.assign_tags.add(self.t3)
+        ct1.assign_view_users.add(self.user3)
+        ct2.save()
+
+        test_file = self.SAMPLE_DIR / "simple.pdf"
+
+        with mock.patch("documents.tasks.async_to_sync"):
+            tasks.consume_file(
+                ConsumableDocument(
+                    source=DocumentSource.ConsumeFolder,
+                    original_file=test_file,
+                ),
+                None,
+            )
+            m.assert_called_once()
+            _, overrides = m.call_args
+            # template 1
+            self.assertEqual(overrides["override_correspondent_id"], self.c.pk)
+            self.assertEqual(overrides["override_document_type_id"], self.dt.pk)
+            # template 2
+            self.assertEqual(overrides["override_storage_path_id"], self.sp.pk)
+            # template 1 & 2
+            self.assertEqual(
+                overrides["override_tag_ids"],
+                [self.t1.pk, self.t2.pk, self.t3.pk],
+            )
+            self.assertEqual(
+                overrides["override_view_users"],
+                [self.user2.pk, self.user3.pk],
+            )
+
+    @mock.patch("documents.consumer.Consumer.try_consume_file")
+    def test_consumption_template_no_match(self, m):
+        """
+        GIVEN:
+            - Existing consumption template
+        WHEN:
+            - File that does not match is consumed
+        THEN:
+            - Template overrides are not applied
+        """
+        ConsumptionTemplate.objects.create(
+            name="Template 1",
+            order=0,
+            sources=f"{int(DocumentSource.ApiUpload)},{int(DocumentSource.ConsumeFolder)},{int(DocumentSource.MailFetch)}",
+            filter_filename="*foobar*",
+            filter_path=None,
+            assign_title="Doc from {correspondent}",
+            assign_correspondent=self.c,
+            assign_document_type=self.dt,
+            assign_storage_path=self.sp,
+            assign_owner=self.user2,
+        )
+
+        test_file = self.SAMPLE_DIR / "simple.pdf"
+
+        with mock.patch("documents.tasks.async_to_sync"):
+            tasks.consume_file(
+                ConsumableDocument(
+                    source=DocumentSource.ConsumeFolder,
+                    original_file=test_file,
+                ),
+                None,
+            )
+            m.assert_called_once()
+            _, overrides = m.call_args
+            self.assertIsNone(overrides["override_correspondent_id"])
+            self.assertIsNone(overrides["override_document_type_id"])
+            self.assertIsNone(overrides["override_tag_ids"])
+            self.assertIsNone(overrides["override_storage_path_id"])
+            self.assertIsNone(overrides["override_owner_id"])
+            self.assertIsNone(overrides["override_view_users"])
+            self.assertIsNone(overrides["override_view_groups"])
+            self.assertIsNone(overrides["override_change_users"])
+            self.assertIsNone(overrides["override_change_groups"])
+            self.assertIsNone(overrides["override_title"])