diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b334a7c0..ab1fbf56 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.9.10 + rev: v0.11.4 hooks: - id: ruff # just check the code, and print the errors - id: ruff # actually fix the fixable errors, but print nothing diff --git a/antispam/forms.py b/antispam/forms.py index a2f97124..0e327f2a 100644 --- a/antispam/forms.py +++ b/antispam/forms.py @@ -1,5 +1,3 @@ -import re - from django import forms from django.core.validators import EmailValidator from django.utils.translation import gettext_lazy as _ @@ -7,12 +5,18 @@ from django.utils.translation import gettext_lazy as _ from antispam.models import ToxicDomain +class AntiSpamEmailValidator(EmailValidator): + def __call__(self, value: str): + super().__call__(value) + domain_part = value.rsplit("@", 1)[1] + if ToxicDomain.objects.filter(domain=domain_part).exists(): + raise forms.ValidationError(_("Email domain is not allowed.")) + + +validate_antispam_email = AntiSpamEmailValidator() + + class AntiSpamEmailField(forms.EmailField): """An email field that email addresses with a known toxic domain.""" - def run_validators(self, value: str): - super().run_validators(value) - # Domain part should exist since email validation is guaranteed to run first - domain = re.search(EmailValidator.domain_regex, value) - if ToxicDomain.objects.filter(domain=domain[0]).exists(): - raise forms.ValidationError(_("Email domain is not allowed.")) + default_validators = [validate_antispam_email] diff --git a/antispam/management/commands/update_spam_database.py b/antispam/management/commands/update_spam_database.py index 4e9d0ec8..2761c649 100644 --- a/antispam/management/commands/update_spam_database.py +++ b/antispam/management/commands/update_spam_database.py @@ -34,7 +34,7 @@ class Command(BaseCommand): f"Source {provider} responded with code {res.status_code}" ) continue - domains |= set(res.content.decode().splitlines()) + domains |= set(res.text.splitlines()) return domains def _update_domains(self, domains: set[str]): diff --git a/club/forms.py b/club/forms.py index ab8f5757..da0f5fb7 100644 --- a/club/forms.py +++ b/club/forms.py @@ -196,9 +196,7 @@ class ClubMemberForm(forms.Form): self.request_user = kwargs.pop("request_user") self.club_members = kwargs.pop("club_members", None) if not self.club_members: - self.club_members = ( - self.club.members.filter(end_date=None).order_by("-role").all() - ) + self.club_members = self.club.members.ongoing().order_by("-role").all() self.request_user_membership = self.club.get_membership_for(self.request_user) super().__init__(*args, **kwargs) diff --git a/club/templates/club/club_members.jinja b/club/templates/club/club_members.jinja index a64d43ce..0778b486 100644 --- a/club/templates/club/club_members.jinja +++ b/club/templates/club/club_members.jinja @@ -11,7 +11,7 @@ {{ select_all_checkbox("users_old") }}

{% endif %} - +
diff --git a/club/tests/base.py b/club/tests/base.py index 47f50264..8ae8f3f4 100644 --- a/club/tests/base.py +++ b/club/tests/base.py @@ -6,8 +6,10 @@ from django.test import TestCase from django.urls import reverse from django.utils.timezone import now from model_bakery import baker +from model_bakery.recipe import Recipe from club.models import Club, Membership +from core.baker_recipes import old_subscriber_user, subscriber_user from core.models import User @@ -17,8 +19,9 @@ class TestClub(TestCase): The generated dataset is the one created by the populate command, plus the following modifications : - - `self.club` is a dummy club recreated for each test - - `self.club` has two board members : skia (role 3) and comptable (role 10) + - `self.club` is a dummy club + - `self.club` has two board members : + simple_board_member (role 3) and president (role 10) - `self.club` has one regular member : richard - `self.club` has one former member : sli (who had role 2) - None of the `self.club` members are in the AE club. @@ -27,44 +30,30 @@ class TestClub(TestCase): @classmethod def setUpTestData(cls): # subscribed users - initial members - cls.skia = User.objects.get(username="skia") - # by default, Skia is in the AE, which creates side effect - cls.skia.memberships.all().delete() + cls.president, cls.simple_board_member = subscriber_user.make(_quantity=2) cls.richard = User.objects.get(username="rbatsbak") - cls.comptable = User.objects.get(username="comptable") cls.sli = User.objects.get(username="sli") - cls.root = User.objects.get(username="root") + cls.root = baker.make(User, is_superuser=True) + cls.old_subscriber = old_subscriber_user.make() + cls.public = baker.make(User) - # subscribed users - not initial members + # subscribed users - not initial member cls.krophil = User.objects.get(username="krophil") - cls.subscriber = User.objects.get(username="subscriber") - - # old subscriber - cls.old_subscriber = User.objects.get(username="old_subscriber") - - # not subscribed - cls.public = User.objects.get(username="public") + cls.subscriber = subscriber_user.make() cls.ae = Club.objects.get(pk=settings.SITH_MAIN_CLUB_ID) cls.club = baker.make(Club) cls.members_url = reverse("club:club_members", kwargs={"club_id": cls.club.id}) a_month_ago = now() - timedelta(days=30) yesterday = now() - timedelta(days=1) - Membership.objects.create( - club=cls.club, user=cls.skia, start_date=a_month_ago, role=3 + membership_recipe = Recipe(Membership, club=cls.club) + membership_recipe.make( + user=cls.simple_board_member, start_date=a_month_ago, role=3 ) - Membership.objects.create(club=cls.club, user=cls.richard, role=1) - Membership.objects.create( - club=cls.club, user=cls.comptable, start_date=a_month_ago, role=10 - ) - - # sli was a member but isn't anymore - Membership.objects.create( - club=cls.club, - user=cls.sli, - start_date=a_month_ago, - end_date=yesterday, - role=2, + membership_recipe.make(user=cls.richard, role=1) + membership_recipe.make(user=cls.president, start_date=a_month_ago, role=10) + membership_recipe.make( # sli was a member but isn't anymore + user=cls.sli, start_date=a_month_ago, end_date=yesterday, role=2 ) def setUp(self): diff --git a/club/tests/test_mailing.py b/club/tests/test_mailing.py index 7076ac0c..e8ea3a9b 100644 --- a/club/tests/test_mailing.py +++ b/club/tests/test_mailing.py @@ -38,7 +38,7 @@ class TestMailingForm(TestCase): self.assertRedirects(response, self.mail_url) response = self.client.get(self.mail_url) assert response.status_code == 200 - assert "Liste de diffusion foyer@utbm.fr" in response.content.decode() + assert "Liste de diffusion foyer@utbm.fr" in response.text # Test with Root self.client.force_login(self.root) @@ -48,7 +48,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - assert "Liste de diffusion mde@utbm.fr" in response.content.decode() + assert "Liste de diffusion mde@utbm.fr" in response.text def test_mailing_list_add_moderation(self): self.client.force_login(self.rbatsbak) @@ -58,7 +58,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "Liste de diffusion mde@utbm.fr" not in content assert "

Listes de diffusions en attente de modération

" in content assert "
  • mde@utbm.fr" in content @@ -90,7 +90,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - assert "skia@git.an" not in response.content.decode() + assert "skia@git.an" not in response.text def test_add_new_subscription_success(self): # Prepare mailing list @@ -111,7 +111,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - assert "skia@git.an" in response.content.decode() + assert "skia@git.an" in response.text # Add multiple users self.client.post( @@ -124,7 +124,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "richard@git.an" in content assert "comunity@git.an" in content assert "skia@git.an" in content @@ -140,7 +140,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "richard@git.an" in content assert "comunity@git.an" in content assert "skia@git.an" in content @@ -158,7 +158,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "richard@git.an" in content assert "comunity@git.an" in content assert "skia@git.an" in content @@ -185,7 +185,7 @@ class TestMailingForm(TestCase): assert response.status_code self.assertInHTML( _("You must specify at least an user or an email address"), - response.content.decode(), + response.text, ) # No mailing specified @@ -197,7 +197,7 @@ class TestMailingForm(TestCase): }, ) assert response.status_code == 200 - assert _("This field is required") in response.content.decode() + assert _("This field is required") in response.text # One of the selected users doesn't exist response = self.client.post( @@ -211,7 +211,7 @@ class TestMailingForm(TestCase): assert response.status_code == 200 self.assertInHTML( _("You must specify at least an user or an email address"), - response.content.decode(), + response.text, ) # An user has no email address @@ -229,7 +229,7 @@ class TestMailingForm(TestCase): assert response.status_code == 200 self.assertInHTML( _("One of the selected users doesn't have an email address"), - response.content.decode(), + response.text, ) self.krophil.email = "krophil@git.an" @@ -257,7 +257,7 @@ class TestMailingForm(TestCase): assert response.status_code == 200 self.assertInHTML( _("This email is already suscribed in this mailing"), - response.content.decode(), + response.text, ) def test_remove_subscription_success(self): @@ -283,7 +283,7 @@ class TestMailingForm(TestCase): response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "comunity@git.an" in content assert "richard@git.an" in content @@ -299,7 +299,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "comunity@git.an" in content assert "richard@git.an" in content @@ -320,7 +320,7 @@ class TestMailingForm(TestCase): ) response = self.client.get(self.mail_url) assert response.status_code == 200 - content = response.content.decode() + content = response.text assert "comunity@git.an" not in content assert "richard@git.an" not in content diff --git a/club/tests/test_membership.py b/club/tests/test_membership.py index dd77e52e..ff668498 100644 --- a/club/tests/test_membership.py +++ b/club/tests/test_membership.py @@ -1,9 +1,12 @@ +from bs4 import BeautifulSoup from django.conf import settings from django.core.cache import cache +from django.db.models import Max from django.urls import reverse from django.utils.timezone import localdate, localtime, now from model_bakery import baker +from club.forms import ClubMemberForm from club.models import Membership from club.tests.base import TestClub from core.baker_recipes import subscriber_user @@ -17,8 +20,8 @@ class TestMembershipQuerySet(TestClub): """ current_members = list(self.club.members.ongoing().order_by("id")) expected = [ - self.skia.memberships.get(club=self.club), - self.comptable.memberships.get(club=self.club), + self.simple_board_member.memberships.get(club=self.club), + self.president.memberships.get(club=self.club), self.richard.memberships.get(club=self.club), ] expected.sort(key=lambda i: i.id) @@ -30,8 +33,8 @@ class TestMembershipQuerySet(TestClub): self.richard.memberships.filter(club=self.club).update(end_date=today) current_members = list(self.club.members.ongoing().order_by("id")) expected = [ - self.skia.memberships.get(club=self.club), - self.comptable.memberships.get(club=self.club), + self.simple_board_member.memberships.get(club=self.club), + self.president.memberships.get(club=self.club), ] expected.sort(key=lambda i: i.id) assert current_members == expected @@ -42,8 +45,8 @@ class TestMembershipQuerySet(TestClub): """ board_members = list(self.club.members.board().order_by("id")) expected = [ - self.skia.memberships.get(club=self.club), - self.comptable.memberships.get(club=self.club), + self.simple_board_member.memberships.get(club=self.club), + self.president.memberships.get(club=self.club), # sli is no more member, but he was in the board self.sli.memberships.get(club=self.club), ] @@ -56,17 +59,17 @@ class TestMembershipQuerySet(TestClub): """ members = list(self.club.members.ongoing().board().order_by("id")) expected = [ - self.skia.memberships.get(club=self.club), - self.comptable.memberships.get(club=self.club), + self.simple_board_member.memberships.get(club=self.club), + self.president.memberships.get(club=self.club), ] expected.sort(key=lambda i: i.id) assert members == expected def test_update_invalidate_cache(self): """Test that the `update` queryset method properly invalidate cache.""" - mem_skia = self.skia.memberships.get(club=self.club) + mem_skia = self.simple_board_member.memberships.get(club=self.club) cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia) - self.skia.memberships.update(end_date=localtime(now()).date()) + self.simple_board_member.memberships.update(end_date=localtime(now()).date()) assert ( cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}") == "not_member" @@ -104,14 +107,14 @@ class TestMembershipQuerySet(TestClub): def test_delete_invalidate_cache(self): """Test that the `delete` queryset properly invalidate cache.""" - mem_skia = self.skia.memberships.get(club=self.club) - mem_comptable = self.comptable.memberships.get(club=self.club) + mem_skia = self.simple_board_member.memberships.get(club=self.club) + mem_comptable = self.president.memberships.get(club=self.club) cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia) cache.set( f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}", mem_comptable ) - # should delete the subscriptions of skia and comptable + # should delete the subscriptions of simple_board_member and president self.club.members.ongoing().board().delete() for membership in (mem_skia, mem_comptable): @@ -167,36 +170,43 @@ class TestMembership(TestClub): """Test that a GET request return a page where the requested information are displayed. """ - self.client.force_login(self.skia) + self.client.force_login(self.simple_board_member) response = self.client.get(self.members_url) assert response.status_code == 200 - expected_html = ( - "
    • {% trans %}User{% endtrans %}
    " - "" - "" - "" - ) + soup = BeautifulSoup(response.text, "lxml") + table = soup.find("table", id="club_members_table") + assert [r.text for r in table.find("thead").find_all("td")] == [ + "Utilisateur", + "Rôle", + "Description", + "Depuis", + "Marquer comme ancien", + ] + rows = table.find("tbody").find_all("tr") memberships = self.club.members.ongoing().order_by("-role") - input_id = 0 - for membership in memberships.select_related("user"): + for row, membership in zip( + rows, memberships.select_related("user"), strict=False + ): user = membership.user - expected_html += ( - f'" - f"" - f"" - f"" - expected_html += "
    UtilisateurRôleDescriptionDepuisMarquer comme ancien
    ' - f"{user.get_display_name()}{settings.SITH_CLUB_ROLES[membership.role]}{membership.description}{membership.start_date}" - ) - if membership.role <= 3: # 3 is the role of skia - expected_html += ( - '' - ) - input_id += 1 - expected_html += "
    " - self.assertInHTML(expected_html, response.content.decode()) + user_url = reverse("core:user_profile", args=[user.id]) + cols = row.find_all("td") + user_link = cols[0].find("a") + assert user_link.attrs["href"] == user_url + assert user_link.text == user.get_display_name() + assert cols[1].text == settings.SITH_CLUB_ROLES[membership.role] + assert cols[2].text == membership.description + assert cols[3].text == str(membership.start_date) + + if membership.role <= 3: # 3 is the role of simple_board_member + form_input = cols[4].find("input") + expected_attrs = { + "type": "checkbox", + "name": "users_old", + "value": str(user.id), + } + assert form_input.attrs.items() >= expected_attrs.items() + else: + assert cols[4].find_all() == [] def test_root_add_one_club_member(self): """Test that root users can add members to clubs, one at a time.""" @@ -228,64 +238,61 @@ class TestMembership(TestClub): """Test that users who are not currently subscribed cannot be members of clubs. """ - self.client.force_login(self.root) - response = self.client.post( - self.members_url, - {"users": self.public.id, "role": 1}, - ) - assert not self.public.memberships.filter(club=self.club).exists() - assert '