From 0b9ccf6a57d2710bdde294f298922f30140e96ab Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Mon, 5 Aug 2024 19:25:30 +0200
Subject: [PATCH 1/8] paginate GET /api/sas/picture

---
 core/templates/core/user_pictures.jinja | 31 +++++++++++++++++++++----
 core/views/user.py                      |  1 +
 sas/api.py                              | 14 +++++------
 sas/schemas.py                          | 16 ++++++++++---
 sas/tests/test_api.py                   | 28 ++++++++++------------
 5 files changed, 58 insertions(+), 32 deletions(-)

diff --git a/core/templates/core/user_pictures.jinja b/core/templates/core/user_pictures.jinja
index ae01ef87..c3815552 100644
--- a/core/templates/core/user_pictures.jinja
+++ b/core/templates/core/user_pictures.jinja
@@ -23,7 +23,7 @@
         <button
           :disabled="in_progress"
           class="btn btn-blue"
-          @click="download('{{ url("api:pictures") }}?users_identified={{ object.id }}')"
+          @click="download_zip()"
         >
           <i class="fa fa-download"></i>
           {% trans %}Download all my pictures{% endtrans %}
@@ -86,13 +86,34 @@
         Alpine.data("picture_download", () => ({
           in_progress: false,
 
-          async download(url) {
+          /**
+           * @return {Promise<Picture[]>}
+           */
+          async get_pictures() {
+            {# The API forbids to get more than 199 items at once
+             from paginated routes.
+             In order to download all the user pictures, it may be needed
+             to performs multiple requests #}
+            const max_per_page = 1;
+            const url = "{{ url("api:pictures") }}"
+            + "?users_identified={{ object.id }}"
+            + `&page_size=${max_per_page}`;
+            let promises = [];
+            const nb_pages = Math.ceil({{ nb_pictures }} / max_per_page);
+            for (let i = 1; i <= nb_pages; i++) {
+              promises.push(
+                fetch(url + `&page=${i}`).then(res => res.json().then(json => json.results))
+              );
+            }
+            return (await Promise.all(promises)).flat()
+          },
+
+
+          async download_zip(){
             this.in_progress = true;
             const bar = this.$refs.progress;
             bar.value = 0;
-
-            /** @type Picture[] */
-            const pictures = await (await fetch(url)).json();
+            const pictures = await this.get_pictures();
             bar.max = pictures.length;
 
             const fileHandle = await window.showSaveFilePicker({
diff --git a/core/views/user.py b/core/views/user.py
index d50671c8..87a4b6a3 100644
--- a/core/views/user.py
+++ b/core/views/user.py
@@ -319,6 +319,7 @@ class UserPicturesView(UserTabsMixin, CanViewMixin, DetailView):
             .order_by("-parent__date", "-date")
             .annotate(album=F("parent__name"))
         )
+        kwargs["nb_pictures"] = len(pictures)
         kwargs["albums"] = {
             album: list(picts)
             for album, picts in itertools.groupby(pictures, lambda i: i.album)
diff --git a/sas/api.py b/sas/api.py
index 807339f3..bb4a2857 100644
--- a/sas/api.py
+++ b/sas/api.py
@@ -1,9 +1,11 @@
 from django.conf import settings
 from django.db.models import F
 from ninja import Query
-from ninja_extra import ControllerBase, api_controller, route
+from ninja_extra import ControllerBase, api_controller, paginate, route
 from ninja_extra.exceptions import PermissionDenied
+from ninja_extra.pagination import PageNumberPaginationExtra
 from ninja_extra.permissions import IsAuthenticated
+from ninja_extra.schemas import PaginatedResponseSchema
 from pydantic import NonNegativeInt
 
 from core.models import User
@@ -15,10 +17,11 @@ from sas.schemas import PictureFilterSchema, PictureSchema
 class PicturesController(ControllerBase):
     @route.get(
         "",
-        response=list[PictureSchema],
+        response=PaginatedResponseSchema[PictureSchema],
         permissions=[IsAuthenticated],
         url_name="pictures",
     )
+    @paginate(PageNumberPaginationExtra, page_size=100)
     def fetch_pictures(self, filters: Query[PictureFilterSchema]):
         """Find pictures viewable by the user corresponding to the given filters.
 
@@ -42,7 +45,7 @@ class PicturesController(ControllerBase):
             # User can view any moderated picture if he/she is subscribed.
             # If not, he/she can view only the one he/she has been identified on
             raise PermissionDenied
-        pictures = list(
+        return (
             filters.filter(
                 Picture.objects.filter(is_moderated=True, asked_for_removal=False)
             )
@@ -50,11 +53,6 @@ class PicturesController(ControllerBase):
             .order_by("-date")
             .annotate(album=F("parent__name"))
         )
-        for picture in pictures:
-            picture.full_size_url = picture.get_download_url()
-            picture.compressed_url = picture.get_download_compressed_url()
-            picture.thumb_url = picture.get_download_thumb_url()
-        return pictures
 
 
 @api_controller("/sas/relation", tags="User identification on SAS pictures")
diff --git a/sas/schemas.py b/sas/schemas.py
index b9af0d72..eaeaedc2 100644
--- a/sas/schemas.py
+++ b/sas/schemas.py
@@ -3,7 +3,6 @@ from datetime import datetime
 from ninja import FilterSchema, ModelSchema, Schema
 from pydantic import Field, NonNegativeInt
 
-from core.schemas import SimpleUserSchema
 from sas.models import PeoplePictureRelation, Picture
 
 
@@ -17,14 +16,25 @@ class PictureFilterSchema(FilterSchema):
 class PictureSchema(ModelSchema):
     class Meta:
         model = Picture
-        fields = ["id", "name", "date", "size"]
+        fields = ["id", "name", "date", "size", "is_moderated"]
 
-    author: SimpleUserSchema = Field(validation_alias="owner")
     full_size_url: str
     compressed_url: str
     thumb_url: str
     album: str
 
+    @staticmethod
+    def resolve_full_size_url(obj: Picture) -> str:
+        return obj.get_download_url()
+
+    @staticmethod
+    def resolve_compressed_url(obj: Picture) -> str:
+        return obj.get_download_compressed_url()
+
+    @staticmethod
+    def resolve_thumb_url(obj: Picture) -> str:
+        return obj.get_download_thumb_url()
+
 
 class PictureCreateRelationSchema(Schema):
     user_id: NonNegativeInt
diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py
index 58079e70..a9cb80ad 100644
--- a/sas/tests/test_api.py
+++ b/sas/tests/test_api.py
@@ -44,12 +44,8 @@ class TestPictureSearch(TestSas):
         self.client.force_login(self.user_b)
         res = self.client.get(reverse("api:pictures") + f"?album_id={self.album_a.id}")
         assert res.status_code == 200
-        expected = list(
-            self.album_a.children_pictures.order_by("-date").values_list(
-                "id", flat=True
-            )
-        )
-        assert [i["id"] for i in res.json()] == expected
+        expected = list(self.album_a.children_pictures.values_list("id", flat=True))
+        assert [i["id"] for i in res.json()["results"]] == expected
 
     def test_filter_by_user(self):
         self.client.force_login(self.user_b)
@@ -58,11 +54,11 @@ class TestPictureSearch(TestSas):
         )
         assert res.status_code == 200
         expected = list(
-            self.user_a.pictures.order_by("-picture__date").values_list(
-                "picture_id", flat=True
-            )
+            self.user_a.pictures.order_by(
+                "-picture__parent__date", "picture__date"
+            ).values_list("picture_id", flat=True)
         )
-        assert [i["id"] for i in res.json()] == expected
+        assert [i["id"] for i in res.json()["results"]] == expected
 
     def test_filter_by_multiple_user(self):
         self.client.force_login(self.user_b)
@@ -73,10 +69,10 @@ class TestPictureSearch(TestSas):
         assert res.status_code == 200
         expected = list(
             self.user_a.pictures.union(self.user_b.pictures.all())
-            .order_by("-picture__date")
+            .order_by("-picture__parent__date", "picture__date")
             .values_list("picture_id", flat=True)
         )
-        assert [i["id"] for i in res.json()] == expected
+        assert [i["id"] for i in res.json()["results"]] == expected
 
     def test_not_subscribed_user(self):
         """Test that a user that is not subscribed can only its own pictures."""
@@ -86,11 +82,11 @@ class TestPictureSearch(TestSas):
         )
         assert res.status_code == 200
         expected = list(
-            self.user_a.pictures.order_by("-picture__date").values_list(
-                "picture_id", flat=True
-            )
+            self.user_a.pictures.order_by(
+                "-picture__parent__date", "picture__date"
+            ).values_list("picture_id", flat=True)
         )
-        assert [i["id"] for i in res.json()] == expected
+        assert [i["id"] for i in res.json()["results"]] == expected
 
         # trying to access the pictures of someone else
         res = self.client.get(

From da6bd84cdf6e746bc6574b72e3db26656bd8ac01 Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Mon, 5 Aug 2024 23:40:11 +0200
Subject: [PATCH 2/8] restify album view

---
 core/static/core/js/script.js           | 18 ++++++
 core/static/core/pagination.scss        |  1 +
 core/static/sas/album.scss              |  6 +-
 pedagogy/templates/pedagogy/guide.jinja | 16 -----
 sas/api.py                              |  2 +-
 sas/templates/sas/album.jinja           | 85 ++++++++++++++++---------
 sas/views.py                            |  8 ---
 7 files changed, 75 insertions(+), 61 deletions(-)

diff --git a/core/static/core/js/script.js b/core/static/core/js/script.js
index 1ab7887e..f2c365ff 100644
--- a/core/static/core/js/script.js
+++ b/core/static/core/js/script.js
@@ -65,3 +65,21 @@ function display_notif() {
 function getCSRFToken() {
     return $("[name=csrfmiddlewaretoken]").val();
 }
+
+
+const initialUrlParams = new URLSearchParams(window.location.search);
+
+function update_query_string(key, value) {
+    const url = new URL(window.location.href);
+    if (!value) {
+        // If the value is null, undefined or empty => delete it
+        url.searchParams.delete(key)
+    } else if (Array.isArray(value)) {
+
+        url.searchParams.delete(key)
+        value.forEach((v) => url.searchParams.append(key, v))
+    } else {
+        url.searchParams.set(key, value);
+    }
+    history.pushState(null, document.title, url.toString());
+}
diff --git a/core/static/core/pagination.scss b/core/static/core/pagination.scss
index 2d3f3a8d..911018a4 100644
--- a/core/static/core/pagination.scss
+++ b/core/static/core/pagination.scss
@@ -3,6 +3,7 @@
 .pagination {
   text-align: center;
   gap: 10px;
+  margin: 30px;
 
   button {
     background-color: $secondary-neutral-light-color;
diff --git a/core/static/sas/album.scss b/core/static/sas/album.scss
index 7b744183..635821e3 100644
--- a/core/static/sas/album.scss
+++ b/core/static/sas/album.scss
@@ -161,17 +161,13 @@ main {
     > .album {
       box-sizing: border-box;
       background-color: #333333;
-      background-size: cover;
+      background-size: contain;
       background-repeat: no-repeat;
       background-position: center center;
 
       width: calc(16 / 9 * 128px);
       height: 128px;
 
-      &.vertical {
-        background-size: contain;
-      }
-
       margin: 0;
       padding: 0;
       box-shadow: none;
diff --git a/pedagogy/templates/pedagogy/guide.jinja b/pedagogy/templates/pedagogy/guide.jinja
index 067d1c18..a9941562 100644
--- a/pedagogy/templates/pedagogy/guide.jinja
+++ b/pedagogy/templates/pedagogy/guide.jinja
@@ -126,22 +126,6 @@
     </nav>
   </div>
   <script>
-    const initialUrlParams = new URLSearchParams(window.location.search);
-
-    function update_query_string(key, value) {
-      const url = new URL(window.location.href);
-      if (!value) {
-            {# If the value is null, undefined or empty => delete it #}
-        url.searchParams.delete(key)
-      } else if (Array.isArray(value)) {
-        url.searchParams.delete(key)
-        value.forEach((v) => url.searchParams.append(key, v))
-      } else {
-        url.searchParams.set(key, value);
-      }
-      history.pushState(null, document.title, url.toString());
-    }
-
     {#
     How does this work :
 
diff --git a/sas/api.py b/sas/api.py
index bb4a2857..170894d1 100644
--- a/sas/api.py
+++ b/sas/api.py
@@ -50,7 +50,7 @@ class PicturesController(ControllerBase):
                 Picture.objects.filter(is_moderated=True, asked_for_removal=False)
             )
             .distinct()
-            .order_by("-date")
+            .order_by("-parent__date", "date")
             .annotate(album=F("parent__name"))
         )
 
diff --git a/sas/templates/sas/album.jinja b/sas/templates/sas/album.jinja
index 60f9dff9..ca68379b 100644
--- a/sas/templates/sas/album.jinja
+++ b/sas/templates/sas/album.jinja
@@ -3,6 +3,7 @@
 
 {%- block additional_css -%}
   <link rel="stylesheet" href="{{ scss('sas/album.scss') }}">
+  <link rel="stylesheet" href="{{ scss('core/pagination.scss') }}">
 {%- endblock -%}
 
 {% block title %}
@@ -81,39 +82,38 @@
     <br>
   {% endif %}
 
-  <h4>{% trans %}Pictures{% endtrans %}</h4>
-  {% if pictures | length != 0 %}
+  <div x-data="pictures">
+    <h4>{% trans %}Pictures{% endtrans %}</h4>
     <div class="photos">
-      {% for p in pictures %}
-        {% if p.can_be_viewed_by(user) %}
-          <a href="{{ url('sas:picture', picture_id=p.id) }}#pict">
-            <div
-              class="photo {% if p.is_vertical %}vertical{% endif %}"
-              style="background-image: url('{{ p.get_download_thumb_url() }}')"
-            >
-              {% if not p.is_moderated %}
-                <div class="overlay">&nbsp;</div>
-                <div class="text">{% trans %}To be moderated{% endtrans %}</div>
-              {% else %}
-                <div class="text">&nbsp;</div>
-              {% endif %}
-            </div>
-            {% if edit_mode %}
-              <input type="checkbox" name="file_list" value="{{ p.id }}">
-            {% endif %}
-          </a>
-        {% endif %}
-      {% endfor %}
+      <template x-for="picture in pictures.results">
+        <a :href="`/sas/picture/${picture.id}#pict`">
+          <div class="photo" :style="`background-image: url(${picture.thumb_url})`">
+            <template x-if="!picture.is_moderated">
+              <div class="overlay">&nbsp;</div>
+              <div class="text">{% trans %}To be moderated{% endtrans %}</div>
+            </template>
+            <template x-if="picture.is_moderated">
+              <div class="text">&nbsp;</div>
+            </template>
+          </div>
+          {% if edit_mode %}
+            <input type="checkbox" name="file_list" :value="picture.id">
+          {% endif %}
+        </a>
+      </template>
     </div>
-  {% else %}
-    {% trans %}This album does not contain any photos.{% endtrans %}
-  {% endif %}
-
-  {% if pictures.has_previous() or pictures.has_next() %}
-    <div class="paginator">
-      {{ paginate(pictures, paginator) }}
-    </div>
-  {% endif %}
+    <nav class="pagination" x-show="nb_pages() > 1">
+      <button @click="page--" :disabled="page <= 1">
+        <i class="fa fa-caret-left"></i>
+      </button>
+      <template x-for="i in nb_pages()">
+        <button x-text="i" @click="page = i":class="{active: page === i}"></button>
+      </template>
+      <button @click="page++" :disabled="page >= nb_pages()">
+        <i class="fa fa-caret-right"></i>
+      </button>
+    </nav>
+  </div>
 
   {% if edit_mode %}
     </form>
@@ -140,6 +140,29 @@
 {% block script %}
   {{ super() }}
   <script>
+    document.addEventListener("alpine:init", () => {
+      Alpine.data("pictures", () => ({
+        pictures: {},
+        page: parseInt(initialUrlParams.get("page")) || 1,
+        async init() {
+          await this.fetch_pictures();
+          this.$watch("page", () => this.fetch_pictures());
+        },
+
+        async fetch_pictures() {
+          update_query_string("page", this.page === 1 ? null : this.page);
+          const url = "{{ url("api:pictures") }}"
+          +"?album_id={{ album.id }}"
+          +`&page=${this.page}`
+          +"&page_size={{ settings.SITH_SAS_IMAGES_PER_PAGE }}";
+          this.pictures = await (await fetch(url)).json();
+        },
+
+        nb_pages() {
+          return Math.ceil(this.pictures.count / {{ settings.SITH_SAS_IMAGES_PER_PAGE }});
+        }
+      }))
+    })
     $("form#upload_form").submit(function (event) {
       let formData = new FormData($(this)[0]);
 
diff --git a/sas/views.py b/sas/views.py
index 9a885538..1ef7afe0 100644
--- a/sas/views.py
+++ b/sas/views.py
@@ -236,7 +236,6 @@ class AlbumView(CanViewMixin, DetailView, FormMixin):
     form_class = SASForm
     pk_url_kwarg = "album_id"
     template_name = "sas/album.jinja"
-    paginate_by = settings.SITH_SAS_IMAGES_PER_PAGE
 
     def dispatch(self, request, *args, **kwargs):
         try:
@@ -283,13 +282,6 @@ class AlbumView(CanViewMixin, DetailView, FormMixin):
 
     def get_context_data(self, **kwargs):
         kwargs = super().get_context_data(**kwargs)
-        kwargs["paginator"] = Paginator(
-            self.object.children_pictures.order_by("id"), self.paginate_by
-        )
-        try:
-            kwargs["pictures"] = kwargs["paginator"].page(self.asked_page)
-        except InvalidPage as e:
-            raise Http404 from e
         kwargs["form"] = self.form
         kwargs["clipboard"] = SithFile.objects.filter(
             id__in=self.request.session["clipboard"]

From 4506440a62870094f700fb9c1dccb876e6c214bb Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Tue, 6 Aug 2024 12:37:50 +0200
Subject: [PATCH 3/8] add `PictureQuerySet.viewable_by(user)` method

---
 sas/api.py            |  6 ++----
 sas/models.py         | 23 ++++++++++++++++++++---
 sas/tests/test_api.py |  3 ++-
 sas/views.py          |  5 +----
 4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/sas/api.py b/sas/api.py
index 170894d1..f4b98fc3 100644
--- a/sas/api.py
+++ b/sas/api.py
@@ -41,14 +41,12 @@ class PicturesController(ControllerBase):
             cf. https://ae.utbm.fr/user/32663/pictures/)
         """
         user: User = self.context.request.user
-        if not user.is_subscribed and filters.users_identified != {user.id}:
+        if not user.was_subscribed and filters.users_identified != {user.id}:
             # User can view any moderated picture if he/she is subscribed.
             # If not, he/she can view only the one he/she has been identified on
             raise PermissionDenied
         return (
-            filters.filter(
-                Picture.objects.filter(is_moderated=True, asked_for_removal=False)
-            )
+            filters.filter(Picture.objects.viewable_by(user))
             .distinct()
             .order_by("-parent__date", "date")
             .annotate(album=F("parent__name"))
diff --git a/sas/models.py b/sas/models.py
index 7330d823..c93e0a0d 100644
--- a/sas/models.py
+++ b/sas/models.py
@@ -14,6 +14,7 @@
 #
 
 from io import BytesIO
+from typing import Self
 
 from django.conf import settings
 from django.core.cache import cache
@@ -27,9 +28,25 @@ from core.models import SithFile, User
 from core.utils import exif_auto_rotate, resize_image
 
 
+class PictureQuerySet(models.QuerySet):
+    def viewable_by(self, user: User) -> Self:
+        """Filter the pictures that this user can view.
+
+        Warnings:
+            Calling this queryset method may add several additional requests.
+        """
+        if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID):
+            return self.all()
+        if user.was_subscribed:
+            return self.filter(is_moderated=True)
+        return self.filter(people__user_id=user.id)
+
+
 class SASPictureManager(models.Manager):
     def get_queryset(self):
-        return super().get_queryset().filter(is_in_sas=True, is_folder=False)
+        return PictureQuerySet(self.model, using=self._db).filter(
+            is_in_sas=True, is_folder=False
+        )
 
 
 class SASAlbumManager(models.Manager):
@@ -41,7 +58,7 @@ class Picture(SithFile):
     class Meta:
         proxy = True
 
-    objects = SASPictureManager()
+    objects = SASPictureManager.from_queryset(PictureQuerySet)()
 
     @property
     def is_vertical(self):
@@ -58,7 +75,7 @@ class Picture(SithFile):
         cache.set("%d_can_edit_pictures" % (user.id), perm, timeout=4)
         return perm
 
-    def can_be_viewed_by(self, user):
+    def can_be_viewed_by(self, user: User) -> bool:
         # SAS pictures are visible to old subscribers
         # Result is cached 4s for this user
         if user.is_anonymous:
diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py
index a9cb80ad..3a21ef12 100644
--- a/sas/tests/test_api.py
+++ b/sas/tests/test_api.py
@@ -75,7 +75,8 @@ class TestPictureSearch(TestSas):
         assert [i["id"] for i in res.json()["results"]] == expected
 
     def test_not_subscribed_user(self):
-        """Test that a user that is not subscribed can only its own pictures."""
+        """Test that a user that never subscribed can only its own pictures."""
+        self.user_a.subscriptions.all().delete()
         self.client.force_login(self.user_a)
         res = self.client.get(
             reverse("api:pictures") + f"?users_identified={self.user_a.id}"
diff --git a/sas/views.py b/sas/views.py
index 1ef7afe0..d45c13d1 100644
--- a/sas/views.py
+++ b/sas/views.py
@@ -18,7 +18,6 @@ from ajax_select.fields import AutoCompleteSelectMultipleField
 from django import forms
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
-from django.core.paginator import InvalidPage, Paginator
 from django.http import Http404, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse, reverse_lazy
@@ -318,9 +317,7 @@ class ModerationView(TemplateView):
         kwargs["albums_to_moderate"] = Album.objects.filter(
             is_moderated=False, is_in_sas=True, is_folder=True
         ).order_by("id")
-        kwargs["pictures"] = Picture.objects.filter(
-            is_moderated=False, is_in_sas=True, is_folder=False
-        )
+        kwargs["pictures"] = Picture.objects.filter(is_moderated=False)
         kwargs["albums"] = Album.objects.filter(
             id__in=kwargs["pictures"].values("parent").distinct("parent")
         )

From d3b203a4a166907fdb84cdbe619561422fec8b0c Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Tue, 6 Aug 2024 13:23:34 +0200
Subject: [PATCH 4/8] change cache on picture download

---
 core/models.py          |  2 +-
 sas/api.py              |  3 ++-
 sas/models.py           | 27 ++++++++++++-----------
 sas/tests/test_model.py | 48 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 15 deletions(-)
 create mode 100644 sas/tests/test_model.py

diff --git a/core/models.py b/core/models.py
index 3803e3ca..1f5b2109 100644
--- a/core/models.py
+++ b/core/models.py
@@ -982,7 +982,7 @@ class SithFile(models.Model):
             return True
         if self.is_in_sas and user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID):
             return True
-        return user.id == self.owner.id
+        return user.id == self.owner_id
 
     def can_be_viewed_by(self, user):
         if hasattr(self, "profile_of"):
diff --git a/sas/api.py b/sas/api.py
index f4b98fc3..dffded87 100644
--- a/sas/api.py
+++ b/sas/api.py
@@ -45,12 +45,13 @@ class PicturesController(ControllerBase):
             # User can view any moderated picture if he/she is subscribed.
             # If not, he/she can view only the one he/she has been identified on
             raise PermissionDenied
-        return (
+        pictures = list(
             filters.filter(Picture.objects.viewable_by(user))
             .distinct()
             .order_by("-parent__date", "date")
             .annotate(album=F("parent__name"))
         )
+        return pictures
 
 
 @api_controller("/sas/relation", tags="User identification on SAS pictures")
diff --git a/sas/models.py b/sas/models.py
index c93e0a0d..f7ed8b02 100644
--- a/sas/models.py
+++ b/sas/models.py
@@ -13,8 +13,9 @@
 #
 #
 
+from __future__ import annotations
+
 from io import BytesIO
-from typing import Self
 
 from django.conf import settings
 from django.core.cache import cache
@@ -29,7 +30,7 @@ from core.utils import exif_auto_rotate, resize_image
 
 
 class PictureQuerySet(models.QuerySet):
-    def viewable_by(self, user: User) -> Self:
+    def viewable_by(self, user: User) -> PictureQuerySet:
         """Filter the pictures that this user can view.
 
         Warnings:
@@ -39,7 +40,7 @@ class PictureQuerySet(models.QuerySet):
             return self.all()
         if user.was_subscribed:
             return self.filter(is_moderated=True)
-        return self.filter(people__user_id=user.id)
+        return self.filter(people__user_id=user.id, is_moderated=True)
 
 
 class SASPictureManager(models.Manager):
@@ -76,19 +77,19 @@ class Picture(SithFile):
         return perm
 
     def can_be_viewed_by(self, user: User) -> bool:
-        # SAS pictures are visible to old subscribers
-        # Result is cached 4s for this user
         if user.is_anonymous:
             return False
 
-        perm = cache.get("%d_can_view_pictures" % (user.id), False)
-        if not perm:
-            perm = user.was_subscribed
-
-        cache.set("%d_can_view_pictures" % (user.id), perm, timeout=4)
-        return (perm and self.is_moderated and self.is_in_sas) or self.can_be_edited_by(
-            user
-        )
+        cache_key = f"sas:pictures_viewable_by_{user.id}_in_{self.parent_id}"
+        viewable: list[int] | None = cache.get(cache_key)
+        if viewable is None:
+            viewable = list(
+                Picture.objects.filter(parent_id=self.parent_id)
+                .viewable_by(user)
+                .values_list("pk", flat=True)
+            )
+            cache.set(cache_key, viewable, timeout=10)
+        return self.id in viewable
 
     def get_download_url(self):
         return reverse("sas:download", kwargs={"picture_id": self.id})
diff --git a/sas/tests/test_model.py b/sas/tests/test_model.py
new file mode 100644
index 00000000..76d3b2cb
--- /dev/null
+++ b/sas/tests/test_model.py
@@ -0,0 +1,48 @@
+from django.test import TestCase
+from model_bakery import baker, seq
+
+from core.baker_recipes import old_subscriber_user, subscriber_user
+from core.models import User
+from sas.models import Picture
+
+
+class TestPictureQuerySet(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        Picture.objects.all().delete()
+        cls.pictures = baker.make(
+            Picture,
+            is_moderated=True,
+            is_in_sas=True,
+            is_folder=False,
+            name=seq(""),
+            _quantity=10,
+            _bulk_create=True,
+        )
+        Picture.objects.filter(pk=cls.pictures[0].id).update(is_moderated=False)
+
+    def test_root(self):
+        root = baker.make(User, is_superuser=True)
+        pictures = list(Picture.objects.viewable_by(root))
+        self.assertCountEqual(pictures, self.pictures)
+
+    def test_subscriber(self):
+        subscriber = subscriber_user.make()
+        old_subcriber = old_subscriber_user.make()
+        for user in (subscriber, old_subcriber):
+            pictures = list(Picture.objects.viewable_by(user))
+            self.assertCountEqual(pictures, self.pictures[1:])
+
+    def test_not_subscribed_identified(self):
+        user = baker.make(
+            # This is the guy who asked the feature of making pictures
+            # available for tagged users, even if not subscribed
+            User,
+            first_name="Pierrick",
+            last_name="Dheilly",
+            nick_name="Sahmer",
+        )
+        user.pictures.create(picture=self.pictures[0])
+        user.pictures.create(picture=self.pictures[1])
+        pictures = list(Picture.objects.viewable_by(user))
+        assert pictures == [self.pictures[1]]

From 00dc03a235e7edf713e2bbe3fbcaf831a9b5d65f Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Thu, 8 Aug 2024 13:15:19 +0200
Subject: [PATCH 5/8] fix rights on albums and next/previous pictures

---
 sas/api.py                      |   7 +-
 sas/models.py                   | 139 ++++++++++++++++----------------
 sas/templates/sas/picture.jinja |  28 ++-----
 sas/tests/test_api.py           |  32 +++++---
 sas/views.py                    |  16 +++-
 5 files changed, 110 insertions(+), 112 deletions(-)

diff --git a/sas/api.py b/sas/api.py
index dffded87..717025c1 100644
--- a/sas/api.py
+++ b/sas/api.py
@@ -41,17 +41,12 @@ class PicturesController(ControllerBase):
             cf. https://ae.utbm.fr/user/32663/pictures/)
         """
         user: User = self.context.request.user
-        if not user.was_subscribed and filters.users_identified != {user.id}:
-            # User can view any moderated picture if he/she is subscribed.
-            # If not, he/she can view only the one he/she has been identified on
-            raise PermissionDenied
-        pictures = list(
+        return (
             filters.filter(Picture.objects.viewable_by(user))
             .distinct()
             .order_by("-parent__date", "date")
             .annotate(album=F("parent__name"))
         )
-        return pictures
 
 
 @api_controller("/sas/relation", tags="User identification on SAS pictures")
diff --git a/sas/models.py b/sas/models.py
index f7ed8b02..cbb669e6 100644
--- a/sas/models.py
+++ b/sas/models.py
@@ -20,6 +20,7 @@ from io import BytesIO
 from django.conf import settings
 from django.core.cache import cache
 from django.db import models
+from django.db.models import Exists, OuterRef
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -29,6 +30,36 @@ from core.models import SithFile, User
 from core.utils import exif_auto_rotate, resize_image
 
 
+class SasFile(SithFile):
+    """Proxy model for any file in the SAS.
+
+    May be used to have logic that should be shared by both
+    [Picture][sas.models.Picture] and [Album][sas.models.Album].
+    """
+
+    class Meta:
+        proxy = True
+
+    def can_be_viewed_by(self, user):
+        if user.is_anonymous:
+            return False
+        cache_key = (
+            f"sas:{self._meta.model_name}_viewable_by_{user.id}_in_{self.parent_id}"
+        )
+        viewable: list[int] | None = cache.get(cache_key)
+        if viewable is None:
+            viewable = list(
+                self.__class__.objects.filter(parent_id=self.parent_id)
+                .viewable_by(user)
+                .values_list("pk", flat=True)
+            )
+            cache.set(cache_key, viewable, timeout=10)
+        return self.id in viewable
+
+    def can_be_edited_by(self, user):
+        return user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
+
+
 class PictureQuerySet(models.QuerySet):
     def viewable_by(self, user: User) -> PictureQuerySet:
         """Filter the pictures that this user can view.
@@ -45,17 +76,10 @@ class PictureQuerySet(models.QuerySet):
 
 class SASPictureManager(models.Manager):
     def get_queryset(self):
-        return PictureQuerySet(self.model, using=self._db).filter(
-            is_in_sas=True, is_folder=False
-        )
+        return super().get_queryset().filter(is_in_sas=True, is_folder=False)
 
 
-class SASAlbumManager(models.Manager):
-    def get_queryset(self):
-        return super().get_queryset().filter(is_in_sas=True, is_folder=True)
-
-
-class Picture(SithFile):
+class Picture(SasFile):
     class Meta:
         proxy = True
 
@@ -68,29 +92,6 @@ class Picture(SithFile):
             (w, h) = im.size
             return (w / h) < 1
 
-    def can_be_edited_by(self, user):
-        perm = cache.get("%d_can_edit_pictures" % (user.id), None)
-        if perm is None:
-            perm = user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
-
-        cache.set("%d_can_edit_pictures" % (user.id), perm, timeout=4)
-        return perm
-
-    def can_be_viewed_by(self, user: User) -> bool:
-        if user.is_anonymous:
-            return False
-
-        cache_key = f"sas:pictures_viewable_by_{user.id}_in_{self.parent_id}"
-        viewable: list[int] | None = cache.get(cache_key)
-        if viewable is None:
-            viewable = list(
-                Picture.objects.filter(parent_id=self.parent_id)
-                .viewable_by(user)
-                .values_list("pk", flat=True)
-            )
-            cache.set(cache_key, viewable, timeout=10)
-        return self.id in viewable
-
     def get_download_url(self):
         return reverse("sas:download", kwargs={"picture_id": self.id})
 
@@ -142,48 +143,53 @@ class Picture(SithFile):
 
     def get_next(self):
         if self.is_moderated:
-            return (
-                self.parent.children.filter(
-                    is_moderated=True,
-                    asked_for_removal=False,
-                    is_folder=False,
-                    id__gt=self.id,
-                )
-                .order_by("id")
-                .first()
+            pictures_qs = self.parent.children.filter(
+                is_moderated=True,
+                asked_for_removal=False,
+                is_folder=False,
+                id__gt=self.id,
             )
         else:
-            return (
-                Picture.objects.filter(id__gt=self.id, is_moderated=False)
-                .order_by("id")
-                .first()
-            )
+            pictures_qs = Picture.objects.filter(id__gt=self.id, is_moderated=False)
+        return pictures_qs.order_by("id").first()
 
     def get_previous(self):
         if self.is_moderated:
-            return (
-                self.parent.children.filter(
-                    is_moderated=True,
-                    asked_for_removal=False,
-                    is_folder=False,
-                    id__lt=self.id,
-                )
-                .order_by("id")
-                .last()
+            pictures_qs = self.parent.children.filter(
+                is_moderated=True,
+                asked_for_removal=False,
+                is_folder=False,
+                id__lt=self.id,
             )
         else:
-            return (
-                Picture.objects.filter(id__lt=self.id, is_moderated=False)
-                .order_by("-id")
-                .first()
-            )
+            pictures_qs = Picture.objects.filter(id__lt=self.id, is_moderated=False)
+        return pictures_qs.order_by("-id").first()
 
 
-class Album(SithFile):
+class AlbumQuerySet(models.QuerySet):
+    def viewable_by(self, user: User) -> PictureQuerySet:
+        """Filter the albums that this user can view.
+
+        Warnings:
+            Calling this queryset method may add several additional requests.
+        """
+        if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID):
+            return self.all()
+        return self.filter(
+            Exists(Picture.objects.filter(parent_id=OuterRef("pk")).viewable_by(user))
+        )
+
+
+class SASAlbumManager(models.Manager):
+    def get_queryset(self):
+        return super().get_queryset().filter(is_in_sas=True, is_folder=True)
+
+
+class Album(SasFile):
     class Meta:
         proxy = True
 
-    objects = SASAlbumManager()
+    objects = SASAlbumManager.from_queryset(AlbumQuerySet)()
 
     @property
     def children_pictures(self):
@@ -193,15 +199,6 @@ class Album(SithFile):
     def children_albums(self):
         return Album.objects.filter(parent=self)
 
-    def can_be_edited_by(self, user):
-        return user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
-
-    def can_be_viewed_by(self, user):
-        # file = SithFile.objects.filter(id=self.id).first()
-        return self.can_be_edited_by(user) or (
-            self.is_in_sas and self.is_moderated and user.was_subscribed
-        )  # or user.can_view(file)
-
     def get_absolute_url(self):
         return reverse("sas:album", kwargs={"album_id": self.id})
 
diff --git a/sas/templates/sas/picture.jinja b/sas/templates/sas/picture.jinja
index a2a300af..7a2ddc77 100644
--- a/sas/templates/sas/picture.jinja
+++ b/sas/templates/sas/picture.jinja
@@ -4,22 +4,6 @@
   <link rel="stylesheet" href="{{ scss('sas/picture.scss') }}">
 {%- endblock -%}
 
-{% block head %}
-  {{ super() }}
-
-  {% if picture.get_previous() %}
-    <link
-      rel="preload"
-      as="image"
-      href="{{ url("sas:download_compressed", picture_id=picture.get_previous().id) }}"
-    >
-  {% endif %}
-  {% if picture.get_next() %}
-    <link rel="preload" as="image" href="{{ url("sas:download_compressed", picture_id=picture.get_next().id) }}">
-  {% endif %}
-
-{% endblock %}
-
 {% block title %}
   {% trans %}SAS{% endtrans %}
 {% endblock %}
@@ -124,16 +108,16 @@
     <div class="subsection">
       <div class="navigation">
         <div id="prev">
-          {% if picture.get_previous() %}
-            <a href="{{ url( 'sas:picture', picture_id=picture.get_previous().id) }}#pict">
-              <div style="background-image: url('{{ picture.get_previous().as_picture.get_download_thumb_url() }}');"></div>
+          {% if previous_pict %}
+            <a href="{{ url( 'sas:picture', picture_id=previous_pict.id) }}#pict">
+              <div style="background-image: url('{{ previous_pict.get_download_thumb_url() }}');"></div>
             </a>
           {% endif %}
         </div>
         <div id="next">
-          {% if picture.get_next() %}
-            <a href="{{ url( 'sas:picture', picture_id=picture.get_next().id) }}#pict">
-              <div style="background-image: url('{{ picture.get_next().as_picture.get_download_thumb_url() }}');"></div>
+          {% if next_pict %}
+            <a href="{{ url( 'sas:picture', picture_id=next_pict.id) }}#pict">
+              <div style="background-image: url('{{ next_pict.get_download_thumb_url() }}');"></div>
             </a>
           {% endif %}
         </div>
diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py
index 3a21ef12..0f84e99f 100644
--- a/sas/tests/test_api.py
+++ b/sas/tests/test_api.py
@@ -89,19 +89,33 @@ class TestPictureSearch(TestSas):
         )
         assert [i["id"] for i in res.json()["results"]] == expected
 
-        # trying to access the pictures of someone else
-        res = self.client.get(
-            reverse("api:pictures") + f"?users_identified={self.user_b.id}"
-        )
-        assert res.status_code == 403
-
-        # trying to access the pictures of someone else shouldn't success,
-        # even if mixed with owned pictures
+        # trying to access the pictures of someone else mixed with owned pictures
+        # should return only owned pictures
         res = self.client.get(
             reverse("api:pictures")
             + f"?users_identified={self.user_a.id}&users_identified={self.user_b.id}"
         )
-        assert res.status_code == 403
+        assert res.status_code == 200
+        assert [i["id"] for i in res.json()["results"]] == expected
+
+        # trying to fetch everything should be the same
+        # as fetching its own pictures for a non-subscriber
+        res = self.client.get(reverse("api:pictures"))
+        assert res.status_code == 200
+        assert [i["id"] for i in res.json()["results"]] == expected
+
+        # trying to access the pictures of someone else should return only
+        # the ones where the non-subscribed user is identified too
+        res = self.client.get(
+            reverse("api:pictures") + f"?users_identified={self.user_b.id}"
+        )
+        assert res.status_code == 200
+        expected = list(
+            self.user_b.pictures.intersection(self.user_a.pictures.all())
+            .order_by("-picture__parent__date", "picture__date")
+            .values_list("picture_id", flat=True)
+        )
+        assert [i["id"] for i in res.json()["results"]] == expected
 
 
 class TestPictureRelation(TestSas):
diff --git a/sas/views.py b/sas/views.py
index d45c13d1..e39201ef 100644
--- a/sas/views.py
+++ b/sas/views.py
@@ -119,10 +119,11 @@ class SASMainView(FormView):
 
     def get_context_data(self, **kwargs):
         kwargs = super().get_context_data(**kwargs)
-        kwargs["categories"] = Album.objects.filter(
-            parent__id=settings.SITH_SAS_ROOT_DIR_ID
-        ).order_by("id")
-        kwargs["latest"] = Album.objects.filter(is_moderated=True).order_by("-id")[:5]
+        albums_qs = Album.objects.viewable_by(self.request.user)
+        kwargs["categories"] = list(
+            albums_qs.filter(parent_id=settings.SITH_SAS_ROOT_DIR_ID).order_by("id")
+        )
+        kwargs["latest"] = list(albums_qs.order_by("-id")[:5])
         return kwargs
 
 
@@ -180,7 +181,14 @@ class PictureView(CanViewMixin, DetailView, FormMixin):
 
     def get_context_data(self, **kwargs):
         kwargs = super().get_context_data(**kwargs)
+        pictures_qs = Picture.objects.viewable_by(self.request.user)
         kwargs["form"] = self.form
+        kwargs["next_pict"] = (
+            pictures_qs.filter(id__gt=self.object.id).order_by("id").first()
+        )
+        kwargs["previous_pict"] = (
+            pictures_qs.filter(id__lt=self.object.id).order_by("-id").first()
+        )
         return kwargs
 
     def get_success_url(self):

From ecb48ce6630f44eaecaf296c85a0cfce8aff4c67 Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Thu, 8 Aug 2024 17:50:23 +0200
Subject: [PATCH 6/8] fix error when uploading image with an alpha channel

---
 core/utils.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/core/utils.py b/core/utils.py
index b336a125..b50578b0 100644
--- a/core/utils.py
+++ b/core/utils.py
@@ -96,10 +96,7 @@ def get_semester_code(d: Optional[date] = None) -> str:
 
 
 def scale_dimension(width, height, long_edge):
-    if width > height:
-        ratio = long_edge * 1.0 / width
-    else:
-        ratio = long_edge * 1.0 / height
+    ratio = long_edge / max(width, height)
     return int(width * ratio), int(height * ratio)
 
 
@@ -107,8 +104,8 @@ def resize_image(im, edge, img_format):
     (w, h) = im.size
     (width, height) = scale_dimension(w, h, long_edge=edge)
     content = BytesIO()
-    # use the lanczos filter for antialiasing
-    im = im.resize((width, height), Resampling.LANCZOS)
+    # use the lanczos filter for antialiasing and discard the alpha channel
+    im = im.resize((width, height), Resampling.LANCZOS).convert("RGB")
     try:
         im.save(
             fp=content,

From 20c015c312a0dd93d81199d1b0b96bc2b26f7ef5 Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Thu, 8 Aug 2024 17:51:33 +0200
Subject: [PATCH 7/8] improved UX

---
 sas/templates/sas/album.jinja   | 38 +++++++--------------------------
 sas/templates/sas/macros.jinja  | 32 +++++++++++++++++++++++++++
 sas/templates/sas/main.jinja    | 38 ++++++---------------------------
 sas/templates/sas/picture.jinja | 19 +++++++----------
 sas/views.py                    | 10 +++++++--
 5 files changed, 62 insertions(+), 75 deletions(-)
 create mode 100644 sas/templates/sas/macros.jinja

diff --git a/sas/templates/sas/album.jinja b/sas/templates/sas/album.jinja
index ca68379b..4453f580 100644
--- a/sas/templates/sas/album.jinja
+++ b/sas/templates/sas/album.jinja
@@ -10,12 +10,7 @@
   {% trans %}SAS{% endtrans %}
 {% endblock %}
 
-{% macro print_path(file) %}
-  {% if file and file.parent %}
-    {{ print_path(file.parent) }}
-    <a href="{{ url('sas:album', album_id=file.id) }}">{{ file.get_display_name() }}</a> /
-  {% endif %}
-{% endmacro %}
+{% from "sas/macros.jinja" import display_album, print_path %}
 
 
 {% block content %}
@@ -23,10 +18,10 @@
     <a href="{{ url('sas:main') }}">SAS</a> / {{ print_path(album.parent) }} {{ album.get_display_name() }}
   </code>
 
-  {% set edit_mode = user.can_edit(album) %}
+  {% set is_sas_admin = user.can_edit(album) %}
   {% set start = timezone.now() %}
 
-  {% if edit_mode %}
+  {% if is_sas_admin %}
     <form action="" method="post" enctype="multipart/form-data">
       {% csrf_token %}
 
@@ -54,28 +49,11 @@
       {% endif %}
   {% endif %}
 
-  {% if album.children_albums.count() > 0 %}
+  {% if children_albums|length > 0 %}
     <h4>{% trans %}Albums{% endtrans %}</h4>
     <div class="albums">
-      {% for a in album.children_albums.order_by('-date') %}
-        {% if a.can_be_viewed_by(user) %}
-          <a href="{{ url('sas:album', album_id=a.id) }}">
-            <div
-              class="album{% if not a.is_moderated %} not_moderated{% endif %}"
-              style="background-image: url('{% if a.file %}{{ a.get_download_url() }}{% else %}{{ static('core/img/sas.jpg') }}{% endif %}');"
-            >
-              {% if not a.is_moderated %}
-                <div class="overlay">&nbsp;</div>
-                <div class="text">{% trans %}To be moderated{% endtrans %}</div>
-              {% else %}
-                <div class="text">{{ a.name }}</div>
-              {% endif %}
-            </div>
-            {% if edit_mode %}
-              <input type="checkbox" name="file_list" value="{{ a.id }}">
-            {% endif %}
-          </a>
-        {% endif %}
+      {% for a in children_albums %}
+        {{ display_album(a, is_sas_admin) }}
       {% endfor %}
     </div>
 
@@ -96,7 +74,7 @@
               <div class="text">&nbsp;</div>
             </template>
           </div>
-          {% if edit_mode %}
+          {% if is_sas_admin %}
             <input type="checkbox" name="file_list" :value="picture.id">
           {% endif %}
         </a>
@@ -115,7 +93,7 @@
     </nav>
   </div>
 
-  {% if edit_mode %}
+  {% if is_sas_admin %}
     </form>
   {% endif %}
 
diff --git a/sas/templates/sas/macros.jinja b/sas/templates/sas/macros.jinja
new file mode 100644
index 00000000..a7f48618
--- /dev/null
+++ b/sas/templates/sas/macros.jinja
@@ -0,0 +1,32 @@
+{% macro display_album(a, edit_mode) %}
+  <a href="{{ url('sas:album', album_id=a.id) }}">
+    {% if a.file %}
+      {% set img = a.get_download_url() %}
+    {% elif a.children.filter(is_folder=False, is_moderated=True).exists() %}
+      {% set img = a.children.filter(is_folder=False).first().as_picture.get_download_thumb_url() %}
+    {% else %}
+      {% set img = static('core/img/sas.jpg') %}
+    {% endif %}
+    <div
+      class="album{% if not a.is_moderated %} not_moderated{% endif %}"
+      style="background-image: url('{{ img }}');"
+    >
+      {% if not a.is_moderated %}
+        <div class="overlay">&nbsp;</div>
+        <div class="text">{% trans %}To be moderated{% endtrans %}</div>
+      {% else %}
+        <div class="text">{{ a.name }}</div>
+      {% endif %}
+    </div>
+    {% if edit_mode %}
+      <input type="checkbox" name="file_list" value="{{ a.id }}">
+    {% endif %}
+  </a>
+{% endmacro %}
+
+{% macro print_path(file) %}
+  {% if file and file.parent %}
+    {{ print_path(file.parent) }}
+    <a href="{{ url('sas:album', album_id=file.id) }}">{{ file.get_display_name() }}</a> /
+  {% endif %}
+{% endmacro %}
\ No newline at end of file
diff --git a/sas/templates/sas/main.jinja b/sas/templates/sas/main.jinja
index 8e37759f..ea704fa6 100644
--- a/sas/templates/sas/main.jinja
+++ b/sas/templates/sas/main.jinja
@@ -8,31 +8,9 @@
   {% trans %}SAS{% endtrans %}
 {% endblock %}
 
-{% set edit_mode = user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
+{% set is_sas_admin = user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
 
-{% macro display_album(a, checkbox) %}
-  <a href="{{ url('sas:album', album_id=a.id) }}">
-    {% if a.file %}
-      {% set img = a.get_download_url() %}
-    {% elif a.children.filter(is_folder=False, is_moderated=True).exists() %}
-      {% set img = a.children.filter(is_folder=False).first().as_picture.get_download_thumb_url() %}
-    {% else %}
-      {% set img = static('core/img/sas.jpg') %}
-    {% endif %}
-
-    <div
-      class="album"
-      style="background-image: url('{{ img }}');"
-    >
-      <div class="text">
-        {{ a.name }}
-      </div>
-    </div>
-        {# {% if edit_mode and checkbox %}
-            <input type="checkbox" name="file_list" value="{{ a.id }}">
-        {% endif %} #}
-  </a>
-{% endmacro %}
+{% from "sas/macros.jinja" import display_album %}
 
 {% block content %}
   <main>
@@ -46,22 +24,18 @@
 
       <div class="albums">
         {% for a in latest %}
-          {{ display_album(a) }}
+          {{ display_album(a, edit_mode=False) }}
         {% endfor %}
       </div>
 
       <br>
 
-      {% if edit_mode %}
+      {% if is_sas_admin %}
         <form action="" method="post" enctype="multipart/form-data">
           {% csrf_token %}
 
           <div class="navbar">
             <h4>{% trans %}All categories{% endtrans %}</h4>
-
-                        {# <div class="toolbar">
-                            <input name="delete" type="submit" value="{% trans %}Delete{% endtrans %}">
-                        </div> #}
           </div>
 
           {% if clipboard %}
@@ -81,11 +55,11 @@
 
       <div class="albums">
         {% for a in categories %}
-          {{ display_album(a, true) }}
+          {{ display_album(a, edit_mode=False) }}
         {% endfor %}
       </div>
 
-      {% if edit_mode %}
+      {% if is_sas_admin %}
         </form>
 
         <br>
diff --git a/sas/templates/sas/picture.jinja b/sas/templates/sas/picture.jinja
index 7a2ddc77..977d4f04 100644
--- a/sas/templates/sas/picture.jinja
+++ b/sas/templates/sas/picture.jinja
@@ -8,12 +8,7 @@
   {% trans %}SAS{% endtrans %}
 {% endblock %}
 
-{% macro print_path(file) %}
-  {% if file and file.parent %}
-    {{ print_path(file.parent) }}
-    <a href="{{ url('sas:album', album_id=file.id) }}">{{ file.get_display_name() }}</a> /
-  {% endif %}
-{% endmacro %}
+{% from "sas/macros.jinja" import print_path %}
 
 {% block content %}
   <code>
@@ -125,11 +120,13 @@
 
       <div class="tags">
         <h5>{% trans %}People{% endtrans %}</h5>
-        <form action="" method="post" enctype="multipart/form-data">
-          {% csrf_token %}
-          {{ form.as_p() }}
-          <input type="submit" value="{% trans %}Go{% endtrans %}" />
-        </form>
+        {% if user.was_subscribed %}
+          <form action="" method="post" enctype="multipart/form-data">
+            {% csrf_token %}
+            {{ form.as_p() }}
+            <input type="submit" value="{% trans %}Go{% endtrans %}" />
+          </form>
+        {% endif %}
         <ul x-data="user_identification">
           <template x-for="item in items" :key="item.id">
             <li>
diff --git a/sas/views.py b/sas/views.py
index e39201ef..399f9360 100644
--- a/sas/views.py
+++ b/sas/views.py
@@ -229,8 +229,9 @@ class AlbumUploadView(CanViewMixin, DetailView, FormMixin):
                     parent=parent,
                     owner=request.user,
                     files=files,
-                    automodere=request.user.is_in_group(
-                        pk=settings.SITH_GROUP_SAS_ADMIN_ID
+                    automodere=(
+                        request.user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
+                        or request.user.is_root
                     ),
                 )
                 if self.form.is_valid():
@@ -293,6 +294,11 @@ class AlbumView(CanViewMixin, DetailView, FormMixin):
         kwargs["clipboard"] = SithFile.objects.filter(
             id__in=self.request.session["clipboard"]
         )
+        kwargs["children_albums"] = list(
+            Album.objects.viewable_by(self.request.user)
+            .filter(parent_id=self.object.id)
+            .order_by("-date")
+        )
         return kwargs
 
 

From 7ea9a5ca2d07736653def9f2fb15c49b669699b3 Mon Sep 17 00:00:00 2001
From: thomas girod <thgirod@hotmail.com>
Date: Thu, 8 Aug 2024 20:20:09 +0200
Subject: [PATCH 8/8] improved feedback when loading ajax content

---
 core/static/core/style.scss             | 26 +++++++++++++++++++
 core/static/sas/album.scss              | 14 ++--------
 pedagogy/templates/pedagogy/guide.jinja |  5 +++-
 sas/templates/sas/album.jinja           | 34 +++++++++++++++++--------
 4 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/core/static/core/style.scss b/core/static/core/style.scss
index 3ba0f569..b08be5fb 100644
--- a/core/static/core/style.scss
+++ b/core/static/core/style.scss
@@ -93,6 +93,32 @@ a:not(.button) {
   }
 }
 
+[aria-busy] {
+  --loading-size: 50px;
+  --loading-stroke: 5px;
+  --loading-duration: 1s;
+  position: relative;
+}
+
+[aria-busy]:after {
+  content: '';
+  position: absolute;
+  top: 50%;
+  left: 50%;
+  width: var(--loading-size);
+  height: var(--loading-size);
+  margin-top: calc(var(--loading-size) / 2 * -1);
+  margin-left: calc(var(--loading-size) / 2 * -1);
+  border: var(--loading-stroke) solid rgba(0, 0, 0, .15);
+  border-radius: 50%;
+  border-top-color: rgba(0, 0, 0, 0.5);
+  animation: rotate calc(var(--loading-duration)) linear infinite;
+}
+
+@keyframes rotate {
+  100% { transform: rotate(360deg); }
+}
+
 .ib {
   display: inline-block;
   padding: 1px;
diff --git a/core/static/sas/album.scss b/core/static/sas/album.scss
index 635821e3..e3c3527a 100644
--- a/core/static/sas/album.scss
+++ b/core/static/sas/album.scss
@@ -102,20 +102,10 @@ main {
   border-radius: 10px;
 }
 
-.paginator {
-  display: flex;
-  justify-content: center;
-  gap: 10px;
-  width: -moz-fit-content;
-  width: fit-content;
-  background-color: rgba(0,0,0,.1);
-  border-radius: 10px;
-  padding: 10px;
-  margin: 10px 0 10px auto;
-}
-
 .photos,
 .albums {
+  margin: 20px;
+  min-height: 50px;  // To contain the aria-busy loading wheel, even if empty
   box-sizing: border-box;
   display: flex;
   flex-direction: row;
diff --git a/pedagogy/templates/pedagogy/guide.jinja b/pedagogy/templates/pedagogy/guide.jinja
index a9941562..1aa713e4 100644
--- a/pedagogy/templates/pedagogy/guide.jinja
+++ b/pedagogy/templates/pedagogy/guide.jinja
@@ -96,7 +96,7 @@
           {% endif %}
         </tr>
       </thead>
-      <tbody id="dynamic_view_content">
+      <tbody id="dynamic_view_content" :aria-busy="loading">
         <template x-for="uv in uvs.results" :key="uv.id">
           <tr @click="window.location.href = `/pedagogy/uv/${uv.id}`" class="clickable">
             <td><a :href="`/pedagogy/uv/${uv.id}`" x-text="uv.code"></a></td>
@@ -140,6 +140,7 @@
     document.addEventListener("alpine:init", () => {
       Alpine.data("uv_search", () => ({
         uvs: [],
+        loading: false,
         page: parseInt(initialUrlParams.get("page")) || page_default,
         page_size: parseInt(initialUrlParams.get("page_size")) || page_size_default,
         search: initialUrlParams.get("search") || "",
@@ -171,8 +172,10 @@
         },
 
         async fetch_data() {
+          this.loading = true;
           const url = "{{ url("api:fetch_uvs") }}" + window.location.search;
           this.uvs = await (await fetch(url)).json();
+          this.loading = false;
         },
 
         max_page() {
diff --git a/sas/templates/sas/album.jinja b/sas/templates/sas/album.jinja
index 4453f580..fed66e66 100644
--- a/sas/templates/sas/album.jinja
+++ b/sas/templates/sas/album.jinja
@@ -1,5 +1,4 @@
 {% extends "core/base.jinja" %}
-{% from "core/macros.jinja" import paginate %}
 
 {%- block additional_css -%}
   <link rel="stylesheet" href="{{ scss('sas/album.scss') }}">
@@ -62,7 +61,7 @@
 
   <div x-data="pictures">
     <h4>{% trans %}Pictures{% endtrans %}</h4>
-    <div class="photos">
+    <div class="photos" :aria-busy="loading">
       <template x-for="picture in pictures.results">
         <a :href="`/sas/picture/${picture.id}#pict`">
           <div class="photo" :style="`background-image: url(${picture.thumb_url})`">
@@ -81,13 +80,24 @@
       </template>
     </div>
     <nav class="pagination" x-show="nb_pages() > 1">
-      <button @click="page--" :disabled="page <= 1">
+      {# Adding the prevent here is important, because otherwise,
+      clicking on the pagination buttons could submit the picture management form
+      and reload the page #}
+      <button
+        @click.prevent="page--"
+        :disabled="page <= 1"
+        @keyup.right.window="page = Math.min(nb_pages(), page + 1)"
+      >
         <i class="fa fa-caret-left"></i>
       </button>
       <template x-for="i in nb_pages()">
-        <button x-text="i" @click="page = i":class="{active: page === i}"></button>
+        <button x-text="i" @click.prevent="page = i" :class="{active: page === i}"></button>
       </template>
-      <button @click="page++" :disabled="page >= nb_pages()">
+      <button
+        @click.prevent="page++"
+        :disabled="page >= nb_pages()"
+        @keyup.left.window="page = Math.max(1, page - 1)"
+      >
         <i class="fa fa-caret-right"></i>
       </button>
     </nav>
@@ -95,9 +105,6 @@
 
   {% if is_sas_admin %}
     </form>
-  {% endif %}
-
-  {% if user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
     <form class="add-files" id="upload_form" action="" method="post" enctype="multipart/form-data">
       {% csrf_token %}
       <div class="inputs">
@@ -122,18 +129,24 @@
       Alpine.data("pictures", () => ({
         pictures: {},
         page: parseInt(initialUrlParams.get("page")) || 1,
+        loading: false,
+
         async init() {
           await this.fetch_pictures();
-          this.$watch("page", () => this.fetch_pictures());
+          this.$watch("page", () => {
+            update_query_string("page", this.page === 1 ? null : this.page);
+            this.fetch_pictures()
+          });
         },
 
         async fetch_pictures() {
-          update_query_string("page", this.page === 1 ? null : this.page);
+          this.loading=true;
           const url = "{{ url("api:pictures") }}"
           +"?album_id={{ album.id }}"
           +`&page=${this.page}`
           +"&page_size={{ settings.SITH_SAS_IMAGES_PER_PAGE }}";
           this.pictures = await (await fetch(url)).json();
+          this.loading=false;
         },
 
         nb_pages() {
@@ -141,6 +154,7 @@
         }
       }))
     })
+
     $("form#upload_form").submit(function (event) {
       let formData = new FormData($(this)[0]);