Merge branch '628-after-changing-the-user-of-a-person-groups-are-not-synced' into 'master'

Resolve "After changing the user of a person, groups are not synced"

Closes #628

See merge request !935

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
 -------------------

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()

aleksis/core/tests/regression/test_regression.py

+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"