Fix naming, fix tooltip and cosmetic changes

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,11 +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 = {"app": "club"}
+    permission_required = "com.view_poster"
 
     def get_queryset(self):
         return super().get_queryset().filter(club=self.club.id)
@@ -770,6 +772,17 @@ class PosterListView(ClubTabsMixin, PosterListBaseView):
     def get_object(self):
         return self.club
 
+    def get_context_data(self, **kwargs):
+        return super().get_context_data(**kwargs) | {
+            "create_url": reverse_lazy(
+                "club:poster_create", kwargs={"club_id": self.club.id}
+            ),
+            "get_edit_url": lambda poster: reverse(
+                "club:poster_edit",
+                kwargs={"club_id": self.club.id, "poster_id": poster.id},
+            ),
+        }
+
 
 class PosterCreateView(ClubTabsMixin, PosterCreateBaseView):
     """Create communication poster."""

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,34 +20,8 @@
       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%);
-        }
-      }
-    }
   }
 
   #posters,
@@ -143,43 +117,15 @@
         }
       }
 
-      .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;
-        }
-      }
-
-      .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;
+      .actions {
+        display: flex;
+        flex-direction: column;
+        align-items: stretch;
+        form {
+          margin: unset;
+          padding: unset;
+          button {
+            width: 100%;
           }
         }
       }

com/templates/com/poster_list.jinja

@@ -13,22 +13,15 @@
 
     <div id="title">
       <h3>{% trans %}Posters{% endtrans %}</h3>
-      <div id="links" class="right">
-        {% if app == "com" %}
-          <a id="create" class="link" href="{{ url(app + ":poster_create") }}">{% trans %}Create{% endtrans %}</a>
-          <a id="moderation" class="link" href="{{ url("com:poster_moderate_list") }}">{% trans %}Moderation{% endtrans %}</a>
-        {% elif app == "club" %}
-          <a id="create" class="link" href="{{ url(app + ":poster_create", club.id) }}">{% trans %}Create{% endtrans %}</a>
-        {% endif %}
+      <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>
@@ -36,30 +29,37 @@
             class="image"
             hover="{% trans %}Click to expand{% endtrans %}"
             @click="active = $el.firstElementChild"
+            tooltip="{%- for screen in poster.screens.all() -%}
+                       {{ screen }}
+                     {% endfor %}"
           >
-              <img src="{{ poster.file.url }}"></img>
+            <img src="{{ poster.file.url }}" alt="{{ poster.name }}">
           </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>
-            {% if app == "com" %}
-              <a class="edit" href="{{ url(app + ":poster_edit", poster.id) }}">{% trans %}Edit{% endtrans %}</a>
-            {% elif app == "club" %}
-              <a class="edit" href="{{ url(app + ":poster_edit", club.id, poster.id) }}">{% trans %}Edit{% endtrans %}</a>
+          <div class="actions">
+            {% if poster.is_editable %}
+              <a class="btn btn-blue" href="{{ get_edit_url(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-green">
+                  <i class="fa fa-check"></i>
+                  {% trans %}Moderate{% endtrans %}
+                </button>
+              </form>
             {% endif %}
-            <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 %}
-
-      {% endif %}
-
     </div>
 
     <div
@@ -68,7 +68,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/templates/com/poster_moderate.jinja

@@ -1,43 +0,0 @@
-{% extends "core/base.jinja" %}
-
-{% block script %}
-  {{ super() }}
-  <script src="{{ static('com/js/poster_list.js') }}"></script>
-{% endblock %}
-
-{% block additional_css %}
-  <link rel="stylesheet" href="{{ static('com/css/posters.scss') }}">
-{% endblock %}
-
-{% block content %}
-  <div id="poster_list">
-
-    <div id="title">
-      <div id="links" class="left">
-        <a id="list" class="link" href="{{ url("com:poster_list") }}">{% trans %}List{% endtrans %}</a>
-      </div>
-      <h3>{% trans %}Posters - moderation{% endtrans %}</h3>
-    </div>
-
-    <div id="posters">
-
-      {% if object_list.count == 0 %}
-        <div id="no-posters">{% trans %}No objects{% endtrans %}</div>
-      {% else %}
-
-        {% for poster in object_list %}
-          <div class="poster{% if not poster.is_moderated %} not_moderated{% endif %}">
-            <div class="name"> {{ poster.name }} </div>
-            <div class="image"> <img src="{{ poster.file.url }}"></img> </div>
-            <a class="moderate" href="{{ url("com:poster_moderate", object_id=poster.id) }}">Moderate</a>
-          </div>
-        {% endfor %}
-
-      {% endif %}
-
-    </div>
-
-    <div id="view"><div id="placeholder"></div></div>
-
-  </div>
-{% endblock %}

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,21 +644,17 @@ class PosterDeleteBaseView(
     permission_required = "com.delete_poster"
 
 
-class PosterListView(ComTabsMixin, PosterListBaseView):
+class PosterListView(PermissionRequiredMixin, ComTabsMixin, PosterListBaseView):
     """List communication posters."""
 
     current_tab = "posters"
-
-    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)
-
-    def get_context_data(self, **kwargs):
-        kwargs = super().get_context_data(**kwargs)
-        kwargs["app"] = "com"
-        return kwargs
+    extra_context = {
+        "create_url": reverse_lazy("com:poster_create"),
+        "get_edit_url": lambda poster: reverse(
+            "com:poster_edit", kwargs={"poster_id": poster.id}
+        ),
+    }
+    permission_required = "com.view_poster"
 
 
 class PosterCreateView(ComTabsMixin, PosterCreateBaseView):
@@ -672,17 +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_moderate.jinja"
-    queryset = Poster.objects.filter(is_moderated=False).all()
-    permission_required = "com.moderate_poster"
-    extra_context = {"app": "com"}
-
-
 class PosterModerateView(PermissionRequiredMixin, ComTabsMixin, View):
     """Moderate communication poster."""
 
@@ -690,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):