From 7042cc41f0f273d4ef27279c7ba9e3648fae46a8 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 11 Nov 2025 13:49:33 +0100 Subject: [PATCH 01/22] remove unused `SelectUser` --- core/views/forms.py | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/core/views/forms.py b/core/views/forms.py index fdfe61cf..acc13880 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -42,7 +42,6 @@ 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 @@ -86,30 +85,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 From c942ff6aecdc3c97676ab595599fdec6639c4f07 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 7 Nov 2025 18:32:56 +0100 Subject: [PATCH 02/22] don't show hidden users in picture identifications --- sas/api.py | 5 ++++- sas/models.py | 13 +++++++++++++ sas/tests/test_api.py | 23 +++++++++++++++++++++++ sas/tests/test_model.py | 25 ++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/sas/api.py b/sas/api.py index 3b87325f..62fe1e5d 100644 --- a/sas/api.py +++ b/sas/api.py @@ -136,11 +136,14 @@ class PicturesController(ControllerBase): "/{picture_id}/identified", permissions=[CanView], response=list[IdentifiedUserSchema], + url_name="picture_identifications", ) def fetch_identifications(self, picture_id: int): """Fetch the users that have been identified on the given picture.""" picture = self.get_object_or_exception(Picture, pk=picture_id) - return picture.people.select_related("user") + return picture.people.viewable_by(self.context.request.user).select_related( + "user" + ) @route.put("/{picture_id}/identified", permissions=[CanView]) def identify_users(self, picture_id: NonNegativeInt, users: set[NonNegativeInt]): diff --git a/sas/models.py b/sas/models.py index 0355c7da..0e8d5b88 100644 --- a/sas/models.py +++ b/sas/models.py @@ -265,6 +265,17 @@ def sas_notification_callback(notif: Notification): notif.param = str(count) +class PeoplePictureRelationQuerySet(models.QuerySet): + def viewable_by(self, user: User) -> Self: + if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID): + return self + if user.was_subscribed: + return self.filter( + Q(user_id=user.id) | Q(user__is_subscriber_viewable=True) + ) + return self.filter(user_id=user.id) + + class PeoplePictureRelation(models.Model): """The PeoplePictureRelation class makes the connection between User and Picture.""" @@ -281,6 +292,8 @@ class PeoplePictureRelation(models.Model): on_delete=models.CASCADE, ) + objects = PeoplePictureRelationQuerySet.as_manager() + class Meta: unique_together = ["user", "picture"] diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py index 5ac3af1f..8811a035 100644 --- a/sas/tests/test_api.py +++ b/sas/tests/test_api.py @@ -186,6 +186,29 @@ class TestPictureRelation(TestSas): assert res.status_code == 404 assert PeoplePictureRelation.objects.count() == relation_count + def test_fetch_relations_including_hidden_users(self): + """Test that normal subscribers users cannot see hidden profiles""" + picture = self.album_a.children_pictures.last() + self.user_a.is_subscriber_viewable = False + self.user_a.save() + url = reverse("api:picture_identifications", kwargs={"picture_id": picture.id}) + + # a normal subscriber user shouldn't see user_a as identified + self.client.force_login(subscriber_user.make()) + response = self.client.get(url) + data = {user["user"]["id"] for user in response.json()} + assert data == {self.user_b.id, self.user_c.id} + + # an admin should see everyone + self.client.force_login( + baker.make( + User, groups=[Group.objects.get(id=settings.SITH_GROUP_SAS_ADMIN_ID)] + ) + ) + response = self.client.get(url) + data = {user["user"]["id"] for user in response.json()} + assert data == {self.user_a.id, self.user_b.id, self.user_c.id} + class TestPictureModeration(TestSas): @classmethod diff --git a/sas/tests/test_model.py b/sas/tests/test_model.py index dd2299f1..5a5c5fe8 100644 --- a/sas/tests/test_model.py +++ b/sas/tests/test_model.py @@ -1,10 +1,11 @@ +import pytest from django.test import TestCase from model_bakery import baker from core.baker_recipes import old_subscriber_user, subscriber_user from core.models import User from sas.baker_recipes import picture_recipe -from sas.models import Picture +from sas.models import PeoplePictureRelation, Picture class TestPictureQuerySet(TestCase): @@ -44,3 +45,25 @@ class TestPictureQuerySet(TestCase): user.pictures.create(picture=self.pictures[1]) # moderated pictures = list(Picture.objects.viewable_by(user)) assert pictures == [self.pictures[1]] + + +@pytest.mark.django_db +def test_identifications_viewable_by_user(): + picture = baker.make(Picture) + identifications = baker.make( + PeoplePictureRelation, picture=picture, _quantity=10, _bulk_create=True + ) + identifications[0].user.is_subscriber_viewable = False + identifications[0].user.save() + + assert ( + list(picture.people.viewable_by(old_subscriber_user.make())) + == identifications[1:] + ) + assert ( + list(picture.people.viewable_by(baker.make(User, is_superuser=True))) + == identifications + ) + assert list(picture.people.viewable_by(identifications[1].user)) == [ + identifications[1] + ] From 04702335e20748c408cf8ef4b361cc6d28b7d44f Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 9 Nov 2025 18:25:51 +0100 Subject: [PATCH 03/22] rename `User.is_subscriber_viewable` => `User.is_viewable` --- com/views.py | 2 +- core/migrations/0048_alter_user_options.py | 33 ++++++++++++++++++ core/models.py | 23 ++++++++++--- core/static/user/user_edit.scss | 27 +++------------ core/templates/core/user_edit.jinja | 15 +++++--- core/tests/test_family.py | 2 +- core/views/forms.py | 6 ++-- .../templates/election/election_detail.jinja | 2 +- galaxy/models.py | 2 +- locale/fr/LC_MESSAGES/django.po | 34 ++++++++++++------- matmat/views.py | 4 +-- sas/models.py | 4 +-- sas/tests/test_api.py | 2 +- sas/tests/test_model.py | 2 +- 14 files changed, 101 insertions(+), 57 deletions(-) create mode 100644 core/migrations/0048_alter_user_options.py diff --git a/com/views.py b/com/views.py index 2d5045d9..87ebe5e6 100644 --- a/com/views.py +++ b/com/views.py @@ -243,7 +243,7 @@ class NewsListView(TemplateView): User.objects.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/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..55669007 100644 --- a/core/models.py +++ b/core/models.py @@ -271,13 +271,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 +562,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 """ 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/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_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/views/forms.py b/core/views/forms.py index acc13880..524ba723 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -177,7 +177,7 @@ class UserProfileForm(forms.ModelForm): "school", "promo", "forum_signature", - "is_subscriber_viewable", + "is_viewable", ] widgets = { "date_of_birth": SelectDate, @@ -186,8 +186,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 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 @@
From 32e1f09d469b92d320f0f47a6a6a26a9a99d11d6 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 16 Nov 2025 15:05:10 +0100 Subject: [PATCH 21/22] prevent csrf on `MembershipSetOldView` --- club/tests/test_membership.py | 31 +++++++++++++++++++++++++++- club/views.py | 29 +++++++------------------- core/templates/core/user_clubs.jinja | 19 ++++++++++++++--- 3 files changed, 53 insertions(+), 26 deletions(-) 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/views.py b/club/views.py index 5a3535a6..9077d0d7 100644 --- a/club/views.py +++ b/club/views.py @@ -34,7 +34,7 @@ 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 @@ -43,6 +43,7 @@ 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 ( @@ -544,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): @@ -582,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/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 %} From 3f4a41ba4207663df3578cfd085c3ae87c5c75d5 Mon Sep 17 00:00:00 2001 From: imperosol Date: Wed, 19 Nov 2025 13:51:38 +0100 Subject: [PATCH 22/22] refactor detection of the need to merge `PageRev` --- core/models.py | 32 +++++++++++++++++++++++++++++++- core/views/forms.py | 23 +++++------------------ 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/core/models.py b/core/models.py index b84139ae..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 @@ -1344,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) @@ -1385,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/views/forms.py b/core/views/forms.py index c39d36b2..58b5c598 100644 --- a/core/views/forms.py +++ b/core/views/forms.py @@ -20,9 +20,9 @@ # Place - Suite 330, Boston, MA 02111-1307, USA. # # -import difflib import re -from datetime import date, datetime, timedelta +from copy import copy +from datetime import date, datetime from io import BytesIO from captcha.fields import CaptchaField @@ -390,14 +390,11 @@ class PageRevisionForm(forms.ModelForm): - less than 20 minutes ago - by the same author - - with a diff ratio higher than 20% + - with a similarity ratio higher than 80% then the latter will be edited and the new revision won't be created. """ - TIME_THRESHOLD = timedelta(minutes=20) - DIFF_THRESHOLD = 0.2 - class Meta: model = PageRev fields = ["title", "content"] @@ -409,21 +406,11 @@ class PageRevisionForm(forms.ModelForm): super().__init__(*args, instance=instance, **kwargs) self.author = author self.page = page - self.initial_content = instance.content if instance else "" - - def diff_ratio(self, new_str: str) -> float: - return difflib.SequenceMatcher( - None, self.initial_content, new_str - ).quick_ratio() + self.initial_obj: PageRev = copy(self.instance) def save(self, commit=True): # noqa FBT002 revision: PageRev = self.instance - if ( - revision._state.adding - or revision.author != self.author - or revision.date + self.TIME_THRESHOLD < now() - or self.diff_ratio(revision.content) < (1 - self.DIFF_THRESHOLD) - ): + 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