From 0f26114ca0ebdd868ca67ad6b6801b8a3458d7dc Mon Sep 17 00:00:00 2001
From: Trenton H <797416+stumpylog@users.noreply.github.com>
Date: Tue, 1 Oct 2024 13:48:28 -0700
Subject: [PATCH] Fixes some tests, and adds migration for StoragePaths

---
 src/documents/file_handling.py                |  4 +-
 .../1054_alter_storage_path_templates.py      | 53 +++++++++++
 src/documents/tests/test_file_handling.py     | 94 ++-----------------
 3 files changed, 62 insertions(+), 89 deletions(-)
 create mode 100644 src/documents/migrations/1054_alter_storage_path_templates.py

diff --git a/src/documents/file_handling.py b/src/documents/file_handling.py
index 38cf6c9a1..1d21b1358 100644
--- a/src/documents/file_handling.py
+++ b/src/documents/file_handling.py
@@ -195,7 +195,7 @@ def get_basic_metadata_context(
 
 def get_tags_context(tags: Iterable[Tag]) -> dict[str, str]:
     return {
-        "tags_list": pathvalidate.sanitize_filename(
+        "tag_list": pathvalidate.sanitize_filename(
             ",".join(
                 sorted(tag.name for tag in tags),
             ),
@@ -327,7 +327,7 @@ def generate_filename(
         )  # backward compatibility
 
         rendered_filename = (
-            rendered_filename.strip(os.sep).replace("\n", "").replace("\r", "")
+            rendered_filename.strip(os.sep).replace("\n", "").replace("\r", "").strip()
         )
 
         return rendered_filename
diff --git a/src/documents/migrations/1054_alter_storage_path_templates.py b/src/documents/migrations/1054_alter_storage_path_templates.py
new file mode 100644
index 000000000..4831d0dbb
--- /dev/null
+++ b/src/documents/migrations/1054_alter_storage_path_templates.py
@@ -0,0 +1,53 @@
+# Generated by Django 5.1.1 on 2024-10-01 20:42
+
+import re
+
+from django.db import migrations
+from django.db import transaction
+
+
+def convert_from_format_to_template(apps, schema_editor):
+    # TODO: Is there a signal to disable?  I don't want documents getting moved while this is running
+
+    StoragePath = apps.get_model("documents", "StoragePath")
+
+    def convert_to_django_template_format(old_format):
+        """
+        Converts old Python string format (with {}) to Django template style (with {{ }}),
+        while ignoring existing {{ ... }} placeholders.
+
+        :param old_format: The old style format string (e.g., "{title} by {author}")
+        :return: Converted string in Django Template style (e.g., "{{ title }} by {{ author }}")
+        """
+
+        # Step 1: Match placeholders with single curly braces but not those with double braces
+        pattern = r"(?<!\{)\{(\w*)\}(?!\})"  # Matches {var} but not {{var}}
+
+        # Step 2: Replace the placeholders with {{ var }} or {{ }}
+        def replace_with_django(match):
+            variable = match.group(1)  # The variable inside the braces
+            return f"{{{{ {variable} }}}}"  # Convert to {{ variable }}
+
+        # Apply the substitution
+        converted_format = re.sub(pattern, replace_with_django, old_format)
+
+        return converted_format
+
+    with transaction.atomic():
+        for storage_path in StoragePath.objects.all():
+            storage_path.path = convert_to_django_template_format(storage_path.path)
+            storage_path.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("documents", "1053_document_page_count"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            convert_from_format_to_template,
+            # This is a one way migration
+            migrations.RunPython.noop,
+        ),
+    ]
diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py
index 1890625a4..73a7698ee 100644
--- a/src/documents/tests/test_file_handling.py
+++ b/src/documents/tests/test_file_handling.py
@@ -290,88 +290,6 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         self.assertEqual(generate_filename(d1), "652 - the_doc.pdf")
         self.assertEqual(generate_filename(d2), "none - the_doc.pdf")
 
-    @override_settings(FILENAME_FORMAT="{tags[type]}")
-    def test_tags_with_underscore(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        # Add tag to document
-        document.tags.create(name="type_demo")
-        document.tags.create(name="foo_bar")
-        document.save()
-
-        # Ensure that filename is properly generated
-        self.assertEqual(generate_filename(document), "demo.pdf")
-
-    @override_settings(FILENAME_FORMAT="{tags[type]}")
-    def test_tags_with_dash(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        # Add tag to document
-        document.tags.create(name="type-demo")
-        document.tags.create(name="foo-bar")
-        document.save()
-
-        # Ensure that filename is properly generated
-        self.assertEqual(generate_filename(document), "demo.pdf")
-
-    @override_settings(FILENAME_FORMAT="{tags[type]}")
-    def test_tags_malformed(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        # Add tag to document
-        document.tags.create(name="type:demo")
-        document.tags.create(name="foo:bar")
-        document.save()
-
-        # Ensure that filename is properly generated
-        self.assertEqual(generate_filename(document), "none.pdf")
-
-    @override_settings(FILENAME_FORMAT="{tags[0]}")
-    def test_tags_all(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        # Add tag to document
-        document.tags.create(name="demo")
-        document.save()
-
-        # Ensure that filename is properly generated
-        self.assertEqual(generate_filename(document), "demo.pdf")
-
-    @override_settings(FILENAME_FORMAT="{tags[1]}")
-    def test_tags_out_of_bounds(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        # Add tag to document
-        document.tags.create(name="demo")
-        document.save()
-
-        # Ensure that filename is properly generated
-        self.assertEqual(generate_filename(document), "none.pdf")
-
-    @override_settings(FILENAME_FORMAT="{tags}")
-    def test_tags_without_args(self):
-        document = Document()
-        document.mime_type = "application/pdf"
-        document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED
-        document.save()
-
-        self.assertEqual(generate_filename(document), f"{document.pk:07}.pdf")
-
     @override_settings(FILENAME_FORMAT="{title} {tag_list}")
     def test_tag_list(self):
         doc = Document.objects.create(title="doc1", mime_type="application/pdf")
@@ -978,7 +896,7 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
             pk=2,
             checksum="2",
-            storage_path=StoragePath.objects.create(path="{asn} - {created}"),
+            storage_path=StoragePath.objects.create(path="{{asn}} - {{created}}"),
         )
         self.assertEqual(generate_filename(doc), "none - 2020-06-25.pdf")
 
@@ -1003,7 +921,9 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
             pk=2,
             checksum="2",
-            storage_path=StoragePath.objects.create(path="TestFolder/{asn}/{created}"),
+            storage_path=StoragePath.objects.create(
+                path="TestFolder/{{asn}}/{{created}}",
+            ),
         )
         self.assertEqual(generate_filename(doc), "TestFolder/2020-06-25.pdf")
 
@@ -1025,7 +945,7 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
             archive_serial_number=4,
             storage_path=StoragePath.objects.create(
                 name="sp1",
-                path="ThisIsAFolder/{asn}/{created}",
+                path="ThisIsAFolder/{{asn}}/{{created}}",
             ),
         )
         doc_b = Document.objects.create(
@@ -1036,7 +956,7 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
             checksum="abcde",
             storage_path=StoragePath.objects.create(
                 name="sp2",
-                path="SomeImportantNone/{created}",
+                path="SomeImportantNone/{{created}}",
             ),
         )
 
@@ -1072,7 +992,7 @@ class TestFilenameGeneration(DirectoriesMixin, TestCase):
             checksum="abcde",
             storage_path=StoragePath.objects.create(
                 name="sp2",
-                path="SomeImportantNone/{created}",
+                path="SomeImportantNone/{{created}}",
             ),
         )