SYS-1598: Allow deletion of media files (#125)

* add delete link for derivative files
* deletion logic, tests, visual updates, and deployment info
* fix file deletion for child MediaFiles
* update wording on popup buttons
* fix file deletion logic
* add deletion logging and refactor deletion method
* tweaked missing file warning log message

charts/prod-ohstaff-values.yaml

@@ -6,7 +6,7 @@ replicaCount: 1
 
 image:
   repository: uclalibrary/oral-history-staff-ui
-  tag: v1.1.5a
+  tag: v1.1.6
   pullPolicy: Always
 
 nameOverride: ""

oh_staff_ui/templates/oh_staff_ui/release_notes.html

@@ -4,6 +4,12 @@
 <h3>Release Notes</h3>
 <hr/>
 
+<h4>1.1.6</h4>
+<p><i>May 22, 2024</i></p>
+<ul>
+   <li>Added ability to delete files from item records.</li>
+</ul>
+
 <h4>1.1.5</h4>
 <p><i>May 21, 2024</i></p>
 <ul>
@@ -139,4 +145,4 @@ <h4>1.0.0</h4>
     </li>
 </ul>
 
-{% endblock %}
+{% endblock %}

oh_staff_ui/templates/oh_staff_ui/upload_file.html

@@ -35,7 +35,6 @@
       </td>
   </tr>
 </table>
-
 {% if files %}
 <hr>
 <table class="header-metadata caption-top">
@@ -47,8 +46,25 @@
     <th>New name & location</th>
     <th>Size</th>
     <th>Sequence</th>
+    {% if staff_status %}
+    <th>Actions</th>
+    {% endif %}
   </tr>
   {% for file in files %}
+  <div class = "confirm-delete-popup" id="confirm-delete-popup-{{file.id}}">
+    <h3>Delete file?</h3>
+    <p>Are you sure you want to delete this file: {{file.file_name_only}}?</p>
+    {% if file.children %}
+    <p>This file has derivative files. Deleting this file will also delete the following files:</p>
+    <ul>
+      {% for child in file.children %}
+      <li>{{ child.file_name_only }}</li>
+      {% endfor %}
+    </ul>
+    {% endif %}
+    <a href="#" class="btn btn-primary" id="cancel-{{file.id}}" onclick="hideConfirmDeletePopup('{{file.id}}')">Cancel</a>
+    <a href="{% url 'delete_file' file.id %}" class="btn btn-secondary">Delete</a> 
+  </div>
   <tr>
     <td>{{ file.file_type }}</td>
     <td>{{ file.create_date }} by {{ file.created_by }}</td>
@@ -62,6 +78,11 @@
     </td>
     <td>{{ file.file_size|floatformat:"g" }} bytes</td>
     <td>{{ file.sequence }}</td>
+    {% if staff_status %}  
+    <td>
+      <button class="delete-link btn btn-primary" onclick="showConfirmDeletePopup('{{ file.id }}')">Delete</button>
+    </td>
+    {% endif %}
   </tr>
   {% endfor %}
 </table>
@@ -125,4 +146,5 @@ <h4>Please fix the following errors</h4>
   </table>
     {% bootstrap_button button_type="submit" content="Upload" %}
 </form>
+
 {% endblock %}

oh_staff_ui/tests.py

@@ -57,6 +57,7 @@
     get_records_oai,
     get_bad_arg_error_xml,
     get_bad_verb_error_xml,
+    delete_file_and_children,
 )
 from oh_staff_ui.management.commands.reprocess_derivative_images import (
     reprocess_derivative_images,
@@ -1709,3 +1710,130 @@ def tearDown(self) -> None:
         # delete MediaFile objects starting with the parent - CASCADE will delete children
         for parent_mf in media_files.filter(parent__isnull=True):
             parent_mf.delete()
+
+
+class FileDeletionTestCase(TestCase):
+    fixtures = [
+        "item-status-data.json",
+        "item-type-data.json",
+        "media-file-type-data.json",
+    ]
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user("tester")
+        cls.mock_request = HttpRequest()
+        cls.mock_request.user = User.objects.get(username=cls.user.username)
+
+    def create_master_and_derivatives(self, user, mock_request):
+        item = ProjectItem.objects.create(
+            ark="fake/abcdef",
+            created_by=user,
+            last_modified_by=user,
+            title="Fake title",
+            type=ItemType.objects.get(type="Audio"),
+        )
+        file_type = MediaFileType.objects.get(file_code="image_master")
+        master_image_file = OralHistoryFile(
+            item.id,
+            "samples/sample_marbles.tif",
+            file_type,
+            "master",
+            mock_request,
+        )
+
+        handler = ImageFileHandler(master_image_file)
+        handler.process_files()
+
+        # get created date from derivative images, so we can check if they are updated
+        thumbnail_create_date = MediaFile.objects.get(
+            item=item, file_type__file_code="image_thumbnail"
+        ).create_date
+        submaster_create_date = MediaFile.objects.get(
+            item=item, file_type__file_code="image_submaster"
+        ).create_date
+
+        return item, thumbnail_create_date, submaster_create_date
+
+    def test_delete_file_and_children(self):
+        self.item, self.thumbnail_created_date, self.submaster_created_date = (
+            self.create_master_and_derivatives(self.user, self.mock_request)
+        )
+
+        # check that the thumbnail and submaster images are created
+        self.assertTrue(MediaFile.objects.filter(item=self.item).count() == 3)
+
+        # check that the thumbnail and submaster images exist on disk
+        thumbnail_path = thumbnail_path = Path(
+            f"{settings.MEDIA_ROOT}/oh_static/nails/fake-abcdef-1-thumbnail.jpg"
+        )
+        submaster_path = Path(
+            f"{settings.MEDIA_ROOT}/oh_static/submasters/fake-abcdef-1-submaster.jpg"
+        )
+        self.assertTrue(thumbnail_path.is_file())
+        self.assertTrue(submaster_path.is_file())
+
+        # get our master Mediafile
+        image_mediafile = MediaFile.objects.get(
+            item=self.item, file_type__file_code="image_master"
+        )
+        # delete Master and derivative files
+        delete_file_and_children(image_mediafile, self.user)
+
+        # check that all 3 MediaFiles are deleted
+        self.assertFalse(MediaFile.objects.filter(item=self.item).exists())
+        # check that the files on disk are deleted
+        self.assertFalse(thumbnail_path.is_file())
+        self.assertFalse(submaster_path.is_file())
+
+    def test_delete_file_and_children_no_children(self):
+        self.item = ProjectItem.objects.create(
+            ark="fake/abcdef",
+            created_by=self.user,
+            last_modified_by=self.user,
+            title="Fake title",
+            type=ItemType.objects.get(type="Audio"),
+        )
+        file_type = MediaFileType.objects.get(file_code="image_master")
+        master_image_file = OralHistoryFile(
+            self.item.id,
+            "samples/sample_marbles.tif",
+            file_type,
+            "master",
+            self.mock_request,
+        )
+
+        handler = ImageFileHandler(master_image_file)
+        handler.process_files()
+
+        # check that the thumbnail and submaster images are created
+        self.assertTrue(MediaFile.objects.filter(item=self.item).count() == 3)
+
+        submaster_mediafile = MediaFile.objects.get(
+            item=self.item, file_type__file_code="image_submaster"
+        )
+        # delete files
+        delete_file_and_children(submaster_mediafile, self.user)
+
+        # check that master and thumbnail files are still there (in DB and on disk)
+        self.assertTrue(MediaFile.objects.filter(item=self.item).count() == 2)
+        thumbnail_path = Path(
+            f"{settings.MEDIA_ROOT}/oh_static/nails/fake-abcdef-1-thumbnail.jpg"
+        )
+        self.assertTrue(thumbnail_path.is_file())
+
+        # check that the submaster file is deleted
+        submaster_path = Path(
+            f"{settings.MEDIA_ROOT}/oh_static/submasters/fake-abcdef-1-submaster.jpg"
+        )
+        self.assertFalse(submaster_path.is_file())
+
+    def tearDown(self) -> None:
+        media_files = MediaFile.objects.filter(item=self.item)
+        # delete files on disk first
+        for mf in media_files:
+            if mf.file:
+                mf.file.delete()
+        # delete MediaFile objects starting with the parent - CASCADE will delete children
+        for parent_mf in media_files.filter(parent__isnull=True):
+            parent_mf.delete()

oh_staff_ui/urls.py

@@ -17,6 +17,7 @@
     path("logs/", views.show_log, name="show_log"),
     path("logs/<int:line_count>", views.show_log, name="show_log"),
     path("upload_file/<int:item_id>", views.upload_file, name="upload_file"),
+    path("delete_file/<int:file_id>", views.delete_file, name="delete_file"),
     path("order_files/<int:item_id>", views.order_files, name="order_files"),
     path("browse/", views.browse, name="browse"),
     # Allow access to download media files via built-in view django.views.static.serve()

oh_staff_ui/views.py

@@ -14,6 +14,7 @@
 )
 from oh_staff_ui.models import MediaFile, MediaFileError, ProjectItem
 from oh_staff_ui.views_utils import (
+    delete_file_and_children,
     get_ark,
     get_edit_item_context,
     get_all_series_and_interviews,
@@ -25,6 +26,7 @@
     get_records_oai,
     get_bad_arg_error_xml,
     get_bad_verb_error_xml,
+    user_in_oh_staff_group,
 )
 
 logger = logging.getLogger(__name__)
@@ -135,7 +137,12 @@ def upload_file(request: HttpRequest, item_id: int) -> HttpResponse:
     files = MediaFile.objects.filter(item=item).order_by(
         "sequence", "file_type__file_code"
     )
+    # add "children" attribute to each file to hold its derivatives
+    # this is used in the template to display child files before deletion
+    for file in files:
+        file.children = list(MediaFile.objects.filter(parent=file))
     file_errors = MediaFileError.objects.filter(item=item).order_by("create_date")
+    staff_status = user_in_oh_staff_group(request.user)
     if request.method == "POST":
         # Pass item_id and request to submitted form to help with validation.
         form = FileUploadForm(request.POST, item_id=item_id, request=request)
@@ -174,10 +181,24 @@ def upload_file(request: HttpRequest, item_id: int) -> HttpResponse:
             return redirect("upload_file", item_id=item_id)
     else:
         form = FileUploadForm()
-    context = {"item": item, "files": files, "file_errors": file_errors, "form": form}
+    context = {
+        "staff_status": staff_status,
+        "item": item,
+        "files": files,
+        "file_errors": file_errors,
+        "form": form,
+    }
     return render(request, "oh_staff_ui/upload_file.html", context)
 
 
+@login_required
+def delete_file(request: HttpRequest, file_id: int) -> HttpResponse:
+    media_file = MediaFile.objects.get(pk=file_id)
+    item_id = media_file.item.pk
+    delete_file_and_children(media_file, request.user)
+    return redirect("upload_file", item_id=item_id)
+
+
 @login_required
 def order_files(request: HttpRequest, item_id: int) -> HttpResponse:
     item = ProjectItem.objects.get(pk=item_id)

oh_staff_ui/views_utils.py

@@ -5,10 +5,12 @@
 from lxml import etree
 import requests
 import uuid
+from pathlib import Path
 from django.conf import settings
 from django.contrib import messages
 from django.core.management import call_command
 from django.db.models import CharField, Model, Q, QuerySet
+from django.contrib.auth.models import User
 from django.forms import BaseFormSet, Form, formset_factory
 from django.http.request import HttpRequest  # for code completion
 from django.utils import timezone
@@ -34,6 +36,7 @@
     Date,
     Description,
     Format,
+    MediaFile,
     Name,
     ProjectItem,
     Subject,
@@ -608,3 +611,31 @@ def add_oai_envelope_to_mods(ohmods: OralHistoryMods) -> etree.Element:
     record_el.append(metadata_el)
 
     return record_el
+
+
+def user_in_oh_staff_group(user: User) -> bool:
+    return user.groups.filter(name="Oral History Staff").exists()
+
+
+def delete_file_and_children(media_file: MediaFile, user: User) -> None:
+    # if file has child files, delete them first
+    children = MediaFile.objects.filter(parent=media_file)
+    for child in children:
+        delete_file(child, user)
+    # now delete the parent file
+    delete_file(media_file, user)
+
+
+def delete_file(media_file: MediaFile, user: User) -> None:
+    # first delete file from file system
+    # check for file existence with Path before attempting to delete
+    file_name = media_file.file.name
+    file_path = Path(settings.MEDIA_ROOT).joinpath(file_name)
+    if file_path.exists():
+        media_file.file.delete()
+    else:
+        logger.warning(
+            f"File {file_name} does not exist on the file system; deleting media object anyhow."
+        )
+    logger.info(f"File {file_name} deleted by user {user}.")
+    media_file.delete()