Version Bump to 2.6.6a10

Fix API exception handling for delete_user

See merge request allianceauth/allianceauth!1209

.gitignore

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

.gitlab-ci.yml

@@ -1,8 +1,11 @@
 stages:
-- "test"
+- test
 - deploy
 
 before_script:
+- apt-get update && apt-get install redis-server -y
+- redis-server --daemonize yes
+- redis-cli ping
 - python -V
 - pip install wheel tox
 
@@ -47,5 +50,5 @@ deploy_production:
     - python setup.py sdist
     - twine upload dist/*
 
-  only:
-    - tags
+  rules:
+    - if: $CI_COMMIT_TAG

.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/__init__.py

@@ -1,7 +1,7 @@
 # This will make sure the app is always imported when
 # Django starts so that shared_task will use this app.
 
-__version__ = '2.6.6a9'
+__version__ = '2.6.6a10'
 __title__ = 'Alliance Auth'
 __url__ = 'https://gitlab.com/allianceauth/allianceauth'
 NAME = '%s v%s' % (__title__, __version__)

allianceauth/notifications/context_processors.py

@@ -1,5 +1,11 @@
 from .models import Notification
-
+from django.core.cache import cache
 
 def user_notification_count(request):
-    return {'notifications': len(Notification.objects.filter(user__id=request.user.id).filter(viewed=False))}
+    user_id = request.user.id
+    notification_count = cache.get("u-note:{}".format(user_id), -1)
+    if notification_count<0:
+        notification_count = Notification.objects.filter(user__id=user_id).filter(viewed=False).count()
+        cache.set("u-note:{}".format(user_id),notification_count,5)
+
+    return {'notifications': notification_count}

allianceauth/notifications/tests.py

@@ -1 +0,0 @@
-# Create your tests here.

allianceauth/notifications/tests/test_processors.py

@@ -0,0 +1,76 @@
+from unittest import mock
+from django.test import TestCase
+from allianceauth.notifications.context_processors import user_notification_count
+from allianceauth.tests.auth_utils import AuthUtils
+from django.core.cache import cache
+from allianceauth.notifications.models import Notification
+
+class TestNotificationCount(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = AuthUtils.create_user('magic_mike')
+        AuthUtils.add_main_character(cls.user, 'Magic Mike', '1', corp_id='2', corp_name='Pole Riders', corp_ticker='PRIDE', alliance_id='3', alliance_name='RIDERS')
+        cls.user.profile.refresh_from_db()
+
+        ### test notifications for mike
+        Notification.objects.all().delete()
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 1 Failed",
+                                    message="Because it was broken",
+                                    viewed=True)
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 2 Failed",
+                                    message="Because it was broken")
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 3 Failed",
+                                    message="Because it was broken")
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 4 Failed",
+                                    message="Because it was broken")
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 5 Failed",
+                                    message="Because it was broken")
+        Notification.objects.create(user=cls.user,
+                                    level="INFO",
+                                    title="Job 6 Failed",
+                                    message="Because it was broken")
+
+        cls.user2 = AuthUtils.create_user('teh_kid')
+        AuthUtils.add_main_character(cls.user, 'The Kid', '2', corp_id='2', corp_name='Pole Riders', corp_ticker='PRIDE', alliance_id='3', alliance_name='RIDERS')
+        cls.user2.profile.refresh_from_db()
+
+        # Noitification for kid
+        Notification.objects.create(user=cls.user2,
+                                    level="INFO",
+                                    title="Job 6 Failed",
+                                    message="Because it was broken")
+
+
+    def test_no_cache(self):
+        mock_req = mock.MagicMock()
+        mock_req.user.id = self.user.id
+
+        cache.delete("u-note:{}".format(self.user.id)) # force the db to be hit
+        context_dict = user_notification_count(mock_req)
+        self.assertIsInstance(context_dict, dict)
+        self.assertEqual(context_dict.get('notifications'), 5)  # 5 only
+
+
+    @mock.patch('allianceauth.notifications.models.Notification.objects')
+    def test_cache(self, mock_foo):
+        mock_foo.filter.return_value = mock_foo
+        mock_foo.count.return_value = 5
+        mock_req = mock.MagicMock()
+        mock_req.user.id = self.user.id
+
+        cache.set("u-note:{}".format(self.user.id),10,5)
+        context_dict = user_notification_count(mock_req)
+        self.assertIsInstance(context_dict, dict)
+        self.assertEqual(context_dict.get('notifications'), 10) # cached value
+        self.assertEqual(mock_foo.called, 0)  # ensure the DB was not hit

allianceauth/services/modules/discord/auth_hooks.py

@@ -102,7 +102,12 @@ class DiscordService(ServicesHook):
     
     @staticmethod
     def user_has_account(user: User) -> bool:
-        return DiscordUser.objects.user_has_account(user)
+        result = DiscordUser.objects.user_has_account(user)
+        if result:
+            logger.debug('User %s has a Discord account', user)
+        else:
+            logger.debug('User %s does not have a Discord account', user)
+        return result
 
     def validate_user(self, user):
         logger.debug('Validating user %s %s account', user, self.name)

allianceauth/services/modules/discord/discord_client/__init__.py

@@ -1,2 +1,3 @@
 from .client import DiscordClient   # noqa
-from .exceptions import DiscordApiBackoff  # noqa
+from .exceptions import DiscordApiBackoff  # noqa
+from .helpers import DiscordRoles  # noqa

allianceauth/services/modules/discord/discord_client/app_settings.py

@@ -24,12 +24,12 @@ DISCORD_OAUTH_TOKEN_URL = clean_setting(
 # How long the Discord guild names retrieved from the server are 
 # caches locally in milliseconds.
 DISCORD_GUILD_NAME_CACHE_MAX_AGE = clean_setting(
-    'DISCORD_GUILD_NAME_CACHE_MAX_AGE', 3600 * 2 * 1000
+    'DISCORD_GUILD_NAME_CACHE_MAX_AGE', 3600 * 1 * 1000
 )
 
 # How long Discord roles retrieved from the server are caches locally in milliseconds.
 DISCORD_ROLES_CACHE_MAX_AGE = clean_setting(
-    'DISCORD_ROLES_CACHE_MAX_AGE', 3600 * 2 * 1000
+    'DISCORD_ROLES_CACHE_MAX_AGE', 3600 * 1 * 1000
 )
 
 # Turns off creation of new roles. In case the rate limit for creating roles is

allianceauth/services/modules/discord/discord_client/client.py

@@ -1,4 +1,5 @@
 from hashlib import md5
+import json
 import logging
 from time import sleep
 from urllib.parse import urljoin
@@ -22,6 +23,7 @@ from .app_settings import (
     DISCORD_ROLES_CACHE_MAX_AGE,    
 )
 from .exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError
+from .helpers import DiscordRoles
 from ..utils import LoggerAddTag
 
 
@@ -72,8 +74,8 @@ class DiscordClient:
     _KEY_GLOBAL_BACKOFF_UNTIL = 'DISCORD_GLOBAL_BACKOFF_UNTIL'
     _KEY_GLOBAL_RATE_LIMIT_REMAINING = 'DISCORD_GLOBAL_RATE_LIMIT_REMAINING'
     _KEYPREFIX_GUILD_NAME = 'DISCORD_GUILD_NAME'
+    _KEYPREFIX_GUILD_ROLES = 'DISCORD_GUILD_ROLES'
     _KEYPREFIX_ROLE_NAME = 'DISCORD_ROLE_NAME'    
-    _ROLE_NAME_MAX_CHARS = 100
     _NICK_MAX_CHARS = 32
     
     _HTTP_STATUS_CODE_NOT_FOUND = 404
@@ -166,23 +168,7 @@ class DiscordClient:
         )
         return r.json()
    
-    # guild roles
-
-    def create_guild_role(self, guild_id: int, role_name: str, **kwargs) -> dict:
-        """Create a new guild role with the given name. 
-        See official documentation for additional optional parameters.
-
-        Note that Discord allows creating multiple roles with the name name,
-        so it's important to check existing roles before creating new one
-        to avoid duplicates.
-        
-        return a new role object on success
-        """
-        route = f"guilds/{guild_id}/roles"
-        data = {'name': self._sanitize_role_name(role_name)}
-        data.update(kwargs)
-        r = self._api_request(method='post', route=route, data=data)
-        return r.json()
+    # guild
 
     def guild_infos(self, guild_id: int) -> dict:
         """Returns all basic infos about this guild"""
@@ -216,101 +202,134 @@ class DiscordClient:
         gen_key = DiscordClient._generate_hash(f'{guild_id}')
         return f'{cls._KEYPREFIX_GUILD_NAME}__{gen_key}'
 
-    def guild_roles(self, guild_id: int) -> list:        
-        """Returns the list of all roles for this guild"""
+    # guild roles
+
+    def guild_roles(self, guild_id: int, use_cache: bool = True) -> list:
+        """Returns the list of all roles for this guild
+        
+        If use_cache is set to False it will always hit the API to retrieve
+        fresh data and update the cache
+        """
+        cache_key = self._guild_roles_cache_key(guild_id)
+        if use_cache:                        
+            roles_raw = self._redis.get(name=cache_key)
+            if roles_raw:
+                logger.debug('Returning roles for guild %s from cache', guild_id)
+                return json.loads(self._redis_decode(roles_raw))
+            else:
+                logger.debug('No roles for guild %s in cache', guild_id)
+        
         route = f"guilds/{guild_id}/roles"
-        r = self._api_request(method='get', route=route)
-        return r.json()
+        r = self._api_request(method='get', route=route)            
+        roles = r.json()
+        if roles and isinstance(roles, list):
+            self._redis.set(
+                name=cache_key, 
+                value=json.dumps(roles), 
+                px=DISCORD_ROLES_CACHE_MAX_AGE
+            )
+        return roles
+
+    def create_guild_role(self, guild_id: int, role_name: str, **kwargs) -> dict:
+        """Create a new guild role with the given name. 
+        See official documentation for additional optional parameters.
+
+        Note that Discord allows the creation of multiple roles with the same name,
+        so to avoid duplicates it's important to check existing roles 
+        before creating new one
+        
+        returns a new role dict on success
+        """
+        route = f"guilds/{guild_id}/roles"
+        data = {'name': DiscordRoles.sanitize_role_name(role_name)}
+        data.update(kwargs)
+        r = self._api_request(method='post', route=route, data=data)
+        role = r.json()
+        if role:
+            self._invalidate_guild_roles_cache(guild_id)
+        return role
 
     def delete_guild_role(self, guild_id: int, role_id: int) -> bool:
         """Deletes a guild role"""
         route = f"guilds/{guild_id}/roles/{role_id}"
         r = self._api_request(method='delete', route=route)
         if r.status_code == 204:
+            self._invalidate_guild_roles_cache(guild_id)
             return True
         else:
             return False
+        
+    def _invalidate_guild_roles_cache(self, guild_id: int) -> None:        
+        cache_key = self._guild_roles_cache_key(guild_id)        
+        self._redis.delete(cache_key)
+        logger.debug('Guild roles cache invalidated')
 
-    # guild role cache
-
-    def match_guild_roles_to_names(self, guild_id: int, role_names: list) -> list:
+    @classmethod
+    def _guild_roles_cache_key(cls, guild_id: int) -> str:
+        """Returns key for accessing cached roles for a guild"""
+        gen_key = cls._generate_hash(f'{guild_id}')
+        return f'{cls._KEYPREFIX_GUILD_ROLES}__{gen_key}'
+    
+    def match_or_create_roles_from_names(self, guild_id: int, role_names: list) -> list:
         """returns Discord roles matching the given names
         
         Returns as list of tuple of role and created flag
 
         Will try to match with existing roles names
         Non-existing roles will be created, then created flag will be True
-        Roles names are cached to improve performance
+        Params:
+        - guild_id: ID of guild
+        - role_names: list of name strings each defining a role
         """
         roles = list()
-        for role_name in role_names:
-            role, created = self.match_guild_role_to_name(
-                guild_id=guild_id, role_name=self._sanitize_role_name(role_name)
+        guild_roles = DiscordRoles(self.guild_roles(guild_id))
+        role_names_cleaned = {
+            DiscordRoles.sanitize_role_name(name) for name in role_names
+        }
+        for role_name in role_names_cleaned:
+            role, created = self.match_or_create_role_from_name(
+                guild_id=guild_id, 
+                role_name=DiscordRoles.sanitize_role_name(role_name),
+                guild_roles=guild_roles
             )
             if role:
-                roles.append((role, created))            
+                roles.append((role, created))
+            if created:
+                guild_roles = guild_roles.union(DiscordRoles([role]))
         return roles
 
-    def match_guild_role_to_name(self, guild_id: int, role_name: str) -> tuple:
+    def match_or_create_role_from_name(
+        self, guild_id: int, role_name: str, guild_roles: DiscordRoles = None
+    ) -> tuple:
         """returns Discord role matching the given name
 
         Returns as tuple of role and created flag
         
         Will try to match with existing roles names
         Non-existing roles will be created, then created flag will be True
-        Roles names are cached to improve performance    
+        Params:
+        - guild_id: ID of guild
+        - role_name: strings defining name of a role
+        - guild_roles: All known guild roles as DiscordRoles object. 
+        Helps to void redundant lookups of guild roles 
+        when this method is used multiple times.
         """
-        created = False
-        role_name = self._sanitize_role_name(role_name)
-        role_id = self._redis_decode(
-            self._redis.get(name=self._role_cache_key(guild_id, role_name))
-        )
-        if not role_id:
-            role_id = None
-            for role in self.guild_roles(guild_id):
-                self._update_role_cache(guild_id, role)
-                if role['name'] == role_name:
-                    role_id = role['id']
-            
-            if role_id:
-                role = self._create_role(role_id, role_name)
+        if not isinstance(role_name, str):
+            raise TypeError('role_name must be of type string')
 
+        created = False        
+        if guild_roles is None:
+            guild_roles = DiscordRoles(self.guild_roles(guild_id))
+        role = guild_roles.role_by_name(role_name)
+        if not role:
+            if not DISCORD_DISABLE_ROLE_CREATION:
+                logger.debug('Need to create missing role: %s', role_name)
+                role = self.create_guild_role(guild_id, role_name)                
+                created = True
             else:
-                if not DISCORD_DISABLE_ROLE_CREATION:
-                    role_raw = self.create_guild_role(guild_id, role_name)
-                    role = self._create_role(role_raw['id'], role_name)
-                    self._update_role_cache(guild_id, role)                
-                    created = True
-                else:
-                    role = None
-        else:                        
-            role = self._create_role(int(role_id), role_name)
-            
-        return role, created
-
-    @staticmethod
-    def _create_role(role_id: int, role_name: str) -> dict:
-        return {'id': int(role_id), 'name': str(role_name)}
+                role = None
     
-    def _update_role_cache(self, guild_id: int, role: dict) -> bool:
-        """updates role cache with given role
-        
-        Returns True on success, else False or raises exception
-        """
-        if not isinstance(role, dict):
-            raise TypeError('role must be a dict')
-
-        return self._redis.set(
-            name=self._role_cache_key(guild_id=guild_id, role_name=role['name']),
-            value=role['id'],
-            px=DISCORD_ROLES_CACHE_MAX_AGE
-        )
-
-    @classmethod
-    def _role_cache_key(cls, guild_id: int, role_name: str) -> str:        
-        """Returns key for accessing role given by name in the role cache"""
-        gen_key = DiscordClient._generate_hash(f'{guild_id}{role_name}')
-        return f'{cls._KEYPREFIX_ROLE_NAME}__{gen_key}'
+        return role, created
 
     # guild members
 
@@ -524,10 +543,10 @@ class DiscordClient:
             args['json'] = data
         
         logger.info('%s: sending %s request to url \'%s\'', uid, method.upper(), url)
-        logger.debug('%s: request headers:\n%s', uid, headers)
+        logger.debug('%s: request headers: %s', uid, headers)
         r = getattr(requests, method)(**args)
         logger.debug(
-            '%s: returned status code %d with headers:\n%s', 
+            '%s: returned status code %d with headers: %s', 
             uid, 
             r.status_code, 
             r.headers
@@ -589,7 +608,7 @@ class DiscordClient:
             resets_in = self._redis.pttl(self._KEY_GLOBAL_RATE_LIMIT_REMAINING)
             if requests_remaining >= 0:
                 logger.debug(
-                    '%s: Got %d remaining requests until reset in %s ms',                 
+                    '%s: Got one of %d remaining requests until reset in %s ms',
                     uid,                
                     requests_remaining + 1,
                     resets_in
@@ -679,11 +698,6 @@ class DiscordClient:
         """make sure its a list of integers"""
         return [int(role_id) for role_id in list(role_ids)]
 
-    @classmethod
-    def _sanitize_role_name(cls, role_name: str) -> str:
-        """shortens too long strings if necessary"""
-        return str(role_name)[:cls._ROLE_NAME_MAX_CHARS]
-
     @classmethod
     def _sanitize_nick(cls, nick: str) -> str:
         """shortens too long strings if necessary"""

allianceauth/services/modules/discord/discord_client/helpers.py

@@ -0,0 +1,132 @@
+from copy import copy
+
+
+class DiscordRoles:
+    """Container class that helps dealing with Discord roles.
+    
+    Objects of this class are immutable and work in many ways like sets.
+    
+    Ideally objects are initialized from raw API responses, 
+    e.g. from DiscordClient.guild.roles()
+    """
+    _ROLE_NAME_MAX_CHARS = 100
+
+    def __init__(self, roles_lst: list) -> None:
+        """roles_lst must be a list of dict, each defining a role"""
+        if not isinstance(roles_lst, (list, set, tuple)):
+            raise TypeError('roles_lst must be of type list, set or tuple')
+        self._roles = dict()
+        self._roles_by_name = dict()
+        for role in list(roles_lst):
+            self._assert_valid_role(role)
+            self._roles[int(role['id'])] = role
+            self._roles_by_name[self.sanitize_role_name(role['name'])] = role
+        
+    def __eq__(self, other):
+        if isinstance(other, type(self)):
+            return self.ids() == other.ids()
+        return NotImplemented
+
+    def __hash__(self):
+        return hash(tuple(sorted(self._roles.keys())))
+    
+    def __iter__(self):
+        for role in self._roles.values():
+            yield role
+
+    def __contains__(self, item) -> bool:        
+        return int(item) in self._roles
+
+    def __len__(self):        
+        return len(self._roles.keys())
+    
+    def has_roles(self, role_ids: set) -> bool:
+        """returns true if this objects contains all roles defined by given role_ids
+        incl. managed roles
+        """
+        role_ids = {int(id) for id in role_ids}
+        all_role_ids = self._roles.keys()        
+        return role_ids.issubset(all_role_ids)
+    
+    def ids(self) -> set:
+        """return a set of all role IDs"""
+        return set(self._roles.keys())
+
+    def subset(self, role_ids: set = None, managed_only: bool = False) -> object:
+        """returns a new object containing the subset of roles as defined 
+        by given role IDs and/or including managed roles only
+        """
+        if role_ids is not None:
+            role_ids = {int(id) for id in role_ids}
+
+        if role_ids is not None and not managed_only:
+            return type(self)([
+                role for role_id, role in self._roles.items() if role_id in role_ids
+            ])
+
+        elif role_ids is None and managed_only:
+            return type(self)([
+                role for _, role in self._roles.items() if role['managed']
+            ])
+        
+        elif role_ids is not None and managed_only:
+            return type(self)([
+                role for role_id, role in self._roles.items() 
+                if role_id in role_ids and role['managed']
+            ])
+        
+        else:
+            return copy(self)
+
+    def union(self, other: object) -> object:
+        """returns a new roles object that is the union of this roles object 
+        with other"""
+        return type(self)(list(self) + list(other))
+
+    def difference(self, other: object) -> object:
+        """returns a new roles object that only contains the roles 
+        that exist in the current objects, but not in other
+        """
+        new_ids = self.ids().difference(other.ids())
+        return self.subset(role_ids=new_ids)
+
+    def role_by_name(self, role_name: str) -> dict:
+        """returns role if one with matching name is found else an empty dict"""
+        role_name = self.sanitize_role_name(role_name)
+        if role_name in self._roles_by_name:
+            return self._roles_by_name[role_name]
+        else:
+            return dict()
+        
+    @classmethod
+    def create_from_matched_roles(cls, matched_roles: list) -> None:
+        """returns a new object created from the given list of matches roles
+        
+        matches_roles must be a list of tuples in the form: (role, created)
+        """
+        raw_roles = [x[0] for x in matched_roles]
+        return cls(raw_roles)
+
+    @staticmethod
+    def _assert_valid_role(role: dict):
+        if not isinstance(role, dict):
+            raise TypeError('Roles must be of type dict: %s' % role)
+            
+        if 'id' not in role or 'name' not in role or 'managed' not in role:
+            raise ValueError('This role is not valid: %s' % role)
+    
+    @classmethod
+    def sanitize_role_name(cls, role_name: str) -> str:
+        """shortens too long strings if necessary"""
+        return str(role_name)[:cls._ROLE_NAME_MAX_CHARS]
+
+
+def match_or_create_roles_from_names(
+    client: object, guild_id: int, role_names: list
+) -> DiscordRoles:
+    """Shortcut for getting the result of matching role names as DiscordRoles object"""
+    return DiscordRoles.create_from_matched_roles(
+        client.match_or_create_roles_from_names(
+            guild_id=guild_id, role_names=role_names
+        )
+    )

allianceauth/services/modules/discord/discord_client/tests/__init__.py

@@ -0,0 +1,38 @@
+TEST_GUILD_ID = 123456789012345678
+TEST_USER_ID = 198765432012345678
+TEST_USER_NAME = 'Peter Parker'
+TEST_USER_DISCRIMINATOR = '1234'
+TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY'
+TEST_ROLE_ID = 654321012345678912
+
+
+def create_role(id: int, name: str, managed=False):
+    return {
+        'id': int(id),
+        'name': str(name),
+        'managed': bool(managed)
+    }
+
+
+def create_matched_role(role, created=False) -> tuple:
+    return role, created
+
+
+ROLE_ALPHA = create_role(1, 'alpha')
+ROLE_BRAVO = create_role(2, 'bravo')
+ROLE_CHARLIE = create_role(3, 'charlie')
+ROLE_MIKE = create_role(13, 'mike', True)
+
+ALL_ROLES = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE]
+
+
+def create_user_info(
+    id: int = TEST_USER_ID, 
+    username: str = TEST_USER_NAME, 
+    discriminator: str = TEST_USER_DISCRIMINATOR
+):
+    return {
+        'id': str(id),
+        'username': str(username[:32]),
+        'discriminator': str(discriminator[:4])
+    }

allianceauth/services/modules/discord/discord_client/tests/piloting_functionality.py

@@ -50,10 +50,10 @@ class TestDiscordApiLive(TestCase):
         self.client.guild_name(DISCORD_GUILD_ID)
         sleep(RATE_LIMIT_DELAY_SECS)
 
-        self.client.match_guild_role_to_name(DISCORD_GUILD_ID, 'Testrole')
+        self.client.match_or_create_role_from_name(DISCORD_GUILD_ID, 'Testrole')
         sleep(RATE_LIMIT_DELAY_SECS)
 
-        self.client.match_guild_roles_to_names(
+        self.client.match_or_create_roles_from_names(
             DISCORD_GUILD_ID, ['Testrole A', 'Testrole B']
         )
         sleep(RATE_LIMIT_DELAY_SECS)

allianceauth/services/modules/discord/discord_client/tests/piloting_tasks.py

@@ -1,47 +0,0 @@
-"""Load testing Discord services tasks
-
-This script will load test the Discord service tasks.
-Note that his will run against your production Auth.
-To run this test start a bunch of celery workers and then run this script directly.
-
-This script requires a user with a Discord account setup through Auth. 
-Please provide the respective Discord user ID by setting it as environment variable:
-
-export DISCORD_USER_ID="123456789"
-"""
-
-import os
-import sys
-
-myauth_dir = '/home/erik997/dev/python/aa/allianceauth-dev/myauth'
-sys.path.insert(0, myauth_dir)
-
-import django   # noqa: E402
-
-# init and setup django project
-os.environ.setdefault("DJANGO_SETTINGS_MODULE", "myauth.settings.local")
-django.setup()
-
-from uuid import uuid1  # noqa: E402
-
-from django.contrib.auth.models import User     # noqa: E402
-# from allianceauth.services.modules.discord.tasks import update_groups  # noqa: E402
-
-if 'DISCORD_USER_ID' not in os.environ:
-    print('Please set DISCORD_USER_ID')
-    exit()
-
-DISCORD_USER_ID = os.environ['DISCORD_USER_ID']
-
-
-def run_many_updates(runs):        
-    user = User.objects.get(discord__uid=DISCORD_USER_ID)
-    for _ in range(runs):                
-        new_nick = f'Testnick {uuid1().hex}'[:32]
-        user.profile.main_character.character_name = new_nick
-        user.profile.main_character.save()
-        # update_groups.delay(user_pk=user.pk)
-        
-
-if __name__ == "__main__":
-    run_many_updates(20)

allianceauth/services/modules/discord/discord_client/tests/test_client.py

@@ -9,7 +9,22 @@ from requests.exceptions import HTTPError
 
 from allianceauth import __title__ as AUTH_TITLE, __url__, __version__
 
-from ..client import DiscordClient, DURATION_CONTINGENCY, DEFAULT_BACKOFF_DELAY
+from . import (
+    TEST_GUILD_ID,
+    TEST_USER_ID,
+    TEST_USER_NAME,
+    TEST_BOT_TOKEN,
+    TEST_ROLE_ID,    
+    ROLE_ALPHA, 
+    ROLE_BRAVO, 
+    ALL_ROLES, 
+    create_role, 
+    create_matched_role,
+    create_user_info
+)
+from ..client import (
+    DiscordClient, DURATION_CONTINGENCY, DEFAULT_BACKOFF_DELAY, DiscordRoles
+)
 from ..exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError
 from ...utils import set_logger_to_file
 
@@ -19,13 +34,6 @@ logger = set_logger_to_file(
 
 MODULE_PATH = 'allianceauth.services.modules.discord.discord_client.client'
 API_BASE_URL = 'https://discordapp.com/api/'
-TEST_GUILD_ID = 123456789012345678
-TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY'
-TEST_USER_ID = 198765432012345678
-TEST_USER_NAME = 'John Doe'
-TEST_ROLE_ID = 654321012345678912
-
-TEST_ROUTE_KEY = 'abc123'
 
 TEST_RETRY_AFTER = 3000
 
@@ -71,13 +79,6 @@ class TestBasicsAndHelpers(TestCase):
         client = DiscordClient(TEST_BOT_TOKEN, mock_redis, is_rate_limited=True)
         self.assertTrue(client.is_rate_limited)
 
-    def test_sanitize_role_name(self):
-        client = DiscordClient(TEST_BOT_TOKEN, mock_redis)
-        role_name_input = 'x' * 110
-        role_name_expected = 'x' * 100                
-        result = client._sanitize_role_name(role_name_input)
-        self.assertEqual(result, role_name_expected)
-
     @patch(MODULE_PATH + '.caches')
     def test_use_default_redis_if_none_provided(self, mock_caches):
         my_redis = MagicMock(spec=Redis)
@@ -110,7 +111,7 @@ class TestOtherMethods(TestCase):
         self.headers = DEFAULT_REQUEST_HEADERS
 
     def test_user_get_current(self, requests_mocker):        
-        expected = {'id': "123456"}
+        expected = create_user_info()
         headers = {
             'accept': 'application/json', 
             'authorization': 'Bearer accesstoken'
@@ -125,25 +126,6 @@ class TestOtherMethods(TestCase):
         result = client.current_user()
         self.assertDictEqual(result, expected)
 
-    def test_guild_create_role(self, requests_mocker):
-        role_name_input = 'x' * 120
-        role_name_used = 'x' * 100
-        expected = {'name': role_name_used}
-
-        def data_matcher(request):
-            return (json.loads(request.text) == expected)
-                 
-        requests_mocker.post(            
-            f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles',
-            request_headers=self.headers,
-            additional_matcher=data_matcher,
-            text=json.dumps(expected),
-        )        
-        result = self.client.create_guild_role(
-            guild_id=TEST_GUILD_ID, role_name=role_name_input
-        )
-        self.assertDictEqual(result, expected)
-    
     def test_get_infos(self, requests_mocker):          
         expected = {
             'id': TEST_GUILD_ID, 
@@ -157,19 +139,93 @@ class TestOtherMethods(TestCase):
         result = self.client.guild_infos(TEST_GUILD_ID)
         self.assertDictEqual(result, expected)
 
-    def test_get_roles(self, requests_mocker):        
-        expected = [
-            {'id': 1, 'name': 'alpha'},
-            {'id': 2, 'name': 'bravo'}
-        ]
-        requests_mocker.get(            
-            f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles',
-            request_headers=self.headers,
-            json=expected
-        )        
-        result = self.client.guild_roles(TEST_GUILD_ID)
-        self.assertListEqual(result, expected)
+    
+@requests_mock.Mocker()
+class TestGuildRoles(TestCase):    
 
+    def setUp(self):
+        self.url = f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles'
+
+    def test_without_cache(self, requests_mocker):        
+        expected = [ROLE_ALPHA, ROLE_BRAVO]
+        my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        requests_mocker.get(     
+            url=self.url,
+            request_headers=DEFAULT_REQUEST_HEADERS,
+            json=expected
+        )
+        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
+        result = client.guild_roles(TEST_GUILD_ID, use_cache=False)
+        self.assertListEqual(result, expected)
+        self.assertTrue(my_mock_redis.set.called)
+
+    def test_return_from_cache_if_in_cache(self, requests_mocker):        
+        expected = [ROLE_ALPHA, ROLE_BRAVO]
+        my_mock_redis = MagicMock(**{
+            'get.return_value': json.dumps(expected).encode('utf8')
+        })
+        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)                
+        result = client.guild_roles(TEST_GUILD_ID)
+        self.assertEqual(result, expected)
+        self.assertFalse(my_mock_redis.set.called)
+
+    def test_return_from_api_and_save_to_cache_if_not_in_cache(
+        self, requests_mocker
+    ):
+        expected = [ROLE_ALPHA, ROLE_BRAVO]
+        my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        requests_mocker.get(     
+            url=self.url,
+            request_headers=DEFAULT_REQUEST_HEADERS,
+            json=expected
+        )
+        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)                
+        result = client.guild_roles(TEST_GUILD_ID)
+        self.assertEqual(result, expected)
+        self.assertTrue(my_mock_redis.set.called)
+
+    def test_dont_save_in_cache_if_api_returns_invalid_response_1(
+        self, requests_mocker
+    ):
+        expected = {}
+        my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        requests_mocker.get(     
+            url=self.url,
+            request_headers=DEFAULT_REQUEST_HEADERS,
+            json=expected
+        )
+        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)                
+        result = client.guild_roles(TEST_GUILD_ID)
+        self.assertEqual(result, expected)
+        self.assertFalse(my_mock_redis.set.called)
+
+    def test_dont_save_in_cache_if_api_returns_invalid_response_2(
+        self, requests_mocker
+    ):
+        expected = "api returns string"
+        my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        requests_mocker.get(     
+            url=self.url,
+            request_headers=DEFAULT_REQUEST_HEADERS,
+            json=expected
+        )
+        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)                
+        result = client.guild_roles(TEST_GUILD_ID)
+        self.assertEqual(result, expected)
+        self.assertFalse(my_mock_redis.set.called)
+    
 
 @requests_mock.Mocker()
 class TestGuildMember(TestCase):
@@ -179,7 +235,7 @@ class TestGuildMember(TestCase):
         self.headers = DEFAULT_REQUEST_HEADERS
 
     def test_return_guild_member_when_ok(self, requests_mocker):
-        expected = {'id': TEST_USER_ID, 'name': 'John Doe'}                 
+        expected = create_user_info()
         requests_mocker.get(            
             f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}',
             request_headers=self.headers,
@@ -243,37 +299,86 @@ class TestGuildGetName(TestCase):
         self.assertFalse(my_mock_redis.set.called)
 
 
+@requests_mock.Mocker()
+class TestCreateGuildRole(TestCase):
+
+    def setUp(self):        
+        self.request_url = f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles'
+        self.my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        self.client = DiscordClient2(TEST_BOT_TOKEN, self.my_mock_redis)
+
+    def test_guild_create_role_normal(self, requests_mocker):
+        role_name_input = 'x' * 120
+        role_name_used = 'x' * 100
+        expected = {'name': role_name_used}
+
+        def data_matcher(request):
+            return (json.loads(request.text) == expected)
+                 
+        requests_mocker.post(            
+            self.request_url,
+            request_headers=DEFAULT_REQUEST_HEADERS,
+            additional_matcher=data_matcher,
+            text=json.dumps(expected),
+        )        
+        result = self.client.create_guild_role(
+            guild_id=TEST_GUILD_ID, role_name=role_name_input
+        )
+        self.assertDictEqual(result, expected)
+        self.assertTrue(self.my_mock_redis.delete.called)
+
+    def test_guild_create_role_empty_response(self, requests_mocker):                 
+        expected = {}
+        requests_mocker.post(
+            self.request_url,
+            request_headers=DEFAULT_REQUEST_HEADERS,        
+            text=json.dumps(expected),
+        )        
+        result = self.client.create_guild_role(
+            guild_id=TEST_GUILD_ID, role_name='dummy'
+        )
+        self.assertDictEqual(result, expected)
+        self.assertFalse(self.my_mock_redis.delete.called)
+    
+
 @requests_mock.Mocker()
 class TestGuildDeleteRole(TestCase):
 
-    def setUp(self):
-        self.access_token = 'accesstoken'        
-        self.headers = DEFAULT_REQUEST_HEADERS
+    def setUp(self):        
         self.request_url = \
-            f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles/{TEST_ROLE_ID}'
-        self.client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+            f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles/{TEST_ROLE_ID}'        
+        self.my_mock_redis = MagicMock(**{
+            'get.return_value': None,
+            'pttl.return_value': -1,
+        })
+        self.client = DiscordClient2(TEST_BOT_TOKEN, self.my_mock_redis)
 
     def test_guild_delete_role_success(self, requests_mocker):
         requests_mocker.delete(            
             self.request_url,
-            request_headers=self.headers,
+            request_headers=DEFAULT_REQUEST_HEADERS,
             status_code=204
         )
         result = self.client.delete_guild_role(
             guild_id=TEST_GUILD_ID, role_id=TEST_ROLE_ID
         )
         self.assertTrue(result)
+        self.assertTrue(self.my_mock_redis.delete.called)
 
     def test_guild_delete_role_failed(self, requests_mocker):
         requests_mocker.delete(            
             self.request_url,
-            request_headers=self.headers,
+            request_headers=DEFAULT_REQUEST_HEADERS,
             status_code=200
         )
         result = self.client.delete_guild_role(
             guild_id=TEST_GUILD_ID, role_id=TEST_ROLE_ID
         )
         self.assertFalse(result)
+        self.assertFalse(self.my_mock_redis.delete.called)
 
 
 @requests_mock.Mocker()
@@ -735,141 +840,138 @@ class TestGuildMemberRemoveRole(TestCase):
 
 @patch(MODULE_PATH + '.DiscordClient.create_guild_role')
 @patch(MODULE_PATH + '.DiscordClient.guild_roles')
-class TestGuildGetOrCreateRoles(TestCase):
+class TestMatchGuildRolesToName(TestCase):
             
-    def test_return_id_if_role_in_cache(
-        self, mock_guild_get_roles, mock_guild_create_role,
-    ):        
-        role_name = 'alpha'
-        my_mock_redis = MagicMock(**{'get.return_value': b'1'})
-        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
-        mock_guild_get_roles.side_effect = RuntimeError
-        mock_guild_create_role.side_effect = RuntimeError
-
-        expected = ({'id': 1, 'name': 'alpha'}, False)
-        result = client.match_guild_role_to_name(TEST_GUILD_ID, role_name)
-        self.assertEqual(result, expected)
-
-    def test_return_id_for_role_known_by_api(
-        self, mock_guild_get_roles, mock_guild_create_role,
-    ):                
-        my_mock_redis = MagicMock(**{'get.return_value': None})
-        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
-        mock_guild_get_roles.return_value = [
-            {'id': 1, 'name': 'alpha'},
-            {'id': 2, 'name': 'bravo'}
-        ]
-        mock_guild_create_role.side_effect = RuntimeError
-        
-        expected = ({'id': 1, 'name': 'alpha'}, False)
-        result = client.match_guild_role_to_name(TEST_GUILD_ID, 'alpha')
-        self.assertEqual(result, expected)
-
-        expected = ({'id': 2, 'name': 'bravo'}, False)
-        result = client.match_guild_role_to_name(TEST_GUILD_ID, 'bravo')
-        self.assertEqual(result, expected)
-    
-    @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', False)
-    def test_create_role_for_role_not_known_by_api(
+    def test_return_role_if_known(
         self, mock_guild_get_roles, mock_guild_create_role,
     ):
-        my_mock_redis = MagicMock(**{'get.return_value': None})
-        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
-        mock_guild_get_roles.return_value = [
-            {'id': 1, 'name': 'alpha'},
-            {'id': 2, 'name': 'bravo'}
-        ]
-        mock_guild_create_role.return_value = {'id': 3, 'name': 'charlie'}
-        
-        expected = ({'id': 3, 'name': 'charlie'}, True)
-        result = client.match_guild_role_to_name(TEST_GUILD_ID, 'charlie')
+        role_name = 'alpha'        
+        mock_guild_get_roles.return_value = ALL_ROLES
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name)
+        expected = (ROLE_ALPHA, False)
         self.assertEqual(result, expected)
+        self.assertFalse(mock_guild_create_role.called)
+
+    def test_create_role_if_not_known_and_return_it(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_name = 'echo'
+        new_role = create_role(5, 'echo')
+        mock_guild_get_roles.return_value = ALL_ROLES
+        mock_guild_create_role.return_value = new_role
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name)
+        expected = (new_role, True)
+        self.assertEqual(result, expected)
+        self.assertTrue(mock_guild_create_role.called)
 
     @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', True)
     def test_return_none_if_role_creation_is_disabled(
         self, mock_guild_get_roles, mock_guild_create_role,
     ):
-        my_mock_redis = MagicMock(**{'get.return_value': None})
-        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
-        mock_guild_get_roles.return_value = [
-            {'id': 1, 'name': 'alpha'},
-            {'id': 2, 'name': 'bravo'}
-        ]
-        mock_guild_create_role.return_value = {'id': 3, 'name': 'charlie'}
-                
-        result = client.match_guild_role_to_name(TEST_GUILD_ID, 'charlie')
-        self.assertIsNone(result[0])
-        self.assertFalse(result[1])
-
-    def test_return_ids_if_role_in_cache(
-        self, mock_guild_get_roles, mock_guild_create_role,   
-    ):
-        def my_cache_get(name):
-            map = {
-                DiscordClient._role_cache_key(TEST_GUILD_ID, 'alpha'): b'1',
-                DiscordClient._role_cache_key(TEST_GUILD_ID, 'bravo'): b'2',
-                DiscordClient._role_cache_key(TEST_GUILD_ID, 'charlie'): b'3'
-            }
-            if name in map:
-                return map[name]
-            else:
-                return None
-        
-        my_mock_redis = MagicMock(**{'get.side_effect': my_cache_get})
-        client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis)
-        mock_guild_get_roles.side_effect = RuntimeError
-        mock_guild_create_role.side_effect = RuntimeError
-        
-        expected = [
-            ({'id': 1, 'name': 'alpha'}, False), ({'id': 3, 'name': 'charlie'}, False)
-        ]
-        result = client.match_guild_roles_to_names(TEST_GUILD_ID, ['alpha', 'charlie'])
-        self.assertEqual(result, expected)
-
-    @patch(MODULE_PATH + '.DiscordClient.match_guild_role_to_name')
-    def test_ignore_none_roles_in_guild_get_or_create_roles(
-        self, 
-        mock_guild_get_or_create_role, 
-        mock_guild_get_roles, 
-        mock_guild_create_role,
-    ):
-        def my_guild_get_or_create_role(guild_id, role_name):
-            if role_name == 'alpha':
-                return {'id': 1, 'name': 'alpha'}, False
-            elif role_name == 'charlie':
-                return None, False
-            else:
-                raise ValueError('Unknown role')
-        
-        mock_guild_get_or_create_role.side_effect = my_guild_get_or_create_role
-
+        role_name = 'echo'        
+        mock_guild_get_roles.return_value = ALL_ROLES        
         client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
-        result = client.match_guild_roles_to_names(TEST_GUILD_ID, ['alpha', 'charlie'])
-        expected = [
-            ({'id': 1, 'name': 'alpha'}, False),         
-        ]
+        result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name)
+        expected = (None, False)
         self.assertEqual(result, expected)
-        
-
-class TestUpdateRoleCache(TestCase):
+        self.assertFalse(mock_guild_create_role.called)
     
-    def test_can_update_cache(self):
-        my_mock_redis = MagicMock()
-        client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis)
-        role = {'id': 1, 'name': 'alpha'}
-        client._update_role_cache(TEST_GUILD_ID, role)
-        self.assertTrue(my_mock_redis.set.called)
-
-    def test_raises_exception_if_wrong_role_type(self):
-        my_mock_redis = MagicMock()
-        client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis)
-        role = 'abc'
+    def test_raise_exception_if_name_has_invalid_type(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_name = ['echo']
+        mock_guild_get_roles.return_value = ALL_ROLES        
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
         with self.assertRaises(TypeError):
-            client._update_role_cache(TEST_GUILD_ID, role)
-
-        self.assertFalse(my_mock_redis.set.called)
+            client.match_or_create_role_from_name(TEST_GUILD_ID, role_name)        
         
 
+@patch(MODULE_PATH + '.DiscordClient.create_guild_role')
+@patch(MODULE_PATH + '.DiscordClient.guild_roles')
+class TestMatchGuildRolesToNames(TestCase):
+
+    def test_return_roles_if_known(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_names = ['alpha', 'bravo']
+        mock_guild_get_roles.return_value = ALL_ROLES
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names)
+        expected = [create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)]
+        self.assertEqual(
+            DiscordRoles.create_from_matched_roles(result), 
+            DiscordRoles.create_from_matched_roles(expected)
+        )
+        self.assertFalse(mock_guild_create_role.called)
+
+    def test_return_roles_if_known_and_create_if_not_known(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_names = ['alpha', 'echo']
+        new_role = create_role(5, 'echo')
+        mock_guild_get_roles.return_value = ALL_ROLES
+        mock_guild_create_role.return_value = new_role
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names)
+        expected = \
+            [create_matched_role(ROLE_ALPHA), create_matched_role(new_role, True)]
+        self.assertEqual(
+            DiscordRoles.create_from_matched_roles(result), 
+            DiscordRoles.create_from_matched_roles(expected)
+        )
+        self.assertTrue(mock_guild_create_role.called)
+
+    @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', True)
+    def test_exclude_non_roles_from_result_list(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_names = ['alpha', 'echo']
+        new_role = create_role(5, 'echo')
+        mock_guild_get_roles.return_value = ALL_ROLES
+        mock_guild_create_role.return_value = new_role
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names)
+        expected = [create_matched_role(ROLE_ALPHA)]
+        self.assertEqual(
+            DiscordRoles.create_from_matched_roles(result), 
+            DiscordRoles.create_from_matched_roles(expected)
+        )
+        self.assertFalse(mock_guild_create_role.called)
+
+    def test_consolidate_roles_of_same_name(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        role_names = ['alpha', 'bravo', 'alpha']
+        mock_guild_get_roles.return_value = ALL_ROLES
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names)
+        expected = [create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)]
+        self.assertEqual(
+            DiscordRoles.create_from_matched_roles(result), 
+            DiscordRoles.create_from_matched_roles(expected)
+        )
+        self.assertFalse(mock_guild_create_role.called)
+
+    def test_consolidate_roles_of_same_name_after_sanitation(
+        self, mock_guild_get_roles, mock_guild_create_role,
+    ):
+        base_role_name = 'x' * 100
+        new_role = create_role(77, base_role_name)
+        role_names = [base_role_name + '1', base_role_name + '2']
+        mock_guild_get_roles.return_value = ALL_ROLES + [new_role]
+        mock_guild_create_role.return_value = new_role
+        client = DiscordClient2(TEST_BOT_TOKEN, mock_redis)
+        result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names)
+        expected = [create_matched_role(new_role)]
+        self.assertEqual(
+            DiscordRoles.create_from_matched_roles(result), 
+            DiscordRoles.create_from_matched_roles(expected)
+        )
+        self.assertFalse(mock_guild_create_role.called)
+
+
 class TestApiRequestBasics(TestCase):
 
     def setUp(self):
@@ -885,7 +987,7 @@ class TestApiRequestBasics(TestCase):
 @requests_mock.Mocker()
 class TestRateLimitMechanic(TestCase):
     
-    my_role = {'id': 1, 'name': 'alpha'}
+    my_role = ROLE_ALPHA
     
     @staticmethod
     def my_redis_pttl(name: str):
@@ -1067,7 +1169,7 @@ class TestRateLimitMechanic(TestCase):
 @requests_mock.Mocker()
 class TestBackoffHandling(TestCase):
     
-    my_role = {'id': 1, 'name': 'alpha'}
+    my_role = ROLE_ALPHA
 
     def test_dont_raise_exception_when_no_global_backoff(
         self, mock_redis_decr_or_set, requests_mocker

allianceauth/services/modules/discord/discord_client/tests/test_helpers.py

@@ -0,0 +1,238 @@
+from unittest import TestCase
+
+from . import ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ALL_ROLES, create_role
+from .. import DiscordRoles
+
+
+MODULE_PATH = 'allianceauth.services.modules.discord.discord_client.client'
+
+
+class TestDiscordRoles(TestCase):
+
+    def setUp(self):
+        self.all_roles = DiscordRoles(ALL_ROLES)
+
+    def test_can_create_simple(self):
+        roles_raw = [ROLE_ALPHA]
+        roles = DiscordRoles(roles_raw)
+        self.assertListEqual(list(roles), roles_raw)
+
+    def test_can_create_empty(self):
+        roles_raw = []
+        roles = DiscordRoles(roles_raw)
+        self.assertListEqual(list(roles), [])
+
+    def test_raises_exception_if_roles_raw_of_wrong_type(self):
+        with self.assertRaises(TypeError):
+            DiscordRoles({'id': 1})
+    
+    def test_raises_exception_if_list_contains_non_dict(self):
+        roles_raw = [ROLE_ALPHA, 'not_valid']
+        with self.assertRaises(TypeError):
+            DiscordRoles(roles_raw)
+
+    def test_raises_exception_if_invalid_role_1(self):
+        roles_raw = [{'name': 'alpha', 'managed': False}]
+        with self.assertRaises(ValueError):
+            DiscordRoles(roles_raw)
+
+    def test_raises_exception_if_invalid_role_2(self):
+        roles_raw = [{'id': 1, 'managed': False}]
+        with self.assertRaises(ValueError):
+            DiscordRoles(roles_raw)
+
+    def test_raises_exception_if_invalid_role_3(self):
+        roles_raw = [{'id': 1, 'name': 'alpha'}]
+        with self.assertRaises(ValueError):
+            DiscordRoles(roles_raw)
+    
+    def test_roles_are_equal(self):
+        roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_b = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        self.assertEqual(roles_a, roles_b)
+
+    def test_roles_are_not_equal(self):
+        roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_b = DiscordRoles([ROLE_ALPHA])
+        self.assertNotEqual(roles_a, roles_b)
+
+    def test_different_objects_are_not_equal(self):
+        roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])        
+        self.assertFalse(roles_a == "invalid")
+
+    def test_len(self):        
+        self.assertEqual(len(self.all_roles), 4)
+
+    def test_contains(self):
+        self.assertTrue(1 in self.all_roles)
+        self.assertFalse(99 in self.all_roles)
+
+    def test_sanitize_role_name(self):        
+        role_name_input = 'x' * 110
+        role_name_expected = 'x' * 100                
+        result = DiscordRoles.sanitize_role_name(role_name_input)
+        self.assertEqual(result, role_name_expected)
+
+    def test_objects_are_hashable(self):
+        roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_b = DiscordRoles([ROLE_BRAVO, ROLE_ALPHA])
+        roles_c = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE])
+        self.assertIsNotNone(hash(roles_a))
+        self.assertEqual(hash(roles_a), hash(roles_b))
+        self.assertNotEqual(hash(roles_a), hash(roles_c))
+
+    def test_create_from_matched_roles(self):
+        matched_roles = [
+            (ROLE_ALPHA, True),
+            (ROLE_BRAVO, False)
+        ]
+        roles = DiscordRoles.create_from_matched_roles(matched_roles)
+        self.assertSetEqual(roles.ids(), {1, 2})
+
+
+class TestIds(TestCase):
+
+    def setUp(self):
+        self.all_roles = DiscordRoles(ALL_ROLES)
+        
+    def test_return_role_ids_default(self):        
+        result = self.all_roles.ids()
+        expected = {1, 2, 3, 13}
+        self.assertSetEqual(result, expected)
+
+    def test_return_role_ids_empty(self):
+        roles = DiscordRoles([])
+        self.assertSetEqual(roles.ids(), set())
+
+
+class TestSubset(TestCase):
+
+    def setUp(self):
+        self.all_roles = DiscordRoles(ALL_ROLES)
+
+    def test_ids_only(self):
+        role_ids = {1, 3}
+        roles_subset = self.all_roles.subset(role_ids)
+        expected = {1, 3}
+        self.assertSetEqual(roles_subset.ids(), expected)
+
+    def test_ids_as_string_work_too(self):
+        role_ids = {'1', '3'}
+        roles_subset = self.all_roles.subset(role_ids)
+        expected = {1, 3}
+        self.assertSetEqual(roles_subset.ids(), expected)
+
+    def test_managed_only(self):
+        roles = self.all_roles.subset(managed_only=True)
+        expected = {13}
+        self.assertSetEqual(roles.ids(), expected)
+
+    def test_ids_and_managed_only(self):
+        role_ids = {1, 3, 13}
+        roles_subset = self.all_roles.subset(role_ids, managed_only=True)
+        expected = {13}
+        self.assertSetEqual(roles_subset.ids(), expected)
+
+    def test_ids_are_empty(self):
+        roles = self.all_roles.subset([])
+        expected = set()
+        self.assertSetEqual(roles.ids(), expected)
+
+    def test_no_parameters(self):
+        roles = self.all_roles.subset()
+        expected = {1, 2, 3, 13}
+        self.assertSetEqual(roles.ids(), expected)
+
+
+class TestHasRoles(TestCase):
+
+    def setUp(self):
+        self.all_roles = DiscordRoles(ALL_ROLES)
+
+    def test_true_if_all_roles_exit(self):
+        self.assertTrue(self.all_roles.has_roles([1, 2]))
+
+    def test_true_if_all_roles_exit_str(self):
+        self.assertTrue(self.all_roles.has_roles(['1', '2']))
+    
+    def test_false_if_role_does_not_exit(self):
+        self.assertFalse(self.all_roles.has_roles([99]))
+
+    def test_false_if_one_role_does_not_exit(self):
+        self.assertFalse(self.all_roles.has_roles([1, 99]))
+
+    def test_true_for_empty_roles(self):
+        self.assertTrue(self.all_roles.has_roles([]))
+        
+
+class TestGetMatchingRolesByName(TestCase):
+
+    def setUp(self):
+        self.all_roles = DiscordRoles(ALL_ROLES)
+
+    def test_return_role_if_matches(self):
+        role_name = 'alpha'
+        expected = ROLE_ALPHA
+        result = self.all_roles.role_by_name(role_name)
+        self.assertEqual(result, expected)
+
+    def test_return_role_if_matches_and_limit_max_length(self):
+        role_name = 'x' * 120
+        expected = create_role(77, 'x' * 100)
+        roles = DiscordRoles([expected])
+        result = roles.role_by_name(role_name)
+        self.assertEqual(result, expected)
+
+    def test_return_empty_if_not_matches(self):
+        role_name = 'lima'
+        expected = {}
+        result = self.all_roles.role_by_name(role_name)
+        self.assertEqual(result, expected)
+
+
+class TestUnion(TestCase):
+
+    def test_distinct_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_CHARLIE, ROLE_MIKE])
+        roles_3 = roles_1.union(roles_2)
+        expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE])
+        self.assertEqual(roles_3, expected)
+
+    def test_overlapping_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_BRAVO, ROLE_MIKE])
+        roles_3 = roles_1.union(roles_2)
+        expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE])
+        self.assertEqual(roles_3, expected)
+
+    def test_identical_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_3 = roles_1.union(roles_2)
+        expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        self.assertEqual(roles_3, expected)
+
+
+class TestDifference(TestCase):
+
+    def test_distinct_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_CHARLIE, ROLE_MIKE])
+        roles_3 = roles_1.difference(roles_2)
+        expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        self.assertEqual(roles_3, expected)
+
+    def test_overlapping_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_BRAVO, ROLE_MIKE])
+        roles_3 = roles_1.difference(roles_2)
+        expected = DiscordRoles([ROLE_ALPHA])
+        self.assertEqual(roles_3, expected)
+
+    def test_identical_sets(self):
+        roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_2 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO])
+        roles_3 = roles_1.difference(roles_2)
+        expected = DiscordRoles([])
+        self.assertEqual(roles_3, expected)

allianceauth/services/modules/discord/managers.py

@@ -20,6 +20,7 @@ from .app_settings import (
     DISCORD_SYNC_NAMES
 )
 from .discord_client import DiscordClient, DiscordApiBackoff
+from .discord_client.helpers import match_or_create_roles_from_names
 from .utils import LoggerAddTag
 
 
@@ -62,10 +63,12 @@ class DiscordUserManager(models.Manager):
             user_id = discord_user['id']
             bot_client = self._bot_client(is_rate_limited=is_rate_limited)
                 
-            if group_names:
-                role_ids = self.model._guild_get_or_create_role_ids(
-                    bot_client, group_names
-                )
+            if group_names:                
+                role_ids = match_or_create_roles_from_names(
+                    client=bot_client, 
+                    guild_id=DISCORD_GUILD_ID, 
+                    role_names=group_names
+                ).ids()
             else:
                 role_ids = None
                         
@@ -133,7 +136,9 @@ class DiscordUserManager(models.Manager):
         
         only checks locally, does not hit the API
         """
-        return True if hasattr(user, self.model.USER_RELATED_NAME) else False
+        if not isinstance(user, User):
+            return False
+        return self.filter(user=user).select_related('user').exists()
 
     @classmethod
     def generate_bot_add_url(cls):

allianceauth/services/modules/discord/models.py

@@ -10,7 +10,8 @@ from allianceauth.notifications import notify
 
 from . import __title__
 from .app_settings import DISCORD_GUILD_ID
-from .discord_client import DiscordClient, DiscordApiBackoff
+from .discord_client import DiscordApiBackoff, DiscordRoles
+from .discord_client.helpers import match_or_create_roles_from_names
 from .managers import DiscordUserManager
 from .utils import LoggerAddTag
 
@@ -99,26 +100,94 @@ class DiscordUser(models.Model):
         - True on success
         - None if user is no longer a member of the Discord server
         - False on error or raises exception
-        """
-        role_names = DiscordUser.objects.user_group_names(self.user)        
-        client = DiscordUser.objects._bot_client()
-        requested_role_ids = self._guild_get_or_create_role_ids(client, role_names)
-        logger.debug(
-            'Requested to update groups for user %s: %s', self.user, requested_role_ids
-        )        
-        success = client.modify_guild_member(
-            guild_id=DISCORD_GUILD_ID,
-            user_id=self.uid,
-            role_ids=requested_role_ids
-        )
-        if success:
-            logger.info('Groups for %s have been updated', self.user)
+        """        
+        client = DiscordUser.objects._bot_client()                                
+        member_info = client.guild_member(guild_id=DISCORD_GUILD_ID, user_id=self.uid)
+        if member_info is None:            
+            # User is no longer a member
+            return None
+        
+        guild_roles = DiscordRoles(client.guild_roles(guild_id=DISCORD_GUILD_ID))
+        logger.debug('Current guild roles: %s', guild_roles.ids())
+        if 'roles' in member_info:
+            if not guild_roles.has_roles(member_info['roles']):
+                guild_roles = DiscordRoles(
+                    client.guild_roles(guild_id=DISCORD_GUILD_ID, use_cache=False)
+                )
+                if not guild_roles.has_roles(member_info['roles']):
+                    raise RuntimeError(
+                        'Member %s has unknown roles: %s' % (
+                            self.user, 
+                            set(member_info['roles']).difference(guild_roles.ids())
+                        )
+                    )
+            member_roles = guild_roles.subset(member_info['roles'])
         else:
-            logger.warning('Failed to update groups for %s', self.user)
-        return success
+            raise RuntimeError('member_info from %s is not valid' % self.user)
+                
+        requested_roles = match_or_create_roles_from_names(
+            client=client, 
+            guild_id=DISCORD_GUILD_ID, 
+            role_names=DiscordUser.objects.user_group_names(self.user)
+        )
+        logger.debug(
+            'Requested roles for user %s: %s', self.user, requested_roles.ids()
+        )
+        logger.debug('Current roles user %s: %s', self.user, member_roles.ids())        
+        member_roles_managed = member_roles.subset(managed_only=True)
+        if requested_roles != member_roles.difference(member_roles_managed):
+            logger.debug('Need to update roles for user %s', self.user)
+            new_roles = requested_roles.union(member_roles_managed)
+            success = client.modify_guild_member(
+                guild_id=DISCORD_GUILD_ID,
+                user_id=self.uid,
+                role_ids=list(new_roles.ids())
+            )
+            if success:
+                logger.info('Groups for %s have been updated', self.user)
+            else:
+                logger.warning('Failed to update groups for %s', self.user)
+            return success
 
+        else:
+            logger.info('No need to update groups for user %s', self.user)
+            return True
+
+    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
+        self, 
+        notify_user: bool = False, 
+        is_rate_limited: bool = True,
+        handle_api_exceptions: bool = False
     ) -> bool:
         """Deletes the Discount user both on the server and locally
 
@@ -126,6 +195,8 @@ class DiscordUser(models.Model):
         - notify_user: When True will sent a notification to the user 
         informing him about the deleting of his account
         - is_rate_limited: When False will disable default rate limiting (use with care)
+        - handle_api_exceptions: When True method will return False 
+        when an API exception occurs
         
         Returns True when successful, otherwise False or raises exceptions
         Return None if user does no longer exist
@@ -162,18 +233,10 @@ class DiscordUser(models.Model):
                 return False
         
         except (HTTPError, ConnectionError, DiscordApiBackoff) as ex:
-            logger.exception(
-                'Failed to remove user %s from Discord server: %s', self.user, ex
-            )
-            return False        
-
-    @staticmethod
-    def _guild_get_or_create_role_ids(client: DiscordClient, role_names: list) -> list:
-        """wrapper for DiscordClient.match_guild_roles_to_names()
-        that only returns the list of IDs
-        """
-        return [
-            x[0]['id'] for x in client.match_guild_roles_to_names(
-                guild_id=DISCORD_GUILD_ID, role_names=role_names
-            )
-        ]
+            if handle_api_exceptions:
+                logger.exception(
+                    'Failed to remove user %s from Discord server: %s', self.user, ex
+                )
+                return False        
+            else:
+                raise ex

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/templates/services/discord/discord_service_ctrl.html

@@ -15,20 +15,20 @@
     </td>
     <td class="text-center">
         {% if not user_has_account %}
-            <a href="{% url 'discord:activate' %}" title="Activate" class="btn btn-warning">
+            <a href="{% url 'discord:activate' %}" title="{% trans 'Join the Discord server' %}" class="btn btn-primary">
                 <span class="glyphicon glyphicon-ok"></span>
             </a>
         {% else %}
-            <a href="{% url 'discord:reset' %}" title="Reset" class="btn btn-primary">
+            <a href="{% url 'discord:reset' %}" title="{% trans 'Leave- and rejoin the Discord Server (Reset)' %}" class="btn btn-warning">
                 <span class="glyphicon glyphicon-refresh"></span>
             </a>
-            <a href="{% url 'discord:deactivate' %}" title="Deactivate" class="btn btn-danger">
+            <a href="{% url 'discord:deactivate' %}" title="{% trans 'Leave the Discord server' %}" class="btn btn-danger">
                 <span class="glyphicon glyphicon-remove"></span>
             </a>
         {% endif %}
         {% if request.user.is_superuser %}
             <div class="text-center" style="padding-top:5px;">
-                <a type="button" class="btn btn-success" href="{% url 'discord:add_bot' %}">
+                <a type="button" class="btn btn-default" href="{% url 'discord:add_bot' %}">
                     {% trans "Link Discord Server" %}
                 </a>
             </div>

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

@@ -1,17 +1,26 @@
-from django.contrib.auth.models import Group, Permission
+from django.contrib.auth.models import Group
 from allianceauth.tests.auth_utils import AuthUtils
+from ..discord_client.tests import (    # noqa
+    TEST_GUILD_ID,
+    TEST_USER_ID,
+    TEST_USER_NAME,
+    TEST_USER_DISCRIMINATOR,
+    create_role, 
+    ROLE_ALPHA, 
+    ROLE_BRAVO, 
+    ROLE_CHARLIE, 
+    ROLE_MIKE, 
+    create_user_info
+)   
 
 DEFAULT_AUTH_GROUP = 'Member'
 MODULE_PATH = 'allianceauth.services.modules.discord'
 
-TEST_GUILD_ID = 123456789012345678
-TEST_USER_ID = 198765432012345678
-TEST_USER_NAME = 'Peter Parker'
 TEST_MAIN_NAME = 'Spiderman'
 TEST_MAIN_ID = 1005
 
 
 def add_permissions_to_members():
-    permission = Permission.objects.get(codename='access_discord')
+    permission = AuthUtils.get_permission_by_name('discord.access_discord')
     members = Group.objects.get_or_create(name=DEFAULT_AUTH_GROUP)[0]
     AuthUtils.add_permissions_to_groups([permission], [members])

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

@@ -0,0 +1,83 @@
+# flake8: noqa
+
+"""Concurrency testing Discord service tasks
+
+This script will run many Discord service tasks in parallel to test concurrency
+Note that it will run against your main Auth database and not test!
+
+Check allianceauth.log for the results.
+
+To run this test start a bunch of celery workers and then run this script directly.
+Make sure to also set the environment variable AUTH_PROJECT_PATH to your Auth path
+and DJANGO_SETTINGS_MODULE to the relative location of your settings:
+
+Example:
+export AUTH_PROJECT_PATH="/home/erik997/dev/python/aa/allianceauth-dev/myauth"
+export DJANGO_SETTINGS_MODULE="myauth.settings.local"
+
+Careful: This script will utilize all existing Discord users and make changes!
+"""
+# start django project
+import os
+import sys
+if not 'AUTH_PROJECT_PATH' in os.environ:
+    print('AUTH_PROJECT_PATH is not set')
+    exit(1)
+if not 'DJANGO_SETTINGS_MODULE' in os.environ:
+    print('DJANGO_SETTINGS_MODULE is not set')
+    exit(1)
+sys.path.insert(0, os.environ['AUTH_PROJECT_PATH'])
+import django
+django.setup()
+
+# normal imports
+import logging
+from uuid import uuid1
+import random
+
+from django.core.cache import caches
+from django.contrib.auth.models import User, Group
+
+from allianceauth.services.modules.discord.models import DiscordUser
+
+logger = logging.getLogger('allianceauth')
+MAX_RUNS = 3
+
+def clear_cache():
+    default_cache = caches['default']
+    redis = default_cache.get_master_client()
+    redis.flushall()
+    logger.info('Cache flushed')
+
+def run_many_updates(runs):
+    logger.info('Starting piloting_tasks for %d runs', runs)
+    users = list()
+    all_groups = Group.objects.all()
+    for i in range(runs):        
+        if not users:
+            users = list(User.objects.filter(discord__isnull=False))
+        user = users.pop()
+        logger.info('%d/%d: Starting run with user %s', i + 1, runs, user)
+        # force change of nick        
+        new_nick = f'Testnick {uuid1().hex}'[:32]
+        logger.info(
+            '%d/%d: Changing nickname of %s to "%s"', i + 1, runs, user, new_nick
+        )
+        user.profile.main_character.character_name = new_nick
+        user.profile.main_character.save()        
+        
+        # force change of groups
+        user_groups = user.groups.all()        
+        user.groups.remove(random.choice(user_groups))
+        while True:
+            new_group = random.choice(all_groups)
+            if new_group not in user_groups:
+                break
+        logger.info('%d/%d: Adding group "%s" to user %s', i + 1, runs, new_group, user)
+        user.groups.add(new_group)
+
+    logger.info('All %d runs have been started', runs)
+
+if __name__ == "__main__":
+    clear_cache()
+    run_many_updates(MAX_RUNS)

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

@@ -7,7 +7,8 @@ from allianceauth.tests.auth_utils import AuthUtils
 
 from . import TEST_USER_NAME, TEST_USER_ID, add_permissions_to_members, MODULE_PATH
 from ..auth_hooks import DiscordService
-from ..models import DiscordUser, DiscordClient
+from ..discord_client import DiscordClient
+from ..models import DiscordUser
 from ..utils import set_logger_to_file
 
 

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

@@ -1,22 +1,287 @@
-from django_webtest import WebTest
-from unittest.mock import patch
+"""Integration tests
 
+Testing all components of the service, with the exception of the Discord API.
+
+Please note that these tests require Redis and will flush it
+"""
+from collections import namedtuple
+import logging
+from unittest.mock import patch, Mock
+from uuid import uuid1
+
+from django_webtest import WebTest
+from requests.exceptions import HTTPError
+import requests_mock
+
+from django.contrib.auth.models import Group, User
+from django.core.cache import caches
 from django.shortcuts import reverse
+from django.test import TransactionTestCase
+from django.test.utils import override_settings
 
 from allianceauth.tests.auth_utils import AuthUtils
 
 from . import (
-    add_permissions_to_members, 
-    MODULE_PATH, 
-    TEST_USER_NAME,    
-    TEST_MAIN_NAME,
-    TEST_MAIN_ID
+    TEST_GUILD_ID,
+    TEST_USER_NAME, 
+    TEST_USER_ID,
+    TEST_USER_DISCRIMINATOR, 
+    TEST_MAIN_NAME, 
+    TEST_MAIN_ID, 
+    MODULE_PATH,
+    add_permissions_to_members,
+    ROLE_ALPHA,
+    ROLE_BRAVO,
+    ROLE_CHARLIE,
+    ROLE_MIKE,
+    create_role,
+    create_user_info
+)
+from ..discord_client.app_settings import DISCORD_API_BASE_URL
+from ..models import DiscordUser
+
+logger = logging.getLogger('allianceauth')
+
+ROLE_MEMBER = create_role(99, 'Member')
+
+# Putting all requests to Discord into objects so we can compare them better
+DiscordRequest = namedtuple('DiscordRequest', ['method', 'url'])
+user_get_current_request = DiscordRequest(
+    method='GET',
+    url=f'{DISCORD_API_BASE_URL}users/@me'
+)
+guild_infos_request = DiscordRequest(
+    method='GET',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}'
+)
+guild_roles_request = DiscordRequest(
+    method='GET',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/roles'
+)
+create_guild_role_request = DiscordRequest(
+    method='POST',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/roles'
+)
+guild_member_request = DiscordRequest(
+    method='GET',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}'
+)
+add_guild_member_request = DiscordRequest(
+    method='PUT',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}'
+)            
+modify_guild_member_request = DiscordRequest(
+    method='PATCH',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}'
+)            
+remove_guild_member_request = DiscordRequest(
+    method='DELETE',
+    url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}'
 )
 
 
-class TestServiceUserActivation(WebTest):
+def clear_cache():
+    default_cache = caches['default']
+    redis = default_cache.get_master_client()
+    redis.flushall()
+    logger.info('Cache flushed')
+
+
+@patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID)
+@override_settings(CELERY_ALWAYS_EAGER=True)
+@requests_mock.Mocker()
+class TestServiceFeatures(TransactionTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.maxDiff = None
+        
+    def setUp(self):
+        clear_cache()
+        AuthUtils.disconnect_signals()
+        Group.objects.all().delete()
+        User.objects.all().delete()
+        AuthUtils.connect_signals()
+        self.group_3 = Group.objects.create(name='charlie')        
+        self.user = AuthUtils.create_member(TEST_USER_NAME)
+        AuthUtils.add_main_character_2(
+            self.user, 
+            TEST_MAIN_NAME, 
+            TEST_MAIN_ID, 
+            corp_id='2', 
+            corp_name='test_corp', 
+            corp_ticker='TEST',
+            disconnect_signals=True
+        )
+        self.discord_user = DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        add_permissions_to_members()
+        
+    def test_name_of_main_changes(self, requests_mocker):      
+        # modify_guild_member()        
+        requests_mocker.patch(modify_guild_member_request.url, status_code=204)
+        
+        # changing nick to trigger signals
+        new_nick = f'Testnick {uuid1().hex}'[:32]
+        self.user.profile.main_character.character_name = new_nick
+        self.user.profile.main_character.save()
+
+        # Need to have called modify_guild_member two times only
+        # Once for sync nickname
+        # Once for change of main character        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+
+        expected = [modify_guild_member_request, modify_guild_member_request]        
+        self.assertListEqual(requests_made, expected)
+
+    def test_name_of_main_changes_but_user_deleted(self, requests_mocker):      
+        # modify_guild_member()        
+        requests_mocker.patch(
+            modify_guild_member_request.url, status_code=404, json={'code': 10007}
+        )
+        # remove_guild_member()
+        requests_mocker.delete(remove_guild_member_request.url, status_code=204)
+        
+        # changing nick to trigger signals
+        new_nick = f'Testnick {uuid1().hex}'[:32]
+        self.user.profile.main_character.character_name = new_nick
+        self.user.profile.main_character.save()
+
+        # Need to have called modify_guild_member two times only
+        # Once for sync nickname
+        # Once for change of main character        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+
+        expected = [
+            modify_guild_member_request, 
+            remove_guild_member_request,            
+        ]            
+        self.assertListEqual(requests_made, expected)
+        # self.assertFalse(DiscordUser.objects.user_has_account(self.user))
+
+    def test_name_of_main_changes_but_user_rate_limited(
+        self, requests_mocker
+    ):
+        # modify_guild_member()        
+        requests_mocker.patch(modify_guild_member_request.url, status_code=204)
+        
+        # exhausting rate limit
+        client = DiscordUser.objects._bot_client()
+        client._redis.set(
+            name=client._KEY_GLOBAL_RATE_LIMIT_REMAINING,
+            value=0,
+            px=2000
+        )
+        
+        # changing nick to trigger signals
+        new_nick = f'Testnick {uuid1().hex}'[:32]
+        self.user.profile.main_character.character_name = new_nick
+        self.user.profile.main_character.save()
+
+        # should not have called the API
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+        
+        expected = list()  
+        self.assertListEqual(requests_made, expected)
+
+    def test_user_demoted_to_guest(self, requests_mocker):        
+        # remove_guild_member()        
+        requests_mocker.delete(remove_guild_member_request.url, status_code=204)
+        self.user.groups.clear()
+
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+        
+        # compare the list of made requests with expected
+        expected = [remove_guild_member_request]
+        self.assertListEqual(requests_made, expected)
+
+    def test_adding_group_to_user_role_exists(self, requests_mocker):
+        # guild_member()                
+        requests_mocker.get(
+            guild_member_request.url,
+            json={
+                'user': create_user_info(),
+                'roles': ['1', '13', '99']
+            }
+        )        
+        # guild_roles()        
+        requests_mocker.get(
+            guild_roles_request.url, 
+            json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_MEMBER]
+        )        
+        # create_guild_role()        
+        requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE)
+        # modify_guild_member()                
+        requests_mocker.patch(modify_guild_member_request.url, status_code=204)
+                
+        # adding new group to trigger signals
+        self.user.groups.add(self.group_3)        
+        self.user.refresh_from_db()
+        
+        # compare the list of made requests with expected
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+
+        expected = [
+            guild_member_request,
+            guild_roles_request,            
+            modify_guild_member_request
+        ]        
+        self.assertListEqual(requests_made, expected)
+
+    def test_adding_group_to_user_role_does_not_exist(self, requests_mocker):    
+        # guild_member()                
+        requests_mocker.get(
+            guild_member_request.url,
+            json={
+                'user': {'id': str(TEST_USER_ID), 'username': TEST_MAIN_NAME},
+                'roles': ['1', '13', '99']
+            }
+        )        
+        # guild_roles()        
+        requests_mocker.get(
+            guild_roles_request.url, 
+            json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER]
+        )        
+        # create_guild_role()        
+        requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE)
+        # modify_guild_member()                
+        requests_mocker.patch(modify_guild_member_request.url, status_code=204)
+                
+        # adding new group to trigger signals
+        self.user.groups.add(self.group_3)        
+        self.user.refresh_from_db()
+        
+        # compare the list of made requests with expected
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            requests_made.append(DiscordRequest(r.method, r.url))
+
+        expected = [
+            guild_member_request,
+            guild_roles_request,
+            create_guild_role_request,
+            modify_guild_member_request
+        ]        
+        self.assertListEqual(requests_made, expected)
+    
+
+@patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID)
+@patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID)
+@requests_mock.Mocker()
+class TestUserFeatures(WebTest):
     
     def setUp(self):
+        clear_cache()
         self.member = AuthUtils.create_member(TEST_USER_NAME)
         AuthUtils.add_main_character_2(
             self.member, 
@@ -25,15 +290,28 @@ class TestServiceUserActivation(WebTest):
             disconnect_signals=True
         )
         add_permissions_to_members()
-    
-    @patch(MODULE_PATH + '.views.messages')
-    @patch(MODULE_PATH + '.models.DiscordUser.objects.add_user')
+        
+    @patch(MODULE_PATH + '.views.messages')    
     @patch(MODULE_PATH + '.managers.OAuth2Session')
-    def test_user_activation(
-        self, mock_OAuth2Session, mock_add_user, mock_messages
+    def test_user_activation_normal(
+        self, requests_mocker, mock_OAuth2Session, mock_messages
     ): 
-        authentication_code = 'auth_code'
-        mock_add_user.return_value = True
+        # user_get_current()
+        requests_mocker.get(
+            user_get_current_request.url, 
+            json=create_user_info(
+                TEST_USER_ID, TEST_USER_NAME, TEST_USER_DISCRIMINATOR
+            )
+        )        
+        # guild_roles()        
+        requests_mocker.get(
+            guild_roles_request.url, 
+            json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER]
+        )        
+        # add_guild_member()
+        requests_mocker.put(add_guild_member_request.url, status_code=201)
+        
+        authentication_code = 'auth_code'        
         oauth_url = 'https://www.example.com/oauth'
         state = ''
         mock_OAuth2Session.return_value.authorization_url.return_value = \
@@ -54,9 +332,145 @@ class TestServiceUserActivation(WebTest):
         response = self.app.get(
             reverse('discord:callback'), params={'code': authentication_code}
         )
+        
+        # user got a success message
+        self.assertTrue(mock_messages.success.called)
+        self.assertFalse(mock_messages.error.called)
+        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            obj = DiscordRequest(r.method, r.url)            
+            requests_made.append(obj)
+                
+        expected = [
+            user_get_current_request, guild_roles_request, add_guild_member_request
+        ]
+        self.assertListEqual(requests_made, expected)
 
-        # user was added to Discord
-        self.assertTrue(mock_add_user.called)
+    @patch(MODULE_PATH + '.views.messages')    
+    @patch(MODULE_PATH + '.managers.OAuth2Session')
+    def test_user_activation_failed(
+        self, requests_mocker, mock_OAuth2Session, mock_messages
+    ): 
+        # user_get_current()
+        requests_mocker.get(
+            user_get_current_request.url, 
+            json=create_user_info(
+                TEST_USER_ID, TEST_USER_NAME, TEST_USER_DISCRIMINATOR
+            )
+        )        
+        # guild_roles()        
+        requests_mocker.get(
+            guild_roles_request.url, 
+            json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER]
+        )        
+        # add_guild_member()
+        mock_exception = HTTPError('error')
+        mock_exception.response = Mock()
+        mock_exception.response.status_code = 503
+        requests_mocker.put(add_guild_member_request.url, exc=mock_exception)
+        
+        authentication_code = 'auth_code'        
+        oauth_url = 'https://www.example.com/oauth'
+        state = ''
+        mock_OAuth2Session.return_value.authorization_url.return_value = \
+            oauth_url, state
+        
+        # login
+        self.app.set_user(self.member)
+        
+        # click activate on the service page
+        response = self.app.get(reverse('discord:activate'))
+        
+        # check we got a redirect to Discord OAuth        
+        self.assertRedirects(
+            response, expected_url=oauth_url, fetch_redirect_response=False
+        )
+
+        # simulate Discord callback
+        response = self.app.get(
+            reverse('discord:callback'), params={'code': authentication_code}
+        )
+        
+        # user got a success message
+        self.assertFalse(mock_messages.success.called)
+        self.assertTrue(mock_messages.error.called)
+        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            obj = DiscordRequest(r.method, r.url)            
+            requests_made.append(obj)
+                
+        expected = [
+            user_get_current_request, guild_roles_request, add_guild_member_request
+        ]
+        self.assertListEqual(requests_made, expected)
+
+    @patch(MODULE_PATH + '.views.messages')
+    def test_user_deactivation_normal(self, requests_mocker, mock_messages): 
+        # guild_infos()
+        requests_mocker.get(
+            guild_infos_request.url, json={'id': TEST_GUILD_ID, 'name': 'Test Guild'})
+
+        # remove_guild_member()
+        requests_mocker.delete(remove_guild_member_request.url, status_code=204)
+        
+        # user needs have an account
+        DiscordUser.objects.create(user=self.member, uid=TEST_USER_ID)
+        
+        # login
+        self.app.set_user(self.member)
+        
+        # click deactivate on the service page
+        response = self.app.get(reverse('discord:deactivate'))
+        
+        # check we got a redirect to service page
+        self.assertRedirects(response, expected_url=reverse('services:services'))
 
         # user got a success message
         self.assertTrue(mock_messages.success.called)
+        self.assertFalse(mock_messages.error.called)
+        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            obj = DiscordRequest(r.method, r.url)            
+            requests_made.append(obj)
+                
+        expected = [remove_guild_member_request, guild_infos_request]
+        self.assertListEqual(requests_made, expected)
+
+    @patch(MODULE_PATH + '.views.messages')
+    def test_user_deactivation_fails(self, requests_mocker, mock_messages): 
+        # guild_infos()
+        requests_mocker.get(
+            guild_infos_request.url, json={'id': TEST_GUILD_ID, 'name': 'Test Guild'})
+
+        # remove_guild_member()        
+        mock_exception = HTTPError('error')
+        mock_exception.response = Mock()
+        mock_exception.response.status_code = 503
+        requests_mocker.delete(remove_guild_member_request.url, exc=mock_exception)
+        
+        # user needs have an account
+        DiscordUser.objects.create(user=self.member, uid=TEST_USER_ID)
+        
+        # login
+        self.app.set_user(self.member)
+        
+        # click deactivate on the service page
+        response = self.app.get(reverse('discord:deactivate'))
+        
+        # check we got a redirect to service page
+        self.assertRedirects(response, expected_url=reverse('services:services'))
+
+        # user got a success message
+        self.assertFalse(mock_messages.success.called)
+        self.assertTrue(mock_messages.error.called)
+        
+        requests_made = list()
+        for r in requests_mocker.request_history:            
+            obj = DiscordRequest(r.method, r.url)            
+            requests_made.append(obj)
+                
+        expected = [remove_guild_member_request, guild_infos_request]
+        self.assertListEqual(requests_made, expected)

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

@@ -14,8 +14,12 @@ from . import (
     TEST_USER_ID, 
     TEST_MAIN_NAME, 
     TEST_MAIN_ID, 
-    MODULE_PATH
+    MODULE_PATH,
+    ROLE_ALPHA,
+    ROLE_BRAVO,
+    ROLE_CHARLIE
 )
+from ..discord_client.tests import create_matched_role
 from ..app_settings import (
     DISCORD_APP_ID, 
     DISCORD_APP_SECRET,     
@@ -32,7 +36,6 @@ logger = set_logger_to_file(MODULE_PATH + '.managers', __file__)
 @patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID)
 @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
 @patch(MODULE_PATH + '.models.DiscordUser.objects._exchange_auth_code_for_token')
-@patch(MODULE_PATH + '.models.DiscordUser.objects.model._guild_get_or_create_role_ids')
 @patch(MODULE_PATH + '.models.DiscordUser.objects.user_group_names')
 @patch(MODULE_PATH + '.models.DiscordUser.objects.user_formatted_nick')
 class TestAddUser(TestCase):
@@ -50,16 +53,16 @@ class TestAddUser(TestCase):
     def test_can_create_user_no_roles_no_nick(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):
         mock_user_formatted_nick.return_value = None
-        mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = None
+        mock_user_group_names.return_value = []                
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.return_value = True
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -79,16 +82,20 @@ class TestAddUser(TestCase):
         self, 
         mock_user_formatted_nick,
         mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):
-        role_ids = [1, 2, 3]
+        roles = [
+            create_matched_role(ROLE_ALPHA), 
+            create_matched_role(ROLE_BRAVO), 
+            create_matched_role(ROLE_CHARLIE)
+        ]
         mock_user_formatted_nick.return_value = None
         mock_user_group_names.return_value = ['a', 'b', 'c']        
-        mock_guild_get_or_create_role_ids.return_value = role_ids
-        mock_exchange_auth_code_for_token.return_value = self.access_token
+        mock_exchange_auth_code_for_token.return_value = self.access_token        
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = roles
         mock_DiscordClient.return_value.add_guild_member.return_value = True
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -101,7 +108,7 @@ class TestAddUser(TestCase):
         self.assertEqual(kwargs['guild_id'], TEST_GUILD_ID)
         self.assertEqual(kwargs['user_id'], TEST_USER_ID)
         self.assertEqual(kwargs['access_token'], self.access_token)
-        self.assertEqual(kwargs['role_ids'], role_ids)
+        self.assertSetEqual(set(kwargs['role_ids']), {1, 2, 3})
         self.assertIsNone(kwargs['nick'])
 
     @patch(MODULE_PATH + '.managers.DISCORD_SYNC_NAMES', True)
@@ -109,15 +116,15 @@ class TestAddUser(TestCase):
         self, 
         mock_user_formatted_nick,
         mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):        
         mock_user_formatted_nick.return_value = TEST_MAIN_NAME
         mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = []
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.return_value = True
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -137,16 +144,16 @@ class TestAddUser(TestCase):
     def test_can_create_user_no_roles_and_without_nick_if_turned_off(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):        
         mock_user_formatted_nick.return_value = TEST_MAIN_NAME
         mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = []
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.return_value = True
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -165,16 +172,16 @@ class TestAddUser(TestCase):
     def test_can_activate_existing_guild_member(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):
         mock_user_formatted_nick.return_value = None
-        mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = None
+        mock_user_group_names.return_value = []                
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.return_value = None
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -187,16 +194,16 @@ class TestAddUser(TestCase):
     def test_return_false_when_user_creation_fails(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):        
         mock_user_formatted_nick.return_value = None
-        mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = None
+        mock_user_group_names.return_value = []                
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.return_value = False
         
         result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
@@ -209,16 +216,16 @@ class TestAddUser(TestCase):
     def test_return_false_when_on_api_backoff(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):        
         mock_user_formatted_nick.return_value = None
         mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = None
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_DiscordClient.return_value.add_guild_member.side_effect = \
             DiscordApiBackoff(999)
         
@@ -232,16 +239,16 @@ class TestAddUser(TestCase):
     def test_return_false_on_http_error(
         self, 
         mock_user_formatted_nick,
-        mock_user_group_names,        
-        mock_guild_get_or_create_role_ids,
+        mock_user_group_names,                
         mock_exchange_auth_code_for_token, 
         mock_DiscordClient
     ):        
         mock_user_formatted_nick.return_value = None
         mock_user_group_names.return_value = []        
-        mock_guild_get_or_create_role_ids.return_value = None
         mock_exchange_auth_code_for_token.return_value = self.access_token
         mock_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = []
         mock_exception = HTTPError('error')
         mock_exception.response = Mock()
         mock_exception.response.status_code = 500

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

@@ -6,8 +6,19 @@ from django.test import TestCase
 
 from allianceauth.tests.auth_utils import AuthUtils
 
-from . import TEST_USER_NAME, TEST_USER_ID, TEST_MAIN_NAME, TEST_MAIN_ID, MODULE_PATH
+from . import (
+    TEST_USER_NAME, 
+    TEST_USER_ID, 
+    TEST_MAIN_NAME, 
+    TEST_MAIN_ID, 
+    MODULE_PATH,
+    ROLE_ALPHA, 
+    ROLE_BRAVO, 
+    ROLE_CHARLIE, 
+    ROLE_MIKE    
+)
 from ..discord_client import DiscordClient, DiscordApiBackoff
+from ..discord_client.tests import create_matched_role
 from ..models import DiscordUser
 from ..utils import set_logger_to_file
 
@@ -28,36 +39,16 @@ class TestBasicsAndHelpers(TestCase):
         discord_user = DiscordUser.objects.create(user=user, uid=TEST_USER_ID)
         expected = 'DiscordUser(user=\'Peter Parker\', uid=198765432012345678)'
         self.assertEqual(repr(discord_user), expected)
-    
-    def test_guild_get_or_create_role_ids(self):
-        mock_client = Mock(spec=DiscordClient)
-        mock_client.match_guild_roles_to_names.return_value = \
-            [({'id': 1, 'name': 'alpha'}, True), ({'id': 2, 'name': 'bravo'}, True)]
-        
-        result = DiscordUser._guild_get_or_create_role_ids(mock_client, [])
-        excepted = [1, 2]
-        self.assertEqual(set(result), set(excepted))
 
 
 @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 +65,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(
-        self, mock_DiscordClient
-    ):
+    def test_return_none_if_user_no_longer_a_member(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 +82,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): 
-        self.user = AuthUtils.create_user(TEST_USER_NAME)        
+    @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
         )
@@ -148,25 +219,48 @@ class TestDeleteUser(TestCase):
         )
         self.assertTrue(mock_DiscordClient.return_value.remove_guild_member.called)
         self.assertFalse(mock_notify.called)
-
-    def test_return_false_on_api_backoff(self, mock_DiscordClient, mock_notify):
+    
+    def test_raise_exception_on_api_backoff(
+        self, mock_DiscordClient, mock_notify
+    ):
         mock_DiscordClient.return_value.remove_guild_member.side_effect = \
             DiscordApiBackoff(999)
-        result = self.discord_user.delete_user()
+        with self.assertRaises(DiscordApiBackoff):
+            self.discord_user.delete_user()
+        
+    def test_return_false_on_api_backoff_and_exception_handling_on(
+        self, mock_DiscordClient, mock_notify
+    ):
+        mock_DiscordClient.return_value.remove_guild_member.side_effect = \
+            DiscordApiBackoff(999)
+        result = self.discord_user.delete_user(handle_api_exceptions=True)
         self.assertFalse(result)
 
-    def test_return_false_on_http_error(self, mock_DiscordClient, mock_notify):
+    def test_raise_exception_on_http_error(
+        self, mock_DiscordClient, mock_notify
+    ):
         mock_exception = HTTPError('error')
         mock_exception.response = Mock()
         mock_exception.response.status_code = 500
         mock_DiscordClient.return_value.remove_guild_member.side_effect = \
             mock_exception
-        result = self.discord_user.delete_user()
+        
+        with self.assertRaises(HTTPError):
+            self.discord_user.delete_user()        
+
+    def test_return_false_on_http_error_and_exception_handling_on(
+        self, mock_DiscordClient, mock_notify
+    ):
+        mock_exception = HTTPError('error')
+        mock_exception.response = Mock()
+        mock_exception.response.status_code = 500
+        mock_DiscordClient.return_value.remove_guild_member.side_effect = \
+            mock_exception
+        result = self.discord_user.delete_user(handle_api_exceptions=True)
         self.assertFalse(result)
 
 
 @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
-@patch(MODULE_PATH + '.models.DiscordUser._guild_get_or_create_role_ids')
 @patch(MODULE_PATH + '.models.DiscordUser.objects.user_group_names')
 class TestUpdateGroups(TestCase):
 
@@ -175,48 +269,169 @@ class TestUpdateGroups(TestCase):
         self.discord_user = DiscordUser.objects.create(
             user=self.user, uid=TEST_USER_ID
         )
+        self.guild_roles = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE]
+        self.roles_requested = [
+            create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)
+        ]  
         
-    def test_can_update(
+    def test_update_if_needed(
         self, 
-        mock_user_group_names, 
-        mock_guild_get_or_create_role_ids, 
+        mock_user_group_names,         
         mock_DiscordClient
-    ):        
-        roles_requested = [1, 2, 3]
+    ):
+        roles_current = [1]        
         mock_user_group_names.return_value = []
-        mock_guild_get_or_create_role_ids.return_value = roles_requested
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
         mock_DiscordClient.return_value.modify_guild_member.return_value = True
         
         result = self.discord_user.update_groups()
         self.assertTrue(result)
         self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args
+        self.assertEqual(set(kwargs['role_ids']), {1, 2})
+
+    def test_update_if_needed_and_preserve_managed_roles(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):
+        roles_current = [1, 13]          
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        
+        result = self.discord_user.update_groups()
+        self.assertTrue(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args
+        self.assertEqual(set(kwargs['role_ids']), {1, 2, 13})
+
+    def test_dont_update_if_not_needed(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):
+        roles_current = [1, 2, 13]        
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+                
+        result = self.discord_user.update_groups()
+        self.assertTrue(result)
+        self.assertFalse(mock_DiscordClient.return_value.modify_guild_member.called)
+
+    def test_update_if_user_has_no_roles_on_discord(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):
+        roles_current = []        
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        
+        result = self.discord_user.update_groups()
+        self.assertTrue(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args
+        self.assertEqual(set(kwargs['role_ids']), {1, 2})
     
     def test_return_none_if_user_no_longer_a_member(
-        self,
-        mock_user_group_names, 
-        mock_guild_get_or_create_role_ids, 
+        self, 
+        mock_user_group_names,         
         mock_DiscordClient
-    ):        
-        roles_requested = [1, 2, 3]
-        mock_user_group_names.return_value = []
-        mock_guild_get_or_create_role_ids.return_value = roles_requested
-        mock_DiscordClient.return_value.modify_guild_member.return_value = None
+    ):                        
+        mock_DiscordClient.return_value.guild_member.return_value = None
                 
         result = self.discord_user.update_groups()
         self.assertIsNone(result)
-        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+        self.assertFalse(mock_DiscordClient.return_value.modify_guild_member.called)
 
     def test_return_false_if_api_returns_false(
         self, 
-        mock_user_group_names, 
-        mock_guild_get_or_create_role_ids, 
+        mock_user_group_names,         
         mock_DiscordClient
     ):
-        roles_requested = [1, 2, 3]
+        roles_current = [1]        
         mock_user_group_names.return_value = []
-        mock_guild_get_or_create_role_ids.return_value = roles_requested
-        mock_DiscordClient.return_value.modify_guild_member.return_value = False      
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = False
         
         result = self.discord_user.update_groups()
         self.assertFalse(result)
         self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+
+    def test_raise_exception_if_member_has_unknown_roles(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):
+        roles_current = [99]        
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        
+        with self.assertRaises(RuntimeError):
+            self.discord_user.update_groups()
+
+    def test_refresh_guild_roles_user_roles_dont_not_match(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):                
+        def my_guild_roles(guild_id, use_cache=True):
+            if use_cache:
+                return [ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE]
+            else:
+                return [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE]
+        
+        roles_current = [3]
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.side_effect = my_guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'roles': roles_current}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        result = self.discord_user.update_groups()
+        self.assertTrue(result)
+        self.assertEqual(mock_DiscordClient.return_value.guild_roles.call_count, 2)
+
+    def test_raise_exception_if_member_info_is_invalid(
+        self, 
+        mock_user_group_names,         
+        mock_DiscordClient
+    ):        
+        mock_user_group_names.return_value = []
+        mock_DiscordClient.return_value.match_or_create_roles_from_names\
+            .return_value = self.roles_requested
+        mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles
+        mock_DiscordClient.return_value.guild_member.return_value = \
+            {'user': 'dummy'}
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        
+        with self.assertRaises(RuntimeError):
+            self.discord_user.update_groups()

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

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

@@ -7,7 +7,8 @@ from django.urls import reverse
 from allianceauth.tests.auth_utils import AuthUtils
 
 from . import MODULE_PATH, add_permissions_to_members, TEST_USER_NAME, TEST_USER_ID
-from ..models import DiscordUser, DiscordClient
+from ..discord_client import DiscordClient
+from ..models import DiscordUser
 from ..utils import set_logger_to_file
 from ..views import (
     discord_callback, 

allianceauth/services/modules/discord/views.py

@@ -23,7 +23,9 @@ ACCESS_PERM = 'discord.access_discord'
 @permission_required(ACCESS_PERM)
 def deactivate_discord(request):
     logger.debug("deactivate_discord called by user %s", request.user)
-    if request.user.discord.delete_user(is_rate_limited=False):
+    if request.user.discord.delete_user(
+        is_rate_limited=False, handle_api_exceptions=True
+    ):
         logger.info("Successfully deactivated discord for user %s", request.user)
         messages.success(request, _('Deactivated Discord account.'))
     else:
@@ -40,7 +42,9 @@ def deactivate_discord(request):
 @permission_required(ACCESS_PERM)
 def reset_discord(request):
     logger.debug("reset_discord called by user %s", request.user)
-    if request.user.discord.delete_user(is_rate_limited=False):
+    if request.user.discord.delete_user(
+        is_rate_limited=False, handle_api_exceptions=True
+    ):
         logger.info(
             "Successfully deleted discord user for user %s - "
             "forwarding to discord activation.", 

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 changes 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_GUILD_ID = ''
-    DISCORD_CALLBACK_URL = ''
-    DISCORD_APP_ID = ''
-    DISCORD_APP_SECRET = ''
-    DISCORD_BOT_TOKEN = ''
-    DISCORD_SYNC_NAMES = False
+# Discord Configuration
+DISCORD_GUILD_ID = ''
+DISCORD_CALLBACK_URL = ''
+DISCORD_APP_ID = ''
+DISCORD_APP_SECRET = ''
+DISCORD_BOT_TOKEN = ''
+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,52 @@ 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
--- | -- | --
-`DISCORD_APP_ID` | Oauth client ID for the Discord Auth app | `''`
-`DISCORD_APP_SECRET` | Oauth client secret for the Discord Auth app | `''`
-`DISCORD_BOT_TOKEN` | Generated bot token for the Discord Auth app | `''`
-`DISCORD_CALLBACK_URL` | Oauth callback URL | `''`
-`DISCORD_GUILD_ID` | Discord ID of your Discord server | `''`
-`DISCORD_ROLES_CACHE_MAX_AGE` | How long roles retrieved from the Discord server are caches locally in milliseconds | `7200000`
-`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_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`
+```eval_rst
+=================================== ============================================================================================= =======
+Name                                Description                                                                                   Default
+=================================== ============================================================================================= =======
+`DISCORD_APP_ID`                    Oauth client ID for the Discord Auth app                                                      `''`
+`DISCORD_APP_SECRET`                Oauth client secret for the Discord Auth app                                                  `''`
+`DISCORD_BOT_TOKEN`                 Generated bot token for the Discord Auth app                                                  `''`
+`DISCORD_CALLBACK_URL`              Oauth callback URL                                                                            `''`
+`DISCORD_GUILD_ID`                  Discord ID of your Discord server                                                             `''`
+`DISCORD_GUILD_NAME_CACHE_MAX_AGE`  How long the Discord server name is cached locally in milliseconds                            `3600000`
+`DISCORD_ROLES_CACHE_MAX_AGE`       How long roles retrieved from the Discord server are cached locally in milliseconds           `3600000`
+`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
 

tests/settings_all.py

@@ -41,7 +41,15 @@ INSTALLED_APPS += [
 
 ROOT_URLCONF = 'tests.urls'
 
-CACHES['default'] = {'BACKEND': 'django.core.cache.backends.db.DatabaseCache'}
+CACHES = {
+    "default": {
+        "BACKEND": "redis_cache.RedisCache",
+        "LOCATION": "localhost:6379",
+        "OPTIONS": {
+            "DB": 1,
+        }
+    }
+}
 
 ########################
 # XenForo Configuration

tests/settings_core.py

@@ -24,7 +24,15 @@ INSTALLED_APPS += [
 
 ROOT_URLCONF = 'tests.urls'
 
-CACHES['default'] = {'BACKEND': 'django.core.cache.backends.db.DatabaseCache'}
+CACHES = {
+    "default": {
+        "BACKEND": "redis_cache.RedisCache",
+        "LOCATION": "localhost:6379",
+        "OPTIONS": {
+            "DB": 1,
+        }
+    }
+}
 
 PASSWORD_HASHERS = [
     'django.contrib.auth.hashers.MD5PasswordHasher',