From 72ea6b6fddb3c985f625728af3dcc78d9ddfd9ae Mon Sep 17 00:00:00 2001 From: thomas girod Date: Sat, 10 Aug 2024 00:43:15 +0200 Subject: [PATCH] fix crash on album fetch & test --- sas/baker_recipes.py | 18 +++++++++++ sas/models.py | 2 +- sas/tests/test_api.py | 46 ++++++++++++++++------------ sas/tests/test_model.py | 13 ++------ sas/tests/test_views.py | 66 +++++++++++++++++++++++++++++++++++++++++ sas/tests/tests.py | 16 ---------- 6 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 sas/baker_recipes.py create mode 100644 sas/tests/test_views.py delete mode 100644 sas/tests/tests.py diff --git a/sas/baker_recipes.py b/sas/baker_recipes.py new file mode 100644 index 00000000..1f7a7667 --- /dev/null +++ b/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 +""" diff --git a/sas/models.py b/sas/models.py index 5b6467f5..af1b30ff 100644 --- a/sas/models.py +++ b/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, diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py index 0f84e99f..f16adf29 100644 --- a/sas/tests/test_api.py +++ b/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, is_in_sas=True, is_folder=False, owner=owner, is_moderated=True - ) + 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) 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( - reverse("api:pictures") + f"?users_identified={self.user_a.id}" - ) + res = self.client.get(self.url + 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( - reverse("api:pictures") + f"?users_identified={self.user_a.id}" - ) + res = self.client.get(f"{self.url}?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( - reverse("api:pictures") + f"?users_identified={self.user_b.id}" - ) + res = self.client.get(f"{self.url}?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): diff --git a/sas/tests/test_model.py b/sas/tests/test_model.py index 76d3b2cb..5cbbc9c0 100644 --- a/sas/tests/test_model.py +++ b/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( - Picture, - is_moderated=True, - is_in_sas=True, - is_folder=False, - name=seq(""), - _quantity=10, - _bulk_create=True, - ) + cls.pictures = picture_recipe.make(_quantity=10, _bulk_create=True) Picture.objects.filter(pk=cls.pictures[0].id).update(is_moderated=False) def test_root(self): diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py new file mode 100644 index 00000000..ac83f199 --- /dev/null +++ b/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 diff --git a/sas/tests/tests.py b/sas/tests/tests.py deleted file mode 100644 index 48d8f1f6..00000000 --- a/sas/tests/tests.py +++ /dev/null @@ -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.