From 118a08372fecf004d2624336b41fc715068e5a75 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 31 Oct 2025 17:15:16 +0100 Subject: [PATCH] simplify poster moderation --- club/views.py | 40 ++++------- com/models.py | 4 +- com/static/com/css/posters.scss | 54 +++------------ com/templates/com/poster_list.jinja | 87 ++++++++++++++---------- com/tests/test_views.py | 40 ++++++++++- com/urls.py | 6 -- com/views.py | 102 ++++++++++------------------ 7 files changed, 148 insertions(+), 185 deletions(-) diff --git a/club/views.py b/club/views.py index ff09a5f5..2c67a56a 100644 --- a/club/views.py +++ b/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): diff --git a/com/models.py b/com/models.py index 6485d73b..3d67ffec 100644 --- a/com/models.py +++ b/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) diff --git a/com/static/com/css/posters.scss b/com/static/com/css/posters.scss index e3863bdb..072f1d6d 100644 --- a/com/static/com/css/posters.scss +++ b/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%; + } } } diff --git a/com/templates/com/poster_list.jinja b/com/templates/com/poster_list.jinja index fe247af5..a7ce4f5a 100644 --- a/com/templates/com/poster_list.jinja +++ b/com/templates/com/poster_list.jinja @@ -12,47 +12,58 @@
-

{{ links["title"] }}

-
- - {% if poster_list.count() == 0 %} -
{% trans %}No posters{% endtrans %}
- {% else %} - - {% for poster in poster_list %} -
-
{{ poster.name }}
-
- -
-
-
{{ poster.date_begin | localtime | date("d/M/Y H:m") }}
-
{{ poster.date_end | localtime | date("d/M/Y H:m") }}
-
- {{ action["label"] }} -
-
    - {% for screen in poster.screens.all() %} -
  • {{ screen }}
    • - {% endfor %} -
-
+ {% for poster in poster_list %} +
+
{{ poster.name }}
+
+ {{ poster.name }}
- {% endfor %} - - {% endif %} - +
+
{{ poster.date_begin | localtime | date("d/M/Y H:m") }}
+
{{ poster.date_end | localtime | date("d/M/Y H:m") }}
+
+
+ {% if poster.is_editable %} + + + {% trans %}Edit{% endtrans %} + + {% endif %} + {% if not poster.is_moderated and user.has_perm("com.moderate_poster") %} +
+ {% csrf_token %} + +
+ {% endif %} +
+
+
    + {% for screen in poster.screens.all() %} +
  • {{ screen }}
    • + {% endfor %} +
+
+
+ {% else %} +
{% trans %}No posters{% endtrans %}
+ {% endfor %}
-
+
+ +
diff --git a/com/tests/test_views.py b/com/tests/test_views.py index 02081597..0e48f033 100644 --- a/com/tests/test_views.py +++ b/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 diff --git a/com/urls.py b/com/urls.py index 8afbfd12..51861316 100644 --- a/com/urls.py +++ b/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//moderate/", PosterModerateView.as_view(), diff --git a/com/views.py b/com/views.py index ee37d5cb..04da8564 100644 --- a/com/views.py +++ b/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):