diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4379d91a..08e322ca 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.11.13 + rev: v0.14.4 hooks: - id: ruff-check # just check the code, and print the errors - id: ruff-check # actually fix the fixable errors, but print nothing @@ -14,7 +14,7 @@ repos: - id: biome-check additional_dependencies: ["@biomejs/biome@1.9.4"] - repo: https://github.com/rtts/djhtml - rev: 3.0.7 + rev: 3.0.10 hooks: - id: djhtml name: format templates diff --git a/api/auth.py b/api/auth.py index 787234a6..aac8cf40 100644 --- a/api/auth.py +++ b/api/auth.py @@ -6,6 +6,8 @@ from api.models import ApiClient, ApiKey class ApiKeyAuth(APIKeyHeader): + """Authentication through client api keys.""" + param_name = "X-APIKey" def authenticate(self, request: HttpRequest, key: str | None) -> ApiClient | None: diff --git a/api/tests/test_mixed_auth.py b/api/tests/test_mixed_auth.py new file mode 100644 index 00000000..52144bb7 --- /dev/null +++ b/api/tests/test_mixed_auth.py @@ -0,0 +1,48 @@ +import pytest +from django.test import Client +from django.urls import path +from model_bakery import baker +from ninja import NinjaAPI +from ninja.security import SessionAuth + +from api.auth import ApiKeyAuth +from api.hashers import generate_key +from api.models import ApiClient, ApiKey + +api = NinjaAPI() + + +@api.post("", auth=[ApiKeyAuth(), SessionAuth()]) +def post_method(*args, **kwargs) -> None: + """Dummy POST route authenticated by either api key or session cookie.""" + pass + + +urlpatterns = [path("", api.urls)] + + +@pytest.mark.django_db +@pytest.mark.urls(__name__) +@pytest.mark.parametrize("user_logged_in", [False, True]) +def test_csrf_token(user_logged_in): + """Test that CSRF check happens only when no api key is used.""" + client = Client(enforce_csrf_checks=True) + key, hashed = generate_key() + api_client = baker.make(ApiClient) + baker.make(ApiKey, client=api_client, hashed_key=hashed) + if user_logged_in: + client.force_login(api_client.owner) + + response = client.post("") + assert response.status_code == 403 + assert response.json()["detail"] == "CSRF check Failed" + + # if using a valid API key, CSRF check should not occur + response = client.post("", headers={"X-APIKey": key}) + assert response.status_code == 200 + + # if using a wrong API key, ApiKeyAuth should fail, + # leading to a fallback into SessionAuth and a CSRF check + response = client.post("", headers={"X-APIKey": generate_key()[0]}) + assert response.status_code == 403 + assert response.json()["detail"] == "CSRF check Failed" diff --git a/api/urls.py b/api/urls.py index 2c3f12ff..50300453 100644 --- a/api/urls.py +++ b/api/urls.py @@ -1,3 +1,4 @@ +from ninja.security import SessionAuth from ninja_extra import NinjaExtraAPI api = NinjaExtraAPI( @@ -5,6 +6,6 @@ api = NinjaExtraAPI( description="Portail Interactif de Communication avec les Outils Numériques", version="0.2.0", urls_namespace="api", - csrf=True, + auth=[SessionAuth()], ) api.auto_discover_controllers() diff --git a/club/api.py b/club/api.py index 2e59d3e5..1479ee5c 100644 --- a/club/api.py +++ b/club/api.py @@ -1,7 +1,5 @@ -from typing import Annotated - -from annotated_types import MinLen from django.db.models import Prefetch +from ninja import Query from ninja.security import SessionAuth from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.pagination import PageNumberPaginationExtra @@ -10,7 +8,7 @@ from ninja_extra.schemas import PaginatedResponseSchema from api.auth import ApiKeyAuth from api.permissions import CanAccessLookup, HasPerm from club.models import Club, Membership -from club.schemas import ClubSchema, SimpleClubSchema +from club.schemas import ClubSchema, ClubSearchFilterSchema, SimpleClubSchema @api_controller("/club") @@ -18,18 +16,18 @@ class ClubController(ControllerBase): @route.get( "/search", response=PaginatedResponseSchema[SimpleClubSchema], - auth=[SessionAuth(), ApiKeyAuth()], + auth=[ApiKeyAuth(), SessionAuth()], permissions=[CanAccessLookup], url_name="search_club", ) @paginate(PageNumberPaginationExtra, page_size=50) - def search_club(self, search: Annotated[str, MinLen(1)]): - return Club.objects.filter(name__icontains=search).values() + def search_club(self, filters: Query[ClubSearchFilterSchema]): + return filters.filter(Club.objects.all()) @route.get( "/{int:club_id}", response=ClubSchema, - auth=[SessionAuth(), ApiKeyAuth()], + auth=[ApiKeyAuth(), SessionAuth()], permissions=[HasPerm("club.view_club")], url_name="fetch_club", ) diff --git a/club/schemas.py b/club/schemas.py index b0601af8..5a7ccccb 100644 --- a/club/schemas.py +++ b/club/schemas.py @@ -1,9 +1,26 @@ -from ninja import ModelSchema +from typing import Annotated + +from annotated_types import MinLen +from django.db.models import Q +from ninja import Field, FilterSchema, ModelSchema from club.models import Club, Membership from core.schemas import SimpleUserSchema +class ClubSearchFilterSchema(FilterSchema): + search: Annotated[str, MinLen(1)] | None = Field(None, q="name__icontains") + is_active: bool | None = None + parent_id: int | None = None + parent_name: str | None = Field(None, q="parent__name__icontains") + exclude_ids: set[int] | None = None + + def filter_exclude_ids(self, value: set[int] | None): + if value is None: + return Q() + return ~Q(id__in=value) + + class SimpleClubSchema(ModelSchema): class Meta: model = Club diff --git a/club/templates/club/club_detail.jinja b/club/templates/club/club_detail.jinja index 33760695..168c4b6c 100644 --- a/club/templates/club/club_detail.jinja +++ b/club/templates/club/club_detail.jinja @@ -9,6 +9,18 @@ {{ club.short_description }} {%- endblock %} +{% block metatags %} + + + + + {% if club.logo %} + + {% else %} + + {% endif %} +{% endblock %} + {% block content %}
{% if club.logo %} @@ -17,7 +29,7 @@ {% if page_revision %} {{ page_revision|markdown }} {% else %} -

{% trans %}Club{% endtrans %}

+

{{ club.name }}

{% endif %}
{% endblock %} diff --git a/club/templates/club/club_sellings.jinja b/club/templates/club/club_sellings.jinja index 3733d0c8..59edd18e 100644 --- a/club/templates/club/club_sellings.jinja +++ b/club/templates/club/club_sellings.jinja @@ -6,11 +6,11 @@ because it works with a somewhat dynamic form, but was written before Alpine was introduced in the project. TODO : rewrite the pagination used in this template an Alpine one #} -{% macro paginate(page_obj, paginator, js_action) %} - {% set js = js_action|default('') %} +{% macro paginate(page_obj, paginator) %} + {% set js = "formPagination(this)" %} {% if page_obj.has_previous() or page_obj.has_next() %} {% if page_obj.has_previous() %} - {% trans %}Previous{% endtrans %} + {% trans %}Previous{% endtrans %} {% else %} {% trans %}Previous{% endtrans %} {% endif %} @@ -18,11 +18,11 @@ TODO : rewrite the pagination used in this template an Alpine one {% if page_obj.number == i %} {{ i }} ({% trans %}current{% endtrans %}) {% else %} - {{ i }} + {{ i }} {% endif %} {% endfor %} {% if page_obj.has_next() %} - {% trans %}Next{% endtrans %} + {% trans %}Next{% endtrans %} {% else %} {% trans %}Next{% endtrans %} {% endif %} @@ -81,6 +81,10 @@ TODO : rewrite the pagination used in this template an Alpine one {% endfor %} + {{ paginate(paginated_result, paginator) }} +{% endblock %} + +{% block script %} - {{ paginate(paginated_result, paginator, "formPagination(this)") }} {% endblock %} diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index 18a3aef1..500dbe6a 100644 --- a/club/tests/test_club_controller.py +++ b/club/tests/test_club_controller.py @@ -1,7 +1,8 @@ from datetime import date, timedelta import pytest -from django.test import Client +from django.contrib.auth.models import Permission +from django.test import Client, TestCase from django.urls import reverse from model_bakery import baker from model_bakery.recipe import Recipe @@ -9,6 +10,54 @@ from pytest_django.asserts import assertNumQueries from club.models import Club, Membership from core.baker_recipes import subscriber_user +from core.models import Group, Page, User + + +class TestClubSearch(TestCase): + @classmethod + def setUpTestData(cls): + cls.url = reverse("api:search_club") + cls.user = baker.make( + User, user_permissions=[Permission.objects.get(codename="access_lookup")] + ) + # delete existing clubs to avoid side effect + groups = list( + Group.objects.exclude(club=None, club_board=None).values_list( + "id", flat=True + ) + ) + Page.objects.exclude(club=None).delete() + Club.objects.all().delete() + Group.objects.filter(id__in=groups).delete() + + cls.clubs = baker.make( + Club, + _quantity=5, + name=iter(["AE", "ae 1", "Troll", "Dev AE", "pdf"]), + is_active=True, + ) + + def test_inactive_club(self): + self.client.force_login(self.user) + inactive_ids = {self.clubs[0].id, self.clubs[2].id} + Club.objects.filter(id__in=inactive_ids).update(is_active=False) + response = self.client.get(self.url, {"is_active": False}) + assert response.status_code == 200 + assert {d["id"] for d in response.json()["results"]} == inactive_ids + + def test_excluded_id(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"exclude_ids": [self.clubs[1].id]}) + assert response.status_code == 200 + ids = {d["id"] for d in response.json()["results"]} + assert ids == {c.id for c in [self.clubs[0], *self.clubs[2:]]} + + def test_club_search(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"search": "AE"}) + assert response.status_code == 200 + ids = {d["id"] for d in response.json()["results"]} + assert ids == {c.id for c in [self.clubs[0], self.clubs[1], self.clubs[3]]} @pytest.mark.django_db diff --git a/club/views.py b/club/views.py index a14b71cd..e6c86a7c 100644 --- a/club/views.py +++ b/club/views.py @@ -23,6 +23,7 @@ # import csv +import itertools from typing import Any from django.conf import settings @@ -30,14 +31,14 @@ from django.contrib.auth.mixins import PermissionRequiredMixin from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError from django.core.paginator import InvalidPage, Paginator -from django.db.models import Q, Sum +from django.db.models import F, Q, Sum from django.http import Http404, HttpResponseRedirect, StreamingHttpResponse from django.shortcuts import get_object_or_404, redirect from django.urls import reverse, reverse_lazy from django.utils import timezone from django.utils.safestring import SafeString from django.utils.timezone import now -from django.utils.translation import gettext as _t +from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView, View from django.views.generic.edit import CreateView, DeleteView, UpdateView @@ -59,7 +60,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 @@ -370,7 +371,7 @@ class ClubOldMembersView(ClubTabsMixin, PermissionRequiredMixin, DetailView): class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): - """Sellings of a club.""" + """Sales of a club.""" model = Club pk_url_kwarg = "club_id" @@ -396,9 +397,8 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): def get_context_data(self, **kwargs): kwargs = super().get_context_data(**kwargs) - qs = Selling.objects.filter(club=self.object) - kwargs["result"] = qs[:0] + kwargs["result"] = Selling.objects.none() kwargs["paginated_result"] = kwargs["result"] kwargs["total"] = 0 kwargs["total_quantity"] = 0 @@ -406,6 +406,7 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): form = self.get_form() if form.is_valid(): + qs = Selling.objects.filter(club=self.object) if not len([v for v in form.cleaned_data.values() if v is not None]): qs = Selling.objects.none() if form.cleaned_data["begin_date"]: @@ -425,18 +426,18 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): if len(selected_products) > 0: qs = qs.filter(product__in=selected_products) + kwargs["total"] = qs.annotate( + price=F("quantity") * F("unit_price") + ).aggregate(total=Sum("price", default=0))["total"] kwargs["result"] = qs.select_related( "counter", "counter__club", "customer", "customer__user", "seller" ).order_by("-id") - kwargs["total"] = sum([s.quantity * s.unit_price for s in kwargs["result"]]) - total_quantity = qs.all().aggregate(Sum("quantity")) - if total_quantity["quantity__sum"]: - kwargs["total_quantity"] = total_quantity["quantity__sum"] - benefit = ( - qs.exclude(product=None).all().aggregate(Sum("product__purchase_price")) - ) - if benefit["product__purchase_price__sum"]: - kwargs["benefit"] = benefit["product__purchase_price__sum"] + kwargs["total_quantity"] = qs.aggregate(total=Sum("quantity", default=0))[ + "total" + ] + kwargs["benefit"] = qs.exclude(product=None).aggregate( + res=Sum("product__purchase_price", default=0) + )["res"] kwargs["paginator"] = Paginator(kwargs["result"], self.paginate_by) try: @@ -487,40 +488,40 @@ class ClubSellingCSVView(ClubSellingView): kwargs = self.get_context_data(**kwargs) # Use the StreamWriter class instead of request for streaming - pseudo_buffer = self.StreamWriter() - writer = csv.writer( - pseudo_buffer, delimiter=";", lineterminator="\n", quoting=csv.QUOTE_ALL - ) + writer = csv.writer(self.StreamWriter()) - writer.writerow([_t("Quantity"), kwargs["total_quantity"]]) - writer.writerow([_t("Total"), kwargs["total"]]) - writer.writerow([_t("Benefit"), kwargs["benefit"]]) - writer.writerow( + first_rows = [ + [gettext("Quantity"), kwargs["total_quantity"]], + [gettext("Total"), kwargs["total"]], + [gettext("Benefit"), kwargs["benefit"]], [ - _t("Date"), - _t("Counter"), - _t("Barman"), - _t("Customer"), - _t("Label"), - _t("Quantity"), - _t("Total"), - _t("Payment method"), - _t("Selling price"), - _t("Purchase price"), - _t("Benefit"), - ] - ) + gettext("Date"), + gettext("Counter"), + gettext("Barman"), + gettext("Customer"), + gettext("Label"), + gettext("Quantity"), + gettext("Total"), + gettext("Payment method"), + gettext("Selling price"), + gettext("Purchase price"), + gettext("Benefit"), + ], + ] # Stream response response = StreamingHttpResponse( - ( - writer.writerow(self.write_selling(selling)) - for selling in kwargs["result"] + itertools.chain( + (writer.writerow(r) for r in first_rows), + ( + writer.writerow(self.write_selling(selling)) + for selling in kwargs["result"] + ), ), content_type="text/csv", ) - name = _("Sellings") + "_" + self.object.name + ".csv" - response["Content-Disposition"] = "filename=" + name + name = f"{gettext('Sellings')}_{self.object.name}.csv" + response["Content-Disposition"] = f"attachment; filename={name}" return response @@ -758,11 +759,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 +773,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.""" diff --git a/com/api.py b/com/api.py index b01eef0e..d487695e 100644 --- a/com/api.py +++ b/com/api.py @@ -5,7 +5,6 @@ from django.utils.cache import add_never_cache_headers from ninja import Query from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.pagination import PageNumberPaginationExtra -from ninja_extra.permissions import IsAuthenticated from ninja_extra.schemas import PaginatedResponseSchema from api.permissions import HasPerm @@ -17,17 +16,13 @@ from core.views.files import send_raw_file @api_controller("/calendar") class CalendarController(ControllerBase): - @route.get("/internal.ics", url_name="calendar_internal") + @route.get("/internal.ics", auth=None, url_name="calendar_internal") def calendar_internal(self): response = send_raw_file(IcsCalendar.get_internal()) add_never_cache_headers(response) return response - @route.get( - "/unpublished.ics", - permissions=[IsAuthenticated], - url_name="calendar_unpublished", - ) + @route.get("/unpublished.ics", url_name="calendar_unpublished") def calendar_unpublished(self): response = HttpResponse( IcsCalendar.get_unpublished(self.context.request.user), @@ -74,6 +69,7 @@ class NewsController(ControllerBase): @route.get( "/date", + auth=None, url_name="fetch_news_dates", response=PaginatedResponseSchema[NewsDateSchema], ) diff --git a/com/models.py b/com/models.py index 6485d73b..6a9100b5 100644 --- a/com/models.py +++ b/com/models.py @@ -144,7 +144,7 @@ class News(models.Model): ), groups__id=settings.SITH_GROUP_COM_ADMIN_ID, ) - notif_url = reverse("com:news_admin_list") + notif_url = reverse("com:news_admin_list", fragment="moderation") new_notifs = [ Notification(user=user, url=notif_url, type="NEWS_MODERATION") for user in admins_without_notif @@ -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..757d151f 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,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%; } } } diff --git a/com/templates/com/news_admin_list.jinja b/com/templates/com/news_admin_list.jinja index 2ddbb69d..ea54caaf 100644 --- a/com/templates/com/news_admin_list.jinja +++ b/com/templates/com/news_admin_list.jinja @@ -131,7 +131,7 @@ {% endfor %} -
{% trans %}Events to moderate{% endtrans %}
+
{% trans %}Events to moderate{% endtrans %}
@@ -165,6 +165,3 @@
{% endblock %} - - - diff --git a/com/templates/com/news_detail.jinja b/com/templates/com/news_detail.jinja index 9ab6acff..a4c9309c 100644 --- a/com/templates/com/news_detail.jinja +++ b/com/templates/com/news_detail.jinja @@ -1,15 +1,20 @@ {% extends "core/base.jinja" %} -{% from 'core/macros.jinja' import user_profile_link, facebook_share, tweet, link_news_logo, gen_news_metatags %} +{% from 'core/macros.jinja' import user_profile_link, link_news_logo %} {% from "com/macros.jinja" import news_moderation_alert %} {% block title %} - {% trans %}News{% endtrans %} - - {{ object.title }} + {% trans %}News{% endtrans %} - {{ object.title }} {% endblock %} -{% block head %} - {{ super() }} - {{ gen_news_metatags(news) }} +{% block description %}{{ news.summary }}{% endblock %} + +{% block metatags %} + + + + + + {% endblock %} @@ -44,8 +49,14 @@
{{ news.summary|markdown }}

{{ news.content|markdown }}
- {{ facebook_share(news) }} - {{ tweet(news) }} + + {% trans %}Share on Facebook{% endtrans %} +

{% trans %}Author: {% endtrans %}{{ user_profile_link(news.author) }}

{% if news.moderator %} diff --git a/com/templates/com/poster_list.jinja b/com/templates/com/poster_list.jinja index d723c248..5460f54e 100644 --- a/com/templates/com/poster_list.jinja +++ b/com/templates/com/poster_list.jinja @@ -13,53 +13,53 @@

{% trans %}Posters{% endtrans %}

-
- - {% 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") }}
-
- {% if app == "com" %} - {% trans %}Edit{% endtrans %} - {% elif app == "club" %} - {% trans %}Edit{% endtrans %} - {% endif %} -
-
    - {% 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 %} +
+
+ {% else %} +
{% trans %}No posters{% endtrans %}
+ {% endfor %}
-
+
+ +
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/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 a1897b12..2d5045d9 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,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("com:poster_list") class ScreenListView(PermissionRequiredMixin, ComTabsMixin, ListView): diff --git a/core/api.py b/core/api.py index af4daff5..ab69f86e 100644 --- a/core/api.py +++ b/core/api.py @@ -99,7 +99,7 @@ class SithFileController(ControllerBase): @route.get( "/search", response=PaginatedResponseSchema[SithFileSchema], - auth=[SessionAuth(), ApiKeyAuth()], + auth=[ApiKeyAuth(), SessionAuth()], permissions=[CanAccessLookup], ) @paginate(PageNumberPaginationExtra, page_size=50) @@ -112,7 +112,7 @@ class GroupController(ControllerBase): @route.get( "/search", response=PaginatedResponseSchema[GroupSchema], - auth=[SessionAuth(), ApiKeyAuth()], + auth=[ApiKeyAuth(), SessionAuth()], permissions=[CanAccessLookup], ) @paginate(PageNumberPaginationExtra, page_size=50) diff --git a/core/auth/mixins.py b/core/auth/mixins.py index c4e603a9..917200ed 100644 --- a/core/auth/mixins.py +++ b/core/auth/mixins.py @@ -24,7 +24,6 @@ from __future__ import annotations import types -import warnings from typing import TYPE_CHECKING, Any, LiteralString from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin @@ -147,45 +146,6 @@ class GenericContentPermissionMixinBuilder(View): return super().dispatch(request, *arg, **kwargs) -class CanCreateMixin(View): - """Protect any child view that would create an object. - - Raises: - PermissionDenied: - If the user has not the necessary permission - to create the object of the view. - """ - - def __init_subclass__(cls, **kwargs): - warnings.warn( - f"{cls.__name__} is deprecated and should be replaced " - "by other permission verification mecanism.", - DeprecationWarning, - stacklevel=2, - ) - super().__init_subclass__(**kwargs) - - def __init__(self, *args, **kwargs): - warnings.warn( - f"{self.__class__.__name__} is deprecated and should be replaced " - "by other permission verification mecanism.", - DeprecationWarning, - stacklevel=2, - ) - super().__init__(*args, **kwargs) - - def dispatch(self, request, *arg, **kwargs): - if not request.user.is_authenticated: - raise PermissionDenied - return super().dispatch(request, *arg, **kwargs) - - def form_valid(self, form): - obj = form.instance - if can_edit_prop(obj, self.request.user): - return super().form_valid(form) - raise PermissionDenied - - class CanEditPropMixin(GenericContentPermissionMixinBuilder): """Ensure the user has owner permissions on the child view object. diff --git a/core/management/commands/check_fs.py b/core/management/commands/check_fs.py deleted file mode 100644 index 8e970ced..00000000 --- a/core/management/commands/check_fs.py +++ /dev/null @@ -1,40 +0,0 @@ -# -# Copyright 2018 -# - Skia -# -# Ce fichier fait partie du site de l'Association des Étudiants de l'UTBM, -# http://ae.utbm.fr. -# -# This program is free software; you can redistribute it and/or modify it under -# the terms of the GNU General Public License a published by the Free Software -# Foundation; either version 3 of the License, or (at your option) any later -# version. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -# 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 -# Place - Suite 330, Boston, MA 02111-1307, USA. -# -# - -from django.core.management.base import BaseCommand - -from core.models import SithFile - - -class Command(BaseCommand): - help = "Recursively check the file system with respect to the DB" - - def add_arguments(self, parser): - parser.add_argument( - "ids", metavar="ID", type=int, nargs="+", help="The file IDs to process" - ) - - def handle(self, *args, **options): - files = SithFile.objects.filter(id__in=options["ids"]).all() - for f in files: - f._check_fs() diff --git a/core/management/commands/repair_fs.py b/core/management/commands/repair_fs.py deleted file mode 100644 index cf88d108..00000000 --- a/core/management/commands/repair_fs.py +++ /dev/null @@ -1,41 +0,0 @@ -# -# Copyright 2018 -# - Skia -# -# Ce fichier fait partie du site de l'Association des Étudiants de l'UTBM, -# http://ae.utbm.fr. -# -# This program is free software; you can redistribute it and/or modify it under -# the terms of the GNU General Public License a published by the Free Software -# Foundation; either version 3 of the License, or (at your option) any later -# version. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -# 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 -# Place - Suite 330, Boston, MA 02111-1307, USA. -# -# - - -from django.core.management.base import BaseCommand - -from core.models import SithFile - - -class Command(BaseCommand): - help = "Recursively repair the file system with respect to the DB" - - def add_arguments(self, parser): - parser.add_argument( - "ids", metavar="ID", type=int, nargs="+", help="The file IDs to process" - ) - - def handle(self, *args, **options): - files = SithFile.objects.filter(id__in=options["ids"]).all() - for f in files: - f._repair_fs() diff --git a/core/models.py b/core/models.py index 0506364a..a624ebf6 100644 --- a/core/models.py +++ b/core/models.py @@ -23,14 +23,12 @@ # from __future__ import annotations -import logging -import os import string import unicodedata from datetime import timedelta from io import BytesIO from pathlib import Path -from typing import TYPE_CHECKING, Optional, Self +from typing import TYPE_CHECKING, Self from uuid import uuid4 from django.conf import settings @@ -97,48 +95,6 @@ def validate_promo(value: int) -> None: ) -def get_group(*, pk: int | None = None, name: str | None = None) -> Group | None: - """Search for a group by its primary key or its name. - Either one of the two must be set. - - The result is cached for the default duration (should be 5 minutes). - - Args: - pk: The primary key of the group - name: The name of the group - - Returns: - The group if it exists, else None - - Raises: - ValueError: If no group matches the criteria - """ - if pk is None and name is None: - raise ValueError("Either pk or name must be set") - - # replace space characters to hide warnings with memcached backend - pk_or_name: str | int = pk if pk is not None else name.replace(" ", "_") - group = cache.get(f"sith_group_{pk_or_name}") - - if group == "not_found": - # Using None as a cache value is a little bit tricky, - # so we use a special string to represent None - return None - elif group is not None: - return group - # if this point is reached, the group is not in cache - if pk is not None: - group = Group.objects.filter(pk=pk).first() - else: - group = Group.objects.filter(name=name).first() - if group is not None: - name = group.name.replace(" ", "_") - cache.set_many({f"sith_group_{group.id}": group, f"sith_group_{name}": group}) - else: - cache.set(f"sith_group_{pk_or_name}", "not_found") - return group - - class BanGroup(AuthGroup): """An anti-group, that removes permissions instead of giving them. @@ -382,19 +338,18 @@ class User(AbstractUser): Returns: True if the user is the group, else False """ - if pk is not None: - group: Optional[Group] = get_group(pk=pk) - elif name is not None: - group: Optional[Group] = get_group(name=name) - else: + if not pk and not name: raise ValueError("You must either provide the id or the name of the group") - if group is None: + group_id: int | None = ( + pk or Group.objects.filter(name=name).values_list("id", flat=True).first() + ) + if group_id is None: return False - if group.id == settings.SITH_GROUP_SUBSCRIBERS_ID: + if group_id == settings.SITH_GROUP_SUBSCRIBERS_ID: return self.is_subscribed - if group.id == settings.SITH_GROUP_ROOT_ID: + if group_id == settings.SITH_GROUP_ROOT_ID: return self.is_root - return group in self.cached_groups + return any(g.id == group_id for g in self.cached_groups) @cached_property def cached_groups(self) -> list[Group]: @@ -454,14 +409,6 @@ class User(AbstractUser): else: raise ValidationError(_("A user with that username already exists")) - def get_profile(self): - return { - "last_name": self.last_name, - "first_name": self.first_name, - "nick_name": self.nick_name, - "date_of_birth": self.date_of_birth, - } - def get_short_name(self): """Returns the short name for the user.""" if self.nick_name: @@ -689,8 +636,8 @@ class AnonymousUser(AuthAnonymousUser): if pk is not None: return pk == allowed_id elif name is not None: - group = get_group(name=name) - return group is not None and group.id == allowed_id + group = Group.objects.get(id=allowed_id) + return group.name == name else: raise ValueError("You must either provide the id or the name of the group") @@ -1016,63 +963,6 @@ class SithFile(models.Model): self.clean() self.save() - def _repair_fs(self): - """Rebuilds recursively the filesystem as it should be regarding the DB tree.""" - if self.is_folder: - for c in self.children.all(): - c._repair_fs() - return - elif not self._check_path_consistence(): - # First get future parent path and the old file name - # Prepend "." so that we match all relative handling of Django's - # file storage - parent_path = "." + self.parent.get_full_path() - parent_full_path = settings.MEDIA_ROOT + parent_path - os.makedirs(parent_full_path, exist_ok=True) - old_path = self.file.name # Should be relative: "./users/skia/bleh.jpg" - new_path = "." + self.get_full_path() - try: - # Make this atomic, so that a FS problem rolls back the DB change - with transaction.atomic(): - # Set the new filesystem path - self.file.name = new_path - self.save() - # Really move at the FS level - if os.path.exists(parent_full_path): - os.rename( - settings.MEDIA_ROOT + old_path, - settings.MEDIA_ROOT + new_path, - ) - # Empty directories may remain, but that's not really a - # problem, and that can be solved with a simple shell - # command: `find . -type d -empty -delete` - except Exception as e: - logging.error(e) - - def _check_path_consistence(self): - file_path = str(self.file) - file_full_path = settings.MEDIA_ROOT + file_path - db_path = ".%s" % self.get_full_path() - if not os.path.exists(file_full_path): - print("%s: WARNING: real file does not exists!" % self.id) # noqa T201 - print("file path: %s" % file_path, end="") # noqa T201 - print(" db path: %s" % db_path) # noqa T201 - return False - if file_path != db_path: - print("%s: " % self.id, end="") # noqa T201 - print("file path: %s" % file_path, end="") # noqa T201 - print(" db path: %s" % db_path) # noqa T201 - return False - return True - - def _check_fs(self): - if self.is_folder: - for c in self.children.all(): - c._check_fs() - return - else: - self._check_path_consistence() - @property def is_file(self): return not self.is_folder @@ -1157,8 +1047,6 @@ class QuickUploadImage(models.Model): identifier = str(uuid4()) name = Path(image.name).stem[: cls.IMAGE_NAME_SIZE - 1] file = File(convert_image(image), name=f"{identifier}.webp") - width, height = Image.open(file).size - return cls.objects.create( uuid=identifier, name=name, diff --git a/core/static/bundled/alpine-index.js b/core/static/bundled/alpine-index.js index daed0b27..ef273300 100644 --- a/core/static/bundled/alpine-index.js +++ b/core/static/bundled/alpine-index.js @@ -1,8 +1,9 @@ +import { limitedChoices } from "#core:alpine/limited-choices"; import { alpinePlugin as notificationPlugin } from "#core:utils/notifications"; import sort from "@alpinejs/sort"; import Alpine from "alpinejs"; -Alpine.plugin(sort); +Alpine.plugin([sort, limitedChoices]); Alpine.magic("notifications", notificationPlugin); window.Alpine = Alpine; diff --git a/core/static/bundled/alpine/limited-choices.ts b/core/static/bundled/alpine/limited-choices.ts new file mode 100644 index 00000000..211441d0 --- /dev/null +++ b/core/static/bundled/alpine/limited-choices.ts @@ -0,0 +1,69 @@ +import type { Alpine as AlpineType } from "alpinejs"; + +export function limitedChoices(Alpine: AlpineType) { + /** + * Directive to limit the number of elements + * that can be selected in a group of checkboxes. + * + * When the max numbers of selectable elements is reached, + * new elements will still be inserted, but oldest ones will be deselected. + * For example, if checkboxes A, B and C have been selected and the max + * number of selections is 3, then selecting D will result in having + * B, C and D selected. + * + * # Example in template + * ```html + *
+ * + * + * + * + * + *
+ * ``` + */ + Alpine.directive( + "limited-choices", + (el, { expression }, { evaluateLater, effect }) => { + const getMaxChoices = evaluateLater(expression); + let maxChoices: number; + const inputs: HTMLInputElement[] = Array.from( + el.querySelectorAll("input[type='checkbox']"), + ); + const checked = [] as HTMLInputElement[]; + + const manageDequeue = () => { + if (checked.length <= maxChoices) { + // There isn't too many checkboxes selected. Nothing to do + return; + } + const popped = checked.splice(0, checked.length - maxChoices); + for (const p of popped) { + p.checked = false; + } + }; + + for (const input of inputs) { + input.addEventListener("change", (_e) => { + if (input.checked) { + checked.push(input); + } else { + checked.splice(checked.indexOf(input), 1); + } + manageDequeue(); + }); + } + effect(() => { + getMaxChoices((value: string) => { + const previousValue = maxChoices; + maxChoices = Number.parseInt(value); + if (maxChoices < previousValue) { + // The maximum number of selectable items has been lowered. + // Some currently selected elements may need to be removed + manageDequeue(); + } + }); + }); + }, + ); +} diff --git a/core/static/core/footer.scss b/core/static/core/footer.scss index aa2e048f..3c0306e0 100644 --- a/core/static/core/footer.scss +++ b/core/static/core/footer.scss @@ -65,7 +65,7 @@ footer.bottom-links { flex-wrap: wrap; align-items: center; background-color: $primary-neutral-dark-color; - box-shadow: $shadow-color 0 0 15px; + box-shadow: black 0 8px 15px; a { color: $white-color; diff --git a/core/static/core/header.scss b/core/static/core/header.scss index 7eca52f9..f43819c3 100644 --- a/core/static/core/header.scss +++ b/core/static/core/header.scss @@ -11,7 +11,8 @@ $hovered-red-text-color: #ff4d4d; .header { box-sizing: border-box; background-color: $deepblue; - box-shadow: 3px 3px 3px 0 #dfdfdf; + box-shadow: black 0 1px 3px 0, + black 0 4px 8px 3px; border-radius: 0; width: 100%; display: flex; @@ -99,7 +100,7 @@ $hovered-red-text-color: #ff4d4d; border-radius: 0; margin: 0; box-sizing: border-box; - background-color: $deepblue; + background-color: transparent; width: 45px; height: 25px; padding: 0; @@ -331,7 +332,7 @@ $hovered-red-text-color: #ff4d4d; padding: 10px; z-index: 100; border-radius: 10px; - box-shadow: 3px 3px 3px 0 #767676; + @include shadow; >ul { list-style-type: none; diff --git a/core/static/core/img/gala25_background.webp b/core/static/core/img/gala25_background.webp new file mode 100644 index 00000000..978e9946 Binary files /dev/null and b/core/static/core/img/gala25_background.webp differ diff --git a/core/static/core/img/gala25_logo.webp b/core/static/core/img/gala25_logo.webp new file mode 100644 index 00000000..3cbdb6f7 Binary files /dev/null and b/core/static/core/img/gala25_logo.webp differ diff --git a/core/static/core/style.scss b/core/static/core/style.scss index 7522666b..c23303a7 100644 --- a/core/static/core/style.scss +++ b/core/static/core/style.scss @@ -271,8 +271,9 @@ body { /*--------------------------------CONTENT------------------------------*/ #content { - padding: 1em 1%; - box-shadow: $shadow-color 0 5px 10px; + padding: 1.5em 2%; + border-radius: 5px; + box-shadow: black 0 8px 15px; background: $white-color; overflow: auto; } diff --git a/core/templates/core/base.jinja b/core/templates/core/base.jinja index 2be6cd54..356abdff 100644 --- a/core/templates/core/base.jinja +++ b/core/templates/core/base.jinja @@ -4,12 +4,22 @@ {% block head %} {% block title %}Association des Étudiants de l'UTBM{% endblock %} - + + + {% block metatags %} + + + + + {% endblock %} @@ -34,6 +44,18 @@ {% block additional_css %}{% endblock %} {% block additional_js %}{% endblock %} + {% endblock %} diff --git a/core/templates/core/base/header.jinja b/core/templates/core/base/header.jinja index cb47a47c..65a15968 100644 --- a/core/templates/core/base/header.jinja +++ b/core/templates/core/base/header.jinja @@ -1,6 +1,6 @@