Merge pull request #210 from co-cddo/feature/file-upload-s3-changes

Remove S3 urls from application UI

docker-compose.yml

@@ -100,6 +100,7 @@ services:
         condition: service_healthy
     networks:
       - clam-net
+      - s3-net
 
 
 volumes:

request_a_govuk_domain/request/admin/model_admins.py

@@ -22,6 +22,50 @@
     Registrar,
 )
 from .forms import ReviewForm
+from ..models.storage_util import s3_root_storage
+
+
+class FileDownloadMixin:
+    """
+    Provide file download support for the given admin class.
+    """
+
+    def download_file_view(self, request, object_id, field_name):
+        """
+        Implement this method to provide the specific retrieval of the object based on the given id
+        :param request: Current request object
+        :param object_id: id of the object containing the field
+        :param field_name: which file field to download from the object
+        :return:
+        """
+        pass
+
+    @property
+    def uid(self):
+        """
+        Unique id for the current subclass
+        :return:
+        """
+        pass
+
+    def get_urls(self):
+        """
+        Generate the custom url for the download_file_view.
+        :return:
+        """
+        urls = super().get_urls()
+        custom_urls = [
+            path(
+                f"download_{self.uid}/<int:object_id>/<str:field_name>/",
+                self.admin_site.admin_view(self.download_file_view),
+                name=f"{self.uid}_download_file",
+            ),
+        ]
+        return custom_urls + urls
+
+    def generate_download_link(self, obj, field_name, link_text):
+        link = reverse(f"admin:{self.uid}_download_file", args=[obj.pk, field_name])
+        return format_html(f'<a href="{link}" target="_blank">{link_text}</a>')
 
 
 class CustomAdminFileWidget(AdminFileWidget):
@@ -59,7 +103,7 @@ def convert_to_local_time(obj):
     )
 
 
-class ReviewAdmin(admin.ModelAdmin):
+class ReviewAdmin(FileDownloadMixin, admin.ModelAdmin):
     model = Review
     form = ReviewForm
     change_form_template = "admin/review_change_form.html"
@@ -81,26 +125,17 @@ class ReviewAdmin(admin.ModelAdmin):
         "application__registrant_org",
     )
 
-    def generate_download_link(self, obj, field_name, link_text):
-        link = reverse("admin:review_download_file", args=[obj.pk, field_name])
-        return format_html(f'<a href="{link}" target="_blank">{link_text}</a>')
-
     def download_file_view(self, request, object_id, field_name):
         review = self.model.objects.get(id=object_id)
         application = review.application
         file = getattr(application, field_name)
-        return FileResponse(file.open("rb"))
+        # We need to use root_storage as the default storage always
+        # refer to the TEMP_STORAGE_ROOT as the parent.
+        return FileResponse(s3_root_storage().open(file.name, "rb"))
 
-    def get_urls(self):
-        urls = super().get_urls()
-        custom_urls = [
-            path(
-                "download/<int:object_id>/<str:field_name>/",
-                self.admin_site.admin_view(self.download_file_view),
-                name="review_download_file",
-            ),
-        ]
-        return custom_urls + urls
+    @property
+    def uid(self):
+        return "review"
 
     def _get_formatted_display_fields(self, display_fields: dict) -> str:
         return render_to_string(
@@ -372,7 +407,7 @@ def has_add_permission(self, request):
         return False
 
 
-class ApplicationAdmin(admin.ModelAdmin):
+class ApplicationAdmin(FileDownloadMixin, admin.ModelAdmin):
     model = Application
     list_display = [
         "reference",
@@ -394,6 +429,15 @@ class ApplicationAdmin(admin.ModelAdmin):
         django.db.models.fields.files.FileField: {"widget": CustomAdminFileWidget},
     }
 
+    def download_file_view(self, request, object_id, field_name):
+        application = self.model.objects.get(id=object_id)
+        file = getattr(application, field_name)
+        return FileResponse(s3_root_storage().open(file.name, "rb"))
+
+    @property
+    def uid(self):
+        return "application"
+
     @admin.display(description="Time Submitted (UK time)")
     def time_submitted_local_time(self, obj):
         return convert_to_local_time(obj.time_submitted)

request_a_govuk_domain/request/db.py

@@ -16,10 +16,10 @@
     RegistryPublishedPerson,
     Review,
 )
+from .models.storage_util import select_storage
 
 from .utils import route_number, is_valid_session_data
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -46,7 +46,13 @@ def sanitised_registration_data(rd: dict, session_id: str) -> dict:
 
     def clear_upload(name: str) -> None:
         rd.pop(name, None)
-        rd.pop(f"{name}_file_uploaded_filename", None)
+        uploaded_file_name = rd.pop(f"{name}_file_uploaded_filename", None)
+        # Delete any temporary files that are no longer needed by the application
+        if uploaded_file_name:
+            storage = select_storage()
+            if storage.exists(uploaded_file_name):
+                storage.delete(uploaded_file_name)
+
         rd.pop(f"{name}_file_original_filename", None)
         rd.pop(f"{name}_file_uploaded_url", None)
 

request_a_govuk_domain/request/management/commands/create_sample_data.py

@@ -1,11 +1,15 @@
+import logging
 import os
 import shutil
 from datetime import datetime
+from pathlib import Path
 
 from django.core.management.base import BaseCommand
 from django.db import IntegrityError
-from request_a_govuk_domain.request import models
 
+from request_a_govuk_domain.request import models
+from request_a_govuk_domain.request.models.storage_util import select_storage
+from request_a_govuk_domain.settings import S3_STORAGE_ENABLED
 
 SCRIPT_PATH = os.path.dirname(os.path.abspath(__file__))
 SEED_DOCS_PATH = os.path.join(SCRIPT_PATH, "..", "..", "..", "..", "seed", "documents")
@@ -38,6 +42,7 @@
 POLICY_TEAM_EXEMPTION_FN = "policy_team_exception.png"
 
 DUMMY_REGISTRARS = ["WeRegister", "WeAlsoRegister", "WeLikeToRegister"]
+logger = logging.getLogger(__name__)
 
 
 def create_sample_application(
@@ -50,6 +55,23 @@ def create_sample_application(
     ministerial_request_file: str | None = None,
     policy_exemption_file: str | None = None,
 ):
+    # Copy the sample data to the temporary storage so the system will assume it is comping from the temporary
+    # location. This is needed as we have overridden the save method of the application to fetch the data
+    # from the TEMP_STORAGE_ROOT root location if we are using S3
+    if S3_STORAGE_ENABLED:
+        for f in [
+            written_permission_file,
+            ministerial_request_file,
+            policy_exemption_file,
+        ]:
+            logger.info("Copying seed file %s", f)
+            if f:
+                with open(
+                    Path(__file__).parent.joinpath(f"../../../media/{f}").resolve(),
+                    "rb",
+                ) as f_content:
+                    select_storage().save(f, f_content)
+
     registrant = models.Registrant.objects.create(name=registrant_name)
 
     registrant_person = models.RegistrantPerson.objects.create(name=person_names[0])

request_a_govuk_domain/request/migrations/0009_alter_application_ministerial_request_evidence_and_more.py

@@ -0,0 +1,46 @@
+# Generated by Django 4.2.13 on 2024-06-12 11:49
+
+from django.db import migrations, models
+import request_a_govuk_domain.request.models.storage_util
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("request", "0008_alter_registrant_type"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="application",
+            name="ministerial_request_evidence",
+            field=models.FileField(
+                blank=True,
+                max_length=255,
+                null=True,
+                storage=request_a_govuk_domain.request.models.storage_util.select_storage,
+                upload_to="",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="application",
+            name="policy_exemption_evidence",
+            field=models.FileField(
+                blank=True,
+                max_length=255,
+                null=True,
+                storage=request_a_govuk_domain.request.models.storage_util.select_storage,
+                upload_to="",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="application",
+            name="written_permission_evidence",
+            field=models.FileField(
+                blank=True,
+                max_length=255,
+                null=True,
+                storage=request_a_govuk_domain.request.models.storage_util.select_storage,
+                upload_to="",
+            ),
+        ),
+    ]

request_a_govuk_domain/request/models/application.py

@@ -1,11 +1,16 @@
-from django.utils.translation import gettext_lazy as _
-from django.db import models
+import logging
+
 from django.contrib.auth.models import User
-from .person import RegistryPublishedPerson, RegistrarPerson, RegistrantPerson
+from django.db import models
+from django.utils.translation import gettext_lazy as _
+
 from .organisation import Registrant, Registrar
-from .storage_util import select_storage
+from .person import RegistryPublishedPerson, RegistrarPerson, RegistrantPerson
+from .storage_util import select_storage, TEMP_STORAGE_ROOT
+from ...settings import S3_STORAGE_ENABLED
 
 REF_NUM_LENGTH = 17
+logger = logging.getLogger(__name__)
 
 
 class ApplicationStatus(models.TextChoices):
@@ -64,14 +69,57 @@ class Application(models.Model):
     registrant_org = models.ForeignKey(Registrant, on_delete=models.CASCADE)
     registrar_org = models.ForeignKey(Registrar, on_delete=models.CASCADE)
     written_permission_evidence = models.FileField(
-        null=True, blank=True, storage=select_storage
+        null=True,
+        blank=True,
+        storage=select_storage,
+        max_length=255,
     )
     ministerial_request_evidence = models.FileField(
-        null=True, blank=True, storage=select_storage
+        null=True,
+        blank=True,
+        storage=select_storage,
+        max_length=255,
     )
     policy_exemption_evidence = models.FileField(
-        null=True, blank=True, storage=select_storage
+        null=True,
+        blank=True,
+        storage=select_storage,
+        max_length=255,
     )
 
     def __str__(self):
         return f"{self.reference} - {self.domain_name}"
+
+    def save(
+        self, force_insert=False, force_update=False, using=None, update_fields=None
+    ):
+        """
+        When the application is saved, move the temporary files to the applications directory
+        :param force_insert:
+        :param force_update:
+        :param using:
+        :param update_fields:
+        :return:
+        """
+        if S3_STORAGE_ENABLED:
+            storage = select_storage()
+            # Move any temporary files in to the application specific folder
+            for file_field in [
+                self.policy_exemption_evidence,
+                self.ministerial_request_evidence,
+                self.written_permission_evidence,
+            ]:
+                if file_field and not file_field.name.startswith("applications"):
+                    from_path = TEMP_STORAGE_ROOT + file_field.name
+                    logger.info(
+                        "Copying temporary file to application folder %s", from_path
+                    )
+                    to_path = f"applications/{self.reference}/" + file_field.name
+                    storage.connection.meta.client.copy_object(
+                        Bucket=storage.bucket_name,
+                        CopySource=storage.bucket_name + "/" + from_path,
+                        Key=to_path,
+                    )
+                    storage.delete(from_path)
+                    file_field.name = to_path
+        super().save(force_insert, force_update, using, update_fields)

request_a_govuk_domain/request/models/storage_util.py

@@ -2,12 +2,26 @@
 from django.core.files.storage import FileSystemStorage
 from storages.backends.s3boto3 import S3Boto3Storage
 
+TEMP_STORAGE_ROOT = "temp_files/"
+
 
 def select_storage():
     """
     Utility method to select the storage type based on where the application is run.
-    When deploying to AWS, We need to set the IS_AWS to true by setting the environment variable
+    When deploying to AWS, We need to set the S3_STORAGE_ENABLED to true by setting the environment variable
     with the same name.
     :return:
     """
-    return S3Boto3Storage() if settings.S3_STORAGE_ENABLED else FileSystemStorage()
+    return (
+        S3Boto3Storage(location=TEMP_STORAGE_ROOT)
+        if settings.S3_STORAGE_ENABLED
+        else FileSystemStorage()
+    )
+
+
+def s3_root_storage():
+    """
+    Storage instance that allows access from the root level on S3
+    :return:
+    """
+    return S3Boto3Storage()

request_a_govuk_domain/request/templates/admin/clearable_file_input.html

@@ -1,6 +1,10 @@
-{% if widget.is_initial %}<p class="file-upload">{{ widget.initial_text }}: <a target="_blank" href="{{ widget.value.url }}">{{ widget.value }} (opens in new tab)</a>{% if not widget.required %}
-<span class="clearable-file-input">
-<input type="checkbox" name="{{ widget.checkbox_name }}" id="{{ widget.checkbox_id }}"{% if widget.attrs.disabled %} disabled{% endif %}>
+{% if widget.is_initial %}<p class="file-upload">{{ widget.initial_text }}:
+    <a target="_blank" href="/admin/request/application/download_application/{{ widget.value.instance.id }}/{{ widget.name }}/">{{ widget.name }}
+        (opens in new tab)</a>{% if not widget.required %}
+    <span class="clearable-file-input">
+<input type="checkbox" name="{{ widget.checkbox_name }}" id="{{ widget.checkbox_id }}"{% if widget.attrs.disabled %}
+       disabled{% endif %}>
 <label for="{{ widget.checkbox_id }}">{{ widget.clear_checkbox_label }}</label></span>{% endif %}<br>
-{{ widget.input_text }}:{% endif %}
-<input type="{{ widget.type }}" name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>{% if widget.is_initial %}</p>{% endif %}
+    {{ widget.input_text }}:{% endif %}
+<input type="{{ widget.type }}" name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>
+{% if widget.is_initial %}</p>{% endif %}