From 3b3e33ed80e3747227fb567f702dddfed940ac7e Mon Sep 17 00:00:00 2001 From: imperosol Date: Wed, 27 May 2026 12:24:27 +0200 Subject: [PATCH] fix: forgotten group assignation on club role update --- club/forms.py | 24 ++++++++++++++++++++++++ club/tests/test_clubrole.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/club/forms.py b/club/forms.py index 7c524f56..adbc9ba3 100644 --- a/club/forms.py +++ b/club/forms.py @@ -392,6 +392,30 @@ class ClubRoleForm(forms.ModelForm): self.instance.order = cleaned_data["ORDER"] - 1 return cleaned_data + def save(self, commit=True): # noqa: FBT002 + instance: ClubRole = super().save(commit=commit) + if commit and "is_board" in self.changed_data: + # if the role was moved from board to simple member, + # remove all users with that role from the club board group. + # If the role became a board role, add users with + # that role to the club board group. + group_id = instance.club.board_group_id + if self.cleaned_data["is_board"]: + User.groups.through.objects.bulk_create( + [ + User.groups.through(user_id=u, group_id=group_id) + for u in Membership.objects.ongoing() + .filter(role=instance) + .values_list("user_id", flat=True) + ], + ignore_conflicts=True, + ) + else: + User.groups.through.objects.filter( + user__memberships__role=instance, group_id=group_id + ).delete() + return instance + class ClubRoleCreateForm(forms.ModelForm): """Form to create a club role. diff --git a/club/tests/test_clubrole.py b/club/tests/test_clubrole.py index 0173c4d6..f6d61e5b 100644 --- a/club/tests/test_clubrole.py +++ b/club/tests/test_clubrole.py @@ -4,6 +4,7 @@ import pytest from django.contrib.auth.models import Permission from django.test import Client, TestCase from django.urls import reverse +from django.utils.timezone import now from model_bakery import baker, seq from model_bakery.recipe import Recipe from pytest_django.asserts import assertRedirects @@ -239,7 +240,7 @@ class TestClubRoleUpdate(TestCase): def test_president_moves_itself_out_of_the_presidency(self): """Test that if the user moves its own role out of the presidency, - then it's redirected to another page and loses access to the update page.""" + then it loses access to the update page.""" self.payload["roles-0-is_presidency"] = False self.client.force_login(self.user) res = self.client.post(self.url, data=self.payload) @@ -251,3 +252,29 @@ class TestClubRoleUpdate(TestCase): res = self.client.get(self.url) assert res.status_code == 403 + + def test_role_stops_being_board(self): + """Test that if a role stops being a board role, + its users lose the club board group.""" + self.payload["roles-0-is_board"] = False + self.payload["roles-0-is_presidency"] = False + self.payload["roles-1-is_board"] = False + formset = ClubRoleFormSet(data=self.payload, instance=self.club) + assert formset.is_valid() + formset.save() + assert not self.user.groups.contains(self.club.board_group) + + def test_role_becomes_board(self): + """Test that if a role becomes a board role, + its active users get the club board group""" + members = [ + baker.make(Membership, club=self.club, role=self.roles[0], end_date=None), + baker.make(Membership, club=self.club, role=self.roles[0], end_date=now()), + ] + self.payload["roles-2-is_board"] = True + formset = ClubRoleFormSet(data=self.payload, instance=self.club) + assert formset.is_valid() + formset.save() + # the second membership is finished, so its user shouldn't get the role + assert members[0].user.groups.contains(self.club.board_group) + assert not members[1].user.groups.contains(self.club.board_group)