test thumbnail management of AlbumEditForm

club/templates/club/club_detail.jinja

@@ -26,9 +26,10 @@
     {% if club.logo %}
       <div class="club_logo"><img src="{{ club.logo.url }}" alt="{{ club.name }}"></div>
     {% endif %}
-    <h3>{{ club.name }}</h3>
     {% if page_revision %}
       {{ page_revision|markdown }}
+    {% else %}
+      <h3>{{ club.name }}</h3>
     {% endif %}
   </div>
 {% endblock %}

com/static/com/css/news-list.scss

@@ -3,7 +3,6 @@
 
 #news {
   display: flex;
-  gap: 1em;
 
   @media (max-width: 800px) {
     flex-direction: column;
@@ -27,14 +26,12 @@
   }
 
   h3 {
-    --box-shadow: rgb(60 64 67 / 30%) 0 1px 3px 0, rgb(60 64 67 / 15%) 0 3px 7px 2px;
+    background: $second-color;
-    background: lighten($second-color, 5%);
+    box-shadow: $shadow-color 1px 1px 1px;
-    box-shadow: var(--box-shadow);
+    padding: 0.4em;
-    padding: .75rem;
     margin: 0 0 0.5em 0;
     text-transform: uppercase;
     font-size: 17px;
-    border-radius: 10px;
 
     &:not(:first-of-type) {
       margin: 2em 0 1em 0;
@@ -42,11 +39,12 @@
 
     .feed {
       float: right;
-      color: #e25512;
+      color: #f26522;
     }
   }
 
   @media screen and (max-width: $small-devices) {
+
     #left_column,
     #right_column {
       flex: 100%;
@@ -59,7 +57,6 @@
     max-height: 600px;
     overflow-y: scroll;
     overflow-x: clip;
-    margin-top: 1em;
 
     #load-more-news-button {
       text-align: center;
@@ -79,11 +76,15 @@
     font-size: 70%;
     margin-bottom: 1em;
 
+    h3 {
+      margin-bottom: 0;
+    }
+
     #links_content {
       overflow: auto;
       box-shadow: $shadow-color 1px 1px 1px;
       min-height: 20em;
-      padding: 1em;
+      padding-bottom: 1em;
 
       h4 {
         margin-left: 5px;
@@ -120,8 +121,6 @@
     }
 
     #birthdays_content {
-      box-shadow: $shadow-color 1px 1px 1px;
-      padding: 1em;
       ul.birthdays_year {
         margin: 0;
         list-style-type: none;
@@ -136,7 +135,8 @@
         }
 
         ul {
-          margin: .5em 0 0 1em;
+          margin: 0;
+          margin-left: 1em;
           list-style-type: square;
           list-style-position: inside;
           font-weight: normal;
@@ -150,13 +150,9 @@
   /* EVENTS TODAY AND NEXT FEW DAYS */
   .news_events_group {
     box-shadow: $shadow-color 1px 1px 1px;
-    margin-left: 0;
+    margin-left: 1em;
     margin-bottom: 0.5em;
 
-    @media screen and (max-width: $small-devices) {
-      margin-left: 3px;
-    }
-
     .news_events_group_date {
       display: table-cell;
       padding: 0.6em;

com/templates/com/news_list.jinja

@@ -23,7 +23,7 @@
         <a target="#" href="{{ url("com:news_feed") }}"><i class="fa fa-rss feed"></i></a>
       </h3>
       {% if user.is_authenticated and (user.is_com_admin or user.memberships.board().ongoing().exists()) %}
-        <a class="btn btn-blue" href="{{ url("com:news_new") }}">
+        <a class="btn btn-blue margin-bottom" href="{{ url("com:news_new") }}">
           <i class="fa fa-plus"></i>
           {% trans %}Create news{% endtrans %}
         </a>

core/static/core/style.scss

@@ -271,7 +271,7 @@ body {
 
   /*--------------------------------CONTENT------------------------------*/
   #content {
-    padding: 1.5em 3%;
+    padding: 1em 1%;
     box-shadow: $shadow-color 0 5px 10px;
     background: $white-color;
     overflow: auto;

core/templates/core/fragment/user_visibility.jinja

@@ -18,7 +18,7 @@
     <span class="helptext">{{ form.is_viewable.help_text }}</span>
     {{ form.is_viewable.errors }}
   </fieldset>
-  <fieldset class="form-group" x-show="!isViewable" x-transition x-cloak>
+  <fieldset class="form-group" x-show="!isViewable">
     {{ form.whitelisted_users.as_field_group() }}
   </fieldset>
   <fieldset class="form-group">

eboutic/static/eboutic/css/eboutic.css

@@ -0,0 +1,163 @@
+#eboutic {
+  display: flex;
+  flex-direction: row-reverse;
+  align-items: flex-start;
+  column-gap: 20px;
+  margin: 0 20px 20px;
+}
+
+#eboutic-title {
+  margin-left: 20px;
+}
+
+#eboutic h3 {
+  margin-left: 0;
+  margin-right: 0;
+}
+
+#basket {
+  min-width: 300px;
+  border-radius: 8px;
+  box-shadow:
+    rgb(60 64 67 / 30%) 0 1px 3px 0,
+    rgb(60 64 67 / 15%) 0 4px 8px 3px;
+  padding: 10px;
+}
+
+#basket h3 {
+  margin-top: 0;
+}
+
+@media screen and (max-width: 765px) {
+  #eboutic {
+    flex-direction: column-reverse;
+    align-items: center;
+    margin: 10px;
+    row-gap: 20px;
+  }
+  #eboutic-title {
+    margin-bottom: 20px;
+    margin-top: 4px;
+  }
+  #basket {
+    width: -webkit-fill-available;
+  }
+}
+
+#eboutic .item-list {
+  margin-left: 0;
+  list-style: none;
+}
+
+#eboutic .item-list li {
+  display: flex;
+  align-items: center;
+  margin-bottom: 10px;
+}
+
+#eboutic .item-row {
+  gap: 10px;
+}
+
+#eboutic .item-name {
+  word-break: break-word;
+  width: 100%;
+  line-height: 100%;
+  white-space: normal;
+}
+
+#eboutic .fa-plus,
+#eboutic .fa-minus {
+  cursor: pointer;
+  background-color: #354a5f;
+  color: white;
+  border-radius: 50%;
+  padding: 5px;
+  font-size: 10px;
+  line-height: 10px;
+  width: 10px;
+  text-align: center;
+}
+
+#eboutic .item-quantity {
+  min-width: 65px;
+  justify-content: space-between;
+  align-items: center;
+  display: flex;
+  gap: 5px;
+}
+
+#eboutic .item-price {
+  min-width: 65px;
+  text-align: right;
+}
+
+/*  CSS du catalogue  */
+
+#eboutic #catalog {
+  display: flex;
+  flex-grow: 1;
+  flex-direction: column;
+  row-gap: 30px;
+}
+
+#eboutic .category-header {
+  margin-bottom: 15px;
+}
+
+#eboutic .product-group {
+  display: flex;
+  flex-wrap: wrap;
+  column-gap: 15px;
+  row-gap: 15px;
+}
+
+#eboutic .card.selected::after {
+  content: "🛒";
+  position: absolute;
+  top: 5px;
+  right: 5px;
+  padding: 5px;
+  border-radius: 50%;
+  box-shadow: 0 0 12px 2px rgb(0 0 0 / 14%);
+  background-color: white;
+  width: 20px;
+  height: 20px;
+  font-size: 16px;
+  line-height: 20px;
+}
+
+#eboutic .catalog-buttons {
+  display: flex;
+  justify-content: center;
+  column-gap: 30px;
+  margin: 30px 0 0;
+}
+
+#eboutic input {
+  all: unset;
+}
+
+#eboutic .catalog-buttons button {
+  min-width: 60px;
+}
+
+#eboutic .catalog-buttons form {
+  margin: 0;
+}
+
+@media screen and (max-width: 765px) {
+  #eboutic #catalog {
+    row-gap: 15px;
+    width: 100%;
+  }
+
+  #eboutic section {
+    text-align: center;
+  }
+
+  #eboutic .product-group {
+    justify-content: space-around;
+    flex-direction: column;
+  }
+}

eboutic/static/eboutic/css/eboutic.scss

@@ -1,162 +0,0 @@
-#eboutic-title {
-  margin-left: 20px;
-}
-
-#eboutic {
-  display: flex;
-  flex-direction: row-reverse;
-  align-items: flex-start;
-  column-gap: 20px;
-  margin: 0 20px 20px;
-
-  h3 {
-    margin-left: 0;
-    margin-right: 0;
-  }
-
-  #basket {
-    --box-shadow:
-    rgb(60 64 67 / 30%) 0 1px 3px 0,
-    rgb(60 64 67 / 15%) 0 4px 8px 3px;
-    min-width: 300px;
-    border-radius: 8px;
-    box-shadow: var(--box-shadow);
-    padding: 10px;
-    h3 {
-      margin-top: 0;
-    }
-  }
-
-  @media screen and (max-width: 765px) {
-    flex-direction: column-reverse;
-    align-items: center;
-    margin: 10px;
-    row-gap: 20px;
-
-    #eboutic-title {
-      margin-bottom: 20px;
-      margin-top: 4px;
-    }
-    #basket {
-      width: -webkit-fill-available;
-    }
-  }
-
-  .item-list {
-    margin-left: 0;
-    list-style: none;
-
-    li {
-      display: flex;
-      align-items: center;
-      margin-bottom: 10px;
-    }
-  }
-
-  .item-row {
-    gap: 10px;
-  }
-
-  .item-name {
-    word-break: break-word;
-    width: 100%;
-    line-height: 100%;
-    white-space: normal;
-  }
-
-  .fa-plus,
-  .fa-minus {
-    cursor: pointer;
-    background-color: #354a5f;
-    color: white;
-    border-radius: 50%;
-    padding: 5px;
-    font-size: 10px;
-    line-height: 10px;
-    width: 10px;
-    text-align: center;
-  }
-
-  .item-quantity {
-    min-width: 65px;
-    justify-content: space-between;
-    align-items: center;
-    display: flex;
-    gap: 5px;
-  }
-
-  .item-price {
-    min-width: 65px;
-    text-align: right;
-  }
-
-/*  CSS du catalogue  */
-
-  #catalog {
-    display: flex;
-    flex-grow: 1;
-    flex-direction: column;
-    row-gap: 30px;
-  }
-
-  .category-header {
-    margin-bottom: 15px;
-  }
-
-  .product-group {
-    display: flex;
-    flex-wrap: wrap;
-    column-gap: 15px;
-    row-gap: 15px;
-  }
-
-  .card.selected::after {
-    --box-shadow: 0 0 12px 2px rgb(0 0 0 / 14%);
-    content: "🛒";
-    position: absolute;
-    top: 5px;
-    right: 5px;
-    padding: 5px;
-    border-radius: 50%;
-    box-shadow: var(--box-shadow);
-    background-color: white;
-    width: 20px;
-    height: 20px;
-    font-size: 16px;
-    line-height: 20px;
-  }
-
-  input {
-    all: unset;
-  }
-  .catalog-buttons {
-    display: flex;
-    justify-content: center;
-    column-gap: 30px;
-    margin: 30px 0 0;
-
-    button {
-      min-width: 60px;
-    }
-    form {
-      margin: 0;
-    }
-  }
-
-
-  @media screen and (max-width: 765px) {
-    #catalog {
-      row-gap: 15px;
-      width: 100%;
-    }
-
-    section {
-      text-align: center;
-    }
-
-    .product-group {
-      justify-content: space-around;
-      flex-direction: column;
-    }
-  }
-}

eboutic/templates/eboutic/eboutic_main.jinja

@@ -15,7 +15,7 @@
 {% endblock %}
 
 {% block additional_css %}
-  <link rel="stylesheet" href="{{ static("eboutic/css/eboutic.scss") }}">
+  <link rel="stylesheet" href="{{ static("eboutic/css/eboutic.css") }}">
   <link rel="stylesheet" href="{{ static("core/components/card.scss") }}">
 {% endblock %}
 
@@ -170,6 +170,8 @@
         {% for category, items in priority_groups.list|groupby('category') %}
           {% if items|count > 0 %}
             <section>
+                            {# I would have wholeheartedly directly used the header element instead
+                            but it has already been made messy in core/style.scss #}
               <div class="category-header">
                 <h3>{{ category }}</h3>
                 {% if items[0].category_comment %}

sas/forms.py

@@ -1,16 +1,23 @@
-from typing import Any
+import copy
+from pathlib import Path
+from typing import TYPE_CHECKING, Any
 
 from django import forms
 from django.core.exceptions import ValidationError
 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:
@@ -49,17 +56,43 @@ class AlbumEditForm(forms.ModelForm):
     class Meta:
         model = Album
         fields = ["name", "date", "file", "parent", "edit_groups"]
-        widgets = {
+        widgets = {"edit_groups": AutoCompleteSelectMultipleGroup, "date": SelectDate}
-            "edit_groups": AutoCompleteSelectMultipleGroup,
-        }
 
     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
     )
 
+    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:
+            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.

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 = (
+        p = self.children_pictures.order_by("?").first()
-            self.children_pictures.order_by("?").first()
+        if p and p.thumbnail:
-            or self.children_albums.exclude(file=None)
+            image = ContentFile(
-            .exclude(file="")
+                name=str(Path(self.name) / "thumb.webp"), content=p.thumbnail.read()
-            .order_by("?")
-            .first()
             )
-        if p and p.file:
-            image = resize_image(Image.open(BytesIO(p.file.read())), 200, "webp")
             self.file = image
-            self.file.name = f"{self.name}/thumb.webp"
             self.save()
 
 

sas/templates/sas/macros.jinja

@@ -2,19 +2,19 @@
   <a href="{{ url('sas:album', album_id=a.id) }}">
     {% 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 %}
     <div
       class="album{% if not a.is_moderated %} not_moderated{% endif %}"
     >
-      <img src="{{ img }}" alt="{{ src }}" loading="lazy" />
+      <img src="{{ img }}" alt="{{ alt }}" loading="lazy" />
       {% if not a.is_moderated %}
         <div class="overlay">&nbsp;</div>
         <div class="text">{% trans %}To be moderated{% endtrans %}</div>

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("<li>Boucle dans l'arborescence des dossiers</li>", 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)

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().strftime("%Y-%m-%d"),
-            "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().strftime("%Y-%m-%d"),
-        }
-        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().strftime("%Y-%m-%d"),
-        }
-        response = client.post(
-            reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload
-        )
-        assertInHTML("<li>Boucle dans l'arborescence des dossiers</li>", 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().strftime("%Y-%m-%d"),
-            "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):

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,9 +153,8 @@ class AlbumView(CanViewMixin, UseFragmentsMixin, DetailView):
 
     def post(self, request, *args, **kwargs):
         self.object = self.get_object()
-        if not self.object.file:
+        if not request.user.can_edit(self.object):
-            self.object.generate_thumbnail()
+            raise PermissionDenied
-        if request.user.can_edit(self.object):  # Handle the copy-paste functions
         FileView.handle_clipboard(request, self.object)
         return HttpResponseRedirect(self.request.path)