Merge pull request #779 from ae-utbm/fix-queryset

fix crash on album fetch & test

sas/baker_recipes.py

@@ -0,0 +1,18 @@
+from model_bakery import seq
+from model_bakery.recipe import Recipe
+
+from sas.models import Picture
+
+picture_recipe = Recipe(
+    Picture,
+    is_in_sas=True,
+    is_folder=False,
+    is_moderated=True,
+    name=seq("Picture "),
+)
+"""A SAS Picture fixture.
+
+Warnings:
+    If you don't `bulk_create` this, you need
+    to explicitly set the parent album, or it won't work
+"""

sas/models.py

@@ -176,7 +176,7 @@ class AlbumQuerySet(models.QuerySet):
         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(moderated=True)
+            return self.filter(is_moderated=True)
         # known bug : if all children of an album are also albums
         # then this album is excluded, even if one of the sub-albums should be visible.
         # The fs-like navigation is likely to be half-broken for non-subscribers,

sas/tests/test_api.py

@@ -3,10 +3,11 @@ 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 sas.baker_recipes import picture_recipe
 from sas.models import Album, PeoplePictureRelation, Picture
 
 
@@ -19,13 +20,11 @@ class TestSas(TestCase):
         cls.user_a = old_subscriber_user.make()
         cls.user_b, cls.user_c = subscriber_user.make(_quantity=2)
 
-        picture_recipe = Recipe(
+        picture = picture_recipe.extend(owner=owner)
-            Picture, is_in_sas=True, is_folder=False, owner=owner, is_moderated=True
-        )
         cls.album_a = baker.make(Album, is_in_sas=True)
         cls.album_b = baker.make(Album, is_in_sas=True)
         for album in cls.album_a, cls.album_b:
-            pictures = picture_recipe.make(parent=album, _quantity=5, _bulk_create=True)
+            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)
@@ -36,22 +35,25 @@ class TestSas(TestCase):
 
 
 class TestPictureSearch(TestSas):
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cls.url = reverse("api:pictures")
+
     def test_anonymous_user_forbidden(self):
-        res = self.client.get(reverse("api:pictures"))
+        res = self.client.get(self.url)
         assert res.status_code == 403
 
     def test_filter_by_album(self):
         self.client.force_login(self.user_b)
-        res = self.client.get(reverse("api:pictures") + f"?album_id={self.album_a.id}")
+        res = self.client.get(self.url + f"?album_id={self.album_a.id}")
         assert res.status_code == 200
         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)
-        res = self.client.get(
+        res = self.client.get(self.url + f"?users_identified={self.user_a.id}")
-            reverse("api:pictures") + f"?users_identified={self.user_a.id}"
-        )
         assert res.status_code == 200
         expected = list(
             self.user_a.pictures.order_by(
@@ -63,7 +65,7 @@ class TestPictureSearch(TestSas):
     def test_filter_by_multiple_user(self):
         self.client.force_login(self.user_b)
         res = self.client.get(
-            reverse("api:pictures")
+            self.url
             + f"?users_identified={self.user_a.id}&users_identified={self.user_b.id}"
         )
         assert res.status_code == 200
@@ -78,9 +80,7 @@ class TestPictureSearch(TestSas):
         """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(
+        res = self.client.get(f"{self.url}?users_identified={self.user_a.id}")
-            reverse("api:pictures") + f"?users_identified={self.user_a.id}"
-        )
         assert res.status_code == 200
         expected = list(
             self.user_a.pictures.order_by(
@@ -92,7 +92,7 @@ class TestPictureSearch(TestSas):
         # trying to access the pictures of someone else mixed with owned pictures
         # should return only owned pictures
         res = self.client.get(
-            reverse("api:pictures")
+            self.url
             + f"?users_identified={self.user_a.id}&users_identified={self.user_b.id}"
         )
         assert res.status_code == 200
@@ -100,15 +100,13 @@ class TestPictureSearch(TestSas):
 
         # trying to fetch everything should be the same
         # as fetching its own pictures for a non-subscriber
-        res = self.client.get(reverse("api:pictures"))
+        res = self.client.get(self.url)
         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(
+        res = self.client.get(f"{self.url}?users_identified={self.user_b.id}")
-            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())
@@ -117,6 +115,16 @@ class TestPictureSearch(TestSas):
         )
         assert [i["id"] for i in res.json()["results"]] == expected
 
+    def test_num_queries(self):
+        """Test that the number of queries is stable."""
+        self.client.force_login(subscriber_user.make())
+        with assertNumQueries(5):
+            # 1 request to fetch the user from the db
+            # 2 requests to check the user permissions
+            # 1 request to fetch the pictures
+            # 1 request to count the total number of items in the pagination
+            self.client.get(self.url)
+
 
 class TestPictureRelation(TestSas):
     def test_delete_relation_route_forbidden(self):

sas/tests/test_model.py

@@ -1,8 +1,9 @@
 from django.test import TestCase
-from model_bakery import baker, seq
+from model_bakery import baker
 
 from core.baker_recipes import old_subscriber_user, subscriber_user
 from core.models import User
+from sas.baker_recipes import picture_recipe
 from sas.models import Picture
 
 
@@ -10,15 +11,7 @@ class TestPictureQuerySet(TestCase):
     @classmethod
     def setUpTestData(cls):
         Picture.objects.all().delete()
-        cls.pictures = baker.make(
+        cls.pictures = picture_recipe.make(_quantity=10, _bulk_create=True)
-            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):

sas/tests/test_views.py

@@ -0,0 +1,66 @@
+#
+# Copyright 2023 © AE UTBM
+# ae@utbm.fr / ae.info@utbm.fr
+#
+# This file is part of the website of the UTBM Student Association (AE UTBM),
+# https://ae.utbm.fr.
+#
+# You can find the source code of the website at https://github.com/ae-utbm/sith3
+#
+# LICENSED UNDER THE GNU GENERAL PUBLIC LICENSE VERSION 3 (GPLv3)
+# SEE : https://raw.githubusercontent.com/ae-utbm/sith3/master/LICENSE
+# OR WITHIN THE LOCAL FILE "LICENSE"
+#
+#
+from typing import Callable
+
+import pytest
+from django.conf import settings
+from django.core.cache import cache
+from django.test import Client
+from django.urls import reverse
+from model_bakery import baker
+
+from core.baker_recipes import old_subscriber_user, subscriber_user
+from core.models import RealGroup, User
+from sas.baker_recipes import picture_recipe
+from sas.models import Album
+
+# Create your tests here.
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "user_factory",
+    [
+        subscriber_user.make,
+        old_subscriber_user.make,
+        lambda: baker.make(User, is_superuser=True),
+        lambda: baker.make(
+            User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)]
+        ),
+        lambda: baker.make(User),
+    ],
+)
+def test_load_main_page(client: Client, user_factory: Callable[[], User]):
+    """Just check that the SAS doesn't crash."""
+    user = user_factory()
+    client.force_login(user)
+    res = client.get(reverse("sas:main"))
+    assert res.status_code == 200
+
+
+@pytest.mark.django_db
+def test_album_access_non_subscriber(client: Client):
+    """Test that non-subscribers can only access albums where they are identified."""
+    album = baker.make(Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID)
+    user = baker.make(User)
+    client.force_login(user)
+    res = client.get(reverse("sas:album", kwargs={"album_id": album.id}))
+    assert res.status_code == 403
+
+    picture = picture_recipe.make(parent=album)
+    picture.people.create(user=user)
+    cache.clear()
+    res = client.get(reverse("sas:album", kwargs={"album_id": album.id}))
+    assert res.status_code == 200

sas/tests/tests.py

@@ -1,16 +0,0 @@
-#
-# Copyright 2023 © AE UTBM
-# ae@utbm.fr / ae.info@utbm.fr
-#
-# This file is part of the website of the UTBM Student Association (AE UTBM),
-# https://ae.utbm.fr.
-#
-# You can find the source code of the website at https://github.com/ae-utbm/sith3
-#
-# LICENSED UNDER THE GNU GENERAL PUBLIC LICENSE VERSION 3 (GPLv3)
-# SEE : https://raw.githubusercontent.com/ae-utbm/sith3/master/LICENSE
-# OR WITHIN THE LOCAL FILE "LICENSE"
-#
-#
-
-# Create your tests here.