diff --git a/website/photos/admin.py b/website/photos/admin.py index eae05478a..42e1e182d 100644 --- a/website/photos/admin.py +++ b/website/photos/admin.py @@ -74,7 +74,11 @@ def save_model(self, request, obj, form, change): # function call. In a real deployment, it has to be called only when the current db # transaction is committed, transaction.on_commit( - process_album_upload.s(archive.temporary_upload.upload_id, obj.id).delay + process_album_upload.s( + archive.temporary_upload.upload_id, + obj.id, + uploader_id=request.user.id, + ).delay ) self.message_user( diff --git a/website/photos/services.py b/website/photos/services.py index 40e1d52e3..2db277206 100644 --- a/website/photos/services.py +++ b/website/photos/services.py @@ -64,18 +64,23 @@ def get_annotated_accessible_albums(request, albums): return albums -def extract_archive(album, archive): +def extract_archive(album, archive) -> tuple[dict[str, str], int]: """Extract zip and tar files.""" + warnings, count = {}, 0 if is_zipfile(archive): archive.seek(0) with ZipFile(archive) as zip_file: for photo in sorted(zip_file.namelist()): if not _has_photo_extension(photo): + warnings[photo] = "has an unknown extension." continue with zip_file.open(photo) as file: - _try_save_photo(album, file, photo) - return + if warning := _try_save_photo(album, file, photo): + warnings[photo] = warning + else: + count += 1 + return warnings, count archive.seek(0) # is_tarfile only supports filenames, so we cannot use that @@ -83,13 +88,19 @@ def extract_archive(album, archive): with tarfile.open(fileobj=archive) as tar_file: for photo in sorted(tar_file.getnames()): if not _has_photo_extension(photo): + warnings[photo] = "has an unknown extension." continue with tar_file.extractfile(photo) as file: - _try_save_photo(album, file, photo) + if warning := _try_save_photo(album, file, photo): + warnings[photo] = warning + else: + count += 1 except tarfile.ReadError as e: raise ValueError(_("The uploaded file is not a zip or tar file.")) from e + return warnings, count + def _has_photo_extension(filename): """Check if the filename has a photo extension.""" @@ -97,15 +108,23 @@ def _has_photo_extension(filename): return extension.lower() in (".jpg", ".jpeg", ".png", ".webp") -def _try_save_photo(album, file, filename): - """Try to save a photo to an album.""" +def _try_save_photo(album, file, filename) -> str | None: + """Try to save a photo to an album. + + Returns None, or a string describing a reason for failure. + """ instance = Photo(album=album) instance.file = File(file, filename) try: with transaction.atomic(): instance.full_clean() instance.save() - except ValidationError: - pass - except UnidentifiedImageError: - pass + except ValidationError as e: + logger.warning(f"Photo '{filename}' could not be read: {e}", exc_info=e) + return "could not be read." + except UnidentifiedImageError as e: + logger.warning(f"Photo '{filename}' could not be read: {e}", exc_info=e) + return "could not be read." + except OSError as e: + logger.warning(f"Photo '{filename}' could not be read: {e}", exc_info=e) + return "could not be read." diff --git a/website/photos/tasks.py b/website/photos/tasks.py index d58689f99..4067e9c2e 100644 --- a/website/photos/tasks.py +++ b/website/photos/tasks.py @@ -1,36 +1,67 @@ +import logging + from django.db import transaction from django.dispatch import Signal from celery import shared_task from django_drf_filepond.models import TemporaryUpload +from mailinglists.services import get_member_email_addresses +from members.models.member import Member from photos.models import Album +from utils.snippets import send_email from .services import extract_archive +logger = logging.getLogger(__name__) album_uploaded = Signal() @shared_task -def process_album_upload(archive_upload_id: str, album_id: int): +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: - return + logger.exception("Album %s does not exist", album_id) + archive.delete() + upload.delete() + + uploader = Member.objects.get(id=uploader_id) if uploader_id is not None else None - upload = TemporaryUpload.objects.get(upload_id=archive_upload_id) - archive = upload.file 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. - extract_archive(album, archive) + warnings, count = extract_archive(album, archive) 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, + }, + ) + except Exception as e: + logger.exception(f"Failed to process album upload: {e}", exc_info=e) + finally: archive.delete() upload.delete() diff --git a/website/photos/templates/photos/emails/upload-processed.txt b/website/photos/templates/photos/emails/upload-processed.txt new file mode 100644 index 000000000..6820b643d --- /dev/null +++ b/website/photos/templates/photos/emails/upload-processed.txt @@ -0,0 +1,16 @@ +Hi {{ name }}, + +You have recently uploaded an archive ('{{ upload_name }}') to this album: {{ album }} +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%} +{{ file }}: {{ warning }} +{% endfor %}{% else %} +There were no issues while processing the upload.

+{% endif %} + +Kisses, + +The website