Merge pull request #1098 from ae-utbm/cached-groups

simplify `User.cached_groups`

core/apps.py

@@ -32,7 +32,6 @@ class SithConfig(AppConfig):
     verbose_name = "Core app of the Sith"
 
     def ready(self):
-        import core.signals  # noqa F401
         from forum.models import Forum
 
         cache.clear()

core/models.py

@@ -396,19 +396,10 @@ class User(AbstractUser):
             return self.is_root
         return group in self.cached_groups
 
-    @property
+    @cached_property
     def cached_groups(self) -> list[Group]:
-        """Get the list of groups this user is in.
-
-        The result is cached for the default duration (should be 5 minutes)
-
-        Returns: A list of all the groups this user is in.
-        """
-        groups = cache.get(f"user_{self.id}_groups")
-        if groups is None:
-            groups = list(self.groups.all())
-            cache.set(f"user_{self.id}_groups", groups)
-        return groups
+        """Get the list of groups this user is in."""
+        return list(self.groups.all())
 
     @cached_property
     def is_root(self) -> bool:

core/signals.py

@@ -1,15 +0,0 @@
-from django.core.cache import cache
-from django.db.models.signals import m2m_changed
-from django.dispatch import receiver
-
-from core.models import User
-
-
-@receiver(m2m_changed, sender=User.groups.through, dispatch_uid="user_groups_changed")
-def user_groups_changed(sender, instance: User, **kwargs):
-    """Clear the cached groups of the user."""
-    # As a m2m relationship doesn't live within the model
-    # but rather on an intermediary table, there is no
-    # model method to override, meaning we must use
-    # a signal to invalidate the cache when a user is removed from a group
-    cache.delete(f"user_{instance.pk}_groups")

core/tests/test_core.py

@@ -411,10 +411,11 @@ class TestUserIsInGroup(TestCase):
         """Test that the number of db queries is stable
         and that less queries are made when making a new call.
         """
-        # make sure Skia is in at least one group
         group_in = baker.make(Group)
         self.public_user.groups.add(group_in)
 
+        # clear the cached property `User.cached_groups`
+        self.public_user.__dict__.pop("cached_groups", None)
         cache.clear()
         # Test when the user is in the group
         with self.assertNumQueries(2):
@@ -423,6 +424,7 @@ class TestUserIsInGroup(TestCase):
             self.public_user.is_in_group(pk=group_in.id)
 
         group_not_in = baker.make(Group)
+        self.public_user.__dict__.pop("cached_groups", None)
         cache.clear()
         # Test when the user is not in the group
         with self.assertNumQueries(2):
@@ -447,24 +449,6 @@ class TestUserIsInGroup(TestCase):
         )
         assert cached_membership == "not_member"
 
-    def test_cache_properly_cleared_group(self):
-        """Test that when a user is removed from a group,
-        the is_in_group_method return False when calling it again.
-        """
-        # testing with pk
-        self.public_user.groups.add(self.com_admin.pk)
-        assert self.public_user.is_in_group(pk=self.com_admin.pk) is True
-
-        self.public_user.groups.remove(self.com_admin.pk)
-        assert self.public_user.is_in_group(pk=self.com_admin.pk) is False
-
-        # testing with name
-        self.public_user.groups.add(self.sas_admin.pk)
-        assert self.public_user.is_in_group(name="SAS admin") is True
-
-        self.public_user.groups.remove(self.sas_admin.pk)
-        assert self.public_user.is_in_group(name="SAS admin") is False
-
     def test_not_existing_group(self):
         """Test that searching for a not existing group
         returns False.