simplify poster moderation

club/views.py

@@ -59,7 +59,7 @@ from com.views import (
     PosterEditBaseView,
     PosterListBaseView,
 )
-from core.auth.mixins import CanEditMixin
+from core.auth.mixins import CanEditMixin, PermissionOrClubBoardRequiredMixin
 from core.models import PageRev
 from core.views import DetailFormView, PageEditViewBase, UseFragmentsMixin
 from core.views.mixins import FragmentMixin, FragmentRenderer, TabedViewMixin
@@ -758,23 +758,13 @@ class MailingAutoGenerationView(View):
         return redirect("club:mailing", club_id=club.id)
 
 
-class PosterListView(ClubTabsMixin, PosterListBaseView):
+class PosterListView(
+    PermissionOrClubBoardRequiredMixin, ClubTabsMixin, PosterListBaseView
+):
     """List communication posters."""
 
     current_tab = "posters"
-    extra_context = {
-        "links": {
-            "title": _("Posters"),
-            "position": "right",
-        },
-        "action": {
-            "class": "edit",
-            "label": _("Edit"),
-            "get_url": lambda club, poster: reverse(
-                "club:poster_edit", kwargs={"club_id": club.id, "poster_id": poster.id}
-            ),
-        },
-    }
+    permission_required = "com.view_poster"
 
     def get_queryset(self):
         return super().get_queryset().filter(club=self.club.id)
@@ -783,17 +773,15 @@ class PosterListView(ClubTabsMixin, PosterListBaseView):
         return self.club
 
     def get_context_data(self, **kwargs):
-        kwargs = super().get_context_data(**kwargs)
-        kwargs["links"]["links"] = [
-            {
-                "pk": "create",
-                "label": _("Create"),
-                "url": reverse_lazy(
-                    "club:poster_create", kwargs={"club_id": self.club.id}
-                ),
-            }
-        ]
-        return kwargs
+        return super().get_context_data(**kwargs) | {
+            "create_url": reverse_lazy(
+                "club:poster_create", kwargs={"club_id": self.club.id}
+            ),
+            "edit_url_factory": lambda poster: reverse(
+                "club:poster_edit",
+                kwargs={"club_id": self.club.id, "poster_id": poster.id},
+            ),
+        }
 
 
 class PosterCreateView(ClubTabsMixin, PosterCreateBaseView):

com/models.py

@@ -402,9 +402,7 @@ class Poster(models.Model):
                 groups__id__in=[settings.SITH_GROUP_COM_ADMIN_ID]
             ):
                 Notification.objects.create(
-                    user=user,
-                    url=reverse("com:poster_moderate_list"),
-                    type="POSTER_MODERATION",
+                    user=user, url=reverse("com:poster_list"), type="POSTER_MODERATION"
                 )
         return super().save(*args, **kwargs)
 

com/static/com/css/posters.scss

@@ -20,33 +20,7 @@
       position: absolute;
       display: flex;
       bottom: 5px;
-
-      &.left {
-        left: 0;
-      }
-
-      &.right {
-        right: 0;
-      }
-
-      .link {
-        padding: 5px;
-        padding-left: 20px;
-        padding-right: 20px;
-        margin-left: 5px;
-        border-radius: 20px;
-        background-color: hsl(40, 100%, 50%);
-        color: black;
-
-        &:hover {
-          color: black;
-          background-color: hsl(40, 58%, 50%);
-        }
-
-        &.delete {
-          background-color: hsl(0, 100%, 40%);
-        }
-      }
+      left: 0;
     }
   }
 
@@ -143,22 +117,16 @@
         }
       }
 
-      .edit,
-      .moderate,
-      .slideshow {
-        padding: 5px;
-        border-radius: 20px;
-        background-color: hsl(40, 100%, 50%);
-        color: black;
-
-        &:hover {
-          color: black;
-          background-color: hsl(40, 58%, 50%);
-        }
-
-        &:nth-child(2n) {
-          margin-top: 5px;
-          margin-bottom: 5px;
+      .actions {
+        display: flex;
+        flex-direction: column;
+        align-items: stretch;
+        form {
+          margin: unset;
+          padding: unset;
+          button {
+            width: 100%;
+          }
         }
       }
 

com/templates/com/poster_list.jinja

@@ -12,47 +12,58 @@
   <div id="poster_list" x-data="{ active: null }">
 
     <div id="title">
-      <h3>{{ links["title"] }}</h3>
-      <div id="links" class="{{ links["position"] }}">
-        {% for link in links["links"] %}
-          <a id="{{ link["pk"] }}" class="link" href="{{ link["url"] }}">{{ link["label"] }}</a>
-        {% endfor %}
+      <h3>{% trans %}Posters{% endtrans %}</h3>
+      <div id="links">
+        <a id="create" class="btn btn-blue" href="{{ create_url }}">
+          <i class="fa fa-plus"></i>
+          {% trans %}Create{% endtrans %}
+        </a>
       </div>
     </div>
 
     <div id="posters">
-
-      {% if poster_list.count() == 0 %}
-        <div id="no-posters">{% trans %}No posters{% endtrans %}</div>
-      {% else %}
-
-        {% for poster in poster_list %}
-          <div class="poster{% if not poster.is_moderated %} not_moderated{% endif %}">
-            <div class="name">{{ poster.name }}</div>
-            <div
-              class="image"
-              hover="{% trans %}Click to expand{% endtrans %}"
-              @click="active = $el.firstElementChild"
-            >
-              <img src="{{ poster.file.url }}"></img>
-            </div>
-            <div class="dates">
-              <div class="begin">{{ poster.date_begin | localtime | date("d/M/Y H:m") }}</div>
-              <div class="end">{{ poster.date_end | localtime | date("d/M/Y H:m") }}</div>
-            </div>
-            <a class="{{ action["class"] }}" href="{{ action["get_url"](club, poster) }}">{{ action["label"] }}</a>
-            <div class="tooltip">
-              <ul>
-                {% for screen in poster.screens.all() %}
-                  <li>{{ screen }}</li>
-                {% endfor %}
-              </ul>
-            </div>
+      {% for poster in poster_list %}
+        <div class="poster{% if not poster.is_moderated %} not_moderated{% endif %}">
+          <div class="name">{{ poster.name }}</div>
+          <div
+            class="image"
+            hover="{% trans %}Click to expand{% endtrans %}"
+            @click="active = $el.firstElementChild"
+          >
+            <img src="{{ poster.file.url }}" alt="{{ poster.name }}">
           </div>
-        {% endfor %}
-
-      {% endif %}
-
+          <div class="dates">
+            <div class="begin">{{ poster.date_begin | localtime | date("d/M/Y H:m") }}</div>
+            <div class="end">{{ poster.date_end | localtime | date("d/M/Y H:m") }}</div>
+          </div>
+          <div class="actions">
+            {% if poster.is_editable %}
+              <a class="btn btn-blue" href="{{ edit_url_factory(poster) }}">
+                <i class="fa fa-pen-to-square"></i>
+                {% trans %}Edit{% endtrans %}
+              </a>
+            {% endif %}
+            {% if not poster.is_moderated and user.has_perm("com.moderate_poster") %}
+              <form action="{{ url("com:poster_moderate", object_id=poster.id) }}" method="post">
+                {% csrf_token %}
+                <button type="submit" class="btn btn-blue">
+                  <i class="fa fa-check"></i>
+                  {% trans %}Moderate{% endtrans %}
+                </button>
+              </form>
+            {% endif %}
+          </div>
+          <div class="tooltip">
+            <ul>
+              {% for screen in poster.screens.all() %}
+                <li>{{ screen }}</li>
+              {% endfor %}
+            </ul>
+          </div>
+        </div>
+      {% else %}
+        <div id="no-posters">{% trans %}No posters{% endtrans %}</div>
+      {% endfor %}
     </div>
 
     <div
@@ -61,7 +72,9 @@
       @click="active = null"
       :class="{active: active !== null}"
     >
-      <div id="placeholder"><img :src="active?.src"></div>
+      <div id="placeholder">
+        <img :src="active?.src" :alt="active?.name">
+      </div>
     </div>
 
   </div>

com/tests/test_views.py

@@ -17,7 +17,9 @@ from unittest.mock import patch
 
 import pytest
 from django.conf import settings
+from django.contrib.auth.models import Permission
 from django.contrib.sites.models import Site
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import Client, TestCase
 from django.urls import reverse
 from django.utils import html
@@ -27,9 +29,10 @@ from model_bakery import baker
 from pytest_django.asserts import assertNumQueries, assertRedirects
 
 from club.models import Club, Membership
-from com.models import News, NewsDate, Sith, Weekmail, WeekmailArticle
+from com.models import News, NewsDate, Poster, Sith, Weekmail, WeekmailArticle
 from core.baker_recipes import subscriber_user
 from core.models import AnonymousUser, Group, User
+from core.utils import RED_PIXEL_PNG
 
 
 @pytest.fixture()
@@ -314,7 +317,6 @@ def test_feed(client: Client):
     [
         reverse("com:poster_list"),
         reverse("com:poster_create"),
-        reverse("com:poster_moderate_list"),
     ],
 )
 def test_poster_management_views_crash_test(client: Client, url: str):
@@ -325,3 +327,37 @@ def test_poster_management_views_crash_test(client: Client, url: str):
     client.force_login(user)
     res = client.get(url)
     assert res.status_code == 200
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "referer",
+    [
+        None,
+        reverse("com:poster_list"),
+        reverse("club:poster_list", kwargs={"club_id": settings.SITH_MAIN_CLUB_ID}),
+    ],
+)
+def test_moderate_poster(client: Client, referer: str | None):
+    poster = baker.make(
+        Poster,
+        is_moderated=False,
+        file=SimpleUploadedFile("test.png", content=RED_PIXEL_PNG),
+        club_id=settings.SITH_MAIN_CLUB_ID,
+    )
+    user = baker.make(
+        User,
+        user_permissions=Permission.objects.filter(
+            codename__in=["view_poster", "moderate_poster"]
+        ),
+    )
+    client.force_login(user)
+    headers = {"REFERER": f"https://{settings.SITH_URL}{referer}"} if referer else {}
+    response = client.post(
+        reverse("com:poster_moderate", kwargs={"object_id": poster.id}), headers=headers
+    )
+    result_url = referer or reverse("com:poster_list")
+    assertRedirects(response, result_url)
+    poster.refresh_from_db()
+    assert poster.is_moderated
+    assert poster.moderator == user

com/urls.py

@@ -33,7 +33,6 @@ from com.views import (
     PosterDeleteView,
     PosterEditView,
     PosterListView,
-    PosterModerateListView,
     PosterModerateView,
     ScreenCreateView,
     ScreenDeleteView,
@@ -102,11 +101,6 @@ urlpatterns = [
         PosterDeleteView.as_view(),
         name="poster_delete",
     ),
-    path(
-        "poster/moderate/",
-        PosterModerateListView.as_view(),
-        name="poster_moderate_list",
-    ),
     path(
         "poster/<int:object_id>/moderate/",
         PosterModerateView.as_view(),

com/views.py

@@ -25,6 +25,7 @@ import itertools
 from datetime import date, timedelta
 from smtplib import SMTPRecipientsRefused
 from typing import Any
+from urllib.parse import urlparse
 
 from dateutil.relativedelta import relativedelta
 from django.conf import settings
@@ -34,7 +35,7 @@ from django.contrib.auth.mixins import (
 )
 from django.contrib.syndication.views import Feed
 from django.core.exceptions import PermissionDenied, ValidationError
-from django.db.models import Max
+from django.db.models import Exists, Max, OuterRef, Value
 from django.forms.models import modelform_factory
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404, redirect
@@ -45,7 +46,7 @@ from django.utils.translation import gettext_lazy as _
 from django.views.generic import DetailView, ListView, TemplateView, View
 from django.views.generic.edit import CreateView, DeleteView, UpdateView
 
-from club.models import Club, Mailing
+from club.models import Club, Mailing, Membership
 from com.forms import NewsDateForm, NewsForm, PosterForm
 from com.ics_calendar import IcsCalendar
 from com.models import News, NewsDate, Poster, Screen, Sith, Weekmail, WeekmailArticle
@@ -561,16 +562,26 @@ class MailingModerateView(View):
         raise PermissionDenied
 
 
-class PosterListBaseView(PermissionOrClubBoardRequiredMixin, ListView):
+class PosterListBaseView(ListView):
     """List communication posters."""
 
     model = Poster
     template_name = "com/poster_list.jinja"
     permission_required = "com.view_poster"
-    ordering = ["-date_begin"]
 
-    def get_context_data(self, **kwargs):
-        return super().get_context_data(**kwargs) | {"club": self.club}
+    def get_queryset(self):
+        qs = Poster.objects.prefetch_related("screens")
+        if self.request.user.has_perm("com.edit_poster"):
+            qs = qs.annotate(is_editable=Value(value=True))
+        else:
+            qs = qs.annotate(
+                is_editable=Exists(
+                    Membership.objects.ongoing()
+                    .board()
+                    .filter(user=self.request.user, club=OuterRef("club_id"))
+                )
+            )
+        return qs.order_by("-date_begin")
 
 
 class PosterCreateBaseView(PermissionOrClubBoardRequiredMixin, CreateView):
@@ -633,41 +644,17 @@ class PosterDeleteBaseView(
     permission_required = "com.delete_poster"
 
 
-class PosterListView(ComTabsMixin, PosterListBaseView):
+class PosterListView(PermissionRequiredMixin, ComTabsMixin, PosterListBaseView):
     """List communication posters."""
 
     current_tab = "posters"
     extra_context = {
-        "links": {
-            "title": _("Posters"),
-            "position": "right",
-            "links": [
-                {
-                    "pk": "create",
-                    "label": _("Create"),
-                    "url": reverse_lazy("com:poster_create"),
-                },
-                {
-                    "pk": "moderation",
-                    "label": "Moderation",
-                    "url": reverse_lazy("com:poster_moderate_list"),
-                },
-            ],
-        },
-        "action": {
-            "class": "edit",
-            "label": _("Edit"),
-            "get_url": lambda club, poster: reverse(
-                "com:poster_edit", kwargs={"poster_id": poster.id}
-            ),
-        },
+        "create_url": reverse_lazy("com:poster_create"),
+        "edit_url_factory": lambda poster: reverse(
+            "com:poster_edit", kwargs={"poster_id": poster.id}
+        ),
     }
-
-    def get_queryset(self):
-        qs = super().get_queryset()
-        if self.request.user.has_perm("com.view_poster"):
-            return qs
-        return qs.filter(club=self.club.id)
+    permission_required = "com.view_poster"
 
 
 class PosterCreateView(ComTabsMixin, PosterCreateBaseView):
@@ -692,36 +679,6 @@ class PosterDeleteView(PosterDeleteBaseView):
     success_url = reverse_lazy("com:poster_list")
 
 
-class PosterModerateListView(PermissionRequiredMixin, ComTabsMixin, ListView):
-    """Moderate list communication poster."""
-
-    current_tab = "posters"
-    model = Poster
-    template_name = "com/poster_list.jinja"
-    queryset = Poster.objects.filter(is_moderated=False).all()
-    permission_required = "com.moderate_poster"
-    extra_context = {
-        "links": {
-            "position": "left",
-            "title": _("Posters - moderation"),
-            "links": [
-                {
-                    "pk": "list",
-                    "label": _("List"),
-                    "url": reverse_lazy("com:poster_list"),
-                }
-            ],
-        },
-        "action": {
-            "class": "moderate",
-            "label": _("Moderate"),
-            "get_url": lambda club, poster: reverse(
-                "com:poster_moderate", kwargs={"object_id": poster.id}
-            ),
-        },
-    }
-
-
 class PosterModerateView(PermissionRequiredMixin, ComTabsMixin, View):
     """Moderate communication poster."""
 
@@ -729,12 +686,21 @@ class PosterModerateView(PermissionRequiredMixin, ComTabsMixin, View):
     permission_required = "com.moderate_poster"
     extra_context = {"app": "com"}
 
-    def get(self, request, *args, **kwargs):
+    def post(self, request, *args, **kwargs):
         obj = get_object_or_404(Poster, pk=kwargs["object_id"])
         obj.is_moderated = True
         obj.moderator = request.user
         obj.save()
-        return redirect("com:poster_moderate_list")
+        # The moderation request may be originated from a club context (/club/poster)
+        # or a global context (/com/poster),
+        # so the redirection URL will be the URL of the page that called this view,
+        # as long as the latter belongs to the sith.
+        referer = self.request.META.get("HTTP_REFERER")
+        if referer:
+            parsed = urlparse(referer)
+            if parsed.netloc == settings.SITH_URL:
+                return redirect(parsed.path)
+        return redirect(reverse("com:poster_list"))
 
 
 class ScreenListView(PermissionRequiredMixin, ComTabsMixin, ListView):