From b8429a510fdca312394decf44e551075f30b6a9a Mon Sep 17 00:00:00 2001 From: Sli Date: Fri, 31 Oct 2025 12:14:33 +0100 Subject: [PATCH 1/3] posters: fix broken moderation view --- club/views.py | 27 ++++++++++++- com/templates/com/poster_list.jinja | 19 +++------ com/templates/com/poster_moderate.jinja | 43 -------------------- com/views.py | 53 +++++++++++++++++++++---- 4 files changed, 78 insertions(+), 64 deletions(-) delete mode 100644 com/templates/com/poster_moderate.jinja diff --git a/club/views.py b/club/views.py index a14b71cd..ff09a5f5 100644 --- a/club/views.py +++ b/club/views.py @@ -762,7 +762,19 @@ class PosterListView(ClubTabsMixin, PosterListBaseView): """List communication posters.""" current_tab = "posters" - extra_context = {"app": "club"} + 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} + ), + }, + } def get_queryset(self): return super().get_queryset().filter(club=self.club.id) @@ -770,6 +782,19 @@ class PosterListView(ClubTabsMixin, PosterListBaseView): def get_object(self): 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 + class PosterCreateView(ClubTabsMixin, PosterCreateBaseView): """Create communication poster.""" diff --git a/com/templates/com/poster_list.jinja b/com/templates/com/poster_list.jinja index d723c248..fe247af5 100644 --- a/com/templates/com/poster_list.jinja +++ b/com/templates/com/poster_list.jinja @@ -12,14 +12,11 @@
-

{% trans %}Posters{% endtrans %}

- @@ -43,11 +40,7 @@
{{ poster.date_begin | localtime | date("d/M/Y H:m") }}
{{ poster.date_end | localtime | date("d/M/Y H:m") }}
- {% if app == "com" %} - {% trans %}Edit{% endtrans %} - {% elif app == "club" %} - {% trans %}Edit{% endtrans %} - {% endif %} + {{ action["label"] }}
    {% for screen in poster.screens.all() %} diff --git a/com/templates/com/poster_moderate.jinja b/com/templates/com/poster_moderate.jinja deleted file mode 100644 index 6370becf..00000000 --- a/com/templates/com/poster_moderate.jinja +++ /dev/null @@ -1,43 +0,0 @@ -{% extends "core/base.jinja" %} - -{% block script %} - {{ super() }} - -{% endblock %} - -{% block additional_css %} - -{% endblock %} - -{% block content %} -
    - -
    - -

    {% trans %}Posters - moderation{% endtrans %}

    -
    - -
    - - {% if object_list.count == 0 %} -
    {% trans %}No objects{% endtrans %}
    - {% else %} - - {% for poster in object_list %} -
    -
    {{ poster.name }}
    -
    - Moderate -
    - {% endfor %} - - {% endif %} - -
    - -
    - -
    -{% endblock %} diff --git a/com/views.py b/com/views.py index a1897b12..ee37d5cb 100644 --- a/com/views.py +++ b/com/views.py @@ -637,6 +637,31 @@ class PosterListView(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} + ), + }, + } def get_queryset(self): qs = super().get_queryset() @@ -644,11 +669,6 @@ class PosterListView(ComTabsMixin, PosterListBaseView): return qs return qs.filter(club=self.club.id) - def get_context_data(self, **kwargs): - kwargs = super().get_context_data(**kwargs) - kwargs["app"] = "com" - return kwargs - class PosterCreateView(ComTabsMixin, PosterCreateBaseView): """Create communication poster.""" @@ -677,10 +697,29 @@ class PosterModerateListView(PermissionRequiredMixin, ComTabsMixin, ListView): current_tab = "posters" model = Poster - template_name = "com/poster_moderate.jinja" + template_name = "com/poster_list.jinja" queryset = Poster.objects.filter(is_moderated=False).all() permission_required = "com.moderate_poster" - extra_context = {"app": "com"} + 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): From 118a08372fecf004d2624336b41fc715068e5a75 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 31 Oct 2025 17:15:16 +0100 Subject: [PATCH 2/3] 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): From 917a2b50cc3dd696289a17239121d0cfb2c57e42 Mon Sep 17 00:00:00 2001 From: Sli Date: Fri, 31 Oct 2025 21:51:12 +0100 Subject: [PATCH 3/3] Fix naming, fix tooltip and cosmetic changes --- club/views.py | 2 +- com/static/com/css/posters.scss | 22 ---------------------- com/templates/com/poster_list.jinja | 14 +++++--------- com/views.py | 2 +- 4 files changed, 7 insertions(+), 33 deletions(-) diff --git a/club/views.py b/club/views.py index 2c67a56a..02e87ba7 100644 --- a/club/views.py +++ b/club/views.py @@ -777,7 +777,7 @@ class PosterListView( "create_url": reverse_lazy( "club:poster_create", kwargs={"club_id": self.club.id} ), - "edit_url_factory": lambda poster: reverse( + "get_edit_url": lambda poster: reverse( "club:poster_edit", kwargs={"club_id": self.club.id, "poster_id": poster.id}, ), diff --git a/com/static/com/css/posters.scss b/com/static/com/css/posters.scss index 072f1d6d..757d151f 100644 --- a/com/static/com/css/posters.scss +++ b/com/static/com/css/posters.scss @@ -130,28 +130,6 @@ } } - .tooltip { - visibility: hidden; - width: 120px; - background-color: hsl(210, 20%, 98%); - color: hsl(0, 0%, 0%); - text-align: center; - padding: 5px 0; - border-radius: 6px; - position: absolute; - z-index: 10; - - ul { - margin-left: 0; - display: inline-block; - - li { - display: list-item; - list-style-type: none; - } - } - } - &.not_moderated { border: 1px solid red; } diff --git a/com/templates/com/poster_list.jinja b/com/templates/com/poster_list.jinja index a7ce4f5a..5460f54e 100644 --- a/com/templates/com/poster_list.jinja +++ b/com/templates/com/poster_list.jinja @@ -29,6 +29,9 @@ class="image" hover="{% trans %}Click to expand{% endtrans %}" @click="active = $el.firstElementChild" + tooltip="{%- for screen in poster.screens.all() -%} + {{ screen }} + {% endfor %}" > {{ poster.name }}
    @@ -38,7 +41,7 @@
    {% if poster.is_editable %} - + {% trans %}Edit{% endtrans %} @@ -46,20 +49,13 @@ {% 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 %}
diff --git a/com/views.py b/com/views.py index 04da8564..eae9bf6a 100644 --- a/com/views.py +++ b/com/views.py @@ -650,7 +650,7 @@ class PosterListView(PermissionRequiredMixin, ComTabsMixin, PosterListBaseView): current_tab = "posters" extra_context = { "create_url": reverse_lazy("com:poster_create"), - "edit_url_factory": lambda poster: reverse( + "get_edit_url": lambda poster: reverse( "com:poster_edit", kwargs={"poster_id": poster.id} ), }