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