Merge branch 'discord_update_usernames' into 'master'

Add update username feature

See merge request allianceauth/allianceauth!1203

.gitignore

@@ -75,3 +75,4 @@ celerybeat-schedule
 
 #other
 .flake8
+.pylintrc

.pylintrc

@@ -1,14 +0,0 @@
-[MASTER]
-ignore-patterns=test_.*.py,__init__.py,generate_.*.py
-
-[BASIC]
-# Good variable names which should always be accepted, separated by a comma
-good-names=i,j,k,x,f,ex
-
-[FORMAT]
-# Maximum number of characters on a single line.
-max-line-length=100
-
-[MESSAGES CONTROL]
-disable=R,C
-

allianceauth/services/modules/discord/models.py

@@ -117,6 +117,36 @@ class DiscordUser(models.Model):
             logger.warning('Failed to update groups for %s', self.user)
         return success
 
+    def update_username(self) -> bool:
+        """Updates the username incl. the discriminator 
+        from the Discord server and saves it
+                
+        Returns:
+        - True on success
+        - None if user is no longer a member of the Discord server
+        - False on error or raises exception
+        """        
+    
+        client = DiscordUser.objects._bot_client()            
+        user_info = client.guild_member(guild_id=DISCORD_GUILD_ID, user_id=self.uid)
+        if user_info is None:
+            success = None
+        elif (
+            user_info 
+            and 'user' in user_info 
+            and 'username' in user_info['user'] 
+            and 'discriminator' in user_info['user']
+        ):            
+            self.username = user_info['user']['username']
+            self.discriminator = user_info['user']['discriminator']
+            self.save()
+            logger.info('Username for %s has been updated', self.user)
+            success = True
+        else:
+            logger.warning('Failed to update username for %s', self.user)
+            success = False
+        return success
+                
     def delete_user(
         self, notify_user: bool = False, is_rate_limited: bool = True
     ) -> bool:

allianceauth/services/modules/discord/tasks.py

@@ -47,6 +47,18 @@ def update_nickname(self, user_pk: int) -> None:
     _task_perform_user_action(self, user_pk, 'update_nickname')
 
 
+@shared_task(
+    bind=True, name='discord.update_username', base=QueueOnce, max_retries=None
+)
+def update_username(self, user_pk: int) -> None:
+    """Update locally stored Discord username from Discord server for given user
+    
+    Params:
+    - user_pk: PK of given user
+    """
+    _task_perform_user_action(self, user_pk, 'update_username')
+
+
 @shared_task(
     bind=True, name='discord.delete_user', base=QueueOnce, max_retries=None
 )
@@ -171,16 +183,45 @@ def _bulk_update_nicknames_for_users(discord_users_qs: QuerySet) -> None:
     chain(update_nicknames_chain).apply_async(priority=BULK_TASK_PRIORITY)
 
 
+@shared_task(name='discord.update_all_usernames')
+def update_all_usernames() -> None:
+    """Update all usernames for all known users with a Discord account."""
+    discord_users_qs = DiscordUser.objects.all()
+    _bulk_update_usernames_for_users(discord_users_qs)
+    
+
+@shared_task(name='discord.update_usernames_bulk')
+def update_usernames_bulk(user_pks: list) -> None:
+    """Update usernames for list of users with a Discord account in bulk."""    
+    discord_users_qs = DiscordUser.objects\
+        .filter(user__pk__in=user_pks)\
+        .select_related()
+    _bulk_update_usernames_for_users(discord_users_qs)
+
+
+def _bulk_update_usernames_for_users(discord_users_qs: QuerySet) -> None:
+    logger.info(
+        "Starting to bulk update discord usernames for %d users", 
+        discord_users_qs.count()
+    )
+    update_usernames_chain = list()
+    for discord_user in discord_users_qs:
+        update_usernames_chain.append(update_username.si(discord_user.user.pk))
+    
+    chain(update_usernames_chain).apply_async(priority=BULK_TASK_PRIORITY)
+
+
 @shared_task(name='discord.update_all')
 def update_all() -> None:
     """Updates groups and nicknames (when activated) for all users."""
     discord_users_qs = DiscordUser.objects.all()
     logger.info(
-        'Starting to bulk update all %s Discord users', discord_users_qs.count()
+        'Starting to bulk update all for %s Discord users', discord_users_qs.count()
     )
     update_all_chain = list()
     for discord_user in discord_users_qs:
         update_all_chain.append(update_groups.si(discord_user.user.pk))
+        update_all_chain.append(update_username.si(discord_user.user.pk))
         if DISCORD_SYNC_NAMES:
             update_all_chain.append(update_nickname.si(discord_user.user.pk))
         

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

@@ -42,22 +42,11 @@ class TestBasicsAndHelpers(TestCase):
 @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
 class TestUpdateNick(TestCase):
 
-    def setUp(self): 
+    def setUp(self):        
         self.user = AuthUtils.create_user(TEST_USER_NAME)
         self.discord_user = DiscordUser.objects.create(
             user=self.user, uid=TEST_USER_ID
         )
-
-    @staticmethod
-    def user_info(nick):
-        return {
-            'user': {
-                'id': TEST_USER_ID,
-                'username': TEST_USER_NAME
-            },
-            'nick': nick,
-            'roles': [1, 2, 3]
-        }
    
     def test_can_update(self, mock_DiscordClient):
         AuthUtils.add_main_character_2(self.user, TEST_MAIN_NAME, TEST_MAIN_ID)        
@@ -74,9 +63,7 @@ class TestUpdateNick(TestCase):
         self.assertFalse(result)
         self.assertFalse(mock_DiscordClient.return_value.modify_guild_member.called)
     
-    def test_return_none_if_user_no_longer_a_member(
+    def test_return_none_if_user_no_longer_a_member(self, mock_DiscordClient):
-        self, mock_DiscordClient
-    ):
         AuthUtils.add_main_character_2(self.user, TEST_MAIN_NAME, TEST_MAIN_ID)
         mock_DiscordClient.return_value.modify_guild_member.return_value = None
         
@@ -93,12 +80,94 @@ class TestUpdateNick(TestCase):
         self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
 
 
+@patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+class TestUpdateUsername(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_user(TEST_USER_NAME)
+    
+    def setUp(self):         
+        self.discord_user = DiscordUser.objects.create(
+            user=self.user, 
+            uid=TEST_USER_ID, 
+            username=TEST_MAIN_NAME, 
+            discriminator='1234'
+        )
+    
+    def test_can_update(self, mock_DiscordClient):        
+        new_username = 'New name'
+        new_discriminator = '9876'
+        user_info = {
+            'user': {
+                'id': str(TEST_USER_ID),
+                'username': new_username,
+                'discriminator': new_discriminator,
+            }
+        }
+        mock_DiscordClient.return_value.guild_member.return_value = user_info
+        
+        result = self.discord_user.update_username()
+        self.assertTrue(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+        self.discord_user.refresh_from_db()
+        self.assertEqual(self.discord_user.username, new_username)
+        self.assertEqual(self.discord_user.discriminator, new_discriminator)
+
+    def test_return_none_if_user_no_longer_a_member(self, mock_DiscordClient):
+        mock_DiscordClient.return_value.guild_member.return_value = None
+        result = self.discord_user.update_username()
+        self.assertIsNone(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+
+    def test_return_false_if_api_returns_false(self, mock_DiscordClient):
+        mock_DiscordClient.return_value.guild_member.return_value = False
+        result = self.discord_user.update_username()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+
+    def test_return_false_if_api_returns_corrput_data_1(self, mock_DiscordClient):
+        mock_DiscordClient.return_value.guild_member.return_value = {'invalid': True}
+        result = self.discord_user.update_username()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+
+    def test_return_false_if_api_returns_corrput_data_2(self, mock_DiscordClient):
+        user_info = {
+            'user': {
+                'id': str(TEST_USER_ID),                
+                'discriminator': '1234',
+            }
+        }
+        mock_DiscordClient.return_value.guild_member.return_value = user_info
+        result = self.discord_user.update_username()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+
+    def test_return_false_if_api_returns_corrput_data_3(self, mock_DiscordClient):
+        user_info = {
+            'user': {
+                'id': str(TEST_USER_ID),                
+                'username': TEST_USER_NAME,
+            }
+        }
+        mock_DiscordClient.return_value.guild_member.return_value = user_info
+        result = self.discord_user.update_username()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.guild_member.called)
+
+
 @patch(MODULE_PATH + '.models.notify')
 @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
 class TestDeleteUser(TestCase):
 
-    def setUp(self): 
+    @classmethod
-        self.user = AuthUtils.create_user(TEST_USER_NAME)        
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_user(TEST_USER_NAME)
+
+    def setUp(self):         
         self.discord_user = DiscordUser.objects.create(
             user=self.user, uid=TEST_USER_ID
         )
@@ -170,8 +239,12 @@ class TestDeleteUser(TestCase):
 @patch(MODULE_PATH + '.models.DiscordUser.objects.user_group_names')
 class TestUpdateGroups(TestCase):
 
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_user(TEST_USER_NAME)
+
     def setUp(self): 
-        self.user = AuthUtils.create_user(TEST_USER_NAME)
         self.discord_user = DiscordUser.objects.create(
             user=self.user, uid=TEST_USER_ID
         )

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

@@ -150,13 +150,29 @@ class TestUpdateNickname(TestCase):
         update_nickname_inner(mock_task, self.user.pk)
         
 
+@patch(MODULE_PATH + '.DiscordUser.update_username')
+class TestUpdateUsername(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_member(TEST_USER_NAME)
+        cls.discord_user = DiscordUser.objects.create(user=cls.user, uid=TEST_USER_ID)
+
+    def test_can_update_username(self, mock_update_username):
+        mock_update_username.return_value = True
+
+        tasks.update_username(self.user.pk)
+        self.assertTrue(mock_update_username.called)
+
+
 @patch(MODULE_PATH + '.DiscordUser.delete_user')
 class TestDeleteUser(TestCase):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        cls.user = AuthUtils.create_member('Peter Parker')    
+        cls.user = AuthUtils.create_member(TEST_USER_NAME)
         cls.discord_user = DiscordUser.objects.create(user=cls.user, uid=TEST_USER_ID)
 
     def test_can_delete_user(self, mock_delete_user):
@@ -270,11 +286,37 @@ class TestBulkTasks(TestCase):
         expected_pks = [du_1.pk, du_2.pk, du_3.pk]
         self.assertSetEqual(set(current_pks), set(expected_pks))
 
+    @patch(MODULE_PATH + '.update_username.si')
+    def test_can_update_username_for_multiple_users(self, mock_update_username):
+        du_1 = DiscordUser.objects.create(user=self.user_1, uid=123)
+        du_2 = DiscordUser.objects.create(user=self.user_2, uid=456)
+        DiscordUser.objects.create(user=self.user_3, uid=789)
+        expected_pks = [du_1.pk, du_2.pk]
+
+        tasks.update_usernames_bulk(expected_pks)
+        self.assertEqual(mock_update_username.call_count, 2)
+        current_pks = [args[0][0] for args in mock_update_username.call_args_list]
+        
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
+    @patch(MODULE_PATH + '.update_username.si')
+    def test_can_update_all_usernames(self, mock_update_username):
+        du_1 = DiscordUser.objects.create(user=self.user_1, uid=123)
+        du_2 = DiscordUser.objects.create(user=self.user_2, uid=456)
+        du_3 = DiscordUser.objects.create(user=self.user_3, uid=789)
+
+        tasks.update_all_usernames()
+        self.assertEqual(mock_update_username.call_count, 3)
+        current_pks = [args[0][0] for args in mock_update_username.call_args_list]
+        expected_pks = [du_1.pk, du_2.pk, du_3.pk]
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
     @patch(MODULE_PATH + '.DISCORD_SYNC_NAMES', True)
     @patch(MODULE_PATH + '.update_nickname')
     @patch(MODULE_PATH + '.update_groups')
+    @patch(MODULE_PATH + '.update_username')
     def test_can_update_all_incl_nicknames(
-        self, mock_update_groups, mock_update_nickname
+        self, mock_update_usernames, mock_update_groups, mock_update_nickname
     ):
         du_1 = DiscordUser.objects.create(user=self.user_1, uid=123)
         du_2 = DiscordUser.objects.create(user=self.user_2, uid=456)
@@ -291,11 +333,17 @@ class TestBulkTasks(TestCase):
         expected_pks = [du_1.pk, du_2.pk, du_3.pk]
         self.assertSetEqual(set(current_pks), set(expected_pks))
 
+        self.assertEqual(mock_update_usernames.si.call_count, 3)        
+        current_pks = [args[0][0] for args in mock_update_usernames.si.call_args_list]
+        expected_pks = [du_1.pk, du_2.pk, du_3.pk]
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
     @patch(MODULE_PATH + '.DISCORD_SYNC_NAMES', False)
     @patch(MODULE_PATH + '.update_nickname')
     @patch(MODULE_PATH + '.update_groups')
+    @patch(MODULE_PATH + '.update_username')
     def test_can_update_all_excl_nicknames(
-        self, mock_update_groups, mock_update_nickname
+        self, mock_update_usernames, mock_update_groups, mock_update_nickname
     ):
         du_1 = DiscordUser.objects.create(user=self.user_1, uid=123)
         du_2 = DiscordUser.objects.create(user=self.user_2, uid=456)
@@ -308,3 +356,8 @@ class TestBulkTasks(TestCase):
         self.assertSetEqual(set(current_pks), set(expected_pks))
 
         self.assertEqual(mock_update_nickname.si.call_count, 0)
+
+        self.assertEqual(mock_update_usernames.si.call_count, 3)        
+        current_pks = [args[0][0] for args in mock_update_usernames.si.call_args_list]
+        expected_pks = [du_1.pk, du_2.pk, du_3.pk]
+        self.assertSetEqual(set(current_pks), set(expected_pks))

docs/features/services/discord.md

@@ -10,19 +10,29 @@ Discord is very popular amongst ad-hoc small groups and larger organizations see
 
 ### Prepare Your Settings File
 
-In your auth project's settings file, do the following:
+Make the following changing in your auth project's settings file (`local.py`):
 
-- Add `'allianceauth.services.modules.discord',` to your `INSTALLED_APPS` list
+- Add `'allianceauth.services.modules.discord',` to `INSTALLED_APPS`
 - Append the following to the bottom of the settings file:
 
 ```python
-    # Discord Configuration
+# Discord Configuration
-    DISCORD_GUILD_ID = ''
+DISCORD_GUILD_ID = ''
-    DISCORD_CALLBACK_URL = ''
+DISCORD_CALLBACK_URL = ''
-    DISCORD_APP_ID = ''
+DISCORD_APP_ID = ''
-    DISCORD_APP_SECRET = ''
+DISCORD_APP_SECRET = ''
-    DISCORD_BOT_TOKEN = ''
+DISCORD_BOT_TOKEN = ''
-    DISCORD_SYNC_NAMES = False
+DISCORD_SYNC_NAMES = False
+
+CELERYBEAT_SCHEDULE['discord.update_all_usernames'] = {
+    'task': 'discord.update_all_usernames',
+    'schedule': crontab(hour='*/12'),
+}
+```
+
+```eval_rst
+.. note::
+   You will have to add most the values for these settings, e.g. your Discord server ID (aka guild ID), later in the setup process.
 ```
 
 ### Creating a Server
@@ -82,21 +92,51 @@ If you want users to have their Discord nickname changed to their in-game charac
 
 Once users link their accounts you’ll notice Roles get populated on Discord. These are the equivalent to groups on every other service. The default permissions should be enough for members to use text and audio communications. Add more permissions to the roles as desired through the server management window.
 
+## Tasks
+
+The Discord service contains a number of tasks that can be run to manually perform updates to all users.
+
+You can run any of these tasks from the command line. Please make sure that you are in your venv and then you can run this command from the same folder that your manage.py is located:
+
+```bash
+celery -A myauth call discord.update_all_groups
+```
+
+```eval_rst
+======================== ====================================================
+Name                     Description
+======================== ====================================================
+`update_all_groups`      Updates groups of all users
+`update_all_nicknames`   Update nicknames of all users (also needs setting)
+`update_all_usernames`   Update locally stored Discord usernames of all users
+`update_all`             Update groups, nicknames, usernames of all users
+======================== ====================================================
+```
+
+```eval_rst
+.. note::
+   Depending on how many users you have, running these tasks can take considerable time to finish. You can calculate roughly 1 sec per user for all tasks, except update_all, which needs roughly 3 secs per user.
+```
+
 ## Settings
 
 You can configure your Discord services with the following settings:
 
-Name | Description | Default
+```eval_rst
--- | -- | --
+============================== ============================================================================================= =======
-`DISCORD_APP_ID` | Oauth client ID for the Discord Auth app | `''`
+Name                           Description                                                                                   Default
-`DISCORD_APP_SECRET` | Oauth client secret for the Discord Auth app | `''`
+============================== ============================================================================================= =======
-`DISCORD_BOT_TOKEN` | Generated bot token for the Discord Auth app | `''`
+`DISCORD_APP_ID`               Oauth client ID for the Discord Auth app                                                      `''`
-`DISCORD_CALLBACK_URL` | Oauth callback URL | `''`
+`DISCORD_APP_SECRET`           Oauth client secret for the Discord Auth app                                                  `''`
-`DISCORD_GUILD_ID` | Discord ID of your Discord server | `''`
+`DISCORD_BOT_TOKEN`            Generated bot token for the Discord Auth app                                                  `''`
-`DISCORD_ROLES_CACHE_MAX_AGE` | How long roles retrieved from the Discord server are caches locally in milliseconds | `7200000`
+`DISCORD_CALLBACK_URL`         Oauth callback URL                                                                            `''`
-`DISCORD_SYNC_NAMES` | When set to True the nicknames of Discord users will automatically be set to the user's main character name when a new user joins Discord | `False`
+`DISCORD_GUILD_ID`             Discord ID of your Discord server                                                             `''`
-`DISCORD_TASKS_RETRY_PAUSE` | Pause in seconds until next retry for tasks after an error occurred | `60`
+`DISCORD_ROLES_CACHE_MAX_AGE`  How long roles retrieved from the Discord server are caches locally in milliseconds           `7200000`
-`DISCORD_TASKS_MAX_RETRIES` | max retries of tasks after an error occurred | `3`
+`DISCORD_SYNC_NAMES`           When set to True the nicknames of Discord users will be set to the user's main character name `False`
+`DISCORD_TASKS_RETRY_PAUSE`    Pause in seconds until next retry for tasks after an error occurred                           `60`
+`DISCORD_TASKS_MAX_RETRIES`    max retries of tasks after an error occurred                                                  `3`
+============================== ============================================================================================= =======
+```
 
 ## Troubleshooting