Merge pull request #769 from ae-utbm/query-sas

Sas picture selection

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"):

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());
+}

core/static/core/pagination.scss

@@ -3,6 +3,7 @@
 .pagination {
   text-align: center;
   gap: 10px;
+  margin: 30px;
 
   button {
     background-color: $secondary-neutral-light-color;

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;

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;
@@ -161,17 +151,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;

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;
-
+            const pictures = await this.get_pictures();
-            /** @type Picture[] */
-            const pictures = await (await fetch(url)).json();
             bar.max = pictures.length;
 
             const fileHandle = await window.showSaveFilePicker({

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 / max(width, height)
-        ratio = long_edge * 1.0 / width
-    else:
-        ratio = long_edge * 1.0 / 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
+    # use the lanczos filter for antialiasing and discard the alpha channel
-    im = im.resize((width, height), Resampling.LANCZOS)
+    im = im.resize((width, height), Resampling.LANCZOS).convert("RGB")
     try:
         im.save(
             fp=content,

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)

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>
@@ -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 :
 
@@ -156,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") || "",
@@ -187,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() {

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.
 
@@ -38,23 +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}:
+        return (
-            # User can view any moderated picture if he/she is subscribed.
+            filters.filter(Picture.objects.viewable_by(user))
-            # If not, he/she can view only the one he/she has been identified on
-            raise PermissionDenied
-        pictures = list(
-            filters.filter(
-                Picture.objects.filter(is_moderated=True, asked_for_removal=False)
-            )
             .distinct()
-            .order_by("-date")
+            .order_by("-parent__date", "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")

sas/models.py

@@ -13,11 +13,14 @@
 #
 #
 
+from __future__ import annotations
+
 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 _
@@ -27,21 +30,60 @@ 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.
+
+        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, is_moderated=True)
+
+
 class SASPictureManager(models.Manager):
     def get_queryset(self):
         return super().get_queryset().filter(is_in_sas=True, is_folder=False)
 
 
-class SASAlbumManager(models.Manager):
+class Picture(SasFile):
-    def get_queryset(self):
-        return super().get_queryset().filter(is_in_sas=True, is_folder=True)
-
-
-class Picture(SithFile):
     class Meta:
         proxy = True
 
-    objects = SASPictureManager()
+    objects = SASPictureManager.from_queryset(PictureQuerySet)()
 
     @property
     def is_vertical(self):
@@ -50,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):
-        # 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
-        )
-
     def get_download_url(self):
         return reverse("sas:download", kwargs={"picture_id": self.id})
 
@@ -124,48 +143,53 @@ class Picture(SithFile):
 
     def get_next(self):
         if self.is_moderated:
-            return (
+            pictures_qs = self.parent.children.filter(
-                self.parent.children.filter(
+                is_moderated=True,
-                    is_moderated=True,
+                asked_for_removal=False,
-                    asked_for_removal=False,
+                is_folder=False,
-                    is_folder=False,
+                id__gt=self.id,
-                    id__gt=self.id,
-                )
-                .order_by("id")
-                .first()
             )
         else:
-            return (
+            pictures_qs = Picture.objects.filter(id__gt=self.id, is_moderated=False)
-                Picture.objects.filter(id__gt=self.id, is_moderated=False)
+        return pictures_qs.order_by("id").first()
-                .order_by("id")
-                .first()
-            )
 
     def get_previous(self):
         if self.is_moderated:
-            return (
+            pictures_qs = self.parent.children.filter(
-                self.parent.children.filter(
+                is_moderated=True,
-                    is_moderated=True,
+                asked_for_removal=False,
-                    asked_for_removal=False,
+                is_folder=False,
-                    is_folder=False,
+                id__lt=self.id,
-                    id__lt=self.id,
-                )
-                .order_by("id")
-                .last()
             )
         else:
-            return (
+            pictures_qs = Picture.objects.filter(id__lt=self.id, is_moderated=False)
-                Picture.objects.filter(id__lt=self.id, is_moderated=False)
+        return pictures_qs.order_by("-id").first()
-                .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):
@@ -175,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})
 

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

sas/templates/sas/album.jinja

@@ -1,20 +1,15 @@
 {% extends "core/base.jinja" %}
-{% from "core/macros.jinja" import paginate %}
 
 {%- block additional_css -%}
   <link rel="stylesheet" href="{{ scss('sas/album.scss') }}">
+  <link rel="stylesheet" href="{{ scss('core/pagination.scss') }}">
 {%- endblock -%}
 
 {% block title %}
   {% trans %}SAS{% endtrans %}
 {% endblock %}
 
-{% macro print_path(file) %}
+{% from "sas/macros.jinja" import display_album, print_path %}
-  {% 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 %}
 
 
 {% block content %}
@@ -22,10 +17,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 %}
 
@@ -53,73 +48,63 @@
       {% 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') %}
+      {% for a in children_albums %}
-        {% if a.can_be_viewed_by(user) %}
+        {{ display_album(a, is_sas_admin) }}
-          <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 %}
       {% endfor %}
     </div>
 
     <br>
   {% endif %}
 
-  <h4>{% trans %}Pictures{% endtrans %}</h4>
+  <div x-data="pictures">
-  {% if pictures | length != 0 %}
+    <h4>{% trans %}Pictures{% endtrans %}</h4>
-    <div class="photos">
+    <div class="photos" :aria-busy="loading">
-      {% for p in pictures %}
+      <template x-for="picture in pictures.results">
-        {% if p.can_be_viewed_by(user) %}
+        <a :href="`/sas/picture/${picture.id}#pict`">
-          <a href="{{ url('sas:picture', picture_id=p.id) }}#pict">
+          <div class="photo" :style="`background-image: url(${picture.thumb_url})`">
-            <div
+            <template x-if="!picture.is_moderated">
-              class="photo {% if p.is_vertical %}vertical{% endif %}"
+              <div class="overlay">&nbsp;</div>
-              style="background-image: url('{{ p.get_download_thumb_url() }}')"
+              <div class="text">{% trans %}To be moderated{% endtrans %}</div>
-            >
+            </template>
-              {% if not p.is_moderated %}
+            <template x-if="picture.is_moderated">
-                <div class="overlay">&nbsp;</div>
+              <div class="text">&nbsp;</div>
-                <div class="text">{% trans %}To be moderated{% endtrans %}</div>
+            </template>
-              {% else %}
+          </div>
-                <div class="text">&nbsp;</div>
+          {% if is_sas_admin %}
-              {% endif %}
+            <input type="checkbox" name="file_list" :value="picture.id">
-            </div>
+          {% endif %}
-            {% if edit_mode %}
+        </a>
-              <input type="checkbox" name="file_list" value="{{ p.id }}">
+      </template>
-            {% endif %}
-          </a>
-        {% endif %}
-      {% endfor %}
     </div>
-  {% else %}
+    <nav class="pagination" x-show="nb_pages() > 1">
-    {% trans %}This album does not contain any photos.{% endtrans %}
+      {# Adding the prevent here is important, because otherwise,
-  {% endif %}
+      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.prevent="page = i" :class="{active: page === i}"></button>
+      </template>
+      <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>
+  </div>
 
-  {% if pictures.has_previous() or pictures.has_next() %}
+  {% if is_sas_admin %}
-    <div class="paginator">
-      {{ paginate(pictures, paginator) }}
-    </div>
-  {% endif %}
-
-  {% if edit_mode %}
     </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">
@@ -140,6 +125,36 @@
 {% block script %}
   {{ super() }}
   <script>
+    document.addEventListener("alpine:init", () => {
+      Alpine.data("pictures", () => ({
+        pictures: {},
+        page: parseInt(initialUrlParams.get("page")) || 1,
+        loading: false,
+
+        async init() {
+          await this.fetch_pictures();
+          this.$watch("page", () => {
+            update_query_string("page", this.page === 1 ? null : this.page);
+            this.fetch_pictures()
+          });
+        },
+
+        async fetch_pictures() {
+          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() {
+          return Math.ceil(this.pictures.count / {{ settings.SITH_SAS_IMAGES_PER_PAGE }});
+        }
+      }))
+    })
+
     $("form#upload_form").submit(function (event) {
       let formData = new FormData($(this)[0]);
 

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 %}

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) %}
+{% from "sas/macros.jinja" import display_album %}
-  <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 %}
 
 {% 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>

sas/templates/sas/picture.jinja

@@ -4,32 +4,11 @@
   <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 %}
 
-{% macro print_path(file) %}
+{% from "sas/macros.jinja" import print_path %}
-  {% 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 %}
 
 {% block content %}
   <code>
@@ -124,16 +103,16 @@
     <div class="subsection">
       <div class="navigation">
         <div id="prev">
-          {% if picture.get_previous() %}
+          {% if previous_pict %}
-            <a href="{{ url( 'sas:picture', picture_id=picture.get_previous().id) }}#pict">
+            <a href="{{ url( 'sas:picture', picture_id=previous_pict.id) }}#pict">
-              <div style="background-image: url('{{ picture.get_previous().as_picture.get_download_thumb_url() }}');"></div>
+              <div style="background-image: url('{{ previous_pict.get_download_thumb_url() }}');"></div>
             </a>
           {% endif %}
         </div>
         <div id="next">
-          {% if picture.get_next() %}
+          {% if next_pict %}
-            <a href="{{ url( 'sas:picture', picture_id=picture.get_next().id) }}#pict">
+            <a href="{{ url( 'sas:picture', picture_id=next_pict.id) }}#pict">
-              <div style="background-image: url('{{ picture.get_next().as_picture.get_download_thumb_url() }}');"></div>
+              <div style="background-image: url('{{ next_pict.get_download_thumb_url() }}');"></div>
             </a>
           {% endif %}
         </div>
@@ -141,11 +120,13 @@
 
       <div class="tags">
         <h5>{% trans %}People{% endtrans %}</h5>
-        <form action="" method="post" enctype="multipart/form-data">
+        {% if user.was_subscribed %}
-          {% csrf_token %}
+          <form action="" method="post" enctype="multipart/form-data">
-          {{ form.as_p() }}
+            {% csrf_token %}
-          <input type="submit" value="{% trans %}Go{% endtrans %}" />
+            {{ form.as_p() }}
-        </form>
+            <input type="submit" value="{% trans %}Go{% endtrans %}" />
+          </form>
+        {% endif %}
         <ul x-data="user_identification">
           <template x-for="item in items" :key="item.id">
             <li>

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(
+        expected = list(self.album_a.children_pictures.values_list("id", flat=True))
-            self.album_a.children_pictures.order_by("-date").values_list(
+        assert [i["id"] for i in res.json()["results"]] == expected
-                "id", flat=True
-            )
-        )
-        assert [i["id"] for i in res.json()] == 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(
+            self.user_a.pictures.order_by(
-                "picture_id", flat=True
+                "-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,38 +69,53 @@ 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."""
+        """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}"
         )
         assert res.status_code == 200
         expected = list(
-            self.user_a.pictures.order_by("-picture__date").values_list(
+            self.user_a.pictures.order_by(
-                "picture_id", flat=True
+                "-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
+        # trying to access the pictures of someone else mixed with owned pictures
-        res = self.client.get(
+        # should return only owned pictures
-            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
         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):

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]]

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
@@ -120,10 +119,11 @@ class SASMainView(FormView):
 
     def get_context_data(self, **kwargs):
         kwargs = super().get_context_data(**kwargs)
-        kwargs["categories"] = Album.objects.filter(
+        albums_qs = Album.objects.viewable_by(self.request.user)
-            parent__id=settings.SITH_SAS_ROOT_DIR_ID
+        kwargs["categories"] = list(
-        ).order_by("id")
+            albums_qs.filter(parent_id=settings.SITH_SAS_ROOT_DIR_ID).order_by("id")
-        kwargs["latest"] = Album.objects.filter(is_moderated=True).order_by("-id")[:5]
+        )
+        kwargs["latest"] = list(albums_qs.order_by("-id")[:5])
         return kwargs
 
 
@@ -181,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):
@@ -222,8 +229,9 @@ class AlbumUploadView(CanViewMixin, DetailView, FormMixin):
                     parent=parent,
                     owner=request.user,
                     files=files,
-                    automodere=request.user.is_in_group(
+                    automodere=(
-                        pk=settings.SITH_GROUP_SAS_ADMIN_ID
+                        request.user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
+                        or request.user.is_root
                     ),
                 )
                 if self.form.is_valid():
@@ -236,7 +244,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,17 +290,15 @@ 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"]
         )
+        kwargs["children_albums"] = list(
+            Album.objects.viewable_by(self.request.user)
+            .filter(parent_id=self.object.id)
+            .order_by("-date")
+        )
         return kwargs
 
 
@@ -326,9 +331,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(
+        kwargs["pictures"] = Picture.objects.filter(is_moderated=False)
-            is_moderated=False, is_in_sas=True, is_folder=False
-        )
         kwargs["albums"] = Album.objects.filter(
             id__in=kwargs["pictures"].values("parent").distinct("parent")
         )