diff --git a/.github/actions/setup_project/action.yml b/.github/actions/setup_project/action.yml index 599fa11e..2e590471 100644 --- a/.github/actions/setup_project/action.yml +++ b/.github/actions/setup_project/action.yml @@ -24,7 +24,7 @@ runs: - name: Install Poetry if: steps.cached-poetry.outputs.cache-hit != 'true' shell: bash - run: curl -sSL https://install.python-poetry.org | python3 - + run: curl -sSL https://install.python-poetry.org | python3 - --version 1.8.5 - name: Check pyproject.toml syntax shell: bash diff --git a/club/admin.py b/club/admin.py index c2444c17..04265245 100644 --- a/club/admin.py +++ b/club/admin.py @@ -20,6 +20,14 @@ from club.models import Club, Membership @admin.register(Club) class ClubAdmin(admin.ModelAdmin): list_display = ("name", "unix_name", "parent", "is_active") + search_fields = ("name", "unix_name") + autocomplete_fields = ( + "parent", + "board_group", + "members_group", + "home", + "page", + ) @admin.register(Membership) diff --git a/club/migrations/0012_club_board_group_club_members_group.py b/club/migrations/0012_club_board_group_club_members_group.py new file mode 100644 index 00000000..f3d3a1e9 --- /dev/null +++ b/club/migrations/0012_club_board_group_club_members_group.py @@ -0,0 +1,106 @@ +# Generated by Django 4.2.16 on 2024-11-20 17:08 + +import django.db.models.deletion +import django.db.models.functions.datetime +from django.conf import settings +from django.db import migrations, models +from django.db.migrations.state import StateApps +from django.db.models import Q +from django.utils.timezone import localdate + + +def migrate_meta_groups(apps: StateApps, schema_editor): + """Attach the existing meta groups to the clubs. + + Until now, the meta groups were not attached to the clubs, + nor to the users. + This creates actual foreign relationships between the clubs + and theirs groups and the users and theirs groups. + + Warnings: + When the meta groups associated with the clubs aren't found, + they are created. + Thus the migration shouldn't fail, and all the clubs will + have their groups. + However, there will probably be some groups that have + not been found but exist nonetheless, + so there will be duplicates and dangling groups. + There must be a manual cleanup after this migration. + """ + Group = apps.get_model("core", "Group") + Club = apps.get_model("club", "Club") + + meta_groups = Group.objects.filter(is_meta=True) + clubs = list(Club.objects.all()) + for club in clubs: + club.board_group = meta_groups.get_or_create( + name=club.unix_name + settings.SITH_BOARD_SUFFIX, + defaults={"is_meta": True}, + )[0] + club.members_group = meta_groups.get_or_create( + name=club.unix_name + settings.SITH_MEMBER_SUFFIX, + defaults={"is_meta": True}, + )[0] + club.save() + club.refresh_from_db() + memberships = club.members.filter( + Q(end_date=None) | Q(end_date__gt=localdate()) + ).select_related("user") + club.members_group.users.set([m.user for m in memberships]) + club.board_group.users.set( + [ + m.user + for m in memberships.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE) + ] + ) + + +# steps of the migration : +# - Create a nullable field for the board group and the member group +# - Edit those new fields to make them point to currently existing meta groups +# - When this data migration is done, make the fields non-nullable +class Migration(migrations.Migration): + dependencies = [ + ("core", "0040_alter_user_options_user_user_permissions_and_more"), + ("club", "0011_auto_20180426_2013"), + ] + + operations = [ + migrations.RemoveField( + model_name="club", + name="edit_groups", + ), + migrations.RemoveField( + model_name="club", + name="owner_group", + ), + migrations.RemoveField( + model_name="club", + name="view_groups", + ), + migrations.AddField( + model_name="club", + name="board_group", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="club_board", + to="core.group", + ), + ), + migrations.AddField( + model_name="club", + name="members_group", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="club", + to="core.group", + ), + ), + migrations.RunPython( + migrate_meta_groups, reverse_code=migrations.RunPython.noop, elidable=True + ), + ] diff --git a/club/migrations/0013_alter_club_board_group_alter_club_members_group_and_more.py b/club/migrations/0013_alter_club_board_group_alter_club_members_group_and_more.py new file mode 100644 index 00000000..21480bda --- /dev/null +++ b/club/migrations/0013_alter_club_board_group_alter_club_members_group_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.17 on 2025-01-04 16:46 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("club", "0012_club_board_group_club_members_group")] + + operations = [ + migrations.AlterField( + model_name="club", + name="board_group", + field=models.OneToOneField( + on_delete=django.db.models.deletion.PROTECT, + related_name="club_board", + to="core.group", + ), + ), + migrations.AlterField( + model_name="club", + name="members_group", + field=models.OneToOneField( + on_delete=django.db.models.deletion.PROTECT, + related_name="club", + to="core.group", + ), + ), + migrations.AddConstraint( + model_name="membership", + constraint=models.CheckConstraint( + check=models.Q(("end_date__gte", models.F("start_date"))), + name="end_after_start", + ), + ), + ] diff --git a/club/models.py b/club/models.py index 5300057d..4184715a 100644 --- a/club/models.py +++ b/club/models.py @@ -23,7 +23,7 @@ # from __future__ import annotations -from typing import Self +from typing import Iterable, Self from django.conf import settings from django.core import validators @@ -31,14 +31,14 @@ from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator, validate_email from django.db import models, transaction -from django.db.models import Exists, OuterRef, Q +from django.db.models import Exists, F, OuterRef, Q from django.urls import reverse from django.utils import timezone from django.utils.functional import cached_property from django.utils.timezone import localdate from django.utils.translation import gettext_lazy as _ -from core.models import Group, MetaGroup, Notification, Page, SithFile, User +from core.models import Group, Notification, Page, SithFile, User # Create your models here. @@ -79,19 +79,6 @@ class Club(models.Model): _("short description"), max_length=1000, default="", blank=True, null=True ) address = models.CharField(_("address"), max_length=254) - - owner_group = models.ForeignKey( - Group, - related_name="owned_club", - default=get_default_owner_group, - on_delete=models.CASCADE, - ) - edit_groups = models.ManyToManyField( - Group, related_name="editable_club", blank=True - ) - view_groups = models.ManyToManyField( - Group, related_name="viewable_club", blank=True - ) home = models.OneToOneField( SithFile, related_name="home_of_club", @@ -103,6 +90,12 @@ class Club(models.Model): page = models.OneToOneField( Page, related_name="club", blank=True, null=True, on_delete=models.CASCADE ) + members_group = models.OneToOneField( + Group, related_name="club", on_delete=models.PROTECT + ) + board_group = models.OneToOneField( + Group, related_name="club_board", on_delete=models.PROTECT + ) class Meta: ordering = ["name", "unix_name"] @@ -112,23 +105,27 @@ class Club(models.Model): @transaction.atomic() def save(self, *args, **kwargs): - old = Club.objects.filter(id=self.id).first() - creation = old is None - if not creation and old.unix_name != self.unix_name: - self._change_unixname(self.unix_name) + creation = self._state.adding + if not creation: + db_club = Club.objects.get(id=self.id) + if self.unix_name != db_club.unix_name: + self.home.name = self.unix_name + self.home.save() + if self.name != db_club.name: + self.board_group.name = f"{self.name} - Bureau" + self.board_group.save() + self.members_group.name = f"{self.name} - Membres" + self.members_group.save() + if creation: + self.board_group = Group.objects.create( + name=f"{self.name} - Bureau", is_manually_manageable=False + ) + self.members_group = Group.objects.create( + name=f"{self.name} - Membres", is_manually_manageable=False + ) super().save(*args, **kwargs) if creation: - board = MetaGroup(name=self.unix_name + settings.SITH_BOARD_SUFFIX) - board.save() - member = MetaGroup(name=self.unix_name + settings.SITH_MEMBER_SUFFIX) - member.save() - subscribers = Group.objects.filter( - name=settings.SITH_MAIN_MEMBERS_GROUP - ).first() self.make_home() - self.home.edit_groups.set([board]) - self.home.view_groups.set([member, subscribers]) - self.home.save() self.make_page() cache.set(f"sith_club_{self.unix_name}", self) @@ -136,7 +133,8 @@ class Club(models.Model): return reverse("club:club_view", kwargs={"club_id": self.id}) @cached_property - def president(self): + def president(self) -> Membership | None: + """Fetch the membership of the current president of this club.""" return self.members.filter( role=settings.SITH_CLUB_ROLES_ID["President"], end_date=None ).first() @@ -154,36 +152,18 @@ class Club(models.Model): def clean(self): self.check_loop() - def _change_unixname(self, old_name, new_name): - c = Club.objects.filter(unix_name=new_name).first() - if c is None: - # Update all the groups names - Group.objects.filter(name=old_name).update(name=new_name) - Group.objects.filter(name=old_name + settings.SITH_BOARD_SUFFIX).update( - name=new_name + settings.SITH_BOARD_SUFFIX - ) - Group.objects.filter(name=old_name + settings.SITH_MEMBER_SUFFIX).update( - name=new_name + settings.SITH_MEMBER_SUFFIX - ) + def make_home(self) -> None: + if self.home: + return + home_root = SithFile.objects.filter(parent=None, name="clubs").first() + root = User.objects.filter(username="root").first() + if home_root and root: + home = SithFile(parent=home_root, name=self.unix_name, owner=root) + home.save() + self.home = home + self.save() - if self.home: - self.home.name = new_name - self.home.save() - - else: - raise ValidationError(_("A club with that unix_name already exists")) - - def make_home(self): - if not self.home: - home_root = SithFile.objects.filter(parent=None, name="clubs").first() - root = User.objects.filter(username="root").first() - if home_root and root: - home = SithFile(parent=home_root, name=self.unix_name, owner=root) - home.save() - self.home = home - self.save() - - def make_page(self): + def make_page(self) -> None: root = User.objects.filter(username="root").first() if not self.page: club_root = Page.objects.filter(name=settings.SITH_CLUB_ROOT_PAGE).first() @@ -213,35 +193,34 @@ class Club(models.Model): self.page.parent = self.parent.page self.page.save(force_lock=True) - def delete(self, *args, **kwargs): + def delete(self, *args, **kwargs) -> tuple[int, dict[str, int]]: # Invalidate the cache of this club and of its memberships for membership in self.members.ongoing().select_related("user"): cache.delete(f"membership_{self.id}_{membership.user.id}") cache.delete(f"sith_club_{self.unix_name}") - super().delete(*args, **kwargs) + self.board_group.delete() + self.members_group.delete() + return super().delete(*args, **kwargs) - def get_display_name(self): + def get_display_name(self) -> str: return self.name - def is_owned_by(self, user): + def is_owned_by(self, user: User) -> bool: """Method to see if that object can be super edited by the given user.""" if user.is_anonymous: return False - return user.is_board_member + return user.is_root or user.is_board_member - def get_full_logo_url(self): - return "https://%s%s" % (settings.SITH_URL, self.logo.url) + def get_full_logo_url(self) -> str: + return f"https://{settings.SITH_URL}{self.logo.url}" - def can_be_edited_by(self, user): + def can_be_edited_by(self, user: User) -> bool: """Method to see if that object can be edited by the given user.""" return self.has_rights_in_club(user) - def can_be_viewed_by(self, user): + def can_be_viewed_by(self, user: User) -> bool: """Method to see if that object can be seen by the given user.""" - sub = User.objects.filter(pk=user.pk).first() - if sub is None: - return False - return sub.was_subscribed + return user.was_subscribed def get_membership_for(self, user: User) -> Membership | None: """Return the current membership the given user. @@ -262,9 +241,8 @@ class Club(models.Model): cache.set(f"membership_{self.id}_{user.id}", membership) return membership - def has_rights_in_club(self, user): - m = self.get_membership_for(user) - return m is not None and m.role > settings.SITH_MAXIMUM_FREE_ROLE + def has_rights_in_club(self, user: User) -> bool: + return user.is_in_group(pk=self.board_group_id) class MembershipQuerySet(models.QuerySet): @@ -283,42 +261,65 @@ class MembershipQuerySet(models.QuerySet): """ return self.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE) - def update(self, **kwargs): - """Refresh the cache for the elements of the queryset. + def update(self, **kwargs) -> int: + """Refresh the cache and edit group ownership. - Besides that, does the same job as a regular update method. + Update the cache, when necessary, remove + users from club groups they are no more in + and add them in the club groups they should be in. - Be aware that this adds a db query to retrieve the updated objects + Be aware that this adds three db queries : + one to retrieve the updated memberships, + one to perform group removal and one to perform + group attribution. """ nb_rows = super().update(**kwargs) - if nb_rows > 0: - # if at least a row was affected, refresh the cache - for membership in self.all(): - if membership.end_date is not None: - cache.set( - f"membership_{membership.club_id}_{membership.user_id}", - "not_member", - ) - else: - cache.set( - f"membership_{membership.club_id}_{membership.user_id}", - membership, - ) + if nb_rows == 0: + # if no row was affected, no need to refresh the cache + return 0 - def delete(self): + cache_memberships = {} + memberships = set(self.select_related("club")) + # delete all User-Group relations and recreate the necessary ones + # It's more concise to write and more reliable + Membership._remove_club_groups(memberships) + Membership._add_club_groups(memberships) + for member in memberships: + cache_key = f"membership_{member.club_id}_{member.user_id}" + if member.end_date is None: + cache_memberships[cache_key] = member + else: + cache_memberships[cache_key] = "not_member" + cache.set_many(cache_memberships) + return nb_rows + + def delete(self) -> tuple[int, dict[str, int]]: """Work just like the default Django's delete() method, but add a cache invalidation for the elements of the queryset - before the deletion. + before the deletion, + and a removal of the user from the club groups. - Be aware that this adds a db query to retrieve the deleted element. - As this first query take place before the deletion operation, - it will be performed even if the deletion fails. + Be aware that this adds some db queries : + + - 1 to retrieve the deleted elements in order to perform + post-delete operations. + As we can't know if a delete will affect rows or not, + this query will always happen + - 1 query to remove the users from the club groups. + If the delete operation affected no row, + this query won't happen. """ - ids = list(self.values_list("club_id", "user_id")) - nb_rows, _ = super().delete() + memberships = set(self.all()) + nb_rows, rows_counts = super().delete() if nb_rows > 0: - for club_id, user_id in ids: - cache.set(f"membership_{club_id}_{user_id}", "not_member") + Membership._remove_club_groups(memberships) + cache.set_many( + { + f"membership_{m.club_id}_{m.user_id}": "not_member" + for m in memberships + } + ) + return nb_rows, rows_counts class Membership(models.Model): @@ -361,6 +362,13 @@ class Membership(models.Model): objects = MembershipQuerySet.as_manager() + class Meta: + constraints = [ + models.CheckConstraint( + check=Q(end_date__gte=F("start_date")), name="end_after_start" + ), + ] + def __str__(self): return ( f"{self.club.name} - {self.user.username} " @@ -370,7 +378,14 @@ class Membership(models.Model): def save(self, *args, **kwargs): super().save(*args, **kwargs) + # a save may either be an update or a creation + # and may result in either an ongoing or an ended membership. + # It could also be a retrogradation from the board to being a simple member. + # To avoid problems, the user is removed from the club groups beforehand ; + # he will be added back if necessary + self._remove_club_groups([self]) if self.end_date is None: + self._add_club_groups([self]) cache.set(f"membership_{self.club_id}_{self.user_id}", self) else: cache.set(f"membership_{self.club_id}_{self.user_id}", "not_member") @@ -378,11 +393,11 @@ class Membership(models.Model): def get_absolute_url(self): return reverse("club:club_members", kwargs={"club_id": self.club_id}) - def is_owned_by(self, user): + def is_owned_by(self, user: User) -> bool: """Method to see if that object can be super edited by the given user.""" if user.is_anonymous: return False - return user.is_board_member + return user.is_root or user.is_board_member def can_be_edited_by(self, user: User) -> bool: """Check if that object can be edited by the given user.""" @@ -392,9 +407,91 @@ class Membership(models.Model): return membership is not None and membership.role >= self.role def delete(self, *args, **kwargs): + self._remove_club_groups([self]) super().delete(*args, **kwargs) cache.delete(f"membership_{self.club_id}_{self.user_id}") + @staticmethod + def _remove_club_groups( + memberships: Iterable[Membership], + ) -> tuple[int, dict[str, int]]: + """Remove users of those memberships from the club groups. + + For example, if a user is in the Troll club board, + he is in the board group and the members group of the Troll. + After calling this function, he will be in neither. + + Returns: + The result of the deletion queryset. + + Warnings: + If this function isn't used in combination + with an actual deletion of the memberships, + it will result in an inconsistent state, + where users will be in the clubs, without + having the associated rights. + """ + clubs = {m.club_id for m in memberships} + users = {m.user_id for m in memberships} + groups = Group.objects.filter(Q(club__in=clubs) | Q(club_board__in=clubs)) + return User.groups.through.objects.filter( + Q(group__in=groups) & Q(user__in=users) + ).delete() + + @staticmethod + def _add_club_groups( + memberships: Iterable[Membership], + ) -> list[User.groups.through]: + """Add users of those memberships to the club groups. + + For example, if a user just joined the Troll club board, + he will be added in both the members group and the board group + of the club. + + Returns: + The created User-Group relations. + + Warnings: + If this function isn't used in combination + with an actual update/creation of the memberships, + it will result in an inconsistent state, + where users will have the rights associated to the + club, without actually being part of it. + """ + # only active membership (i.e. `end_date=None`) + # grant the attribution of club groups. + memberships = [m for m in memberships if m.end_date is None] + if not memberships: + return [] + + if sum(1 for m in memberships if not hasattr(m, "club")) > 1: + # if more than one membership hasn't its `club` attribute set + # it's less expensive to reload the whole query with + # a select_related than perform a distinct query + # to fetch each club. + ids = {m.id for m in memberships} + memberships = list( + Membership.objects.filter(id__in=ids).select_related("club") + ) + club_groups = [] + for membership in memberships: + club_groups.append( + User.groups.through( + user_id=membership.user_id, + group_id=membership.club.members_group_id, + ) + ) + if membership.role > settings.SITH_MAXIMUM_FREE_ROLE: + club_groups.append( + User.groups.through( + user_id=membership.user_id, + group_id=membership.club.board_group_id, + ) + ) + return User.groups.through.objects.bulk_create( + club_groups, ignore_conflicts=True + ) + class Mailing(models.Model): """A Mailing list for a club. diff --git a/club/tests.py b/club/tests.py index ae055bd0..b81aa38d 100644 --- a/club/tests.py +++ b/club/tests.py @@ -21,6 +21,7 @@ from django.urls import reverse from django.utils import timezone from django.utils.timezone import localdate, localtime, now from django.utils.translation import gettext as _ +from model_bakery import baker from club.forms import MailingForm from club.models import Club, Mailing, Membership @@ -164,6 +165,27 @@ class TestMembershipQuerySet(TestClub): assert new_mem != "not_member" assert new_mem.role == 5 + def test_update_change_club_groups(self): + """Test that `update` set the user groups accordingly.""" + user = baker.make(User) + membership = baker.make(Membership, end_date=None, user=user, role=5) + members_group = membership.club.members_group + board_group = membership.club.board_group + assert user.groups.contains(members_group) + assert user.groups.contains(board_group) + + user.memberships.update(role=1) # from board to simple member + assert user.groups.contains(members_group) + assert not user.groups.contains(board_group) + + user.memberships.update(role=5) # from member to board + assert user.groups.contains(members_group) + assert user.groups.contains(board_group) + + user.memberships.update(end_date=localdate()) # end the membership + assert not user.groups.contains(members_group) + assert not user.groups.contains(board_group) + def test_delete_invalidate_cache(self): """Test that the `delete` queryset properly invalidate cache.""" mem_skia = self.skia.memberships.get(club=self.club) @@ -182,6 +204,19 @@ class TestMembershipQuerySet(TestClub): ) assert cached_mem == "not_member" + def test_delete_remove_from_groups(self): + """Test that `delete` removes from club groups""" + user = baker.make(User) + memberships = baker.make(Membership, role=iter([1, 5]), user=user, _quantity=2) + club_groups = { + memberships[0].club.members_group, + memberships[1].club.members_group, + memberships[1].club.board_group, + } + assert set(user.groups.all()) == club_groups + user.memberships.all().delete() + assert user.groups.all().count() == 0 + class TestClubModel(TestClub): def assert_membership_started_today(self, user: User, role: int): @@ -192,10 +227,8 @@ class TestClubModel(TestClub): assert membership.end_date is None assert membership.role == role assert membership.club.get_membership_for(user) == membership - member_group = self.club.unix_name + settings.SITH_MEMBER_SUFFIX - board_group = self.club.unix_name + settings.SITH_BOARD_SUFFIX - assert user.is_in_group(name=member_group) - assert user.is_in_group(name=board_group) + assert user.is_in_group(pk=self.club.members_group_id) + assert user.is_in_group(pk=self.club.board_group_id) def assert_membership_ended_today(self, user: User): """Assert that the given user have a membership which ended today.""" @@ -474,37 +507,35 @@ class TestClubModel(TestClub): assert self.club.members.count() == nb_memberships assert membership == new_mem - def test_delete_remove_from_meta_group(self): - """Test that when a club is deleted, all its members are removed from the - associated metagroup. - """ - memberships = self.club.members.select_related("user") - users = [membership.user for membership in memberships] - meta_group = self.club.unix_name + settings.SITH_MEMBER_SUFFIX + def test_remove_from_club_group(self): + """Test that when a membership ends, the user is removed from club groups.""" + user = baker.make(User) + baker.make(Membership, user=user, club=self.club, end_date=None, role=3) + assert user.groups.contains(self.club.members_group) + assert user.groups.contains(self.club.board_group) + user.memberships.update(end_date=localdate()) + assert not user.groups.contains(self.club.members_group) + assert not user.groups.contains(self.club.board_group) - self.club.delete() - for user in users: - assert not user.is_in_group(name=meta_group) + def test_add_to_club_group(self): + """Test that when a membership begins, the user is added to the club group.""" + assert not self.subscriber.groups.contains(self.club.members_group) + assert not self.subscriber.groups.contains(self.club.board_group) + baker.make(Membership, club=self.club, user=self.subscriber, role=3) + assert self.subscriber.groups.contains(self.club.members_group) + assert self.subscriber.groups.contains(self.club.board_group) - def test_add_to_meta_group(self): - """Test that when a membership begins, the user is added to the meta group.""" - group_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX - board_members = self.club.unix_name + settings.SITH_BOARD_SUFFIX - assert not self.subscriber.is_in_group(name=group_members) - assert not self.subscriber.is_in_group(name=board_members) - Membership.objects.create(club=self.club, user=self.subscriber, role=3) - assert self.subscriber.is_in_group(name=group_members) - assert self.subscriber.is_in_group(name=board_members) - - def test_remove_from_meta_group(self): - """Test that when a membership ends, the user is removed from meta group.""" - group_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX - board_members = self.club.unix_name + settings.SITH_BOARD_SUFFIX - assert self.comptable.is_in_group(name=group_members) - assert self.comptable.is_in_group(name=board_members) - self.comptable.memberships.update(end_date=localtime(now())) - assert not self.comptable.is_in_group(name=group_members) - assert not self.comptable.is_in_group(name=board_members) + def test_change_position_in_club(self): + """Test that when moving from board to members, club group change""" + membership = baker.make( + Membership, club=self.club, user=self.subscriber, role=3 + ) + assert self.subscriber.groups.contains(self.club.members_group) + assert self.subscriber.groups.contains(self.club.board_group) + membership.role = 1 + membership.save() + assert self.subscriber.groups.contains(self.club.members_group) + assert not self.subscriber.groups.contains(self.club.board_group) def test_club_owner(self): """Test that a club is owned only by board members of the main club.""" @@ -517,6 +548,26 @@ class TestClubModel(TestClub): Membership(club=self.ae, user=self.sli, role=3).save() assert self.club.is_owned_by(self.sli) + def test_change_club_name(self): + """Test that changing the club name doesn't break things.""" + members_group = self.club.members_group + board_group = self.club.board_group + initial_members = set(members_group.users.values_list("id", flat=True)) + initial_board = set(board_group.users.values_list("id", flat=True)) + self.club.name = "something else" + self.club.save() + self.club.refresh_from_db() + + # The names should have changed, but not the ids nor the group members + assert self.club.members_group.name == "something else - Membres" + assert self.club.board_group.name == "something else - Bureau" + assert self.club.members_group.id == members_group.id + assert self.club.board_group.id == board_group.id + new_members = set(self.club.members_group.users.values_list("id", flat=True)) + new_board = set(self.club.board_group.users.values_list("id", flat=True)) + assert new_members == initial_members + assert new_board == initial_board + class TestMailingForm(TestCase): """Perform validation tests for MailingForm.""" diff --git a/club/views.py b/club/views.py index e1f3367e..d713a1cc 100644 --- a/club/views.py +++ b/club/views.py @@ -71,14 +71,13 @@ class ClubTabsMixin(TabedViewMixin): return self.object.get_display_name() def get_list_of_tabs(self): - tab_list = [] - tab_list.append( + tab_list = [ { "url": reverse("club:club_view", kwargs={"club_id": self.object.id}), "slug": "infos", "name": _("Infos"), } - ) + ] if self.request.user.can_view(self.object): tab_list.append( { diff --git a/com/api.py b/com/api.py new file mode 100644 index 00000000..e46daea9 --- /dev/null +++ b/com/api.py @@ -0,0 +1,32 @@ +from pathlib import Path + +from django.conf import settings +from django.http import Http404 +from ninja_extra import ControllerBase, api_controller, route + +from com.calendar import IcsCalendar +from core.views.files import send_raw_file + + +@api_controller("/calendar") +class CalendarController(ControllerBase): + CACHE_FOLDER: Path = settings.MEDIA_ROOT / "com" / "calendars" + + @route.get("/external.ics", url_name="calendar_external") + def calendar_external(self): + """Return the ICS file of the AE Google Calendar + + Because of Google's cors rules, we can't just do a request to google ics + from the frontend. Google is blocking CORS request in it's responses headers. + The only way to do it from the frontend is to use Google Calendar API with an API key + This is not especially desirable as your API key is going to be provided to the frontend. + + This is why we have this backend based solution. + """ + if (calendar := IcsCalendar.get_external()) is not None: + return send_raw_file(calendar) + raise Http404 + + @route.get("/internal.ics", url_name="calendar_internal") + def calendar_internal(self): + return send_raw_file(IcsCalendar.get_internal()) diff --git a/com/apps.py b/com/apps.py new file mode 100644 index 00000000..0502c588 --- /dev/null +++ b/com/apps.py @@ -0,0 +1,9 @@ +from django.apps import AppConfig + + +class ComConfig(AppConfig): + name = "com" + verbose_name = "News and communication" + + def ready(self): + import com.signals # noqa F401 diff --git a/com/calendar.py b/com/calendar.py new file mode 100644 index 00000000..9003d6de --- /dev/null +++ b/com/calendar.py @@ -0,0 +1,76 @@ +from datetime import datetime, timedelta +from pathlib import Path +from typing import final + +import urllib3 +from dateutil.relativedelta import relativedelta +from django.conf import settings +from django.urls import reverse +from django.utils import timezone +from ical.calendar import Calendar +from ical.calendar_stream import IcsCalendarStream +from ical.event import Event + +from com.models import NewsDate + + +@final +class IcsCalendar: + _CACHE_FOLDER: Path = settings.MEDIA_ROOT / "com" / "calendars" + _EXTERNAL_CALENDAR = _CACHE_FOLDER / "external.ics" + _INTERNAL_CALENDAR = _CACHE_FOLDER / "internal.ics" + + @classmethod + def get_external(cls, expiration: timedelta = timedelta(hours=1)) -> Path | None: + if ( + cls._EXTERNAL_CALENDAR.exists() + and timezone.make_aware( + datetime.fromtimestamp(cls._EXTERNAL_CALENDAR.stat().st_mtime) + ) + + expiration + > timezone.now() + ): + return cls._EXTERNAL_CALENDAR + return cls.make_external() + + @classmethod + def make_external(cls) -> Path | None: + calendar = urllib3.request( + "GET", + "https://calendar.google.com/calendar/ical/ae.utbm%40gmail.com/public/basic.ics", + ) + if calendar.status != 200: + return None + + cls._CACHE_FOLDER.mkdir(parents=True, exist_ok=True) + with open(cls._EXTERNAL_CALENDAR, "wb") as f: + _ = f.write(calendar.data) + return cls._EXTERNAL_CALENDAR + + @classmethod + def get_internal(cls) -> Path: + if not cls._INTERNAL_CALENDAR.exists(): + return cls.make_internal() + return cls._INTERNAL_CALENDAR + + @classmethod + def make_internal(cls) -> Path: + # Updated through a post_save signal on News in com.signals + calendar = Calendar() + for news_date in NewsDate.objects.filter( + news__is_moderated=True, + end_date__gte=timezone.now() - (relativedelta(months=6)), + ).prefetch_related("news"): + event = Event( + summary=news_date.news.title, + start=news_date.start_date, + end=news_date.end_date, + url=reverse("com:news_detail", kwargs={"news_id": news_date.news.id}), + ) + calendar.events.append(event) + + # Create a file so we can offload the download to the reverse proxy if available + cls._CACHE_FOLDER.mkdir(parents=True, exist_ok=True) + with open(cls._INTERNAL_CALENDAR, "wb") as f: + _ = f.write(IcsCalendarStream.calendar_to_ics(calendar).encode("utf-8")) + return cls._INTERNAL_CALENDAR diff --git a/com/models.py b/com/models.py index f3076174..633c7671 100644 --- a/com/models.py +++ b/com/models.py @@ -17,11 +17,12 @@ # details. # # You should have received a copy of the GNU General Public License along with -# this program; if not, write to the Free Sofware Foundation, Inc., 59 Temple +# this program; if not, write to the Free Software Foundation, Inc., 59 Temple # Place - Suite 330, Boston, MA 02111-1307, USA. # # + from django.conf import settings from django.core.exceptions import ValidationError from django.core.mail import EmailMultiAlternatives diff --git a/com/signals.py b/com/signals.py new file mode 100644 index 00000000..ea004ad8 --- /dev/null +++ b/com/signals.py @@ -0,0 +1,10 @@ +from django.db.models.base import post_save +from django.dispatch import receiver + +from com.calendar import IcsCalendar +from com.models import News + + +@receiver(post_save, sender=News, dispatch_uid="update_internal_ics") +def update_internal_ics(*args, **kwargs): + _ = IcsCalendar.make_internal() diff --git a/com/static/bundled/com/components/ics-calendar-index.ts b/com/static/bundled/com/components/ics-calendar-index.ts new file mode 100644 index 00000000..11a15a32 --- /dev/null +++ b/com/static/bundled/com/components/ics-calendar-index.ts @@ -0,0 +1,194 @@ +import { makeUrl } from "#core:utils/api"; +import { inheritHtmlElement, registerComponent } from "#core:utils/web-components"; +import { Calendar, type EventClickArg } from "@fullcalendar/core"; +import type { EventImpl } from "@fullcalendar/core/internal"; +import enLocale from "@fullcalendar/core/locales/en-gb"; +import frLocale from "@fullcalendar/core/locales/fr"; +import dayGridPlugin from "@fullcalendar/daygrid"; +import iCalendarPlugin from "@fullcalendar/icalendar"; +import listPlugin from "@fullcalendar/list"; +import { calendarCalendarExternal, calendarCalendarInternal } from "#openapi"; + +@registerComponent("ics-calendar") +export class IcsCalendar extends inheritHtmlElement("div") { + static observedAttributes = ["locale"]; + private calendar: Calendar; + private locale = "en"; + + attributeChangedCallback(name: string, _oldValue?: string, newValue?: string) { + if (name !== "locale") { + return; + } + + this.locale = newValue; + } + + isMobile() { + return window.innerWidth < 765; + } + + currentView() { + // Get view type based on viewport + return this.isMobile() ? "listMonth" : "dayGridMonth"; + } + + currentToolbar() { + if (this.isMobile()) { + return { + left: "prev,next", + center: "title", + right: "", + }; + } + return { + left: "prev,next today", + center: "title", + right: "dayGridMonth,dayGridWeek,dayGridDay", + }; + } + + formatDate(date: Date) { + return new Intl.DateTimeFormat(this.locale, { + dateStyle: "medium", + timeStyle: "short", + }).format(date); + } + + createEventDetailPopup(event: EventClickArg) { + // Delete previous popup + const oldPopup = document.getElementById("event-details"); + if (oldPopup !== null) { + oldPopup.remove(); + } + + const makePopupInfo = (info: HTMLElement, iconClass: string) => { + const row = document.createElement("div"); + const icon = document.createElement("i"); + + row.setAttribute("class", "event-details-row"); + + icon.setAttribute("class", `event-detail-row-icon fa-xl ${iconClass}`); + + row.appendChild(icon); + row.appendChild(info); + + return row; + }; + + const makePopupTitle = (event: EventImpl) => { + const row = document.createElement("div"); + row.innerHTML = ` +
{% trans %}Back to news{% endtrans %}
{% trans %}You need an up to date subscription to access this content{% endtrans %}
+{% trans %}You need to subscribe to access this content{% endtrans %}
{%- endif -%}{% trans %}Since{% endtrans %} : {{ user_ban.created_at|date }}
++ {% trans %}Until{% endtrans %} : + {% if user_ban.expires_at %} + {{ user_ban.expires_at|date }} {{ user_ban.expires_at|time }} + {% else %} + {% trans %}not specified{% endtrans %} + {% endif %} +
+{{ user_ban.reason }}
+{% trans %}No active ban.{% endtrans %}
+ {% endfor %} +{% endblock %} \ No newline at end of file diff --git a/rootplace/tests/__init__.py b/rootplace/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/rootplace/tests/test_ban.py b/rootplace/tests/test_ban.py new file mode 100644 index 00000000..4616630e --- /dev/null +++ b/rootplace/tests/test_ban.py @@ -0,0 +1,57 @@ +from datetime import datetime, timedelta + +import pytest +from django.contrib.auth.models import Permission +from django.test import Client +from django.urls import reverse +from django.utils.timezone import localtime +from model_bakery import baker +from pytest_django.asserts import assertRedirects + +from core.models import BanGroup, User, UserBan + + +@pytest.fixture +def operator(db) -> User: + return baker.make( + User, + user_permissions=Permission.objects.filter( + codename__in=["view_userban", "add_userban", "delete_userban"] + ), + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "expires_at", + [None, localtime().replace(second=0, microsecond=0) + timedelta(days=7)], +) +def test_ban_user(client: Client, operator: User, expires_at: datetime): + client.force_login(operator) + user = baker.make(User) + ban_group = BanGroup.objects.first() + data = { + "user": user.id, + "ban_group": ban_group.id, + "reason": "Being naughty", + } + if expires_at is not None: + data["expires_at"] = expires_at.strftime("%Y-%m-%d %H:%M") + response = client.post(reverse("rootplace:ban_create"), data) + assertRedirects(response, expected_url=reverse("rootplace:ban_list")) + bans = list(user.bans.all()) + assert len(bans) == 1 + assert bans[0].expires_at == expires_at + assert bans[0].reason == "Being naughty" + assert bans[0].ban_group == ban_group + + +@pytest.mark.django_db +def test_remove_ban(client: Client, operator: User): + client.force_login(operator) + user = baker.make(User) + ban = baker.make(UserBan, user=user) + assert user.bans.exists() + response = client.post(reverse("rootplace:ban_remove", kwargs={"ban_id": ban.id})) + assertRedirects(response, expected_url=reverse("rootplace:ban_list")) + assert not user.bans.exists() diff --git a/rootplace/tests.py b/rootplace/tests/test_merge_users.py similarity index 100% rename from rootplace/tests.py rename to rootplace/tests/test_merge_users.py diff --git a/rootplace/urls.py b/rootplace/urls.py index 81568558..6ba94c28 100644 --- a/rootplace/urls.py +++ b/rootplace/urls.py @@ -25,6 +25,9 @@ from django.urls import path from rootplace.views import ( + BanCreateView, + BanDeleteView, + BanView, DeleteAllForumUserMessagesView, MergeUsersView, OperationLogListView, @@ -38,4 +41,7 @@ urlpatterns = [ name="delete_forum_messages", ), path("logs/", OperationLogListView.as_view(), name="operation_logs"), + path("ban/", BanView.as_view(), name="ban_list"), + path("ban/new", BanCreateView.as_view(), name="ban_create"), + path("ban/