add og tags to sas main page

Quand quelqu'un qui n'a pas le droit tente d'accéder au SAS, il reçoit un HTTP 200 au lieu d'un 403. C'est pas forcément le plus pertinent, mais autant en profiter pour mettre les tags og.

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

@@ -44,7 +44,7 @@ dependencies = [
     "django-honeypot>=1.3.0,<2",
     "pydantic-extra-types>=2.11.0,<3.0.0",
     "ical>=11.1.0,<12",
-    "redis[hiredis]>=7.4.0,<8.0.0",
+    "redis[hiredis]>=5.3.0,<8.0.0",
     "environs[django]>=14.5.0,<15.0.0",
     "requests>=2.32.5,<3.0.0",
     "honcho>=2.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/static/sas/css/picture.scss

@@ -134,7 +134,7 @@
           --loading-size: 20px
         }
 
-        @media (max-width: 1000px) {
+        @media (min-width: 700px) and (max-width: 1000px) {
           max-width: calc(50% - 5px);
         }
 
@@ -201,57 +201,52 @@
   }
 }
 
-.general {
+#pict .general {
   display: flex;
   flex-direction: row;
-  gap: 20px;
+  gap: 3em;
+  justify-content: space-evenly;
 
   @media (max-width: 1000px) {
+    gap: 1em;
     flex-direction: column;
   }
 
-  >.infos {
+  .infos, .tools {
+    flex: 1;
     display: flex;
     flex-direction: column;
-    width: 50%;
+    gap: .5em;
+    @media (min-width: 700px) {
+      max-width: 350px;
+    }
+  }
+  .infos > div, .tools > div > div {
+    display: flex;
+    flex-direction: column;
+    gap: .35em;
+  }
 
-    >div>div {
+  .tools > div, >.infos >div>div {
     display: flex;
     flex-direction: row;
     justify-content: space-between;
-
-      >*:first-child {
-        min-width: 150px;
-
-        @media (max-width: 1000px) {
-          min-width: auto;
-        }
-      }
-    }
   }
 
   >.tools {
-    display: flex;
-    flex-direction: column;
-    width: 50%;
+    flex: 1;
 
-    >div {
-      display: flex;
-      flex-direction: row;
-      justify-content: space-between;
-
-      >div {
-        >a.button {
-          box-sizing: border-box;
+    >div>div {
+      >a.btn {
         background-color: $primary-neutral-light-color;
         display: flex;
         justify-content: center;
         align-items: center;
-          padding: 10px;
+        padding: 0;
         color: black;
-          border-radius: 5px;
         width: 40px;
         height: 40px;
+        font-size: 20px;
 
         &:hover {
           background-color: #aaa;
@@ -268,9 +263,9 @@
 
       &.buttons {
         display: flex;
+        flex-direction: row;
         gap: 5px;
       }
     }
   }
-  }
 }

sas/templates/sas/main.jinja

@@ -12,6 +12,17 @@
   {% trans %}See all the photos taken during events organised by the AE.{% endtrans %}
 {%- endblock %}
 
+{% block metatags %}
+  <meta property="og:url" content="{{ request.build_absolute_uri() }}" />
+  <meta property="og:type" content="website" />
+  <meta property="og:title" content="Stock à souvenirs" />
+  <meta
+    property="og:description"
+    content="Retrouvez toutes les photos prises durant les événements organisés par l'AE."
+  />
+  <meta property="og:image" content="{{ request.build_absolute_uri(static("core/img/logo_no_text.png")) }}" />
+{% endblock %}
+
 {% set is_sas_admin = user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
 
 {% from "sas/macros.jinja" import display_album %}

sas/templates/sas/picture.jinja

@@ -118,15 +118,20 @@
                 <a class="text" :href="currentPicture.full_size_url">
                   {% trans %}HD version{% endtrans %}
                 </a>
-                <br>
-                <a class="text danger" :href="currentPicture.report_url">
+                <a class="text danger " :href="currentPicture.report_url">
                   {% trans %}Ask for removal{% endtrans %}
                 </a>
               </div>
               <div class="buttons">
-                <a class="button" :href="currentPicture.edit_url"><i class="fa-regular fa-pen-to-square edit-action"></i></a>
-                <a class="button" href="?rotate_left"><i class="fa-solid fa-rotate-left"></i></a>
-                <a class="button" href="?rotate_right"><i class="fa-solid fa-rotate-right"></i></a>
+                <a
+                  class="btn btn-no-text"
+                  :href="currentPicture.edit_url"
+                  x-show="{{ user.has_perm("sas.change_sasfile")|tojson }} || currentPicture.owner.id === {{ user.id }}"
+                >
+                  <i class="fa-regular fa-pen-to-square edit-action"></i>
+                </a>
+                <a class="btn btn-no-text" href="?rotate_left"><i class="fa-solid fa-rotate-left"></i></a>
+                <a class="btn btn-no-text" href="?rotate_right"><i class="fa-solid fa-rotate-right"></i></a>
               </div>
             </div>
           </div>

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,25 @@ 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.
+    # Also, the SAS itself shouldn't be listed in the albums.
+    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"""
@@ -89,6 +110,15 @@ def test_album_access_non_subscriber(client: Client):
     assert res.status_code == 200
 
 
+@pytest.mark.django_db
+def test_accessing_sas_from_album_view_is_404(client: Client):
+    """Test that trying to see the SAS with a regular album view isn't allowed."""
+    res = client.get(
+        reverse("sas:album", kwargs={"album_id": settings.SITH_SAS_ROOT_DIR_ID})
+    )
+    assert res.status_code == 404
+
+
 @pytest.mark.django_db
 class TestAlbumUpload:
     @staticmethod
@@ -133,6 +163,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"