diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f3447c6df245f709fde7d7437ad2d874e2d2bbe3..5e32606622bebac2fd0e217c8a70768b8b2d90ed 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,6 +20,7 @@ Fixed * PDF generation failed with S3 storage due to incompatibility with boto3 * PWA theme colour defaulted to red * Form for editing group type displayed irrelevant fields +* Permission groups could get outdated if re-assigning a user account to a different person `2.7`_ - 2022-01-24 ------------------- diff --git a/aleksis/core/models.py b/aleksis/core/models.py index f46a803b687cc9cde9a893c642586eac232fd84d..d5dc7d7befe1395b222ff492f1a9f8eed3994f69 100644 --- a/aleksis/core/models.py +++ b/aleksis/core/models.py @@ -316,7 +316,7 @@ class Person(ExtensibleModel): def initials(self): return f"{self.first_name[0]}{self.last_name[0]}".upper() - user_info_tracker = FieldTracker(fields=("first_name", "last_name", "email")) + user_info_tracker = FieldTracker(fields=("first_name", "last_name", "email", "user")) @property def member_of_recursive(self) -> QuerySet: @@ -336,16 +336,29 @@ class Person(ExtensibleModel): def save(self, *args, **kwargs): # Determine all fields that were changed since last load - dirty = self.pk is None or bool(self.user_info_tracker.changed()) + changed = self.user_info_tracker.changed() super().save(*args, **kwargs) - if self.user and dirty: - # Synchronise user fields to linked User object to keep it up to date - self.user.first_name = self.first_name - self.user.last_name = self.last_name - self.user.email = self.email - self.user.save() + if self.pk is None or bool(changed): + if "user" in changed: + # Clear groups of previous Django user + previous_user = changed["user"] + if previous_user is not None: + get_user_model().objects.get(pk=previous_user).groups.clear() + + if self.user: + if "first_name" in changed or "last_name" in changed or "email" in changed: + # Synchronise user fields to linked User object to keep it up to date + self.user.first_name = self.first_name + self.user.last_name = self.last_name + self.user.email = self.email + self.user.save() + + if "user" in changed: + # Synchronise groups to Django groups + for group in self.member_of.union(self.owner_of.all()).all(): + group.save(force=True) # Select a primary group if none is set self.auto_select_primary_group() diff --git a/aleksis/core/tests/regression/test_regression.py b/aleksis/core/tests/regression/test_regression.py index 7ac541531db385ea8e28b454dc487458a45d7a66..b9d33e6981312817324b92d4fe67176b96763cda 100644 --- a/aleksis/core/tests/regression/test_regression.py +++ b/aleksis/core/tests/regression/test_regression.py @@ -1,3 +1,12 @@ +from django.contrib.auth import get_user_model + +import pytest + +from aleksis.core.models import Group, Person + +pytestmark = pytest.mark.django_db + + def test_all_settigns_registered(): """Tests for regressions of preferences not being registered. @@ -33,3 +42,43 @@ def test_custom_managers_return_correct_qs(): assert isinstance(Manager.from_queryset(QuerySet)().get_queryset(), QuerySet) _check_get_queryset(managers.GroupManager, managers.GroupQuerySet) + + +def test_reassign_user_to_person(): + """Tests that on re-assigning a user, groups are correctly synced. + + https://edugit.org/AlekSIS/official/AlekSIS-Core/-/issues/628 + """ + + User = get_user_model() + + group1 = Group.objects.create(name="Group 1") + group2 = Group.objects.create(name="Group 2") + + user1 = User.objects.create(username="user1") + user2 = User.objects.create(username="user2") + + person1 = Person.objects.create(first_name="Person", last_name="1", user=user1) + person2 = Person.objects.create(first_name="Person", last_name="2", user=user2) + + person1.member_of.set([group1]) + person2.member_of.set([group2]) + + assert user1.groups.count() == 1 + assert user2.groups.count() == 1 + assert user1.groups.first().name == "Group 1" + assert user2.groups.first().name == "Group 2" + + person1.user = None + person1.save() + assert user1.groups.count() == 0 + + person2.user = user1 + person2.save() + person1.user = user2 + person1.save() + + assert user1.groups.count() == 1 + assert user2.groups.count() == 1 + assert user1.groups.first().name == "Group 2" + assert user2.groups.first().name == "Group 1"