From 4b9a953a20d08c5ba0aec8b34356ac12b7b342ee Mon Sep 17 00:00:00 2001 From: imperosol Date: Thu, 23 Apr 2026 23:52:20 +0200 Subject: [PATCH 1/2] Automatically resize album thumbnail --- sas/forms.py | 44 ++++++++++++++++++++++++++++++---- sas/models.py | 22 ++++++----------- sas/templates/sas/macros.jinja | 8 +++---- sas/views.py | 8 +++---- 4 files changed, 54 insertions(+), 28 deletions(-) diff --git a/sas/forms.py b/sas/forms.py index 5f91477d..4621e8ba 100644 --- a/sas/forms.py +++ b/sas/forms.py @@ -1,18 +1,25 @@ +import copy import datetime -from typing import Any +from pathlib import Path +from typing import TYPE_CHECKING, Any from django import forms from django.core.exceptions import ValidationError from django.utils.timezone import get_current_timezone from django.utils.translation import gettext_lazy as _ +from PIL import Image from core.models import User +from core.utils import resize_image from core.views import MultipleImageField from core.views.forms import SelectDate from core.views.widgets.ajax_select import AutoCompleteSelectMultipleGroup from sas.models import Album, Picture, PictureModerationRequest from sas.widgets.ajax_select import AutoCompleteSelectAlbum +if TYPE_CHECKING: + from django.db.models.fields.files import FieldFile + class AlbumCreateForm(forms.ModelForm): class Meta: @@ -51,12 +58,9 @@ class AlbumEditForm(forms.ModelForm): class Meta: model = Album fields = ["name", "date", "file", "parent", "edit_groups"] - widgets = { - "edit_groups": AutoCompleteSelectMultipleGroup, - } + widgets = {"edit_groups": AutoCompleteSelectMultipleGroup, "date": SelectDate} name = forms.CharField(max_length=Album.NAME_MAX_LENGTH, label=_("file name")) - date = forms.DateField(label=_("Date"), widget=SelectDate, required=True) recursive = forms.BooleanField(label=_("Apply rights recursively"), required=False) parent = forms.ModelChoiceField( Album.objects.all(), required=True, widget=AutoCompleteSelectAlbum @@ -71,6 +75,36 @@ class AlbumEditForm(forms.ModelForm): tzinfo=get_current_timezone(), ) + def clean_file(self): + # if a file was given in the form, resize it + f: FieldFile = self.cleaned_data["file"] + if self.errors or not f or "file" not in self.changed_data: + return f + f.file = resize_image(Image.open(f.file), 200, "WEBP") + return f + + def save(self, commit=True): # noqa: FBT002 + initial_file = copy.copy(self.initial["file"]) + if not self.cleaned_data["file"]: + # if no file is in the form, it can mean either : + # - there was a file initially, but the deletion box was checked + # - there was no file initially, and there still isn't + # in both cases, we procedurally generate the thumbnail + self.instance.generate_thumbnail() + elif "file" in self.changed_data: + # the file was either added or modified + self.instance.file.name = str(Path(self.instance.name) / "thumb.webp") + res = super().save(commit=commit) + if initial_file and ( + not self.instance.file or initial_file.path != self.instance.file.path + ): + # The initial file must be removed from storage + # AFTER the new one has been dealt with, + # in order to be sure that django will generate a different filename. + # Otherwise, the client cache wouldn't be properly busted. + initial_file.delete(save=False) + return res + class PictureModerationRequestForm(forms.ModelForm): """Form to create a PictureModerationRequest. diff --git a/sas/models.py b/sas/models.py index 64f6c15b..d4981839 100644 --- a/sas/models.py +++ b/sas/models.py @@ -22,6 +22,7 @@ from typing import ClassVar, Self from django.conf import settings from django.core.cache import cache +from django.core.files.base import ContentFile from django.db import models from django.db.models import Exists, OuterRef, Q from django.urls import reverse @@ -110,7 +111,7 @@ class Picture(SasFile): def get_absolute_url(self): return reverse("sas:picture", kwargs={"picture_id": self.id}) - def generate_thumbnails(self, *, overwrite=False): + def generate_thumbnails(self): im = Image.open(BytesIO(self.file.read())) with contextlib.suppress(Exception): im = exif_auto_rotate(im) @@ -126,10 +127,6 @@ class Picture(SasFile): file = resize_image(im, max(im.size), extension, optimize=False) thumb = resize_image(im, 200, "webp") compressed = resize_image(im, 1200, "webp") - if overwrite: - self.file.delete() - self.thumbnail.delete() - self.compressed.delete() new_extension_name = str(Path(self.name).with_suffix(".webp")) self.file = file self.file.name = self.name @@ -245,17 +242,12 @@ class Album(SasFile): return reverse("sas:album_preview", kwargs={"album_id": self.id}) def generate_thumbnail(self): - p = ( - self.children_pictures.order_by("?").first() - or self.children_albums.exclude(file=None) - .exclude(file="") - .order_by("?") - .first() - ) - if p and p.file: - image = resize_image(Image.open(BytesIO(p.file.read())), 200, "webp") + p = self.children_pictures.order_by("?").first() + if p and p.thumbnail: + image = ContentFile( + name=str(Path(self.name) / "thumb.webp"), content=p.thumbnail.read() + ) self.file = image - self.file.name = f"{self.name}/thumb.webp" self.save() diff --git a/sas/templates/sas/macros.jinja b/sas/templates/sas/macros.jinja index 36f76584..e0c21bce 100644 --- a/sas/templates/sas/macros.jinja +++ b/sas/templates/sas/macros.jinja @@ -2,19 +2,19 @@ {% if a.file %} {% set img = a.get_download_url() %} - {% set src = a.name %} + {% set alt = a.name %} {% elif a.children.filter(is_folder=False, is_moderated=True).exists() %} {% set picture = a.children.filter(is_folder=False).first().as_picture %} {% set img = picture.get_download_thumb_url() %} - {% set src = picture.name %} + {% set alt = picture.name %} {% else %} {% set img = static('core/img/sas.jpg') %} - {% set src = "sas.jpg" %} + {% set alt = "sas.jpg" %} {% endif %}
- {{ src }} + {{ alt }} {% if not a.is_moderated %}
 
{% trans %}To be moderated{% endtrans %}
diff --git a/sas/views.py b/sas/views.py index 160182e4..7e7f0ba2 100644 --- a/sas/views.py +++ b/sas/views.py @@ -16,6 +16,7 @@ from typing import Any from django.conf import settings from django.contrib.auth.mixins import PermissionRequiredMixin +from django.core.exceptions import PermissionDenied from django.db.models import Count, OuterRef, Subquery from django.http import Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect @@ -152,10 +153,9 @@ class AlbumView(CanViewMixin, UseFragmentsMixin, DetailView): def post(self, request, *args, **kwargs): self.object = self.get_object() - if not self.object.file: - self.object.generate_thumbnail() - if request.user.can_edit(self.object): # Handle the copy-paste functions - FileView.handle_clipboard(request, self.object) + if not request.user.can_edit(self.object): + raise PermissionDenied + FileView.handle_clipboard(request, self.object) return HttpResponseRedirect(self.request.path) def get_fragment_data(self) -> dict[str, dict[str, Any]]: From facfa1be8950f97841ab5a7810f26e744805643d Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 26 Apr 2026 17:10:39 +0200 Subject: [PATCH 2/2] test thumbnail management of AlbumEditForm --- sas/tests/test_update_album.py | 218 +++++++++++++++++++++++++++++++++ sas/tests/test_views.py | 137 +-------------------- 2 files changed, 219 insertions(+), 136 deletions(-) create mode 100644 sas/tests/test_update_album.py diff --git a/sas/tests/test_update_album.py b/sas/tests/test_update_album.py new file mode 100644 index 00000000..cc3fbd7a --- /dev/null +++ b/sas/tests/test_update_album.py @@ -0,0 +1,218 @@ +import random +import string +from pathlib import Path +from typing import Callable +from unittest.mock import patch + +import pytest +from django.conf import settings +from django.core.files.base import ContentFile +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import Client +from django.urls import reverse +from django.utils.datastructures import MultiValueDict +from django.utils.timezone import localdate +from model_bakery import baker +from PIL import Image +from pytest_django.asserts import assertInHTML, assertRedirects + +from core.baker_recipes import subscriber_user +from core.models import Group, User +from core.utils import RED_PIXEL_PNG +from sas.baker_recipes import picture_recipe +from sas.forms import AlbumEditForm +from sas.models import Album + + +@pytest.fixture +def sas_root(db) -> Album: + return Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID) + + +@pytest.fixture +def album(db) -> Album: + name = "".join( + random.choice(string.ascii_letters) for _ in range(Album.NAME_MAX_LENGTH) + ) + return baker.make( + Album, name=name, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True + ) + + +@pytest.mark.parametrize("user", [None, lambda: baker.make(User), subscriber_user.make]) +@pytest.mark.django_db +def test_permission_denied( + client: Client, album: Album, user: Callable[[], User] | None +): + if user: + client.force_login(user()) + url = reverse("sas:album_edit", kwargs={"album_id": album.pk}) + for method in client.get, client.post: + assert method(url).status_code == 403 + + +@pytest.mark.django_db +def test_sas_root_read_only(client: Client, sas_root: Album): + moderator = baker.make( + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + ) + client.force_login(moderator) + url = reverse("sas:album_edit", kwargs={"album_id": sas_root.pk}) + for method in client.get, client.post: + assert method(url).status_code == 404 + + +@pytest.mark.parametrize( + ("excluded", "is_valid"), + [ + ("name", False), + ("date", False), + ("file", True), + ("parent", False), + ("edit_groups", True), + ("recursive", True), + ], +) +@pytest.mark.django_db +def test_form_required(album: Album, excluded: str, is_valid: bool): # noqa: FBT001 + data = { + "name": album.name, + "parent": baker.make(Album, parent=album.parent, is_moderated=True).pk, + "date": localdate(), + "file": "/random/path", + "edit_groups": [settings.SITH_GROUP_SAS_ADMIN_ID], + "recursive": False, + } + del data[excluded] + assert AlbumEditForm(data=data).is_valid() == is_valid + + +@pytest.mark.django_db +def test_form_album_name(album: Album): + data = { + "name": "a" * Album.NAME_MAX_LENGTH, + "parent": album.pk, + "date": localdate(), + } + assert AlbumEditForm(data=data).is_valid() + + data["name"] = "a" * (Album.NAME_MAX_LENGTH + 1) + assert not AlbumEditForm(data=data).is_valid() + + +@pytest.mark.django_db +def test_update_recursive_parent(client: Client, album: Album): + client.force_login(baker.make(User, is_superuser=True)) + + payload = {"name": album.name, "parent": album.pk, "date": localdate()} + response = client.post( + reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload + ) + assertInHTML("
  • Boucle dans l'arborescence des dossiers
    • ", response.text) + assert response.status_code == 200 + + +@pytest.mark.parametrize( + "user", + [ + lambda: baker.make(User, is_superuser=True), + lambda: baker.make( + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + ), + ], +) +@pytest.mark.parametrize( + "parent", + [ + lambda: baker.make( + Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True + ), + lambda: Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID), + ], +) +@pytest.mark.django_db +def test_update( + client: Client, + album: Album, + sas_root: Album, + user: Callable[[], User], + parent: Callable[[], Album], +): + client.force_login(user()) + expected_redirect = reverse("sas:album", kwargs={"album_id": album.pk}) + payload = { + "name": "foo", + "parent": parent().id, + "date": localdate(), + "recursive": False, + } + response = client.post( + reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload + ) + assertRedirects(response, expected_redirect) + album.refresh_from_db() + assert album.name == "foo" + assert album.parent.id == payload["parent"] + assert localdate(album.date) == localdate() + + +class TestAlbumThumbnail: + @pytest.fixture + def files(self): + return MultiValueDict( + {"file": [SimpleUploadedFile(name="foo.png", content=RED_PIXEL_PNG)]} + ) + + def test_thumbnail_resized(self, album, files): + """Test that album thumbnails are resized to the correct dimensions.""" + form = AlbumEditForm( + data={"name": album.name, "date": localdate(), "parent": album.parent.id}, + files=files, + instance=album, + ) + assert form.is_valid() + form.save() + album.refresh_from_db() + assert album.file.name == f"SAS/{album.name}/thumb.webp" + assert Image.open(album.file).size == (200, 200) + + def test_thumbnail_removed(self, album): + """Test the case where the user checks the box to remove the thumbnail""" + album.file = ContentFile(name="foo.png", content=RED_PIXEL_PNG) + album.save() + previous_filename = album.file.name + form = AlbumEditForm( + data={ + "name": "foo", + "date": localdate(), + "parent": album.parent.id, + "file-clear": True, + }, + instance=album, + ) + # as there is now no picture, a thumbnail should be generated + with patch.object(Album, "generate_thumbnail") as mock: + assert form.is_valid() + form.save() + album.refresh_from_db() + assert album.file.storage.exists(album.file.name) + assert not album.file.storage.exists(previous_filename) + mock.assert_called_once() + + def test_generate_thumbnail(self, album): + """Test that if no image is given and the album has pictures, + the thumbnail is automatically generated. + """ + picture = picture_recipe.make( + parent=album, thumbnail=ContentFile(name="foo.png", content=RED_PIXEL_PNG) + ) + form = AlbumEditForm( + data={"name": "foo", "date": localdate(), "parent": album.parent.id}, + instance=album, + ) + assert form.is_valid() + form.save() + album.refresh_from_db() + assert Path(album.file.name) == Path("SAS/foo/thumb.webp") + assert album.file.storage.exists(album.file.name) + assert Image.open(album.file) == Image.open(picture.thumbnail) diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py index 01f18d4b..57a69750 100644 --- a/sas/tests/test_views.py +++ b/sas/tests/test_views.py @@ -20,14 +20,12 @@ from django.conf import settings from django.core.cache import cache from django.test import Client, TestCase from django.urls import reverse -from django.utils.timezone import localdate from model_bakery import baker from pytest_django.asserts import assertHTMLEqual, assertInHTML, assertRedirects from core.baker_recipes import old_subscriber_user, subscriber_user from core.models import Group, User from sas.baker_recipes import picture_recipe -from sas.forms import AlbumEditForm from sas.models import Album, Picture # Create your tests here. @@ -97,6 +95,7 @@ def test_main_page_content_anonymous(client: Client): @pytest.mark.django_db def test_album_access_non_subscriber(client: Client): """Test that non-subscribers can only access albums where they are identified.""" + cache.clear() album = baker.make(Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID) user = baker.make(User) client.force_login(user) @@ -163,140 +162,6 @@ class TestAlbumUpload: assert not album.children.exists() -@pytest.mark.django_db -class TestAlbumEdit: - @pytest.fixture - def sas_root(self) -> Album: - return Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID) - - @pytest.fixture - def album(self) -> Album: - return baker.make( - Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True - ) - - @pytest.mark.parametrize( - "user", - [None, lambda: baker.make(User), subscriber_user.make], - ) - def test_permission_denied( - self, - client: Client, - album: Album, - user: Callable[[], User] | None, - ): - if user: - client.force_login(user()) - - url = reverse("sas:album_edit", kwargs={"album_id": album.pk}) - response = client.get(url) - assert response.status_code == 403 - response = client.post(url) - assert response.status_code == 403 - - def test_sas_root_read_only(self, client: Client, sas_root: Album): - moderator = baker.make( - User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] - ) - client.force_login(moderator) - url = reverse("sas:album_edit", kwargs={"album_id": sas_root.pk}) - response = client.get(url) - assert response.status_code == 404 - response = client.post(url) - assert response.status_code == 404 - - @pytest.mark.parametrize( - ("excluded", "is_valid"), - [ - ("name", False), - ("date", False), - ("file", True), - ("parent", False), - ("edit_groups", True), - ("recursive", True), - ], - ) - def test_form_required(self, album: Album, excluded: str, is_valid: bool): # noqa: FBT001 - data = { - "name": album.name[: Album.NAME_MAX_LENGTH], - "parent": baker.make(Album, parent=album.parent, is_moderated=True).pk, - "date": localdate(), - "file": "/random/path", - "edit_groups": [settings.SITH_GROUP_SAS_ADMIN_ID], - "recursive": False, - } - del data[excluded] - assert AlbumEditForm(data=data).is_valid() == is_valid - - def test_form_album_name(self, album: Album): - data = { - "name": album.name[: Album.NAME_MAX_LENGTH], - "parent": album.pk, - "date": localdate(), - } - assert AlbumEditForm(data=data).is_valid() - - data["name"] = album.name[: Album.NAME_MAX_LENGTH + 1] - assert not AlbumEditForm(data=data).is_valid() - - def test_update_recursive_parent(self, client: Client, album: Album): - client.force_login(baker.make(User, is_superuser=True)) - - payload = { - "name": album.name[: Album.NAME_MAX_LENGTH], - "parent": album.pk, - "date": localdate(), - } - response = client.post( - reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload - ) - assertInHTML("
  • Boucle dans l'arborescence des dossiers
    • ", response.text) - assert response.status_code == 200 - - @pytest.mark.parametrize( - "user", - [ - lambda: baker.make(User, is_superuser=True), - lambda: baker.make( - User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] - ), - ], - ) - @pytest.mark.parametrize( - "parent", - [ - lambda: baker.make( - Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True - ), - lambda: Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID), - ], - ) - def test_update( - self, - client: Client, - album: Album, - sas_root: Album, - user: Callable[[], User], - parent: Callable[[], Album], - ): - client.force_login(user()) - expected_redirect = reverse("sas:album", kwargs={"album_id": album.pk}) - payload = { - "name": album.name[: Album.NAME_MAX_LENGTH], - "parent": parent().id, - "date": localdate(), - "recursive": False, - } - response = client.post( - reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload - ) - assertRedirects(response, expected_redirect) - album.refresh_from_db() - assert album.name == payload["name"] - assert album.parent.id == payload["parent"] - assert localdate(album.date) == localdate() - - class TestSasModeration(TestCase): @classmethod def setUpTestData(cls):