diff --git a/request_a_govuk_domain/request/models/application.py b/request_a_govuk_domain/request/models/application.py index 84a494e0..20aa62a6 100644 --- a/request_a_govuk_domain/request/models/application.py +++ b/request_a_govuk_domain/request/models/application.py @@ -1,4 +1,5 @@ import logging +import re from django.contrib.auth.models import User from django.core.files.uploadedfile import InMemoryUploadedFile @@ -105,6 +106,8 @@ def save( if S3_STORAGE_ENABLED: storage = select_storage() # Move any temporary files in to the application specific folder + # We have to do this custom moving because we store the initially uploaded + # files in the temp location. for file_field in [ self.policy_exemption_evidence, self.ministerial_request_evidence, @@ -126,7 +129,9 @@ def save( Body=file_field.file.read(), ) - to_path = f"applications/{self.reference}/" + file_field.name + to_path = f"applications/{self.reference}/" + re.sub( + r"[^A-Za-z0-9.]+", "_", file_field.name + ) logger.info( "Copying temporary file %s to application folder %s", from_path, diff --git a/tests/test_model_admins.py b/tests/test_model_admins.py index 7792e7c3..dff46d77 100644 --- a/tests/test_model_admins.py +++ b/tests/test_model_admins.py @@ -6,6 +6,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase, Client from django.urls import reverse +from parameterized import parameterized from request_a_govuk_domain.request import db from request_a_govuk_domain.request.admin.model_admins import convert_to_local_time @@ -56,7 +57,17 @@ def test_gmt_time_is_converted_correctly(self): ) self.assertEqual("01 Nov 2024 00:00:00 AM", convert_to_local_time(gmt_date)) - def test_file_is_uploaded_from_admin_screen(self): + @parameterized.expand( + [ + ["new_document.pdf", "new_document.pdf"], + ["new document.pdf", "new_document.pdf"], + [ + "new -document _with more ' spaces.pdf", + "new_document_with_more_spaces.pdf", + ], + ] + ) + def test_file_is_uploaded_from_admin_screen(self, file_name, expected_name): request = Mock() request.session = SessionDict({"registration_data": self.registration_data}) app = db.save_data_in_database("ABCDEFGHIJK", request) @@ -64,7 +75,7 @@ def test_file_is_uploaded_from_admin_screen(self): c.login(username="superuser", password="secret") # pragma: allowlist secret written_permission_evidence = SimpleUploadedFile( - "new_document.pdf", b"file_content", content_type="application/pdf" + file_name, b"file_content", content_type="application/pdf" ) with patch( "request_a_govuk_domain.request.models.application.S3_STORAGE_ENABLED", True @@ -98,15 +109,15 @@ def test_file_is_uploaded_from_admin_screen(self): [ call.connection.meta.client.put_object( Bucket="mock-data-bucket", - Key="temp_files/new_document.pdf", + Key=f"temp_files/{file_name}", Body=b"file_content", ), call.connection.meta.client.copy_object( Bucket="mock-data-bucket", - CopySource="mock-data-bucket/temp_files/new_document.pdf", - Key="applications/ABCDEFGHIJK/new_document.pdf", + CopySource=f"mock-data-bucket/temp_files/{file_name}", + Key=f"applications/ABCDEFGHIJK/{expected_name}", ), - call.delete("temp_files/new_document.pdf"), + call.delete(f"temp_files/{file_name}"), ] ) self.assertEqual(response.status_code, 200)