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 <[email protected]>
svthalia · Sep 20, 2024 · dd06471 · dd06471
1 parent 43eb7a6
commit dd06471
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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>