From 09f8d0fd6da8d3e8d083b2a819ecedfa90a8b05f Mon Sep 17 00:00:00 2001 From: Sli Date: Fri, 3 Apr 2026 16:49:19 +0200 Subject: [PATCH] Fix bug where you can't select /SAS as a parent album --- core/management/commands/populate.py | 2 +- sas/forms.py | 4 +- sas/models.py | 8 +- sas/tests/test_views.py | 176 +++++++++++++++++++++++++++ sas/views.py | 5 +- 5 files changed, 191 insertions(+), 4 deletions(-) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 4c1c5379..52244792 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -110,7 +110,7 @@ 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 = Album.objects.create(name="SAS", owner=root) main_club = Club.objects.create( id=1, name="AE", address="6 Boulevard Anatole France, 90000 Belfort" ) diff --git a/sas/forms.py b/sas/forms.py index af3547c8..a478ce43 100644 --- a/sas/forms.py +++ b/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): diff --git a/sas/models.py b/sas/models.py index 64f6c15b..5d8cee44 100644 --- a/sas/models.py +++ b/sas/models.py @@ -205,7 +205,13 @@ 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(id=settings.SITH_SAS_ROOT_DIR_ID) | Q(is_in_sas=True, is_folder=True) + ) + ) class Album(SasFile): diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py index 8d67997d..aab74512 100644 --- a/sas/tests/test_views.py +++ b/sas/tests/test_views.py @@ -12,6 +12,7 @@ # OR WITHIN THE LOCAL FILE "LICENSE" # # +from datetime import date from typing import Callable import pytest @@ -20,6 +21,7 @@ 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 import timezone from model_bakery import baker from pytest_django.asserts import assertHTMLEqual, assertInHTML, assertRedirects @@ -133,6 +135,180 @@ 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, sas_root: Album) -> Album: + return baker.make(Album, parent=sas_root, is_moderated=True) + + @pytest.fixture + def moderator(self) -> User: + return baker.make( + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + ) + + @pytest.fixture + def subscriber(self) -> User: + return subscriber_user.make() + + @pytest.fixture + def root(self) -> User: + return User.objects.get(username="root") + + @pytest.mark.parametrize( + "user", + [ + None, + "subscriber", + ], + ) + def test_permission_denied( + self, + client: Client, + album: Album, + request: pytest.FixtureRequest, + user: str | None, + ): + if user: + client.force_login(request.getfixturevalue(user)) + response = client.get(reverse("sas:album_edit", kwargs={"album_id": album.pk})) + assert response.status_code == 403 + response = client.post(reverse("sas:album_edit", kwargs={"album_id": album.pk})) + assert response.status_code == 403 + + def test_sas_root_read_only(self, client: Client, sas_root: Album, moderator: User): + client.force_login(moderator) + response = client.get( + reverse("sas:album_edit", kwargs={"album_id": sas_root.pk}) + ) + assert response.status_code == 404 + response = client.post( + reverse("sas:album_edit", kwargs={"album_id": sas_root.pk}) + ) + assert response.status_code == 404 + + @pytest.mark.parametrize( + "user", + [ + "root", + "moderator", + ], + ) + def test_update( + self, + client: Client, + album: Album, + sas_root: Album, + request: pytest.FixtureRequest, + user: str, + ): + client.force_login(request.getfixturevalue(user)) + response = client.get(reverse("sas:album_edit", kwargs={"album_id": album.pk})) + assert response.status_code == 200 + + # Test no changes + response = client.post(reverse("sas:album_edit", kwargs={"album_id": album.pk})) + assert response.status_code == 200 + + updated_album = Album.objects.get(id=album.pk) + assert album.name == updated_album.name + assert album.date == updated_album.date + assert album.parent == updated_album.parent + assert album.edit_groups == updated_album.edit_groups + + # Prepare a good payload + payload = { + "name": album.name[50], + "parent": baker.make(Album, parent=sas_root, is_moderated=True).pk, + "date": date.today().isoformat(), + "recursive": False, + } + # Test missing parent + payload_missing_parent = {**payload} + del payload_missing_parent["parent"] + response = client.post( + reverse( + "sas:album_edit", + kwargs={"album_id": album.pk}, + ), + payload_missing_parent, + ) + assert response.status_code == 200 + updated_album = Album.objects.get(id=album.pk) + assert updated_album.name == album.name + assert updated_album.parent == album.parent + assert updated_album.date == album.date + + # Test missing date + payload_missing_date = {**payload} + del payload_missing_date["date"] + response = client.post( + reverse( + "sas:album_edit", + kwargs={"album_id": album.pk}, + ), + payload_missing_date, + ) + assert response.status_code == 200 + updated_album = Album.objects.get(id=album.pk) + assert updated_album.name == album.name + assert updated_album.parent == album.parent + assert updated_album.date == album.date + + # Test recursive parent + payload_recursive_parent = {**payload} + payload_recursive_parent["parent"] = album.pk + response = client.post( + reverse( + "sas:album_edit", + kwargs={"album_id": album.pk}, + ), + payload_recursive_parent, + ) + assert response.status_code == 200 + updated_album = Album.objects.get(id=album.pk) + assert updated_album.name == album.name + assert updated_album.parent == album.parent + assert updated_album.date == album.date + + # Test successful update + response = client.post( + reverse( + "sas:album_edit", + kwargs={"album_id": album.pk}, + ), + payload, + ) + assert response.status_code == 302 + updated_album = Album.objects.get(id=album.pk) + assert updated_album.name == payload["name"] + assert updated_album.parent.id == payload["parent"] + assert timezone.localdate(updated_album.date) == date.fromisoformat( + payload["date"] + ) + + # Test root album can be used as parent + payload["parent"] = sas_root.pk + response = client.post( + reverse( + "sas:album_edit", + kwargs={"album_id": album.pk}, + ), + payload, + ) + assert response.status_code == 302 + updated_album = Album.objects.get(id=album.pk) + assert updated_album.name == payload["name"] + assert updated_album.parent.id == payload["parent"] + assert timezone.localdate(updated_album.date) == date.fromisoformat( + payload["date"] + ) + + class TestSasModeration(TestCase): @classmethod def setUpTestData(cls): diff --git a/sas/views.py b/sas/views.py index a2145a94..7e58b97b 100644 --- a/sas/views.py +++ b/sas/views.py @@ -37,7 +37,7 @@ from sas.forms import ( PictureModerationRequestForm, PictureUploadForm, ) -from sas.models import Album, PeoplePictureRelation, Picture +from sas.models import Album, AlbumQuerySet, PeoplePictureRelation, Picture class AlbumCreateFragment(FragmentMixin, CreateView): @@ -266,6 +266,9 @@ class AlbumEditView(CanEditMixin, UpdateView): template_name = "core/edit.jinja" pk_url_kwarg = "album_id" + def get_queryset(self) -> AlbumQuerySet: + return super().get_queryset().exclude(id=settings.SITH_SAS_ROOT_DIR_ID) + def form_valid(self, form): ret = super().form_valid(form) if form.cleaned_data["recursive"]: