From dd06471e4b7eec7c446890cfbf941a42989ba3fd Mon Sep 17 00:00:00 2001
From: Dirk Doesburg <dirk.doesburg@gmail.com>
Date: Fri, 20 Sep 2024 13:16:19 +0200
Subject: [PATCH] Fix album processed email warnings (#3792)

* Fix album processed email warnings

This prevents warning about directories existing in the uploaded archive, and fixes  crash because of a bug in the email template.

* Improve robustness a bit more

---------

Co-authored-by: Ties Dirksen <tiesdirksen@gmail.com>
---
 website/photos/services.py                    |  5 ++
 website/photos/tasks.py                       | 49 ++++++++++---------
 .../photos/email/upload-processed.txt         |  2 +-
 3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/website/photos/services.py b/website/photos/services.py
index 2a8e7a5fe..55a353122 100644
--- a/website/photos/services.py
+++ b/website/photos/services.py
@@ -71,6 +71,8 @@ def extract_archive(album, archive) -> tuple[dict[str, str], int]:
         archive.seek(0)
         with ZipFile(archive) as zip_file:
             for photo in sorted(zip_file.namelist()):
+                if zip_file.getinfo(photo).is_dir():
+                    continue
                 if not _has_photo_extension(photo):
                     warnings[photo] = "has an unknown extension."
                     continue
@@ -87,6 +89,9 @@ def extract_archive(album, archive) -> tuple[dict[str, str], int]:
     try:
         with tarfile.open(fileobj=archive) as tar_file:
             for photo in sorted(tar_file.getnames()):
+                if not tar_file.getmember(photo).isfile():
+                    continue
+
                 if not _has_photo_extension(photo):
                     warnings[photo] = "has an unknown extension."
                     continue
diff --git a/website/photos/tasks.py b/website/photos/tasks.py
index 8b696a696..e848acc83 100644
--- a/website/photos/tasks.py
+++ b/website/photos/tasks.py
@@ -25,48 +25,51 @@ def process_album_upload(
     archive_upload_id: str, album_id: int, uploader_id: int | None = None
 ):
     upload = TemporaryUpload.objects.get(upload_id=archive_upload_id)
-    archive = upload.file
 
     try:
         album = Album.objects.get(id=album_id)
     except Album.DoesNotExist:
         logger.exception("Album %s does not exist", album_id)
-        archive.delete()
+        upload.file.delete()
         upload.delete()
 
-    uploader = Member.objects.get(id=uploader_id) if uploader_id is not None else None
+    uploader = (
+        Member.objects.filter(id=uploader_id).first()
+        if uploader_id is not None
+        else None
+    )
 
     try:
         with transaction.atomic():
             # We make the upload atomic separately, so we can keep using the db if it fails.
             # See https://docs.djangoproject.com/en/4.2/topics/db/transactions/#handling-exceptions-within-postgresql-transactions.
-            warnings, count = extract_archive(album, archive)
+            warnings, count = extract_archive(album, upload.file)
             album.is_processing = False
             album.save()
 
-            # Send signal to notify that an album has been uploaded. This is used
-            # by facedetection, and possibly in the future to notify the uploader.
-            album_uploaded.send(sender=None, album=album)
-
-            if uploader is not None:
-                # Notify uploader of the upload result.
-                send_email(
-                    to=get_member_email_addresses(uploader),
-                    subject=("Album upload processed completed."),
-                    txt_template="photos/email/upload-processed.txt",
-                    context={
-                        "name": uploader.first_name,
-                        "album": album,
-                        "upload_name": upload.file.name,
-                        "warnings": warnings,
-                        "num_processed": count,
-                    },
-                )
+        # Send signal to notify that an album has been uploaded. This is used
+        # by facedetection, and possibly in the future to notify the uploader.
+        album_uploaded.send(sender=None, album=album)
+
+        if uploader is not None:
+            # Notify uploader of the upload result.
+            send_email(
+                to=get_member_email_addresses(uploader),
+                subject=("Album upload processed completed."),
+                txt_template="photos/email/upload-processed.txt",
+                context={
+                    "name": uploader.first_name,
+                    "album": album,
+                    "upload_name": upload.upload_name,
+                    "warnings": warnings,
+                    "num_processed": count,
+                },
+            )
     except Exception as e:
         logger.exception(f"Failed to process album upload: {e}", exc_info=e)
 
     finally:
-        archive.delete()
+        upload.file.delete()
         upload.delete()
 
 
diff --git a/website/photos/templates/photos/email/upload-processed.txt b/website/photos/templates/photos/email/upload-processed.txt
index 6820b643d..cf857db02 100644
--- a/website/photos/templates/photos/email/upload-processed.txt
+++ b/website/photos/templates/photos/email/upload-processed.txt
@@ -5,7 +5,7 @@ You can see it here: {% url "photos:album" album.slug %}
 
 {{ num_processed }} photos were successfully processed.
 {% if warnings %}
-The following warnings occurred while processing the upload:{% for file, warning in warnings%}
+The following warnings occurred while processing the upload:{% for file, warning in warnings.items %}
 {{ file }}: {{ warning }}
 {% endfor %}{% else %}
 There were no issues while processing the upload.</p>