Moves the renaming triggered from a storage path change to be a background task

diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py
index 3b4021a2ef7f4dcab36fed3adb9395c81d173e9c..7f6003ae4eae98d158cf1c3efdd14f10c345bf02 100644 (file)
--- a/src/documents/signals/handlers.py
+++ b/src/documents/signals/handlers.py
@@ -21,6 +21,7 @@ from django.utils import timezone
 from filelock import FileLock
 
 from .. import matching
+from ..bulk_edit import bulk_update_documents
 from ..file_handling import create_source_path_directory
 from ..file_handling import delete_empty_directories
 from ..file_handling import generate_unique_filename
@@ -385,7 +386,7 @@ def validate_move(instance, old_path, new_path):
 
 @receiver(models.signals.m2m_changed, sender=Document.tags.through)
 @receiver(models.signals.post_save, sender=Document)
-def update_filename_and_move_files(sender, instance, **kwargs):
+def update_filename_and_move_files(sender, instance: Document, **kwargs):
 
     if not instance.filename:
         # Can't update the filename if there is no filename to begin with
@@ -497,13 +498,14 @@ def update_filename_and_move_files(sender, instance, **kwargs):
 
 
 @receiver(models.signals.post_save, sender=StoragePath)
-def update_document_storage_path(sender, instance, **kwargs):
+def update_document_storage_path(sender, instance: StoragePath, **kwargs):
     """
     Triggers when a storage path is changed, running against any documents using
     the path, and checks to see if they need to be renamed
     """
-    for document in instance.documents.all():
-        update_filename_and_move_files(None, document)
+    doc_ids = [doc.id for doc in instance.documents.all()]
+    if len(doc_ids):
+        bulk_update_documents.delay(doc_ids)
 
 
 def set_log_entry(sender, document=None, logging_group=None, **kwargs):

diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py
index 6b55d9b88f690fec5593bc7c9944a0fb8f30eb93..e97168c23a65a10ae830b3a9ee4fa8bd55fee689 100644 (file)
--- a/src/documents/tests/test_file_handling.py
+++ b/src/documents/tests/test_file_handling.py
@@ -10,6 +10,7 @@ from django.test import override_settings
 from django.test import TestCase
 from django.utils import timezone
 
+from ..bulk_edit import bulk_update_documents
 from ..file_handling import create_source_path_directory
 from ..file_handling import delete_empty_directories
 from ..file_handling import generate_filename
@@ -1032,7 +1033,8 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
         self.assertEqual(generate_filename(doc_a), "0000002.pdf")
         self.assertEqual(generate_filename(doc_b), "SomeImportantNone/2020-07-25.pdf")
 
-    def test_document_storage_path_changed(self):
+    @mock.patch("documents.bulk_edit.bulk_update_documents.delay")
+    def test_document_storage_path_changed_with_doc(self, bulk_update_mock):
         """
         GIVEN:
             - Document with a defined storage path
@@ -1062,6 +1064,9 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
         document.save()
 
         # Change format
+        bulk_update_mock.side_effect = (
+            bulk_update_documents  # call through, no Redis needed
+        )
         sp.path = "NewFolder/{created}"
         sp.save()
 
@@ -1073,6 +1078,50 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
         )
         self.assertTrue(os.path.isfile(document.source_path))
 
+    @mock.patch("documents.bulk_edit.bulk_update_documents.delay")
+    def test_document_storage_path_changed_with_no_doc(self, bulk_update_mock):
+        """
+        GIVEN:
+            - Document with no storage path
+            - Storage path exist
+        WHEN:
+            - The storage path format is updated
+        THEN:
+            - No changes to document
+        """
+        sp = StoragePath.objects.create(path="TestFolder/{created}")
+        document = Document.objects.create(
+            mime_type="application/pdf",
+            created=timezone.make_aware(datetime.datetime(2021, 6, 25, 7, 36, 51, 153)),
+        )
+
+        # Ensure that filename is properly generated
+        document.filename = generate_filename(document)
+        self.assertEqual(
+            document.source_path,
+            os.path.join(settings.ORIGINALS_DIR, f"{document.pk:07}.pdf"),
+        )
+
+        # Actually create the file
+        create_source_path_directory(document.source_path)
+        Path(document.source_path).touch()
+        self.assertTrue(os.path.isfile(document.source_path))
+        document.save()
+
+        # Change format
+        sp.path = "NewFolder/{created}"
+        sp.save()
+
+        bulk_update_mock.assert_not_called()
+
+        document.refresh_from_db()
+
+        self.assertEqual(
+            document.source_path,
+            os.path.join(settings.ORIGINALS_DIR, f"{document.pk:07}.pdf"),
+        )
+        self.assertTrue(os.path.isfile(document.source_path))
+
     @override_settings(
         FILENAME_FORMAT="{created_year_short}/{created_month_name_short}/{created_month_name}/{title}",
     )