diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py
index 5a282143..3d50e90f 100644
--- a/core/management/commands/populate.py
+++ b/core/management/commands/populate.py
@@ -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, is_in_sas=True)
+ 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"
)
diff --git a/core/models.py b/core/models.py
index 43d2e7ca..ffbdebfc 100644
--- a/core/models.py
+++ b/core/models.py
@@ -886,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:
diff --git a/core/tests/test_files.py b/core/tests/test_files.py
index f48f6d0b..3144b1f6 100644
--- a/core/tests/test_files.py
+++ b/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
diff --git a/sas/models.py b/sas/models.py
index 5d8cee44..c5d53410 100644
--- a/sas/models.py
+++ b/sas/models.py
@@ -205,13 +205,7 @@ class AlbumQuerySet(models.QuerySet):
class SASAlbumManager(models.Manager):
def get_queryset(self):
- return (
- super()
- .get_queryset()
- .filter(
- Q(id=settings.SITH_SAS_ROOT_DIR_ID) | Q(is_in_sas=True, is_folder=True)
- )
- )
+ return super().get_queryset().filter(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 c5e86da8..bc2e6cab 100644
--- a/sas/tests/test_views.py
+++ b/sas/tests/test_views.py
@@ -20,7 +20,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 django.utils.timezone import localdate
from model_bakery import baker
from pytest_django.asserts import assertHTMLEqual, assertInHTML, assertRedirects
@@ -66,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"""
@@ -149,11 +167,7 @@ class TestAlbumEdit:
@pytest.mark.parametrize(
"user",
- [
- None,
- lambda: baker.make(User),
- subscriber_user.make,
- ],
+ [None, lambda: baker.make(User), subscriber_user.make],
)
def test_permission_denied(
self,
@@ -164,9 +178,10 @@ class TestAlbumEdit:
if user:
client.force_login(user())
- response = client.get(reverse("sas:album_edit", kwargs={"album_id": album.pk}))
+ url = reverse("sas:album_edit", kwargs={"album_id": album.pk})
+ response = client.get(url)
assert response.status_code == 403
- response = client.post(reverse("sas:album_edit", kwargs={"album_id": album.pk}))
+ response = client.post(url)
assert response.status_code == 403
def test_sas_root_read_only(self, client: Client, sas_root: Album):
@@ -174,13 +189,10 @@ class TestAlbumEdit:
User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)]
)
client.force_login(moderator)
- response = client.get(
- reverse("sas:album_edit", kwargs={"album_id": sas_root.pk})
- )
+ url = reverse("sas:album_edit", kwargs={"album_id": sas_root.pk})
+ response = client.get(url)
assert response.status_code == 404
- response = client.post(
- reverse("sas:album_edit", kwargs={"album_id": sas_root.pk})
- )
+ response = client.post(url)
assert response.status_code == 404
@pytest.mark.parametrize(
@@ -198,7 +210,7 @@ class TestAlbumEdit:
data = {
"name": album.name[: Album.NAME_MAX_LENGTH],
"parent": baker.make(Album, parent=album.parent, is_moderated=True).pk,
- "date": timezone.now().strftime("%Y-%m-%d"),
+ "date": localdate().strftime("%Y-%m-%d"),
"file": "/random/path",
"edit_groups": [settings.SITH_GROUP_SAS_ADMIN_ID],
"recursive": False,
@@ -210,7 +222,7 @@ class TestAlbumEdit:
data = {
"name": album.name[: Album.NAME_MAX_LENGTH],
"parent": album.pk,
- "date": timezone.now().strftime("%Y-%m-%d"),
+ "date": localdate().strftime("%Y-%m-%d"),
}
assert AlbumEditForm(data=data).is_valid()
@@ -223,19 +235,12 @@ class TestAlbumEdit:
payload = {
"name": album.name[: Album.NAME_MAX_LENGTH],
"parent": album.pk,
- "date": timezone.now().strftime("%Y-%m-%d"),
+ "date": localdate().strftime("%Y-%m-%d"),
}
response = client.post(
- reverse(
- "sas:album_edit",
- kwargs={"album_id": album.pk},
- ),
- payload,
- )
- assertInHTML(
- "Boucle dans l'arborescence des dossiers",
- response.text,
+ reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload
)
+ assertInHTML("Boucle dans l'arborescence des dossiers", response.text)
assert response.status_code == 200
@pytest.mark.parametrize(
@@ -247,57 +252,39 @@ class TestAlbumEdit:
),
],
)
+ @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())
-
- # Prepare a good payload
expected_redirect = reverse("sas:album", kwargs={"album_id": album.pk})
- expected_date = timezone.now()
payload = {
"name": album.name[: Album.NAME_MAX_LENGTH],
- "parent": baker.make(Album, parent=sas_root, is_moderated=True).pk,
- "date": expected_date.strftime("%Y-%m-%d"),
+ "parent": parent().id,
+ "date": localdate().strftime("%Y-%m-%d"),
"recursive": False,
}
-
- # Test successful update
response = client.post(
- reverse(
- "sas:album_edit",
- kwargs={"album_id": album.pk},
- ),
- payload,
+ reverse("sas:album_edit", kwargs={"album_id": album.pk}), payload
)
assertRedirects(response, expected_redirect)
- 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) == timezone.localdate(
- expected_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,
- )
- assertRedirects(response, expected_redirect)
- 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) == timezone.localdate(
- expected_date
- )
+ album.refresh_from_db()
+ assert album.name == payload["name"]
+ assert album.parent.id == payload["parent"]
+ assert localdate(album.date) == localdate()
class TestSasModeration(TestCase):
diff --git a/sas/views.py b/sas/views.py
index 7e58b97b..160182e4 100644
--- a/sas/views.py
+++ b/sas/views.py
@@ -37,7 +37,7 @@ from sas.forms import (
PictureModerationRequestForm,
PictureUploadForm,
)
-from sas.models import Album, AlbumQuerySet, PeoplePictureRelation, Picture
+from sas.models import Album, PeoplePictureRelation, Picture
class AlbumCreateFragment(FragmentMixin, CreateView):
@@ -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,13 +267,11 @@ 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"
- 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"]: