Fix state role not always updated due to lazy properties

allianceauth/authentication/models.py

@@ -73,11 +73,17 @@ class UserProfile(models.Model):
             if commit:
                 logger.info('Updating {} state to {}'.format(self.user, self.state))
                 self.save(update_fields=['state'])
-                notify(self.user, _('State Changed'),
-                       _('Your user state has been changed to %(state)s') % ({'state': state}),
-                       'info')
+                notify(
+                    self.user, 
+                    _('State Changed'),
+                    _('Your user state has been changed to %(state)s') 
+                    % ({'state': state}),
+                    'info'
+                )
                 from allianceauth.authentication.signals import state_changed
-                state_changed.send(sender=self.__class__, user=self.user, state=self.state)
+                state_changed.send(
+                    sender=self.__class__, user=self.user, state=self.state
+                )
 
     def __str__(self):
         return str(self.user)

allianceauth/authentication/signals.py

@@ -23,9 +23,7 @@ def trigger_state_check(state):
     check_states = State.objects.filter(priority__lt=state.priority)
     for profile in UserProfile.objects.filter(state__in=check_states):
         if state.available_to_user(profile.user):
-            profile.state = state
-            profile.save(update_fields=['state'])
-            state_changed.send(sender=state.__class__, user=profile.user, state=state)
+            profile.assign_state(state)
 
 
 @receiver(m2m_changed, sender=State.member_characters.through)

allianceauth/services/modules/discord/auth_hooks.py

@@ -70,6 +70,8 @@ class DiscordService(ServicesHook):
             tasks.update_nickname.apply_async(
                 kwargs={
                     'user_pk': user.pk,
+                    # since the new nickname is not yet in the DB we need to 
+                    # provide it manually to the task
                     'nickname': DiscordUser.objects.user_formatted_nick(user)
                 }, 
                 priority=SINGLE_TASK_PRIORITY
@@ -90,10 +92,16 @@ class DiscordService(ServicesHook):
         tasks.update_all_groups.delay()
 
     def update_groups(self, user):        
-        logger.debug('Processing %s groups for %s', self.name, user)
-        if self.user_has_account(user):            
+        logger.debug('Processing %s groups for %s', self.name, user)        
+        if self.user_has_account(user):
             tasks.update_groups.apply_async(
-                kwargs={'user_pk': user.pk}, priority=SINGLE_TASK_PRIORITY
+                kwargs={
+                    'user_pk': user.pk,
+                    # since state changes may not yet be in the DB we need to 
+                    # provide the new state name manually to the task
+                    'state_name': user.profile.state.name
+                }, 
+                priority=SINGLE_TASK_PRIORITY
             )
 
     def update_groups_bulk(self, users: list):

allianceauth/services/modules/discord/managers.py

@@ -127,9 +127,17 @@ class DiscordUserManager(models.Manager):
             return None
 
     @staticmethod
-    def user_group_names(user: User) -> list:
+    def user_group_names(user: User, state_name: str = None) -> list:
         """returns list of group names plus state the given user is a member of"""
-        return [group.name for group in user.groups.all()] + [user.profile.state.name]
+        if not state_name:
+            state_name = user.profile.state.name
+        group_names = (
+            [group.name for group in user.groups.all()] + [state_name]
+        )
+        logger.debug(
+            "Group names for roles updates of user %s are: %s", user, group_names
+        )
+        return group_names
     
     def user_has_account(self, user: User) -> bool:
         """Returns True if the user has an Discord account, else False

allianceauth/services/modules/discord/models.py

@@ -96,10 +96,13 @@ class DiscordUser(models.Model):
         else:
             return False
 
-    def update_groups(self) -> bool:
+    def update_groups(self, state_name: str = None) -> bool:
         """update groups for a user based on his current group memberships. 
         Will add or remove roles of a user as needed.
         
+        Params:
+        - state_name: optional state name to be used
+
         Returns:
         - True on success
         - None if user is no longer a member of the Discord server
@@ -132,7 +135,9 @@ class DiscordUser(models.Model):
         requested_roles = match_or_create_roles_from_names(
             client=client, 
             guild_id=DISCORD_GUILD_ID, 
-            role_names=DiscordUser.objects.user_group_names(self.user)
+            role_names=DiscordUser.objects.user_group_names(
+                user=self.user, state_name=state_name
+            )
         )
         logger.debug(
             'Requested roles for user %s: %s', self.user, requested_roles.ids()
@@ -148,13 +153,13 @@ class DiscordUser(models.Model):
                 role_ids=list(new_roles.ids())
             )
             if success:
-                logger.info('Groups for %s have been updated', self.user)
+                logger.info('Roles for %s have been updated', self.user)
             else:
-                logger.warning('Failed to update groups for %s', self.user)
+                logger.warning('Failed to update roles for %s', self.user)
             return success
 
         else:
-            logger.info('No need to update groups for user %s', self.user)
+            logger.info('No need to update roles for user %s', self.user)
             return True
 
     def update_username(self) -> bool:

allianceauth/services/modules/discord/tasks.py

@@ -6,6 +6,7 @@ from requests.exceptions import HTTPError
 from django.contrib.auth.models import User
 from django.db.models.query import QuerySet
 
+from allianceauth.authentication.models import UserProfile
 from allianceauth.services.tasks import QueueOnce
 
 from . import __title__
@@ -26,13 +27,14 @@ BULK_TASK_PRIORITY = 6
 @shared_task(
     bind=True, name='discord.update_groups', base=QueueOnce, max_retries=None
 )
-def update_groups(self, user_pk: int) -> None:
+def update_groups(self, user_pk: int, state_name: str = None) -> None:
     """Update roles on Discord for given user according to his current groups
     
     Params:
     - user_pk: PK of given user
-    """
-    _task_perform_user_action(self, user_pk, 'update_groups')
+    - state_name: optional state name to be used
+    """    
+    _task_perform_user_action(self, user_pk, 'update_groups', state_name=state_name)
 
 
 @shared_task(
@@ -76,6 +78,7 @@ def _task_perform_user_action(self, user_pk: int, method: str, **kwargs) -> None
     """perform a user related action incl. managing all exceptions"""
     logger.debug("Starting %s for user with pk %s", method, user_pk)
     user = User.objects.get(pk=user_pk)    
+    # logger.debug("user %s has state %s", user, user.profile.state)
     if DiscordUser.objects.user_has_account(user):
         logger.info("Running %s for user %s", method, user)
         try:

allianceauth/services/modules/discord/tests/test_integration.py

@@ -238,15 +238,21 @@ class TestServiceFeatures(TransactionTestCase):
         # request mocks
         requests_mocker.get(
             guild_member_request.url,
-            json={'user': create_user_info(),'roles': ['13', '99']}
+            json={'user': create_user_info(),'roles': ['3', '13', '99']}
         )     
         requests_mocker.get(
             guild_roles_request.url,
-            json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER, ROLE_BLUE]
+            json=[
+                ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_MEMBER, ROLE_BLUE
+            ]
         )                
         requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE) 
         requests_mocker.patch(modify_guild_member_request.url, status_code=204)
         
+        AuthUtils.disconnect_signals()
+        self.user.groups.add(self.group_charlie)
+        AuthUtils.connect_signals()
+
         # demote user to blue state
         self.blue_state.member_characters.add(self.main)
         self.member_state.member_characters.remove(self.main)
@@ -257,7 +263,7 @@ class TestServiceFeatures(TransactionTestCase):
             my_request = DiscordRequest(r.method, r.url)                        
             if my_request == modify_guild_member_request and "roles" in r.json():
                 roles_updated = True
-                self.assertSetEqual(set(r.json()["roles"]), {13, 98})
+                self.assertSetEqual(set(r.json()["roles"]), {3, 13, 98})
                 break
         
         self.assertTrue(roles_updated)