actually fix bug where you can't select /SAS as a parent album

Fix hidden user can't search itself

core/management/commands/populate.py

@@ -16,7 +16,7 @@
 # details.
 #
 # You should have received a copy of the GNU General Public License along with
-# this program; if not, write to the Free Sofware Foundation, Inc., 59 Temple
+# this program; if not, write to the Free Software Foundation, Inc., 59 Temple
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
@@ -110,7 +110,9 @@ class Command(BaseCommand):
         p.save(force_lock=True)
 
         club_root = SithFile.objects.create(name="clubs", owner=root)
-        sas = SithFile.objects.create(name="SAS", owner=root)
+        sas = SithFile.objects.create(
+            name="SAS", owner=root, id=settings.SITH_SAS_ROOT_DIR_ID
+        )
         main_club = Club.objects.create(
             id=1, name="AE", address="6 Boulevard Anatole France, 90000 Belfort"
         )

core/models.py

@@ -131,7 +131,9 @@ class UserQuerySet(models.QuerySet):
         if user.has_perm("core.view_hidden_user"):
             return self
         if user.has_perm("core.view_user"):
-            return self.filter(Q(is_viewable=True) | Q(whitelisted_users=user))
+            return self.filter(
+                Q(is_viewable=True) | Q(whitelisted_users=user) | Q(pk=user.pk)
+            )
         if user.is_anonymous:
             return self.none()
         return self.filter(id=user.id)
@@ -884,8 +886,10 @@ class SithFile(models.Model):
         return self.get_parent_path() + "/" + self.name
 
     def save(self, *args, **kwargs):
-        sas = SithFile.objects.filter(id=settings.SITH_SAS_ROOT_DIR_ID).first()
-        self.is_in_sas = sas in self.get_parent_list() or self == sas
+        sas_id = settings.SITH_SAS_ROOT_DIR_ID
+        self.is_in_sas = self.id == sas_id or any(
+            p.id == sas_id for p in self.get_parent_list()
+        )
         adding = self._state.adding
         super().save(*args, **kwargs)
         if adding:

core/tests/test_files.py

@@ -344,3 +344,14 @@ def test_quick_upload_image(
     assert (
         parsed["name"] == Path(file.name).stem[: QuickUploadImage.IMAGE_NAME_SIZE - 1]
     )
+
+
+@pytest.mark.django_db
+def test_populated_sas_is_in_sas():
+    """Test that, in the data generated by the populate command,
+    the SAS has value is_in_sas=True.
+
+    If it's not the case, it has no incidence in prod, but it's annoying
+    in dev and may cause misunderstandings.
+    """
+    assert SithFile.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID).is_in_sas

core/tests/test_user.py

@@ -410,12 +410,20 @@ class TestUserQuerySetViewableBy:
         assert set(viewable) == set(users)
 
     @pytest.mark.parametrize(
-        "user_factory", [old_subscriber_user.make, subscriber_user.make]
+        "user_factory",
+        [
+            old_subscriber_user.make,
+            lambda: old_subscriber_user.make(is_viewable=False),
+            subscriber_user.make,
+            lambda: subscriber_user.make(is_viewable=False),
+        ],
     )
-    def test_subscriber(self, users: list[User], user_factory):
+    def test_can_search(self, users: list[User], user_factory):
         user = user_factory()
-        viewable = User.objects.filter(id__in=[u.id for u in users]).viewable_by(user)
-        assert set(viewable) == {users[0], users[1]}
+        viewable = User.objects.filter(
+            id__in=[u.id for u in [*users, user]]
+        ).viewable_by(user)
+        assert set(viewable) == {user, users[0], users[1]}
 
     def test_whitelist(self, users: list[User]):
         user = subscriber_user.make()

pyproject.toml

@@ -82,7 +82,7 @@ tests = [
 ]
 docs = [
     "mkdocs<2.0.0,>=1.6.1",
-    "mkdocs-material>=9.7.6,<10.0.0",
+    "mkdocs-material>=9.7.5,<10.0.0",
     "mkdocstrings>=1.0.3,<2.0.0",
     "mkdocstrings-python>=2.0.3,<3.0.0",
     "mkdocs-include-markdown-plugin>=7.2.1,<8.0.0",

sas/forms.py

@@ -50,13 +50,15 @@ class AlbumEditForm(forms.ModelForm):
         model = Album
         fields = ["name", "date", "file", "parent", "edit_groups"]
         widgets = {
-            "parent": AutoCompleteSelectAlbum,
             "edit_groups": AutoCompleteSelectMultipleGroup,
         }
 
     name = forms.CharField(max_length=Album.NAME_MAX_LENGTH, label=_("file name"))
     date = forms.DateField(label=_("Date"), widget=SelectDate, required=True)
     recursive = forms.BooleanField(label=_("Apply rights recursively"), required=False)
+    parent = forms.ModelChoiceField(
+        Album.objects.all(), required=True, widget=AutoCompleteSelectAlbum
+    )
 
 
 class PictureModerationRequestForm(forms.ModelForm):

sas/models.py

@@ -205,7 +205,7 @@ class AlbumQuerySet(models.QuerySet):
 
 class SASAlbumManager(models.Manager):
     def get_queryset(self):
-        return super().get_queryset().filter(is_in_sas=True, is_folder=True)
+        return super().get_queryset().filter(Q(is_in_sas=True, is_folder=True))
 
 
 class Album(SasFile):

sas/tests/test_views.py

@@ -20,12 +20,14 @@ from django.conf import settings
 from django.core.cache import cache
 from django.test import Client, TestCase
 from django.urls import reverse
+from django.utils.timezone import localdate
 from model_bakery import baker
 from pytest_django.asserts import assertHTMLEqual, assertInHTML, assertRedirects
 
 from core.baker_recipes import old_subscriber_user, subscriber_user
 from core.models import Group, User
 from sas.baker_recipes import picture_recipe
+from sas.forms import AlbumEditForm
 from sas.models import Album, Picture
 
 # Create your tests here.
@@ -64,6 +66,24 @@ def test_main_page_no_form_for_regular_users(client: Client):
     assert len(forms) == 0
 
 
+@pytest.mark.django_db
+def test_main_page_displayed_albums(client: Client):
+    """Test that the right data is displayed on the SAS main page"""
+    sas = Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID)
+    Album.objects.exclude(id=sas.id).delete()
+    album_a = baker.make(Album, parent=sas, is_moderated=True)
+    album_b = baker.make(Album, parent=album_a, is_moderated=True)
+    album_c = baker.make(Album, parent=sas, is_moderated=True)
+    baker.make(Album, parent=sas, is_moderated=False)
+    client.force_login(subscriber_user.make())
+    res = client.get(reverse("sas:main"))
+    # album_b is not a direct child of the SAS, so it shouldn't be displayed
+    # in the categories, but it should appear in the latest albums.
+    # album_d isn't moderated, so it shouldn't appear at all for a simple user.
+    assert res.context_data["latest"] == [album_c, album_b, album_a]
+    assert res.context_data["categories"] == [album_a, album_c]
+
+
 @pytest.mark.django_db
 def test_main_page_content_anonymous(client: Client):
     """Test that public users see only an incentive to login"""
@@ -133,6 +153,140 @@ class TestAlbumUpload:
         assert not album.children.exists()
 
 
+@pytest.mark.django_db
+class TestAlbumEdit:
+    @pytest.fixture
+    def sas_root(self) -> Album:
+        return Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID)
+
+    @pytest.fixture
+    def album(self) -> Album:
+        return baker.make(
+            Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True
+        )
+
+    @pytest.mark.parametrize(
+        "user",
+        [None, lambda: baker.make(User), subscriber_user.make],
+    )
+    def test_permission_denied(
+        self,
+        client: Client,
+        album: Album,
+        user: Callable[[], User] | None,
+    ):
+        if user:
+            client.force_login(user())
+
+        url = reverse("sas:album_edit", kwargs={"album_id": album.pk})
+        response = client.get(url)
+        assert response.status_code == 403
+        response = client.post(url)
+        assert response.status_code == 403
+
+    def test_sas_root_read_only(self, client: Client, sas_root: Album):
+        moderator = baker.make(
+            User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)]
+        )
+        client.force_login(moderator)
+        url = reverse("sas:album_edit", kwargs={"album_id": sas_root.pk})
+        response = client.get(url)
+        assert response.status_code == 404
+        response = client.post(url)
+        assert response.status_code == 404
+
+    @pytest.mark.parametrize(
+        ("excluded", "is_valid"),
+        [
+            ("name", False),
+            ("date", False),
+            ("file", True),
+            ("parent", False),
+            ("edit_groups", True),
+            ("recursive", True),
+        ],
+    )
+    def test_form_required(self, album: Album, excluded: str, is_valid: bool):  # noqa: FBT001
+        data = {
+            "name": album.name[: Album.NAME_MAX_LENGTH],
+            "parent": baker.make(Album, parent=album.parent, is_moderated=True).pk,
+            "date": localdate().strftime("%Y-%m-%d"),
+            "file": "/random/path",
+            "edit_groups": [settings.SITH_GROUP_SAS_ADMIN_ID],
+            "recursive": False,
+        }
+        del data[excluded]
+        assert AlbumEditForm(data=data).is_valid() == is_valid
+
+    def test_form_album_name(self, album: Album):
+        data = {
+            "name": album.name[: Album.NAME_MAX_LENGTH],
+            "parent": album.pk,
+            "date": localdate().strftime("%Y-%m-%d"),
+        }
+        assert AlbumEditForm(data=data).is_valid()
+
+        data["name"] = album.name[: Album.NAME_MAX_LENGTH + 1]
+        assert not AlbumEditForm(data=data).is_valid()
+
+    def test_update_recursive_parent(self, client: Client, album: Album):
+        client.force_login(baker.make(User, is_superuser=True))
+
+        payload = {
+            "name": album.name[: Album.NAME_MAX_LENGTH],
+            "parent": album.pk,
+            "date": localdate().strftime("%Y-%m-%d"),
+        }
+        response = client.post(
+            reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload
+        )
+        assertInHTML("<li>Boucle dans l'arborescence des dossiers</li>", response.text)
+        assert response.status_code == 200
+
+    @pytest.mark.parametrize(
+        "user",
+        [
+            lambda: baker.make(User, is_superuser=True),
+            lambda: baker.make(
+                User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)]
+            ),
+        ],
+    )
+    @pytest.mark.parametrize(
+        "parent",
+        [
+            lambda: baker.make(
+                Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID, is_moderated=True
+            ),
+            lambda: Album.objects.get(id=settings.SITH_SAS_ROOT_DIR_ID),
+        ],
+    )
+    def test_update(
+        self,
+        client: Client,
+        album: Album,
+        sas_root: Album,
+        user: Callable[[], User],
+        parent: Callable[[], Album],
+    ):
+        client.force_login(user())
+        expected_redirect = reverse("sas:album", kwargs={"album_id": album.pk})
+        payload = {
+            "name": album.name[: Album.NAME_MAX_LENGTH],
+            "parent": parent().id,
+            "date": localdate().strftime("%Y-%m-%d"),
+            "recursive": False,
+        }
+        response = client.post(
+            reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload
+        )
+        assertRedirects(response, expected_redirect)
+        album.refresh_from_db()
+        assert album.name == payload["name"]
+        assert album.parent.id == payload["parent"]
+        assert localdate(album.date) == localdate()
+
+
 class TestSasModeration(TestCase):
     @classmethod
     def setUpTestData(cls):

sas/views.py

@@ -85,7 +85,9 @@ class SASMainView(UseFragmentsMixin, TemplateView):
         kwargs["categories"] = list(
             albums_qs.filter(parent_id=settings.SITH_SAS_ROOT_DIR_ID).order_by("id")
         )
-        kwargs["latest"] = list(albums_qs.order_by("-id")[:5])
+        kwargs["latest"] = list(
+            albums_qs.exclude(id=settings.SITH_SAS_ROOT_DIR_ID).order_by("-id")[:5]
+        )
         return kwargs
 
 
@@ -126,6 +128,9 @@ def send_thumb(request, picture_id):
 
 class AlbumView(CanViewMixin, UseFragmentsMixin, DetailView):
     model = Album
+    # exclude the SAS from the album accessible with this view
+    # the SAS can be viewed only with SASMainView
+    queryset = Album.objects.exclude(id=settings.SITH_SAS_ROOT_DIR_ID)
     pk_url_kwarg = "album_id"
     template_name = "sas/album.jinja"
 
@@ -262,6 +267,7 @@ class PictureAskRemovalView(CanViewMixin, DetailView, FormView):
 
 class AlbumEditView(CanEditMixin, UpdateView):
     model = Album
+    queryset = Album.objects.exclude(id=settings.SITH_SAS_ROOT_DIR_ID)
     form_class = AlbumEditForm
     template_name = "core/edit.jinja"
     pk_url_kwarg = "album_id"