feat: display moderation requests to moderators

sas/api.py

@@ -9,10 +9,17 @@ from ninja_extra.permissions import IsAuthenticated
 from ninja_extra.schemas import PaginatedResponseSchema
 from pydantic import NonNegativeInt
 
-from core.api_permissions import CanView, IsOwner
+from core.api_permissions import CanView, IsInGroup, IsRoot
 from core.models import Notification, User
 from sas.models import PeoplePictureRelation, Picture
-from sas.schemas import IdentifiedUserSchema, PictureFilterSchema, PictureSchema
+from sas.schemas import (
+    IdentifiedUserSchema,
+    ModerationRequestSchema,
+    PictureFilterSchema,
+    PictureSchema,
+)
+
+IsSasAdmin = IsRoot | IsInGroup(settings.SITH_GROUP_SAS_ADMIN_ID)
 
 
 @api_controller("/sas/picture")
@@ -85,18 +92,35 @@ class PicturesController(ControllerBase):
                 },
             )
 
-    @route.delete("/{picture_id}", permissions=[IsOwner])
+    @route.delete("/{picture_id}", permissions=[IsSasAdmin])
     def delete_picture(self, picture_id: int):
         self.get_object_or_exception(Picture, pk=picture_id).delete()
 
-    @route.patch("/{picture_id}/moderate", permissions=[IsOwner])
+    @route.patch(
+        "/{picture_id}/moderation",
+        permissions=[IsSasAdmin],
+        url_name="picture_moderate",
+    )
     def moderate_picture(self, picture_id: int):
+        """Mark a picture as moderated and remove its pending moderation requests."""
         picture = self.get_object_or_exception(Picture, pk=picture_id)
+        picture.moderation_requests.all().delete()
         picture.is_moderated = True
         picture.moderator = self.context.request.user
         picture.asked_for_removal = False
         picture.save()
 
+    @route.get(
+        "/{picture_id}/moderation",
+        permissions=[IsSasAdmin],
+        response=list[ModerationRequestSchema],
+        url_name="picture_moderation_requests",
+    )
+    def fetch_moderation_requests(self, picture_id: int):
+        """Fetch the moderation requests issued on this picture."""
+        picture = self.get_object_or_exception(Picture, pk=picture_id)
+        return picture.moderation_requests.select_related("author")
+
 
 @api_controller("/sas/relation", tags="User identification on SAS pictures")
 class UsersIdentifiedController(ControllerBase):

sas/schemas.py

@@ -4,8 +4,8 @@ from django.urls import reverse
 from ninja import FilterSchema, ModelSchema, Schema
 from pydantic import Field, NonNegativeInt
 
-from core.schemas import UserProfileSchema
-from sas.models import Picture
+from core.schemas import SimpleUserSchema, UserProfileSchema
+from sas.models import Picture, PictureModerationRequest
 
 
 class PictureFilterSchema(FilterSchema):
@@ -52,3 +52,11 @@ class PictureRelationCreationSchema(Schema):
 class IdentifiedUserSchema(Schema):
     id: int
     user: UserProfileSchema
+
+
+class ModerationRequestSchema(ModelSchema):
+    author: SimpleUserSchema
+
+    class Meta:
+        model = PictureModerationRequest
+        fields = ["id", "created_at", "reason"]

sas/static/webpack/sas/viewer-index.ts

@@ -11,10 +11,12 @@ import {
   type IdentifiedUserSchema,
   type PictureSchema,
   type PicturesFetchIdentificationsResponse,
+  type PicturesFetchModerationRequestsResponse,
   type PicturesFetchPicturesData,
   type UserProfileSchema,
   picturesDeletePicture,
   picturesFetchIdentifications,
+  picturesFetchModerationRequests,
   picturesFetchPictures,
   picturesIdentifyUsers,
   picturesModeratePicture,
@@ -27,18 +29,20 @@ import {
  * able to prefetch its data.
  */
 class PictureWithIdentifications {
-  identifications: PicturesFetchIdentificationsResponse | null = null;
+  identifications: PicturesFetchIdentificationsResponse = null;
   imageLoading = false;
   identificationsLoading = false;
+  moderationLoading = false;
   id: number;
   // biome-ignore lint/style/useNamingConvention: api is in snake_case
   compressed_url: string;
+  moderationRequests: PicturesFetchModerationRequestsResponse = null;
 
   constructor(picture: PictureSchema) {
     Object.assign(this, picture);
   }
 
-  static fromPicture(picture: PictureSchema) {
+  static fromPicture(picture: PictureSchema): PictureWithIdentifications {
     return new PictureWithIdentifications(picture);
   }
 
@@ -46,7 +50,7 @@ class PictureWithIdentifications {
    * If not already done, fetch the users identified on this picture and
    * populate the identifications field
    */
-  async loadIdentifications(options?: { forceReload: boolean }) {
+  async loadIdentifications(options?: { forceReload: boolean }): Promise<void> {
     if (this.identificationsLoading) {
       return; // The users are already being fetched.
     }
@@ -65,11 +69,29 @@ class PictureWithIdentifications {
     this.identificationsLoading = false;
   }
 
+  async loadModeration(options?: { forceReload: boolean }): Promise<void> {
+    if (this.moderationLoading) {
+      return; // The moderation requests are already being fetched.
+    }
+    if (!!this.moderationRequests && !options?.forceReload) {
+      // The moderation requests are already fetched
+      // and the user does not want to force the reload
+      return;
+    }
+    this.moderationLoading = true;
+    this.moderationRequests = (
+      await picturesFetchModerationRequests({
+        // biome-ignore lint/style/useNamingConvention: api is in snake_case
+        path: { picture_id: this.id },
+      })
+    ).data;
+    this.moderationLoading = false;
+  }
+
   /**
    * Preload the photo and the identifications
-   * @return {Promise<void>}
    */
-  async preload() {
+  async preload(): Promise<void> {
     const img = new Image();
     img.src = this.compressed_url;
     if (!img.complete) {
@@ -87,12 +109,12 @@ interface ViewerConfig {
   userId: number;
   /** Url of the current album */
   albumUrl: string;
-  /** Id of the album to displlay */
+  /** Id of the album to display */
   albumId: number;
   /** id of the first picture to load on the page */
   firstPictureId: number;
   /** if the user is sas admin */
-  userIsSasAdmin: number;
+  userIsSasAdmin: boolean;
 }
 
 /**
@@ -103,9 +125,8 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
     Alpine.data("picture_viewer", () => ({
       /**
        * All the pictures that can be displayed on this picture viewer
-       * @type PictureWithIdentifications[]
        **/
-      pictures: [],
+      pictures: [] as PictureWithIdentifications[],
       /**
        * The currently displayed picture
        * Default dummy data are pre-loaded to avoid javascript error
@@ -131,14 +152,12 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
       },
       /**
        * The picture which will be displayed next if the user press the "next" button
-       * @type ?PictureWithIdentifications
        **/
-      nextPicture: null,
+      nextPicture: null as PictureWithIdentifications,
       /**
        * The picture which will be displayed next if the user press the "previous" button
-       * @type ?PictureWithIdentifications
        **/
-      previousPicture: null,
+      previousPicture: null as PictureWithIdentifications,
       /**
        * The select2 component used to identify users
        **/
@@ -148,13 +167,11 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
        **/
       /**
        * Error message when a moderation operation fails
-       * @type string
        **/
       moderationError: "",
       /**
        * Method of pushing new url to the browser history
        * Used by popstate event and always reset to it's default value when used
-       * @type History
        **/
       pushstate: History.Push,
 
@@ -166,7 +183,7 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
           } as PicturesFetchPicturesData)
         ).map(PictureWithIdentifications.fromPicture);
         this.selector = sithSelect2({
-          element: $(this.$refs.search) as unknown as HTMLElement,
+          element: this.$refs.search,
           dataSource: remoteDataSource(await makeUrl(userSearchUsers), {
             excluded: () => [
               ...(this.currentPicture.identifications || []).map(
@@ -213,7 +230,7 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
        * and the previous picture, the next picture and
        * the list of identified users are updated.
        */
-      async updatePicture() {
+      async updatePicture(): Promise<void> {
         const updateArgs = {
           data: { sasPictureId: this.currentPicture.id },
           unused: "",
@@ -231,16 +248,23 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
         }
 
         this.moderationError = "";
-        const index = this.pictures.indexOf(this.currentPicture);
+        const index: number = this.pictures.indexOf(this.currentPicture);
         this.previousPicture = this.pictures[index - 1] || null;
         this.nextPicture = this.pictures[index + 1] || null;
-        await this.currentPicture.loadIdentifications();
         this.$refs.mainPicture?.addEventListener("load", () => {
           // once the current picture is loaded,
           // start preloading the next and previous pictures
           this.nextPicture?.preload();
           this.previousPicture?.preload();
         });
+        if (this.currentPicture.asked_for_removal && config.userIsSasAdmin) {
+          await Promise.all([
+            this.currentPicture.loadIdentifications(),
+            this.currentPicture.loadModeration(),
+          ]);
+        } else {
+          await this.currentPicture.loadIdentifications();
+        }
       },
 
       async moderatePicture() {
@@ -253,7 +277,7 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
           return;
         }
         this.currentPicture.is_moderated = true;
-        this.currentPicture.askedForRemoval = false;
+        this.currentPicture.asked_for_removal = false;
       },
 
       async deletePicture() {
@@ -277,7 +301,7 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
       /**
        * Send the identification request and update the list of identified users.
        */
-      async submitIdentification() {
+      async submitIdentification(): Promise<void> {
         await picturesIdentifyUsers({
           path: {
             // biome-ignore lint/style/useNamingConvention: api is in snake_case
@@ -292,18 +316,15 @@ exportToHtml("loadViewer", (config: ViewerConfig) => {
 
       /**
        * Check if an identification can be removed by the currently logged user
-       * @param {PictureIdentification} identification
-       * @return {boolean}
        */
-      canBeRemoved(identification: IdentifiedUserSchema) {
+      canBeRemoved(identification: IdentifiedUserSchema): boolean {
         return config.userIsSasAdmin || identification.user.id === config.userId;
       },
 
       /**
        * Untag a user from the current picture
-       * @param {PictureIdentification} identification
        */
-      async removeIdentification(identification: IdentifiedUserSchema) {
+      async removeIdentification(identification: IdentifiedUserSchema): Promise<void> {
         const res = await usersidentifiedDeleteRelation({
           // biome-ignore lint/style/useNamingConvention: api is in snake_case
           path: { relation_id: identification.id },

sas/templates/sas/picture.jinja

@@ -2,7 +2,7 @@
 
 {%- block additional_css -%}
   <link rel="stylesheet" href="{{ static('sas/css/picture.scss') }}">
-  <link rel="stylesheet" href="{{ static('webpack/sas/viewer-index.css') }}" defer>
+  <link rel="stylesheet" href="{{ static('webpack/sas/viewer-index.css') }}">
 {%- endblock -%}
 
 {%- block additional_js -%}
@@ -30,10 +30,10 @@
     <br>
 
     <template x-if="!currentPicture.is_moderated">
-      <div class="alert alert-red">
+      <div class="alert alert-red" @click="console.log(currentPicture)">
         <div class="alert-main">
-          <template x-if="currentPicture.askedForRemoval">
-            <span class="important">{% trans %}Asked for removal{% endtrans %}</span>
+          <template x-if="currentPicture.asked_for_removal">
+            <h3 class="alert-title">{% trans %}Asked for removal{% endtrans %}</h3>
           </template>
           <p>
             {% trans trimmed %}
@@ -41,16 +41,33 @@
               It will be hidden to other users until it has been moderated.
             {% endtrans %}
           </p>
+          <template x-if="currentPicture.asked_for_removal">
+            <div>
+              <h5 @click="console.log(currentPicture.moderationRequests)">
+                {% trans %}The following issues have been raised:{% endtrans %}
+              </h5>
+              <template x-for="req in (currentPicture.moderationRequests ?? [])" :key="req.id">
+                <div>
+                  <h6
+                    x-text="`${req.author.first_name} ${req.author.last_name}`"
+                  ></h6>
+                  <i x-text="Intl.DateTimeFormat(
+                             '{{ LANGUAGE_CODE }}',
+                             {dateStyle: 'long', timeStyle: 'short'}
+                             ).format(new Date(req.created_at))"></i>
+                  <blockquote x-text="`> ${req.reason}`"></blockquote>
+                </div>
+              </template>
+            </div>
+          </template>
         </div>
-        <div>
-          <div>
-            <button class="btn btn-blue" @click="moderatePicture()">
-              {% trans %}Moderate{% endtrans %}
-            </button>
-            <button class="btn btn-red" @click.prevent="deletePicture()">
-              {% trans %}Delete{% endtrans %}
-            </button>
-          </div>
+        <div class="alert-aside">
+          <button class="btn btn-blue" @click="moderatePicture()">
+            {% trans %}Moderate{% endtrans %}
+          </button>
+          <button class="btn btn-red" @click.prevent="deletePicture()">
+            {% trans %}Delete{% endtrans %}
+          </button>
           <p x-show="!!moderationError" x-text="moderationError"></p>
         </div>
       </div>
@@ -58,7 +75,6 @@
 
     <div class="container" id="pict">
       <div class="main">
-
         <div class="photo" :aria-busy="currentPicture.imageLoading">
           <img
             :src="currentPicture.compressed_url"

sas/tests/test_api.py

@@ -3,35 +3,45 @@ from django.db import transaction
 from django.test import TestCase
 from django.urls import reverse
 from model_bakery import baker
+from model_bakery.recipe import Recipe
 from pytest_django.asserts import assertNumQueries
 
 from core.baker_recipes import old_subscriber_user, subscriber_user
-from core.models import RealGroup, User
+from core.models import RealGroup, SithFile, User
 from sas.baker_recipes import picture_recipe
-from sas.models import Album, PeoplePictureRelation, Picture
+from sas.models import Album, PeoplePictureRelation, Picture, PictureModerationRequest
 
 
 class TestSas(TestCase):
     @classmethod
     def setUpTestData(cls):
-        Picture.objects.all().delete()
+        sas = SithFile.objects.get(pk=settings.SITH_SAS_ROOT_DIR_ID)
+
+        Picture.objects.exclude(id=sas.id).delete()
         owner = User.objects.get(username="root")
 
         cls.user_a = old_subscriber_user.make()
         cls.user_b, cls.user_c = subscriber_user.make(_quantity=2)
 
         picture = picture_recipe.extend(owner=owner)
-        cls.album_a = baker.make(Album, is_in_sas=True)
-        cls.album_b = baker.make(Album, is_in_sas=True)
+        cls.album_a = baker.make(Album, is_in_sas=True, parent=sas)
+        cls.album_b = baker.make(Album, is_in_sas=True, parent=sas)
+        relation_recipe = Recipe(PeoplePictureRelation)
+        relations = []
         for album in cls.album_a, cls.album_b:
             pictures = picture.make(parent=album, _quantity=5, _bulk_create=True)
-            baker.make(PeoplePictureRelation, picture=pictures[1], user=cls.user_a)
-            baker.make(PeoplePictureRelation, picture=pictures[2], user=cls.user_a)
-            baker.make(PeoplePictureRelation, picture=pictures[2], user=cls.user_b)
-            baker.make(PeoplePictureRelation, picture=pictures[3], user=cls.user_b)
-            baker.make(PeoplePictureRelation, picture=pictures[4], user=cls.user_a)
-            baker.make(PeoplePictureRelation, picture=pictures[4], user=cls.user_b)
-            baker.make(PeoplePictureRelation, picture=pictures[4], user=cls.user_c)
+            relations.extend(
+                [
+                    relation_recipe.prepare(picture=pictures[1], user=cls.user_a),
+                    relation_recipe.prepare(picture=pictures[2], user=cls.user_a),
+                    relation_recipe.prepare(picture=pictures[2], user=cls.user_b),
+                    relation_recipe.prepare(picture=pictures[3], user=cls.user_b),
+                    relation_recipe.prepare(picture=pictures[4], user=cls.user_a),
+                    relation_recipe.prepare(picture=pictures[4], user=cls.user_b),
+                    relation_recipe.prepare(picture=pictures[4], user=cls.user_c),
+                ]
+            )
+        PeoplePictureRelation.objects.bulk_create(relations)
 
 
 class TestPictureSearch(TestSas):
@@ -170,3 +180,49 @@ class TestPictureRelation(TestSas):
         res = self.client.delete(f"/api/sas/relation/{relation.id}")
         assert res.status_code == 404
         assert PeoplePictureRelation.objects.count() == relation_count
+
+
+class TestPictureModeration(TestSas):
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cls.sas_admin = baker.make(
+            User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)]
+        )
+        cls.picture = Picture.objects.filter(parent=cls.album_a)[0]
+        cls.picture.is_moderated = False
+        cls.picture.asked_for_removal = True
+        cls.picture.save()
+        cls.url = reverse("api:picture_moderate", kwargs={"picture_id": cls.picture.id})
+        baker.make(PictureModerationRequest, picture=cls.picture, author=cls.user_a)
+
+    def test_moderation_route_forbidden(self):
+        """Test that basic users (even if owner) cannot moderate a picture."""
+        self.picture.owner = self.user_b
+
+        for user in baker.make(User), subscriber_user.make(), self.user_b:
+            self.client.force_login(user)
+            res = self.client.patch(self.url)
+            assert res.status_code == 403
+
+    def test_moderation_route_authorized(self):
+        """Test that sas admins can moderate a picture."""
+        self.client.force_login(self.sas_admin)
+        res = self.client.patch(self.url)
+        assert res.status_code == 200
+        self.picture.refresh_from_db()
+        assert self.picture.is_moderated
+        assert not self.picture.asked_for_removal
+        assert not self.picture.moderation_requests.exists()
+
+    def test_get_moderation_requests(self):
+        """Test that fetching moderation requests work."""
+
+        url = reverse(
+            "api:picture_moderation_requests", kwargs={"picture_id": self.picture.id}
+        )
+        self.client.force_login(self.sas_admin)
+        res = self.client.get(url)
+        assert res.status_code == 200
+        assert len(res.json()) == 1
+        assert res.json()[0]["author"]["id"] == self.user_a.id

sas/tests/test_views.py

@@ -20,7 +20,7 @@ from django.core.cache import cache
 from django.test import Client, TestCase
 from django.urls import reverse
 from model_bakery import baker
-from pytest_django.asserts import assertRedirects
+from pytest_django.asserts import assertInHTML, assertRedirects
 
 from core.baker_recipes import old_subscriber_user, subscriber_user
 from core.models import RealGroup, User
@@ -70,7 +70,9 @@ def test_album_access_non_subscriber(client: Client):
 class TestSasModeration(TestCase):
     @classmethod
     def setUpTestData(cls):
-        album = baker.make(Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID)
+        album = baker.make(
+            Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True
+        )
         cls.pictures = picture_recipe.make(
             parent=album, _quantity=10, _bulk_create=True
         )
@@ -82,6 +84,9 @@ class TestSasModeration(TestCase):
         )
         cls.simple_user = subscriber_user.make()
 
+    def setUp(self):
+        cache.clear()
+
     def test_moderation_page_sas_admin(self):
         """Test that a moderator can see the pictures needing moderation."""
         self.client.force_login(self.moderator)
@@ -132,3 +137,37 @@ class TestSasModeration(TestCase):
         )
         assert res.status_code == 403
         assert Picture.objects.filter(pk=self.to_moderate.id).exists()
+
+    def test_request_moderation_form_access(self):
+        """Test that regular can access the form to ask for moderation."""
+        self.client.force_login(self.simple_user)
+        res = self.client.get(
+            reverse(
+                "sas:picture_ask_removal", kwargs={"picture_id": self.pictures[1].id}
+            ),
+        )
+        assert res.status_code == 200
+
+    def test_request_moderation_form_submit(self):
+        """Test that moderation requests are created."""
+        self.client.force_login(self.simple_user)
+        message = "J'aime pas cette photo (ni la Cocarde)."
+        url = reverse(
+            "sas:picture_ask_removal", kwargs={"picture_id": self.pictures[1].id}
+        )
+        res = self.client.post(url, data={"reason": message})
+        assertRedirects(
+            res, reverse("sas:album", kwargs={"album_id": self.pictures[1].parent_id})
+        )
+        assert self.pictures[1].moderation_requests.count() == 1
+        assert self.pictures[1].moderation_requests.first().reason == message
+
+        # test that the user cannot ask for moderation twice
+        res = self.client.post(url, data={"reason": message})
+        assert res.status_code == 200
+        assert self.pictures[1].moderation_requests.count() == 1
+        assertInHTML(
+            '<ul class="errorlist nonfield"><li>'
+            "Vous avez déjà déposé une demande de retrait pour cette photo.</li></ul>",
+            res.content.decode(),
+        )