diff --git a/api/hashers.py b/api/hashers.py index 95c16673..42909fde 100644 --- a/api/hashers.py +++ b/api/hashers.py @@ -8,7 +8,7 @@ from django.utils.crypto import constant_time_compare class Sha512ApiKeyHasher(BasePasswordHasher): """ - An API key hasher using the sha256 algorithm. + An API key hasher using the sha512 algorithm. This hasher shouldn't be used in Django's `PASSWORD_HASHERS` setting. It is insecure for use in hashing passwords, but is safe for hashing diff --git a/club/forms.py b/club/forms.py index d2b24dde..dcd270e7 100644 --- a/club/forms.py +++ b/club/forms.py @@ -37,6 +37,7 @@ from core.views.widgets.ajax_select import ( AutoCompleteSelectUser, ) from counter.models import Counter, Selling +from counter.schemas import SaleFilterSchema class ClubEditForm(forms.ModelForm): @@ -191,6 +192,18 @@ class SellingsForm(forms.Form): required=False, ) + def to_filter_schema(self) -> SaleFilterSchema: + products = ( + *self.cleaned_data["products"], + *self.cleaned_data["archived_products"], + ) + return SaleFilterSchema( + after=self.cleaned_data["begin_date"], + before=self.cleaned_data["end_date"], + counters={c.id for c in self.cleaned_data["counters"]} or None, + products={p.id for p in products} or None, + ) + class ClubOldMemberForm(forms.Form): members_old = forms.ModelMultipleChoiceField( diff --git a/club/templates/club/page_history.jinja b/club/templates/club/page_history.jinja index 07c25146..825fa7ab 100644 --- a/club/templates/club/page_history.jinja +++ b/club/templates/club/page_history.jinja @@ -1,12 +1,8 @@ {% extends "core/base.jinja" %} -{% from 'core/macros_pages.jinja' import page_history %} +{% from 'core/page/macros.jinja' import page_history %} {% block content %} - {% if club.page %} - {{ page_history(club.page) }} - {% else %} - {% trans %}No page existing for this club{% endtrans %} - {% endif %} + {{ page_history(club.page) }} {% endblock %} diff --git a/club/templates/club/pagerev_edit.jinja b/club/templates/club/pagerev_edit.jinja index 5253805d..cc7d3038 100644 --- a/club/templates/club/pagerev_edit.jinja +++ b/club/templates/club/pagerev_edit.jinja @@ -1,8 +1,12 @@ {% extends "core/base.jinja" %} -{% from 'core/macros_pages.jinja' import page_edit_form %} {% block content %} - {{ page_edit_form(page, form, url('club:club_edit_page', club_id=page.club.id), csrf_token) }} +

{% trans %}Edit page{% endtrans %}

+
+ {% csrf_token %} + {{ form.as_p() }} +

+
{% endblock %} diff --git a/club/tests/test_membership.py b/club/tests/test_membership.py index a3c0be50..2420043d 100644 --- a/club/tests/test_membership.py +++ b/club/tests/test_membership.py @@ -7,7 +7,7 @@ from django.conf import settings from django.contrib.auth.models import Permission from django.core.cache import cache from django.db.models import Max -from django.test import TestCase +from django.test import Client, TestCase from django.urls import reverse from django.utils.timezone import localdate, localtime, now from model_bakery import baker @@ -532,6 +532,35 @@ class TestMembership(TestClub): assert new_board == initial_board +@pytest.mark.django_db +def test_membership_set_old(client: Client): + membership = baker.make(Membership, end_date=None, user=(subscriber_user.make())) + client.force_login(membership.user) + response = client.post( + reverse("club:membership_set_old", kwargs={"membership_id": membership.id}) + ) + assertRedirects( + response, reverse("core:user_clubs", kwargs={"user_id": membership.user_id}) + ) + membership.refresh_from_db() + assert membership.end_date == localdate() + + +@pytest.mark.django_db +def test_membership_delete(client: Client): + user = baker.make(User, is_superuser=True) + membership = baker.make(Membership) + client.force_login(user) + url = reverse("club:membership_delete", kwargs={"membership_id": membership.id}) + response = client.get(url) + assert response.status_code == 200 + response = client.post(url) + assertRedirects( + response, reverse("core:user_clubs", kwargs={"user_id": membership.user_id}) + ) + assert not Membership.objects.filter(id=membership.id).exists() + + @pytest.mark.django_db class TestJoinClub: @pytest.fixture(autouse=True) diff --git a/club/tests/test_page.py b/club/tests/test_page.py index 7e8c5efc..c368735d 100644 --- a/club/tests/test_page.py +++ b/club/tests/test_page.py @@ -3,9 +3,10 @@ from bs4 import BeautifulSoup from django.test import Client from django.urls import reverse from model_bakery import baker -from pytest_django.asserts import assertHTMLEqual +from pytest_django.asserts import assertHTMLEqual, assertRedirects -from club.models import Club +from club.models import Club, Membership +from core.baker_recipes import subscriber_user from core.markdown import markdown from core.models import PageRev, User @@ -16,7 +17,6 @@ def test_page_display_on_club_main_page(client: Client): club = baker.make(Club) content = "# foo\nLorem ipsum dolor sit amet" baker.make(PageRev, page=club.page, revision=1, content=content) - client.force_login(baker.make(User)) res = client.get(reverse("club:club_view", kwargs={"club_id": club.id})) assert res.status_code == 200 @@ -30,10 +30,42 @@ def test_club_main_page_without_content(client: Client): """Test the club view works, even if the club page is empty""" club = baker.make(Club) club.page.revisions.all().delete() - client.force_login(baker.make(User)) res = client.get(reverse("club:club_view", kwargs={"club_id": club.id})) assert res.status_code == 200 soup = BeautifulSoup(res.text, "lxml") detail_html = soup.find(id="club_detail") assert detail_html.find_all("markdown") == [] + + +@pytest.mark.django_db +def test_page_revision(client: Client): + club = baker.make(Club) + revisions = baker.make( + PageRev, page=club.page, _quantity=3, content=iter(["foo", "bar", "baz"]) + ) + client.force_login(baker.make(User)) + url = reverse( + "club:club_view_rev", kwargs={"club_id": club.id, "rev_id": revisions[1].id} + ) + res = client.get(url) + assert res.status_code == 200 + soup = BeautifulSoup(res.text, "lxml") + detail_html = soup.find(class_="markdown") + assertHTMLEqual(detail_html.decode_contents(), markdown(revisions[1].content)) + + +@pytest.mark.django_db +def test_edit_page(client: Client): + club = baker.make(Club) + user = subscriber_user.make() + baker.make(Membership, user=user, club=club, role=3) + client.force_login(user) + url = reverse("club:club_edit_page", kwargs={"club_id": club.id}) + content = "# foo\nLorem ipsum dolor sit amet" + + res = client.get(url) + assert res.status_code == 200 + res = client.post(url, data={"content": content}) + assertRedirects(res, reverse("club:club_view", kwargs={"club_id": club.id})) + assert club.page.revisions.last().content == content diff --git a/club/tests/test_sales.py b/club/tests/test_sales.py index 6e734f80..457b8967 100644 --- a/club/tests/test_sales.py +++ b/club/tests/test_sales.py @@ -1,3 +1,6 @@ +import csv +import itertools + import pytest from django.test import Client from django.urls import reverse @@ -7,16 +10,20 @@ from club.forms import SellingsForm from club.models import Club from core.models import User from counter.baker_recipes import product_recipe, sale_recipe -from counter.models import Counter, Customer +from counter.models import Counter, Customer, Product, Selling @pytest.mark.django_db def test_sales_page_doesnt_crash(client: Client): + """Basic crashtest on club sales view.""" club = baker.make(Club) + product = baker.make(Product, club=club) admin = baker.make(User, is_superuser=True) client.force_login(admin) - response = client.get(reverse("club:club_sellings", kwargs={"club_id": club.id})) - assert response.status_code == 200 + url = reverse("club:club_sellings", kwargs={"club_id": club.id}) + assert client.get(url).status_code == 200 + assert client.post(url).status_code == 200 + assert client.post(url, data={"products": [product.id]}).status_code == 200 @pytest.mark.django_db @@ -36,3 +43,62 @@ def test_sales_form_counter_filter(): form = SellingsForm(club) form_counters = list(form.fields["counters"].queryset) assert form_counters == [counters[1], counters[2], counters[0]] + + +@pytest.mark.django_db +def test_club_sales_csv(client: Client): + client.force_login(baker.make(User, is_superuser=True)) + club = baker.make(Club) + counter = baker.make(Counter, club=club) + product = product_recipe.make(club=club, counters=[counter], purchase_price=0.5) + customers = baker.make(Customer, amount=100, _quantity=2, _bulk_create=True) + sales: list[Selling] = sale_recipe.make( + club=club, + counter=counter, + quantity=2, + unit_price=1.5, + product=iter([product, product, None]), + customer=itertools.cycle(customers), + _quantity=3, + ) + url = reverse("club:sellings_csv", kwargs={"club_id": club.id}) + response = client.post(url, data={"counters": [counter.id]}) + assert response.status_code == 200 + reader = csv.reader(s.decode() for s in response.streaming_content) + data = list(reader) + sale_rows = [ + [ + str(s.date), + str(counter), + str(s.seller), + s.customer.user.get_display_name(), + s.label, + "2", + "1.50", + "3.00", + "Compte utilisateur", + ] + for s in sales[::-1] + ] + sale_rows[2].extend(["0.50", "1.00"]) + sale_rows[1].extend(["0.50", "1.00"]) + sale_rows[0].extend(["", ""]) + assert data == [ + ["Quantité", "6"], + ["Total", "9"], + ["Bénéfice", "1"], + [ + "Date", + "Comptoir", + "Barman", + "Client", + "Étiquette", + "Quantité", + "Prix unitaire", + "Total", + "Méthode de paiement", + "Prix d'achat", + "Bénéfice", + ], + *sale_rows, + ] diff --git a/club/views.py b/club/views.py index e6c86a7c..9077d0d7 100644 --- a/club/views.py +++ b/club/views.py @@ -22,25 +22,28 @@ # # +from __future__ import annotations + import csv import itertools -from typing import Any +from typing import TYPE_CHECKING, Any from django.conf import settings -from django.contrib.auth.mixins import PermissionRequiredMixin +from django.contrib.auth.mixins import LoginRequiredMixin, 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 F, Q, Sum -from django.http import Http404, HttpResponseRedirect, StreamingHttpResponse +from django.http import Http404, 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.functional import cached_property from django.utils.timezone import now 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.detail import SingleObjectMixin from django.views.generic.edit import CreateView, DeleteView, UpdateView from club.forms import ( @@ -61,11 +64,14 @@ from com.views import ( PosterListBaseView, ) from core.auth.mixins import CanEditMixin, PermissionOrClubBoardRequiredMixin -from core.models import PageRev -from core.views import DetailFormView, PageEditViewBase, UseFragmentsMixin +from core.models import Page, PageRev +from core.views import BasePageEditView, DetailFormView, UseFragmentsMixin from core.views.mixins import FragmentMixin, FragmentRenderer, TabedViewMixin from counter.models import Selling +if TYPE_CHECKING: + from django.utils.safestring import SafeString + class ClubTabsMixin(TabedViewMixin): def get_tabs_title(self): @@ -75,6 +81,8 @@ class ClubTabsMixin(TabedViewMixin): self.object = self.object.page.club elif isinstance(self.object, Poster): self.object = self.object.club + elif hasattr(self, "club"): + self.object = self.club return self.object.get_display_name() def get_list_of_tabs(self): @@ -202,7 +210,7 @@ class ClubView(ClubTabsMixin, DetailView): return kwargs -class ClubRevView(ClubView): +class ClubRevView(LoginRequiredMixin, ClubView): """Display a specific page revision.""" def dispatch(self, request, *args, **kwargs): @@ -216,26 +224,26 @@ class ClubRevView(ClubView): return kwargs -class ClubPageEditView(ClubTabsMixin, PageEditViewBase): +class ClubPageEditView(ClubTabsMixin, BasePageEditView): template_name = "club/pagerev_edit.jinja" current_tab = "page_edit" - def dispatch(self, request, *args, **kwargs): - self.club = get_object_or_404(Club, pk=kwargs["club_id"]) - if not self.club.page: - raise Http404 - return super().dispatch(request, *args, **kwargs) + @cached_property + def club(self): + return get_object_or_404(Club, pk=self.kwargs["club_id"]) - def get_object(self): - self.page = self.club.page - return self._get_revision() + @cached_property + def page(self) -> Page: + page = self.club.page + page.set_lock(self.request.user) + return page def get_success_url(self, **kwargs): return reverse_lazy("club:club_view", kwargs={"club_id": self.club.id}) class ClubPageHistView(ClubTabsMixin, PermissionRequiredMixin, DetailView): - """Modification hostory of the page.""" + """Modification history of the page.""" model = Club pk_url_kwarg = "club_id" @@ -399,33 +407,14 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): kwargs = super().get_context_data(**kwargs) kwargs["result"] = Selling.objects.none() - kwargs["paginated_result"] = kwargs["result"] kwargs["total"] = 0 kwargs["total_quantity"] = 0 kwargs["benefit"] = 0 - 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"]: - qs = qs.filter(date__gte=form.cleaned_data["begin_date"]) - if form.cleaned_data["end_date"]: - qs = qs.filter(date__lte=form.cleaned_data["end_date"]) - - if form.cleaned_data["counters"]: - qs = qs.filter(counter__in=form.cleaned_data["counters"]) - - selected_products = [] - if form.cleaned_data["products"]: - selected_products.extend(form.cleaned_data["products"]) - if form.cleaned_data["archived_products"]: - selected_products.extend(form.cleaned_data["archived_products"]) - - if len(selected_products) > 0: - qs = qs.filter(product__in=selected_products) - + form: SellingsForm = self.get_form() + if form.is_valid() and any(v for v in form.cleaned_data.values()): + filters = form.to_filter_schema() + qs = filters.filter(Selling.objects.filter(club=self.object)) kwargs["total"] = qs.annotate( price=F("quantity") * F("unit_price") ).aggregate(total=Sum("price", default=0))["total"] @@ -472,15 +461,15 @@ class ClubSellingCSVView(ClubSellingView): *row, selling.label, selling.quantity, + selling.unit_price, selling.quantity * selling.unit_price, selling.get_payment_method_display(), ] if selling.product: - row.append(selling.product.selling_price) row.append(selling.product.purchase_price) - row.append(selling.product.selling_price - selling.product.purchase_price) + row.append(selling.unit_price - selling.product.purchase_price) else: - row = [*row, "", "", ""] + row = [*row, "", ""] return row def get(self, request, *args, **kwargs): @@ -501,9 +490,9 @@ class ClubSellingCSVView(ClubSellingView): gettext("Customer"), gettext("Label"), gettext("Quantity"), + gettext("Unit price"), gettext("Total"), gettext("Payment method"), - gettext("Selling price"), gettext("Purchase price"), gettext("Benefit"), ], @@ -556,33 +545,17 @@ class ClubCreateView(PermissionRequiredMixin, CreateView): permission_required = "club.add_club" -class MembershipSetOldView(CanEditMixin, DetailView): - """Set a membership as beeing old.""" +class MembershipSetOldView(CanEditMixin, SingleObjectMixin, View): + """Set a membership as being old.""" model = Membership pk_url_kwarg = "membership_id" - def get(self, request, *args, **kwargs): + def post(self, *_args, **_kwargs): self.object = self.get_object() self.object.end_date = timezone.now() self.object.save() - return HttpResponseRedirect( - reverse( - "club:club_members", - args=self.args, - kwargs={"club_id": self.object.club.id}, - ) - ) - - def post(self, request, *args, **kwargs): - self.object = self.get_object() - return HttpResponseRedirect( - reverse( - "club:club_members", - args=self.args, - kwargs={"club_id": self.object.club.id}, - ) - ) + return redirect("core:user_clubs", user_id=self.object.user_id) class MembershipDeleteView(PermissionRequiredMixin, DeleteView): @@ -594,7 +567,7 @@ class MembershipDeleteView(PermissionRequiredMixin, DeleteView): permission_required = "club.delete_membership" def get_success_url(self): - return reverse_lazy("core:user_clubs", kwargs={"user_id": self.object.user.id}) + return reverse_lazy("core:user_clubs", kwargs={"user_id": self.object.user_id}) class ClubMailingView(ClubTabsMixin, CanEditMixin, DetailFormView): diff --git a/com/tests/test_api.py b/com/tests/test_api.py index 571bc2a0..ce747347 100644 --- a/com/tests/test_api.py +++ b/com/tests/test_api.py @@ -1,4 +1,3 @@ -from dataclasses import dataclass from datetime import timedelta from pathlib import Path @@ -18,16 +17,6 @@ from core.markdown import markdown from core.models import User -@dataclass -class MockResponse: - ok: bool - value: str - - @property - def content(self): - return self.value.encode("utf8") - - def accel_redirect_to_file(response: HttpResponse) -> Path | None: redirect = Path(response.headers.get("X-Accel-Redirect", "")) if not redirect.is_relative_to(Path("/") / settings.MEDIA_ROOT.stem): diff --git a/com/views.py b/com/views.py index 2d5045d9..ea5b742d 100644 --- a/com/views.py +++ b/com/views.py @@ -240,10 +240,11 @@ class NewsListView(TemplateView): if not self.request.user.has_perm("core.view_user"): return [] return itertools.groupby( - User.objects.filter( + User.objects.viewable_by(self.request.user) + .filter( date_of_birth__month=localdate().month, date_of_birth__day=localdate().day, - is_subscriber_viewable=True, + is_viewable=True, ) .filter(role__in=["STUDENT", "FORMER STUDENT"]) .order_by("-date_of_birth"), diff --git a/core/admin.py b/core/admin.py index eff77817..a21086a0 100644 --- a/core/admin.py +++ b/core/admin.py @@ -74,9 +74,19 @@ class UserBanAdmin(admin.ModelAdmin): autocomplete_fields = ("user", "ban_group") +class GroupInline(admin.TabularInline): + model = Group.permissions.through + readonly_fields = ("group",) + extra = 0 + + def has_add_permission(self, request, obj): + return False + + @admin.register(Permission) class PermissionAdmin(admin.ModelAdmin): search_fields = ("codename",) + inlines = (GroupInline,) @admin.register(Page) diff --git a/core/api.py b/core/api.py index ab69f86e..2f2c0fb1 100644 --- a/core/api.py +++ b/core/api.py @@ -1,6 +1,6 @@ from typing import Annotated, Any, Literal -import annotated_types +from annotated_types import Ge, Le, MinLen from django.conf import settings from django.db.models import F from django.http import HttpResponse @@ -28,6 +28,7 @@ from core.schemas import ( UserSchema, ) from core.templatetags.renderer import markdown +from counter.utils import is_logged_in_counter @api_controller("/markdown") @@ -72,9 +73,9 @@ class MailingListController(ControllerBase): @api_controller("/user") class UserController(ControllerBase): - @route.get("", response=list[UserProfileSchema], permissions=[CanAccessLookup]) + @route.get("", response=list[UserProfileSchema]) def fetch_profiles(self, pks: Query[set[int]]): - return User.objects.filter(pk__in=pks) + return User.objects.viewable_by(self.context.request.user).filter(pk__in=pks) @route.get("/{int:user_id}", response=UserSchema, permissions=[CanView]) def fetch_user(self, user_id: int): @@ -85,13 +86,18 @@ class UserController(ControllerBase): "/search", response=PaginatedResponseSchema[UserProfileSchema], url_name="search_users", - permissions=[CanAccessLookup], + # logged in barmen aren't authenticated stricto sensu, so no auth here + auth=None, ) @paginate(PageNumberPaginationExtra, page_size=20) def search_users(self, filters: Query[UserFilterSchema]): - return filters.filter( - User.objects.order_by(F("last_login").desc(nulls_last=True)) - ) + qs = User.objects + # the logged in barmen can see all users (even the hidden one), + # because they have a temporary administrative function during + # which they may have to deal with hidden users + if not is_logged_in_counter(self.context.request): + qs = qs.viewable_by(self.context.request.user) + return filters.filter(qs.order_by(F("last_login").desc(nulls_last=True))) @api_controller("/file") @@ -103,7 +109,7 @@ class SithFileController(ControllerBase): permissions=[CanAccessLookup], ) @paginate(PageNumberPaginationExtra, page_size=50) - def search_files(self, search: Annotated[str, annotated_types.MinLen(1)]): + def search_files(self, search: Annotated[str, MinLen(1)]): return SithFile.objects.filter(is_in_sas=False).filter(name__icontains=search) @@ -116,11 +122,11 @@ class GroupController(ControllerBase): permissions=[CanAccessLookup], ) @paginate(PageNumberPaginationExtra, page_size=50) - def search_group(self, search: Annotated[str, annotated_types.MinLen(1)]): + def search_group(self, search: Annotated[str, MinLen(1)]): return Group.objects.filter(name__icontains=search).values() -DepthValue = Annotated[int, annotated_types.Ge(0), annotated_types.Le(10)] +DepthValue = Annotated[int, Ge(0), Le(10)] DEFAULT_DEPTH = 4 diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index cd0087e7..60c282aa 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -150,7 +150,8 @@ class Command(BaseCommand): Weekmail().save() - # Here we add a lot of test datas, that are not necessary for the Sith, but that provide a basic development environment + # Here we add a lot of test datas, that are not necessary for the Sith, + # but that provide a basic development environment self.now = timezone.now().replace(hour=12, second=0) skia = User.objects.create_user( diff --git a/core/migrations/0048_alter_user_options.py b/core/migrations/0048_alter_user_options.py new file mode 100644 index 00000000..f446273a --- /dev/null +++ b/core/migrations/0048_alter_user_options.py @@ -0,0 +1,33 @@ +# Generated by Django 5.2.8 on 2025-11-09 15:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("core", "0047_alter_notification_date_alter_notification_type")] + + operations = [ + migrations.AlterModelOptions( + name="user", + options={ + "permissions": [("view_hidden_user", "Can view hidden users")], + "verbose_name": "user", + "verbose_name_plural": "users", + }, + ), + migrations.RenameField( + model_name="user", old_name="is_subscriber_viewable", new_name="is_viewable" + ), + migrations.AlterField( + model_name="user", + name="is_viewable", + field=models.BooleanField( + default=True, + verbose_name="Profile visible by subscribers", + help_text=( + "If you disable this option, only admin users " + "will be able to see your profile." + ), + ), + ), + ] diff --git a/core/models.py b/core/models.py index a624ebf6..be3b5cc4 100644 --- a/core/models.py +++ b/core/models.py @@ -23,12 +23,13 @@ # from __future__ import annotations +import difflib import string import unicodedata from datetime import timedelta from io import BytesIO from pathlib import Path -from typing import TYPE_CHECKING, Self +from typing import TYPE_CHECKING, Final, Self from uuid import uuid4 from django.conf import settings @@ -54,6 +55,8 @@ from django.utils.translation import gettext_lazy as _ from phonenumber_field.modelfields import PhoneNumberField from PIL import Image, ImageOps +from core.utils import get_last_promo + if TYPE_CHECKING: from django.core.files.uploadedfile import UploadedFile from pydantic import NonNegativeInt @@ -86,12 +89,11 @@ class Group(AuthGroup): def validate_promo(value: int) -> None: - start_year = settings.SITH_SCHOOL_START_YEAR - delta = (localdate() + timedelta(days=180)).year - start_year - if value < 0 or delta < value: + last_promo = get_last_promo() + if not 0 < value <= last_promo: raise ValidationError( _("%(value)s is not a valid promo (between 0 and %(end)s)"), - params={"value": value, "end": delta}, + params={"value": value, "end": last_promo}, ) @@ -136,6 +138,15 @@ class UserQuerySet(models.QuerySet): Q(Exists(subscriptions)) | Q(Exists(refills)) | Q(Exists(purchases)) ) + def viewable_by(self, user: User) -> Self: + if user.has_perm("core.view_hidden_user"): + return self + if user.has_perm("core.view_user"): + return self.filter(is_viewable=True) + if user.is_anonymous: + return self.none() + return self.filter(id=user.id) + class CustomUserManager(UserManager.from_queryset(UserQuerySet)): # see https://docs.djangoproject.com/fr/stable/topics/migrations/#model-managers @@ -271,13 +282,24 @@ class User(AbstractUser): parent_address = models.CharField( _("parent address"), max_length=128, blank=True, default="" ) - is_subscriber_viewable = models.BooleanField( - _("is subscriber viewable"), default=True + is_viewable = models.BooleanField( + _("Profile visible by subscribers"), + help_text=_( + "If you disable this option, only admin users " + "will be able to see your profile." + ), + default=True, ) godfathers = models.ManyToManyField("User", related_name="godchildren", blank=True) objects = CustomUserManager() + class Meta(AbstractUser.Meta): + abstract = False + permissions = [ + ("view_hidden_user", "Can view hidden users"), + ] + def __str__(self): return self.get_display_name() @@ -551,8 +573,12 @@ class User(AbstractUser): def can_be_edited_by(self, user): return user.is_root or user.is_board_member - def can_be_viewed_by(self, user): - return (user.was_subscribed and self.is_subscriber_viewable) or user.is_root + def can_be_viewed_by(self, user: User) -> bool: + return ( + user.id == self.id + or user.has_perm("core.view_hidden_user") + or (user.has_perm("core.view_user") and self.is_viewable) + ) def get_mini_item(self): return """ @@ -1319,6 +1345,9 @@ class PageRev(models.Model): The content is in PageRev.title and PageRev.content . """ + MERGE_TIME_THRESHOLD: Final[timedelta] = timedelta(minutes=20) + MERGE_DIFF_THRESHOLD: Final[float] = 0.2 + revision = models.IntegerField(_("revision")) title = models.CharField(_("page title"), max_length=255, blank=True) content = models.TextField(_("page content"), blank=True) @@ -1360,6 +1389,32 @@ class PageRev(models.Model): def is_owned_by(self, user: User) -> bool: return any(g.id == self.page.owner_group_id for g in user.cached_groups) + def similarity_ratio(self, text: str) -> float: + """Similarity ratio between this revision's content and the given text. + + The result is a float in [0; 1], 0 meaning the contents are entirely different, + and 1 they are strictly the same. + """ + # cf. https://docs.python.org/3/library/difflib.html#difflib.SequenceMatcher.ratio + return difflib.SequenceMatcher(None, self.content, text).quick_ratio() + + def should_merge(self, other: Self) -> bool: + """Return True if `other` should be merged into `self`, else False. + + It's considered the other revision should be merged into this one if : + + - it was made less than 20 minutes after + - by the same author + - with a similarity ratio higher than 80% + """ + return ( + not self._state.adding # cannot merge if the original rev doesn't exist + and self.author == other.author + and (other.date - self.date) < self.MERGE_TIME_THRESHOLD + and (not other._state.adding or other.revision == self.revision + 1) + and self.similarity_ratio(other.content) >= (1 - other.MERGE_DIFF_THRESHOLD) + ) + def get_notification_types(): return settings.SITH_NOTIFICATIONS diff --git a/core/static/core/colors.scss b/core/static/core/colors.scss index 9ed493ba..490690d9 100644 --- a/core/static/core/colors.scss +++ b/core/static/core/colors.scss @@ -21,6 +21,8 @@ $secondary-neutral-dark-color: hsl(40, 57.6%, 17%); $white-color: hsl(219.6, 20.8%, 98%); $black-color: hsl(0, 0%, 17%); +$red-text-color: #eb2f06; +$hovered-red-text-color: #ff4d4d; $faceblue: hsl(221, 44%, 41%); $twitblue: hsl(206, 82%, 63%); diff --git a/core/static/core/footer.scss b/core/static/core/footer.scss index 3c0306e0..aa2e048f 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: black 0 8px 15px; + box-shadow: $shadow-color 0 0 15px; a { color: $white-color; diff --git a/core/static/core/forms.scss b/core/static/core/forms.scss index d761331c..e1793a69 100644 --- a/core/static/core/forms.scss +++ b/core/static/core/forms.scss @@ -745,4 +745,32 @@ form { background-repeat: no-repeat; background-size: var(--nf-input-size); } + + &.no-margin { + margin:0; + } + + // a submit input that should look like a regular + input[type="submit"], button { + &.link-like { + color: $primary-dark-color; + &:hover { + color: $primary-light-color; + } + + &.link-red { + color: $red-text-color; + &:hover { + color: $hovered-red-text-color; + } + } + font-weight: normal; + font-size: 100%; + margin: auto; + background: none; + border: none; + cursor: pointer; + padding: 0; + } + } } diff --git a/core/static/core/header.scss b/core/static/core/header.scss index f43819c3..01e70403 100644 --- a/core/static/core/header.scss +++ b/core/static/core/header.scss @@ -5,14 +5,10 @@ $text-color: white; $background-color-hovered: #283747; -$red-text-color: #eb2f06; -$hovered-red-text-color: #ff4d4d; - .header { box-sizing: border-box; background-color: $deepblue; - box-shadow: black 0 1px 3px 0, - black 0 4px 8px 3px; + box-shadow: 3px 3px 3px 0 #dfdfdf; border-radius: 0; width: 100%; display: flex; @@ -100,7 +96,7 @@ $hovered-red-text-color: #ff4d4d; border-radius: 0; margin: 0; box-sizing: border-box; - background-color: transparent; + background-color: $deepblue; width: 45px; height: 25px; padding: 0; @@ -252,12 +248,15 @@ $hovered-red-text-color: #ff4d4d; justify-content: flex-start; } + a { + color: $text-color; + } + a, button { font-size: 100%; margin: 0; text-align: right; - color: $text-color; margin-top: auto; &:hover { @@ -269,19 +268,6 @@ $hovered-red-text-color: #ff4d4d; margin: 0; display: inline; } - - #logout-form button { - color: $red-text-color; - - &:hover { - color: $hovered-red-text-color; - } - - background: none; - border: none; - cursor: pointer; - padding: 0; - } } } } @@ -332,7 +318,7 @@ $hovered-red-text-color: #ff4d4d; padding: 10px; z-index: 100; border-radius: 10px; - @include shadow; + box-shadow: 3px 3px 3px 0 #767676; >ul { list-style-type: none; diff --git a/core/static/core/img/gala25_background.webp b/core/static/core/img/gala25_background.webp deleted file mode 100644 index 978e9946..00000000 Binary files a/core/static/core/img/gala25_background.webp and /dev/null differ diff --git a/core/static/core/img/gala25_logo.webp b/core/static/core/img/gala25_logo.webp deleted file mode 100644 index 3cbdb6f7..00000000 Binary files a/core/static/core/img/gala25_logo.webp and /dev/null differ diff --git a/core/static/core/style.scss b/core/static/core/style.scss index c23303a7..b48aa7c1 100644 --- a/core/static/core/style.scss +++ b/core/static/core/style.scss @@ -271,9 +271,8 @@ body { /*--------------------------------CONTENT------------------------------*/ #content { - padding: 1.5em 2%; - border-radius: 5px; - box-shadow: black 0 8px 15px; + padding: 1em 1%; + box-shadow: $shadow-color 0 5px 10px; background: $white-color; overflow: auto; } @@ -520,7 +519,6 @@ th { td { margin: 5px; border-collapse: collapse; - vertical-align: top; overflow: hidden; text-overflow: ellipsis; diff --git a/core/static/user/user_edit.scss b/core/static/user/user_edit.scss index 888ae729..5b20fcee 100644 --- a/core/static/user/user_edit.scss +++ b/core/static/user/user_edit.scss @@ -7,10 +7,13 @@ .profile { &-visible { display: flex; - justify-content: center; + flex-direction: column; align-items: center; gap: 5px; padding-top: 10px; + input[type="checkbox"]+label { + max-width: unset; + } } &-pictures { @@ -116,23 +119,19 @@ display: flex; flex-direction: row; flex-wrap: wrap; - gap: 10px; + gap: var(--nf-input-size) 10px; justify-content: center; } &-field { display: flex; - flex-direction: row; - align-items: center; flex-wrap: wrap; justify-content: center; - gap: 10px; width: 100%; max-width: 330px; min-width: 300px; @media (max-width: 750px) { - gap: 4px; max-width: 100%; } @@ -145,22 +144,6 @@ } } - &-label { - text-align: left !important; - } - - &-content { - > * { - box-sizing: border-box; - text-align: left !important; - margin: 0; - - > * { - text-align: left !important; - } - } - } - textarea { height: 7rem; } diff --git a/core/templates/core/base.jinja b/core/templates/core/base.jinja index 356abdff..34a8040b 100644 --- a/core/templates/core/base.jinja +++ b/core/templates/core/base.jinja @@ -44,18 +44,6 @@ {% 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 65a15968..de0169b9 100644 --- a/core/templates/core/base/header.jinja +++ b/core/templates/core/base/header.jinja @@ -1,6 +1,6 @@
diff --git a/core/templates/core/page.jinja b/core/templates/core/page.jinja deleted file mode 100644 index b676855a..00000000 --- a/core/templates/core/page.jinja +++ /dev/null @@ -1,64 +0,0 @@ -{% extends "core/base.jinja" %} - -{% block title %} - {% if page %} - {{ page.get_display_name() }} - {% elif page_list %} - {% trans %}Page list{% endtrans %} - {% elif new_page %} - {% trans %}Create page{% endtrans %} - {% else %} - {% trans %}Not found{% endtrans %} - {% endif %} -{% endblock %} - -{% block metatags %} - {% if page %} - - - - - - {% else %} - {{ super() }} - {% endif %} -{% endblock %} - -{%- macro print_page_name(page) -%} - {%- if page -%} - {{ print_page_name(page.parent) }} > - {{ page.get_display_name() }} - {%- endif -%} -{%- endmacro -%} - -{% block content %} - {{ print_page_name(page) }} -
-
- {% if page %} - {% if page.club %} - {% trans %}Return to club management{% endtrans %} - {% else %} - {% trans %}View{% endtrans %} - {% endif %} - {% trans %}History{% endtrans %} - {% if can_edit(page, user) %} - {% trans %}Edit{% endtrans %} - {% endif %} - {% if can_edit_prop(page, user) and not page.is_club_page %} - {% trans %}Prop{% endtrans %} - {% endif %} - {% endif %} -
-
-
- - {% if page %} - {% block page %} - {% endblock %} - {% else %} -

{% trans %}Page does not exist{% endtrans %}

-

- {% trans %}Create it?{% endtrans %}

- {% endif %} -{% endblock %} diff --git a/core/templates/core/page/base.jinja b/core/templates/core/page/base.jinja new file mode 100644 index 00000000..3737eb2f --- /dev/null +++ b/core/templates/core/page/base.jinja @@ -0,0 +1,44 @@ +{% extends "core/base.jinja" %} + +{% block title %} + {{ page.get_display_name() }} +{% endblock %} + +{% block metatags %} + + + + + +{% endblock %} + +{%- macro print_page_name(page) -%} + {%- if page -%} + {{ print_page_name(page.parent) }} > + {{ page.get_display_name() }} + {%- endif -%} +{%- endmacro -%} + +{% block content %} + {{ print_page_name(page) }} +
+
+ {% if page.club %} + {% trans %}Return to club management{% endtrans %} + {% else %} + {% trans %}View{% endtrans %} + {% endif %} + {% trans %}History{% endtrans %} + {% if can_edit(page, user) %} + {% trans %}Edit{% endtrans %} + {% endif %} + {% if can_edit_prop(page, user) and not page.is_club_page %} + {% trans %}Prop{% endtrans %} + {% endif %} +
+
+
+ + {% block page %} + {% endblock %} +{% endblock %} diff --git a/core/templates/core/page/detail.jinja b/core/templates/core/page/detail.jinja new file mode 100644 index 00000000..d15510c4 --- /dev/null +++ b/core/templates/core/page/detail.jinja @@ -0,0 +1,17 @@ +{% extends "core/page/base.jinja" %} + +{% block page %} + {% if revision and revision.id != last_revision.id %} +

+ {% trans trimmed rev_id=revision.revision %} + This may not be the last update, you are seeing revision {{ rev_id }}! + {% endtrans %} +

+ {% endif %} + {% set current_revision = revision or last_revision %} +

{{ current_revision.title }}

+
{{ current_revision.content|markdown }}
+{% endblock %} + + + diff --git a/core/templates/core/page/edit.jinja b/core/templates/core/page/edit.jinja new file mode 100644 index 00000000..a4b01cdc --- /dev/null +++ b/core/templates/core/page/edit.jinja @@ -0,0 +1,13 @@ +{% extends "core/page/base.jinja" %} + +{% block page %} +

{% trans %}Edit page{% endtrans %}

+
+ {% csrf_token %} + {{ form.as_p() }} +

+
+{% endblock %} + + + diff --git a/core/templates/core/page_hist.jinja b/core/templates/core/page/history.jinja similarity index 55% rename from core/templates/core/page_hist.jinja rename to core/templates/core/page/history.jinja index 4aac13b1..6956d54f 100644 --- a/core/templates/core/page_hist.jinja +++ b/core/templates/core/page/history.jinja @@ -1,6 +1,6 @@ -{% extends "core/page.jinja" %} +{% extends "core/page/base.jinja" %} -{% from "core/macros_pages.jinja" import page_history %} +{% from "core/page/macros.jinja" import page_history %} {% block page %}

{% trans %}Page history{% endtrans %}

diff --git a/core/templates/core/page_list.jinja b/core/templates/core/page/list.jinja similarity index 100% rename from core/templates/core/page_list.jinja rename to core/templates/core/page/list.jinja diff --git a/core/templates/core/macros_pages.jinja b/core/templates/core/page/macros.jinja similarity index 70% rename from core/templates/core/macros_pages.jinja rename to core/templates/core/page/macros.jinja index 79228ae7..ad2680de 100644 --- a/core/templates/core/macros_pages.jinja +++ b/core/templates/core/page/macros.jinja @@ -17,12 +17,3 @@ {%- endfor -%} {% endmacro %} - -{% macro page_edit_form(page, form, url, token) %} -

{% trans %}Edit page{% endtrans %}

-
- - {{ form.as_p() }} -

-
-{% endmacro %} diff --git a/core/templates/core/page/not_found.jinja b/core/templates/core/page/not_found.jinja new file mode 100644 index 00000000..823d3f44 --- /dev/null +++ b/core/templates/core/page/not_found.jinja @@ -0,0 +1,12 @@ +{% extends "core/base.jinja" %} + +{% block content %} +

{% trans %}Page does not exist{% endtrans %}

+

+ {# This template is rendered when a PageNotFound error is raised, + so the `exception` context variable should always have a page_name attribute #} + + {% trans %}Create it?{% endtrans %} + +

+{% endblock %} \ No newline at end of file diff --git a/core/templates/core/page_prop.jinja b/core/templates/core/page/prop.jinja similarity index 50% rename from core/templates/core/page_prop.jinja rename to core/templates/core/page/prop.jinja index 978ec385..0e6e2dd2 100644 --- a/core/templates/core/page_prop.jinja +++ b/core/templates/core/page/prop.jinja @@ -1,18 +1,13 @@ -{% extends "core/page.jinja" %} +{% extends "core/page/base.jinja" %} -{% block content %} - {% if page %} - {{ super() }} - {% endif %} +{% block page %}

{% trans %}Page properties{% endtrans %}

{% csrf_token %} {{ form.as_p() }}

- {% if page %} - {% trans %}Delete{% endtrans %} - {% endif %} + {% trans %}Delete{% endtrans %} {% endblock %} diff --git a/core/templates/core/page_detail.jinja b/core/templates/core/page_detail.jinja deleted file mode 100644 index 054ee612..00000000 --- a/core/templates/core/page_detail.jinja +++ /dev/null @@ -1,17 +0,0 @@ -{% extends "core/page.jinja" %} - -{% block page %} - {% if rev %} -

{% trans rev_id=rev.revision %}This may not be the last update, you are seeing revision {{ rev_id }}!{% endtrans %}

-

{{ rev.title }}

-
{{ rev.content|markdown }}
- {% else %} - {% if page.revisions.last() %} -

{{ page.revisions.last().title }}

-
{{ page.revisions.last().content|markdown }}
- {% endif %} - {% endif %} -{% endblock %} - - - diff --git a/core/templates/core/pagerev_edit.jinja b/core/templates/core/pagerev_edit.jinja deleted file mode 100644 index 2d1bb6a0..00000000 --- a/core/templates/core/pagerev_edit.jinja +++ /dev/null @@ -1,9 +0,0 @@ -{% extends "core/page.jinja" %} -{% from 'core/macros_pages.jinja' import page_edit_form %} - -{% block page %} - {{ page_edit_form(page, form, url('core:page_edit', page_name=page.get_full_name()), csrf_token) }} -{% endblock %} - - - diff --git a/core/templates/core/user_clubs.jinja b/core/templates/core/user_clubs.jinja index 51b6827f..bd365239 100644 --- a/core/templates/core/user_clubs.jinja +++ b/core/templates/core/user_clubs.jinja @@ -17,7 +17,9 @@ {% trans %}Description{% endtrans %} {% trans %}Since{% endtrans %} - + {% if user.has_perm("club.delete_membership") %} + + {% endif %} @@ -28,7 +30,16 @@ {{ m.description }} {{ m.start_date }} {% if m.can_be_edited_by(user) %} - {% trans %}Mark as old{% endtrans %} + +
+ {% csrf_token %} + +
+ {% endif %} {% if user.has_perm("club.delete_membership") %} {% trans %}Delete{% endtrans %} @@ -48,7 +59,9 @@ {% trans %}Description{% endtrans %} {% trans %}From{% endtrans %} {% trans %}To{% endtrans %} - + {% if user.has_perm("club.delete_membership") %} + + {% endif %} diff --git a/core/templates/core/user_edit.jinja b/core/templates/core/user_edit.jinja index 8d015467..2f069da7 100644 --- a/core/templates/core/user_edit.jinja +++ b/core/templates/core/user_edit.jinja @@ -116,12 +116,12 @@ {# All fields #}
{%- for field in form -%} - {%- if field.name in ["quote","profile_pict","avatar_pict","scrub_pict","is_subscriber_viewable","forum_signature"] -%} + {%- if field.name in ["quote","profile_pict","avatar_pict","scrub_pict","is_viewable","forum_signature"] -%} {%- continue -%} {%- endif -%}
-
{{ field.label }}
+ {{ field.label_tag() }}
{{ field }} {%- if field.errors -%} @@ -136,7 +136,7 @@
{%- for field in [form.quote, form.forum_signature] -%}
-
{{ field.label }}
+ {{ field.label_tag() }}
{{ field }} {%- if field.errors -%} @@ -149,8 +149,13 @@ {# Checkboxes #}
- {{ form.is_subscriber_viewable }} - {{ form.is_subscriber_viewable.label }} +
+ {{ form.is_viewable }} + {{ form.is_viewable.label_tag() }} +
+ + {{ form.is_viewable.help_text }} +
diff --git a/core/tests/test_core.py b/core/tests/test_core.py index 81c979ee..77d3f535 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -23,6 +23,7 @@ from django.contrib.auth.hashers import make_password from django.contrib.auth.models import Permission from django.core import mail from django.core.cache import cache +from django.core.exceptions import ValidationError from django.core.mail import EmailMessage from django.test import Client, RequestFactory, TestCase from django.urls import reverse @@ -35,8 +36,8 @@ from pytest_django.asserts import assertInHTML, assertRedirects from antispam.models import ToxicDomain from club.models import Club, Membership from core.markdown import markdown -from core.models import AnonymousUser, Group, Page, User -from core.utils import get_semester_code, get_start_of_semester +from core.models import AnonymousUser, Group, Page, User, validate_promo +from core.utils import get_last_promo, get_semester_code, get_start_of_semester from core.views import AllowFragment from counter.models import Customer from sith import settings @@ -318,9 +319,8 @@ class TestPageHandling(TestCase): def test_access_page_not_found(self): """Should not display a page correctly.""" response = self.client.get(reverse("core:page", kwargs={"page_name": "swagg"})) - assert response.status_code == 200 - html = response.text - self.assertIn('', html) + assert response.status_code == 404 + assert '' in response.text def test_create_page_markdown_safe(self): """Should format the markdown and escape html correctly.""" @@ -523,6 +523,21 @@ class TestDateUtils(TestCase): assert get_start_of_semester() == autumn_2023 +@pytest.mark.parametrize( + ("current_date", "promo"), + [("2020-10-01", 22), ("2025-03-01", 26), ("2000-11-11", 2)], +) +def test_get_last_promo(current_date: str, promo: int): + with freezegun.freeze_time(current_date): + assert get_last_promo() == promo + + +@pytest.mark.parametrize("promo", [0, 24]) +def test_promo_validator(promo: int): + with freezegun.freeze_time("2021-10-01"), pytest.raises(ValidationError): + validate_promo(promo) + + def test_allow_fragment_mixin(): class TestAllowFragmentView(AllowFragment, ContextMixin, View): def get(self, *args, **kwargs): diff --git a/core/tests/test_family.py b/core/tests/test_family.py index 69844213..b13f3710 100644 --- a/core/tests/test_family.py +++ b/core/tests/test_family.py @@ -55,7 +55,7 @@ class TestFetchFamilyApi(TestCase): assert response.status_code == 403 def test_fetch_family_hidden_user(self): - self.main_user.is_subscriber_viewable = False + self.main_user.is_viewable = False self.main_user.save() for user_to_login, error_code in [ (self.main_user, 200), diff --git a/core/tests/test_page.py b/core/tests/test_page.py index 10c93f2b..8dcfa19c 100644 --- a/core/tests/test_page.py +++ b/core/tests/test_page.py @@ -1,32 +1,122 @@ +from datetime import timedelta + +import freezegun import pytest +from bs4 import BeautifulSoup from django.conf import settings from django.contrib.auth.models import Permission from django.test import Client from django.urls import reverse +from django.utils.timezone import now from model_bakery import baker -from pytest_django.asserts import assertRedirects +from pytest_django.asserts import assertHTMLEqual, assertRedirects +from club.models import Club from core.baker_recipes import board_user, subscriber_user -from core.models import AnonymousUser, Page, User -from sith.settings import SITH_GROUP_OLD_SUBSCRIBERS_ID, SITH_GROUP_SUBSCRIBERS_ID +from core.markdown import markdown +from core.models import AnonymousUser, Page, PageRev, User @pytest.mark.django_db -def test_edit_page(client: Client): - user = board_user.make() +class TestEditPage: + def test_edit_page(self, client: Client): + user = board_user.make() + page = baker.prepare(Page) + page.save(force_lock=True) + page.view_groups.add(user.groups.first()) + page.edit_groups.add(user.groups.first()) + client.force_login(user) + + url = reverse("core:page_edit", kwargs={"page_name": page._full_name}) + res = client.get(url) + assert res.status_code == 200 + + res = client.post(url, data={"content": "Hello World"}) + assertRedirects( + res, reverse("core:page", kwargs={"page_name": page._full_name}) + ) + revision = page.revisions.last() + assert revision.content == "Hello World" + + def test_pagerev_reused(self, client): + """Test that the previous revision is edited, if same author and small time diff""" + user = baker.make(User, is_superuser=True) + page = baker.prepare(Page) + page.save(force_lock=True) + first_rev = baker.make( + PageRev, author=user, page=page, date=now(), content="Hello World" + ) + client.force_login(user) + url = reverse("core:page_edit", kwargs={"page_name": page._full_name}) + client.post(url, data={"content": "Hello World!"}) + assert page.revisions.count() == 1 + assert page.revisions.last() == first_rev + first_rev.refresh_from_db() + assert first_rev.author == user + assert first_rev.content == "Hello World!" + + def test_pagerev_not_reused(self, client): + """Test that a new revision is created if too much time + passed since the last one. + """ + user = baker.make(User, is_superuser=True) + page = baker.prepare(Page) + page.save(force_lock=True) + first_rev = baker.make(PageRev, author=user, page=page, date=now()) + client.force_login(user) + url = reverse("core:page_edit", kwargs={"page_name": page._full_name}) + with freezegun.freeze_time(now() + timedelta(minutes=30)): + client.post(url, data={"content": "Hello World"}) + assert page.revisions.count() == 2 + assert page.revisions.last() != first_rev + + +@pytest.mark.django_db +def test_page_revision(client: Client): + """Test the GET to request to a specific revision page.""" page = baker.prepare(Page) page.save(force_lock=True) - page.view_groups.add(user.groups.first()) - client.force_login(user) - - url = reverse("core:page_edit", kwargs={"page_name": page._full_name}) + page.view_groups.add(settings.SITH_GROUP_SUBSCRIBERS_ID) + revisions = baker.make( + PageRev, page=page, _quantity=3, content=iter(["foo", "bar", "baz"]) + ) + client.force_login(subscriber_user.make()) + url = reverse( + "core:page_rev", + kwargs={"page_name": page._full_name, "rev": revisions[1].id}, + ) res = client.get(url) assert res.status_code == 200 + soup = BeautifulSoup(res.text, "lxml") + detail_html = soup.find(class_="markdown") + assertHTMLEqual(detail_html.decode_contents(), markdown(revisions[1].content)) - res = client.post(url, data={"content": "Hello World"}) - assertRedirects(res, reverse("core:page", kwargs={"page_name": page._full_name})) - revision = page.revisions.last() - assert revision.content == "Hello World" + +@pytest.mark.django_db +def test_page_club_redirection(client: Client): + club = baker.make(Club) + url = reverse("core:page", kwargs={"page_name": club.page._full_name}) + res = client.get(url) + redirection_url = reverse("club:club_view", kwargs={"club_id": club.id}) + assertRedirects(res, redirection_url) + + +@pytest.mark.django_db +def test_page_revision_club_redirection(client: Client): + client.force_login(subscriber_user.make()) + club = baker.make(Club) + revisions = baker.make( + PageRev, page=club.page, _quantity=3, content=iter(["foo", "bar", "baz"]) + ) + url = reverse( + "core:page_rev", + kwargs={"page_name": club.page._full_name, "rev": revisions[1].id}, + ) + res = client.get(url) + redirection_url = reverse( + "club:club_view_rev", kwargs={"club_id": club.id, "rev_id": revisions[1].id} + ) + assertRedirects(res, redirection_url) @pytest.mark.django_db @@ -35,9 +125,9 @@ def test_viewable_by(): Page.objects.all().delete() view_groups = [ [settings.SITH_GROUP_PUBLIC_ID], - [settings.SITH_GROUP_PUBLIC_ID, SITH_GROUP_SUBSCRIBERS_ID], - [SITH_GROUP_SUBSCRIBERS_ID], - [SITH_GROUP_SUBSCRIBERS_ID, SITH_GROUP_OLD_SUBSCRIBERS_ID], + [settings.SITH_GROUP_PUBLIC_ID, settings.SITH_GROUP_SUBSCRIBERS_ID], + [settings.SITH_GROUP_SUBSCRIBERS_ID], + [settings.SITH_GROUP_SUBSCRIBERS_ID, settings.SITH_GROUP_OLD_SUBSCRIBERS_ID], [], ] pages = baker.make(Page, _quantity=len(view_groups), _bulk_create=True) @@ -56,3 +146,11 @@ def test_viewable_by(): ) viewable = Page.objects.viewable_by(root_user).values_list("id", flat=True) assert set(viewable) == {p.id for p in pages} + + +@pytest.mark.django_db +def test_page_list_view(client: Client): + baker.make(Page, _quantity=10, _bulk_create=True) + client.force_login(subscriber_user.make()) + res = client.get(reverse("core:page_list")) + assert res.status_code == 200 diff --git a/core/tests/test_user.py b/core/tests/test_user.py index 8be9bf5f..6d99d2a2 100644 --- a/core/tests/test_user.py +++ b/core/tests/test_user.py @@ -1,8 +1,10 @@ from datetime import timedelta +from unittest import mock import pytest from django.conf import settings from django.contrib import auth +from django.contrib.auth.models import Permission from django.core.management import call_command from django.test import Client, RequestFactory, TestCase from django.urls import reverse @@ -18,10 +20,11 @@ from core.baker_recipes import ( subscriber_user, very_old_subscriber_user, ) -from core.models import Group, User +from core.models import AnonymousUser, Group, User from core.views import UserTabsMixin from counter.baker_recipes import sale_recipe from counter.models import Counter, Customer, Refilling, Selling +from counter.utils import is_logged_in_counter from eboutic.models import Invoice, InvoiceItem @@ -59,7 +62,9 @@ class TestSearchUsersAPI(TestSearchUsers): """Test that users are ordered by last login date.""" self.client.force_login(subscriber_user.make()) - response = self.client.get(reverse("api:search_users") + "?search=First") + response = self.client.get( + reverse("api:search_users", query={"search": "First"}) + ) assert response.status_code == 200 assert response.json()["count"] == 11 # The users are ordered by last login date, so we need to reverse the list @@ -68,7 +73,7 @@ class TestSearchUsersAPI(TestSearchUsers): ] def test_search_case_insensitive(self): - """Test that the search is case insensitive.""" + """Test that the search is case-insensitive.""" self.client.force_login(subscriber_user.make()) expected = [u.id for u in self.users[::-1]] @@ -81,14 +86,19 @@ class TestSearchUsersAPI(TestSearchUsers): assert [r["id"] for r in response.json()["results"]] == expected def test_search_nick_name(self): - """Test that the search can be done on the nick name.""" + """Test that the search can be done on the nickname.""" + # hidden users should not be in the final result, + # even when the nickname matches + self.users[10].is_viewable = False + self.users[10].save() self.client.force_login(subscriber_user.make()) # this should return users with nicknames Nick11, Nick10 and Nick1 - response = self.client.get(reverse("api:search_users") + "?search=Nick1") + response = self.client.get( + reverse("api:search_users", query={"search": "Nick1"}) + ) assert response.status_code == 200 assert [r["id"] for r in response.json()["results"]] == [ - self.users[10].id, self.users[9].id, self.users[0].id, ] @@ -100,10 +110,25 @@ class TestSearchUsersAPI(TestSearchUsers): self.client.force_login(subscriber_user.make()) # this should return users with first names First1 and First10 - response = self.client.get(reverse("api:search_users") + "?search=bél") + response = self.client.get(reverse("api:search_users", query={"search": "bél"})) assert response.status_code == 200 assert [r["id"] for r in response.json()["results"]] == [belix.id] + @mock.create_autospec(is_logged_in_counter, return_value=True) + def test_search_as_barman(self): + # barmen should also see hidden users + self.users[10].is_viewable = False + self.users[10].save() + response = self.client.get( + reverse("api:search_users", query={"search": "Nick1"}) + ) + assert response.status_code == 200 + assert [r["id"] for r in response.json()["results"]] == [ + self.users[10].id, + self.users[9].id, + self.users[0].id, + ] + class TestSearchUsersView(TestSearchUsers): """Test the search user view (`GET /search`).""" @@ -368,3 +393,38 @@ class TestRedirectMe: def test_promo_has_logo(promo): user = baker.make(User, promo=promo) assert user.promo_has_logo() + + +@pytest.mark.django_db +class TestUserQuerySetViewableBy: + @pytest.fixture + def users(self) -> list[User]: + return [ + baker.make(User), + subscriber_user.make(), + subscriber_user.make(is_viewable=False), + ] + + def test_admin_user(self, users: list[User]): + user = baker.make( + User, + user_permissions=[Permission.objects.get(codename="view_hidden_user")], + ) + viewable = User.objects.filter(id__in=[u.id for u in users]).viewable_by(user) + assert set(viewable) == set(users) + + @pytest.mark.parametrize( + "user_factory", [old_subscriber_user.make, subscriber_user.make] + ) + def test_subscriber(self, users: list[User], user_factory): + user = user_factory() + viewable = User.objects.filter(id__in=[u.id for u in users]).viewable_by(user) + assert set(viewable) == {users[0], users[1]} + + @pytest.mark.parametrize( + "user_factory", [lambda: baker.make(User), lambda: AnonymousUser()] + ) + def test_not_subscriber(self, users: list[User], user_factory): + user = user_factory() + viewable = User.objects.filter(id__in=[u.id for u in users]).viewable_by(user) + assert not viewable.exists() diff --git a/core/utils.py b/core/utils.py index c54aad81..0e284768 100644 --- a/core/utils.py +++ b/core/utils.py @@ -112,6 +112,16 @@ def get_semester_code(d: date | None = None) -> str: return "P" + str(start.year)[-2:] +def get_last_promo() -> int: + """Get the latest promo at the time the function is called. + + For example, if called in october 2022 return 24, + if called in march 2026 return 27, etc. + """ + start_year = settings.SITH_SCHOOL_START_YEAR + return (localdate() + timedelta(days=180)).year - start_year + + def is_image(file: UploadedFile): try: im = PIL.Image.open(file.file) @@ -186,7 +196,7 @@ def exif_auto_rotate(image): def get_client_ip(request: HttpRequest) -> str | None: headers = ( - "X_FORWARDED_FOR", # Common header for proixes + "X_FORWARDED_FOR", # Common header for proxies "FORWARDED", # Standard header defined by RFC 7239. "REMOTE_ADDR", # Default IP Address (direct connection) ) diff --git a/core/views/__init__.py b/core/views/__init__.py index 288a28e8..ea7177d8 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -21,10 +21,10 @@ # Place - Suite 330, Boston, MA 02111-1307, USA. # # - from django.http import ( + Http404, + HttpRequest, HttpResponseForbidden, - HttpResponseNotFound, HttpResponseServerError, ) from django.shortcuts import render @@ -33,17 +33,20 @@ from django.views.generic.edit import FormView from sentry_sdk import last_event_id from core.views.forms import LoginForm +from core.views.page import PageNotFound -def forbidden(request, exception): +def forbidden(request: HttpRequest, exception): context = {"next": request.path, "form": LoginForm()} return HttpResponseForbidden(render(request, "core/403.jinja", context=context)) -def not_found(request, exception): - return HttpResponseNotFound( - render(request, "core/404.jinja", context={"exception": exception}) - ) +def not_found(request: HttpRequest, exception: Http404): + if isinstance(exception, PageNotFound): + template_name = "core/page/not_found.jinja" + else: + template_name = "core/404.jinja" + return render(request, template_name, context={"exception": exception}, status=404) def internal_servor_error(request): diff --git a/core/views/forms.py b/core/views/forms.py index fdfe61cf..58b5c598 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -21,6 +21,7 @@ # # import re +from copy import copy from datetime import date, datetime from io import BytesIO @@ -42,13 +43,12 @@ from django.forms import ( Widget, ) from django.utils.timezone import now -from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ from phonenumber_field.widgets import RegionalPhoneNumberWidget from PIL import Image from antispam.forms import AntiSpamEmailField -from core.models import Gift, Group, Page, SithFile, User +from core.models import Gift, Group, Page, PageRev, SithFile, User from core.utils import resize_image from core.views.widgets.ajax_select import ( AutoCompleteSelect, @@ -56,6 +56,7 @@ from core.views.widgets.ajax_select import ( AutoCompleteSelectMultipleGroup, AutoCompleteSelectUser, ) +from core.views.widgets.markdown import MarkdownInput # Widgets @@ -86,30 +87,6 @@ class NFCTextInput(TextInput): return context -class SelectUser(TextInput): - def render(self, name, value, attrs=None, renderer=None): - if attrs: - attrs["class"] = "select_user" - else: - attrs = {"class": "select_user"} - output = ( - '%(content)s
' - % { - "content": super().render(name, value, attrs, renderer), - "title": _("Choose user"), - "name": name, - } - ) - output += ( - '' - + gettext("Choose user") - + "" - ) - return output - - # Fields @@ -202,7 +179,7 @@ class UserProfileForm(forms.ModelForm): "school", "promo", "forum_signature", - "is_subscriber_viewable", + "is_viewable", ] widgets = { "date_of_birth": SelectDate, @@ -211,8 +188,8 @@ class UserProfileForm(forms.ModelForm): "quote": forms.Textarea, } - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, *args, label_suffix: str = "", **kwargs): + super().__init__(*args, label_suffix=label_suffix, **kwargs) # Image fields are injected here to override the file field provided by the model # This would be better if we could have a SithImage sort of model input instead of a generic SithFile @@ -404,6 +381,42 @@ class PageForm(forms.ModelForm): ) +class PageRevisionForm(forms.ModelForm): + """Form to add a new revision to a page. + + Notes: + Saving this form won't always result in a new revision. + If the previous revision on the same page was made : + + - less than 20 minutes ago + - by the same author + - with a similarity ratio higher than 80% + + then the latter will be edited and the new revision won't be created. + """ + + class Meta: + model = PageRev + fields = ["title", "content"] + widgets = {"content": MarkdownInput} + + def __init__( + self, *args, author: User, page: Page, instance: PageRev | None = None, **kwargs + ): + super().__init__(*args, instance=instance, **kwargs) + self.author = author + self.page = page + self.initial_obj: PageRev = copy(self.instance) + + def save(self, commit=True): # noqa FBT002 + revision: PageRev = self.instance + if not self.initial_obj.should_merge(self.instance): + revision.author = self.author + revision.page = self.page + revision.id = None # if id is None, Django will create a new record + return super().save(commit=commit) + + class GiftForm(forms.ModelForm): class Meta: model = Gift diff --git a/core/views/page.py b/core/views/page.py index d1cd87aa..fdfbecc3 100644 --- a/core/views/page.py +++ b/core/views/page.py @@ -13,39 +13,39 @@ # # -from django.contrib.auth.mixins import PermissionRequiredMixin +from django.contrib.auth.mixins import PermissionRequiredMixin, UserPassesTestMixin from django.db.models import F, OuterRef, Subquery from django.db.models.functions import Coalesce - -# This file contains all the views that concern the page model -from django.forms.models import modelform_factory from django.http import Http404 -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.urls import reverse_lazy +from django.utils.functional import cached_property from django.views.generic import DetailView, ListView from django.views.generic.edit import CreateView, DeleteView, UpdateView -from core.auth.mixins import ( - CanEditMixin, - CanEditPropMixin, - CanViewMixin, -) -from core.models import LockError, Page, PageRev -from core.views.forms import PageForm, PagePropForm -from core.views.widgets.markdown import MarkdownInput +from core.auth.mixins import CanEditPropMixin, CanViewMixin +from core.models import Page, PageRev +from core.views.forms import PageForm, PagePropForm, PageRevisionForm -class CanEditPagePropMixin(CanEditPropMixin): - def dispatch(self, request, *args, **kwargs): - res = super().dispatch(request, *args, **kwargs) - if self.object.is_club_page: - raise Http404 - return res +class PageNotFound(Http404): + """Http404 Exception, but specifically for when the not found object is a Page.""" + + def __init__(self, page_name: str): + self.page_name = page_name + + +def get_page_or_404(full_name: str) -> Page: + """Like Django's get_object_or_404, but for Page, and with a custom 404 exception.""" + page = Page.objects.filter(_full_name=full_name).first() + if not page: + raise PageNotFound(full_name) + return page class PageListView(ListView): model = Page - template_name = "core/page_list.jinja" + template_name = "core/page/list.jinja" def get_queryset(self): return ( @@ -64,80 +64,57 @@ class PageListView(ListView): ) -class PageView(CanViewMixin, DetailView): +class BasePageDetailView(CanViewMixin, DetailView): model = Page - template_name = "core/page_detail.jinja" - - def dispatch(self, request, *args, **kwargs): - res = super().dispatch(request, *args, **kwargs) - if self.object and self.object.need_club_redirection: - return redirect("club:club_view", club_id=self.object.club.id) - return res - - def get_object(self): - self.page = Page.get_page_by_full_name(self.kwargs["page_name"]) - return self.page - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - if "page" not in context: - context["new_page"] = self.kwargs["page_name"] - return context - - -class PageHistView(CanViewMixin, DetailView): - model = Page - template_name = "core/page_hist.jinja" - slug_field = "_full_name" slug_url_kwarg = "page_name" _cached_object: Page | None = None def dispatch(self, request, *args, **kwargs): page = self.get_object() if page.need_club_redirection: - return redirect("club:club_hist", club_id=page.club.id) + return redirect("club:club_view", club_id=page.club.id) return super().dispatch(request, *args, **kwargs) def get_object(self, *args, **kwargs): if not self._cached_object: - self._cached_object = super().get_object() + full_name = self.kwargs.get(self.slug_url_kwarg) + self._cached_object = get_page_or_404(full_name) return self._cached_object + def get_context_data(self, **kwargs): + return super().get_context_data(**kwargs) | { + "last_revision": self.object.revisions.last() + } -class PageRevView(CanViewMixin, DetailView): - model = Page - template_name = "core/page_detail.jinja" + +class PageView(BasePageDetailView): + template_name = "core/page/detail.jinja" + + +class PageHistView(BasePageDetailView): + template_name = "core/page/history.jinja" + + +class PageRevView(BasePageDetailView): + template_name = "core/page/detail.jinja" def dispatch(self, request, *args, **kwargs): - res = super().dispatch(request, *args, **kwargs) - self.object = self.get_object() - - if self.object is None: - return redirect("core:page_create", page_name=self.kwargs["page_name"]) - - if self.object.need_club_redirection: + page = self.get_object() + if page.need_club_redirection: return redirect( - "club:club_view_rev", club_id=self.object.club.id, rev_id=kwargs["rev"] + "club:club_view_rev", club_id=page.club.id, rev_id=kwargs["rev"] ) - return res - - def get_object(self, *args, **kwargs): - self.page = Page.get_page_by_full_name(self.kwargs["page_name"]) - return self.page + self.revision = get_object_or_404(page.revisions, id=self.kwargs["rev"]) + return super().dispatch(request, *args, **kwargs) def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - if not self.page: - return context | {"new_page": self.kwargs["page_name"]} - context["page"] = self.page - context["rev"] = self.page.revisions.filter(id=self.kwargs["rev"]).first() - return context + return super().get_context_data(**kwargs) | {"revision": self.revision} class PageCreateView(PermissionRequiredMixin, CreateView): model = Page form_class = PageForm - template_name = "core/page_prop.jinja" + template_name = "core/create.jinja" permission_required = "core.add_page" def get_initial(self): @@ -152,88 +129,67 @@ class PageCreateView(PermissionRequiredMixin, CreateView): init["name"] = page_name[-1] return init - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["new_page"] = True - return context - def form_valid(self, form): form.instance.set_lock(self.request.user) ret = super().form_valid(form) return ret +class CanEditPagePropMixin(CanEditPropMixin): + def dispatch(self, request, *args, **kwargs): + res = super().dispatch(request, *args, **kwargs) + if self.object.is_club_page: + raise Http404 + return res + + class PagePropView(CanEditPagePropMixin, UpdateView): model = Page form_class = PagePropForm - template_name = "core/page_prop.jinja" - slug_field = "_full_name" - slug_url_kwarg = "page_name" + template_name = "core/page/prop.jinja" def get_object(self, queryset=None): - self.page = super().get_object() - try: - self.page.set_lock_recursive(self.request.user) - except LockError as e: - raise e + self.page = get_page_or_404(full_name=self.kwargs["page_name"]) + self.page.set_lock_recursive(self.request.user) return self.page -class PageEditViewBase(CanEditMixin, UpdateView): +class BasePageEditView(UserPassesTestMixin, UpdateView): model = PageRev - form_class = modelform_factory( - model=PageRev, fields=["title", "content"], widgets={"content": MarkdownInput} - ) - template_name = "core/pagerev_edit.jinja" + form_class = PageRevisionForm + template_name = "core/page/edit.jinja" + + def test_func(self): + return self.request.user.can_edit(self.page) + + @cached_property + def page(self) -> Page: + page = get_page_or_404(full_name=self.kwargs["page_name"]) + page.set_lock(self.request.user) + return page def get_object(self, *args, **kwargs): - self.page = Page.get_page_by_full_name(self.kwargs["page_name"]) - return self._get_revision() - - def _get_revision(self): - if self.page is not None: - # First edit - if self.page.revisions.all() is None: - rev = PageRev(author=self.request.user) - rev.save() - self.page.revisions.add(rev) - try: - self.page.set_lock(self.request.user) - except LockError as e: - raise e - return self.page.revisions.last() - return None + return self.page.revisions.last() def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - if self.page is not None: - context["page"] = self.page - else: - context["new_page"] = self.kwargs["page_name"] - return context + return super().get_context_data(**kwargs) | {"page": self.page} - def form_valid(self, form): - # TODO : factor that, but first make some tests - rev = form.instance - new_rev = PageRev(title=rev.title, content=rev.content) - new_rev.author = self.request.user - new_rev.page = self.page - form.instance = new_rev - return super().form_valid(form) + def get_form_kwargs(self): + return super().get_form_kwargs() | { + "author": self.request.user, + "page": self.page, + } -class PageEditView(PageEditViewBase): +class PageEditView(BasePageEditView): def dispatch(self, request, *args, **kwargs): - res = super().dispatch(request, *args, **kwargs) - if self.object and self.object.page.need_club_redirection: - return redirect("club:club_edit_page", club_id=self.object.page.club.id) - return res + if self.page.need_club_redirection: + return redirect("club:club_edit_page", club_id=self.page.club.id) + return super().dispatch(request, *args, **kwargs) class PageDeleteView(CanEditPagePropMixin, DeleteView): model = Page template_name = "core/delete_confirm.jinja" pk_url_kwarg = "page_id" - - def get_success_url(self, **kwargs): - return reverse_lazy("core:page_list") + success_url = reverse_lazy("core:page_list") diff --git a/core/views/user.py b/core/views/user.py index ba7beaa9..4b9c32cb 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -103,9 +103,7 @@ def password_root_change(request, user_id): """Allows a root user to change someone's password.""" if not request.user.is_root: raise PermissionDenied - user = User.objects.filter(id=user_id).first() - if not user: - raise Http404("User not found") + user = get_object_or_404(User, id=user_id) if request.method == "POST": form = views.SetPasswordForm(user=user, data=request.POST) if form.is_valid(): diff --git a/counter/schemas.py b/counter/schemas.py index 6ee9f8b1..a6b55b76 100644 --- a/counter/schemas.py +++ b/counter/schemas.py @@ -1,3 +1,4 @@ +from datetime import datetime from typing import Annotated, Self from annotated_types import MinLen @@ -100,3 +101,10 @@ class ProductFilterSchema(FilterSchema): product_type: set[int] | None = Field(None, q="product_type__in") club: set[int] | None = Field(None, q="club__in") counter: set[int] | None = Field(None, q="counters__in") + + +class SaleFilterSchema(FilterSchema): + before: datetime | None = Field(None, q="date__lt") + after: datetime | None = Field(None, q="date__gt") + counters: set[int] | None = Field(None, q="counter__in") + products: set[int] | None = Field(None, q="product__in") diff --git a/election/templates/election/election_detail.jinja b/election/templates/election/election_detail.jinja index b93ab9b7..b450e2c0 100644 --- a/election/templates/election/election_detail.jinja +++ b/election/templates/election/election_detail.jinja @@ -141,7 +141,7 @@