diff --git a/docker-compose.yml b/docker-compose.yml index f9ac5d70..d674b5f6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -100,6 +100,7 @@ services: condition: service_healthy networks: - clam-net + - s3-net volumes: diff --git a/request_a_govuk_domain/request/admin/model_admins.py b/request_a_govuk_domain/request/admin/model_admins.py index e559fb6f..dd333c24 100644 --- a/request_a_govuk_domain/request/admin/model_admins.py +++ b/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}///", + 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'{link_text}') 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'{link_text}') - 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///", - 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) diff --git a/request_a_govuk_domain/request/db.py b/request_a_govuk_domain/request/db.py index 390277cd..4c342cd9 100644 --- a/request_a_govuk_domain/request/db.py +++ b/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) diff --git a/request_a_govuk_domain/request/management/commands/create_sample_data.py b/request_a_govuk_domain/request/management/commands/create_sample_data.py index 29371c38..5263ff0e 100644 --- a/request_a_govuk_domain/request/management/commands/create_sample_data.py +++ b/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]) diff --git a/request_a_govuk_domain/request/migrations/0009_alter_application_ministerial_request_evidence_and_more.py b/request_a_govuk_domain/request/migrations/0009_alter_application_ministerial_request_evidence_and_more.py new file mode 100644 index 00000000..a33c100c --- /dev/null +++ b/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="", + ), + ), + ] diff --git a/request_a_govuk_domain/request/models/application.py b/request_a_govuk_domain/request/models/application.py index 2fd337c6..728d55b4 100644 --- a/request_a_govuk_domain/request/models/application.py +++ b/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) diff --git a/request_a_govuk_domain/request/models/storage_util.py b/request_a_govuk_domain/request/models/storage_util.py index afed3532..14263f4a 100644 --- a/request_a_govuk_domain/request/models/storage_util.py +++ b/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() diff --git a/request_a_govuk_domain/request/templates/admin/clearable_file_input.html b/request_a_govuk_domain/request/templates/admin/clearable_file_input.html index 72128684..f513cb7d 100644 --- a/request_a_govuk_domain/request/templates/admin/clearable_file_input.html +++ b/request_a_govuk_domain/request/templates/admin/clearable_file_input.html @@ -1,6 +1,10 @@ -{% if widget.is_initial %}

{{ widget.initial_text }}: {{ widget.value }} (opens in new tab){% if not widget.required %} - - +{% if widget.is_initial %}

{{ widget.initial_text }}: + {{ widget.name }} + (opens in new tab){% if not widget.required %} + + {% endif %}
-{{ widget.input_text }}:{% endif %} -{% if widget.is_initial %}

{% endif %} + {{ widget.input_text }}:{% endif %} + +{% if widget.is_initial %}

{% endif %} diff --git a/request_a_govuk_domain/request/templates/confirm.html b/request_a_govuk_domain/request/templates/confirm.html index 54fdb1cf..36827bdc 100644 --- a/request_a_govuk_domain/request/templates/confirm.html +++ b/request_a_govuk_domain/request/templates/confirm.html @@ -100,7 +100,7 @@

Domain request details

{% gds_summary_list_row_value %} {% if registration_data.written_permission == "yes" %} Yes, evidence provided:
- {{ registration_data.exemption_file_original_filename }} + {{ registration_data.exemption_file_original_filename }} {% endif %} {% endgds_summary_list_row_value %} {% gds_summary_list_row_actions %} @@ -118,7 +118,7 @@

Domain request details

{% gds_summary_list_row_value %} {% if registration_data.written_permission == "yes" %} Yes, evidence provided:
- {{ registration_data.written_permission_file_original_filename }} + {{ registration_data.written_permission_file_original_filename }} {% else %} No {# This shouldn't normally happen #} {% endif %} @@ -152,7 +152,7 @@

Domain request details

No evidence provided {% else %} Yes, evidence provided:
- {{ registration_data.minister_file_original_filename }} + {{ registration_data.minister_file_original_filename }} {% endif %} {% endgds_summary_list_row_value %} {% gds_summary_list_row_actions %} diff --git a/request_a_govuk_domain/request/templates/exemption_upload_confirm.html b/request_a_govuk_domain/request/templates/exemption_upload_confirm.html index c2893179..a281f3e9 100644 --- a/request_a_govuk_domain/request/templates/exemption_upload_confirm.html +++ b/request_a_govuk_domain/request/templates/exemption_upload_confirm.html @@ -22,7 +22,7 @@

- {{ registration_data.exemption_file_original_filename }} + {{ registration_data.exemption_file_original_filename }}
diff --git a/request_a_govuk_domain/request/templates/minister_upload_confirm.html b/request_a_govuk_domain/request/templates/minister_upload_confirm.html index 415a7123..7f9338c6 100644 --- a/request_a_govuk_domain/request/templates/minister_upload_confirm.html +++ b/request_a_govuk_domain/request/templates/minister_upload_confirm.html @@ -22,7 +22,7 @@

- {{ registration_data.minister_file_original_filename }} + {{ registration_data.minister_file_original_filename }}
diff --git a/request_a_govuk_domain/request/templates/written_permission_upload_confirm.html b/request_a_govuk_domain/request/templates/written_permission_upload_confirm.html index 855e307d..bb10065b 100644 --- a/request_a_govuk_domain/request/templates/written_permission_upload_confirm.html +++ b/request_a_govuk_domain/request/templates/written_permission_upload_confirm.html @@ -22,7 +22,7 @@

- {{ registration_data.written_permission_file_original_filename }} + {{ registration_data.written_permission_file_original_filename }}
diff --git a/request_a_govuk_domain/request/views.py b/request_a_govuk_domain/request/views.py index d5cbcfb8..bc86a0be 100644 --- a/request_a_govuk_domain/request/views.py +++ b/request_a_govuk_domain/request/views.py @@ -4,9 +4,9 @@ from datetime import datetime from django.db import transaction +from django.http import HttpResponse, HttpRequest, FileResponse, HttpResponseNotFound from django.shortcuts import render, redirect from django.urls import reverse_lazy -from django.http import HttpResponse, HttpRequest from django.views import View from django.views.generic import TemplateView, RedirectView from django.views.generic.edit import FormView @@ -320,6 +320,14 @@ class UploadRemoveView(RedirectView): pattern_name = "" # to be subclassed def get_redirect_url(self, *args, **kwargs): + registration_data = self.request.session.get("registration_data", {}) + if path_to_delete := registration_data.get( + self.page_type + "_file_uploaded_filename" + ): + storage = select_storage() + if storage.exists(path_to_delete): + storage.delete(path_to_delete) + # delete the session data remove_from_session( self.request.session, @@ -329,6 +337,7 @@ def get_redirect_url(self, *args, **kwargs): self.page_type + "_file_uploaded_url", ], ) + return super().get_redirect_url(*args, **kwargs) @@ -500,6 +509,21 @@ def form_valid(self, form): return super().form_valid(form) +def download_file(request, file_type): + """ + Utility method to download a file which was temporarily uploaded during the application process. + :param request: Current request object + :param file_type: Type of file to be downloaded + :return: + """ + registration_data = request.session["registration_data"] + storage = select_storage() + file_name = registration_data[f"{file_type}_file_uploaded_filename"] + if storage.exists(file_name): + return FileResponse(storage.open(file_name, "rb")) + return HttpResponseNotFound("Not Found") + + class UploadView(FormView): page_type = "" template_name = "" diff --git a/request_a_govuk_domain/urls.py b/request_a_govuk_domain/urls.py index d8e0aef2..535c388f 100644 --- a/request_a_govuk_domain/urls.py +++ b/request_a_govuk_domain/urls.py @@ -54,6 +54,7 @@ security_txt_view, robots_txt_view, forbidden_view, + download_file, AccessibilityView, TermsAndConditionsView, ) @@ -178,6 +179,7 @@ ), path(".well-known/security.txt", security_txt_view), path("robots.txt", robots_txt_view), + path("download_file/", download_file, name="download_file"), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)