Add blacklist for groups and ignore blacklisted roles in Discord service

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

@@ -1,4 +1,5 @@
 from copy import copy
+from typing import Set, Iterable
 
 
 class DiscordRoles:
@@ -39,7 +40,7 @@ class DiscordRoles:
     def __len__(self):
         return len(self._roles.keys())
 
-    def has_roles(self, role_ids: set) -> bool:
+    def has_roles(self, role_ids: Set[int]) -> bool:
         """returns true if this objects contains all roles defined by given role_ids
         incl. managed roles
         """
@@ -47,13 +48,22 @@ class DiscordRoles:
         all_role_ids = self._roles.keys()
         return role_ids.issubset(all_role_ids)
 
-    def ids(self) -> set:
+    def ids(self) -> Set[int]:
         """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
+    def subset(
+        self,
+        role_ids: Iterable[int] = None,
+        managed_only: bool = False,
+        role_names: Iterable[str] = None
+    ) -> "DiscordRoles":
+        """returns a new object containing the subset of roles
+
+        Args:
+        - role_ids: role ids must be in the provided list
+        - managed_only: roles must be managed
+        - role_names: role names must match provided list (not case sensitive)
         """
         if role_ids is not None:
             role_ids = {int(id) for id in role_ids}
@@ -74,15 +84,21 @@ class DiscordRoles:
                 if role_id in role_ids and role['managed']
             ])
 
-        else:
-            return copy(self)
+        elif role_ids is None and managed_only is False and role_names is not None:
+            role_names = {self.sanitize_role_name(name).lower() for name in role_names}
+            return type(self)([
+                role for role in self._roles.values()
+                if role["name"].lower() in role_names
+            ])
 
-    def union(self, other: object) -> object:
+        return copy(self)
+
+    def union(self, other: object) -> "DiscordRoles":
         """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:
+    def difference(self, other: object) -> "DiscordRoles":
         """returns a new roles object that only contains the roles
         that exist in the current objects, but not in other
         """
@@ -94,11 +110,10 @@ class DiscordRoles:
         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()
+        return dict()
 
     @classmethod
-    def create_from_matched_roles(cls, matched_roles: list) -> None:
+    def create_from_matched_roles(cls, matched_roles: list) -> "DiscordRoles":
         """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)
@@ -107,7 +122,7 @@ class DiscordRoles:
         return cls(raw_roles)
 
     @staticmethod
-    def _assert_valid_role(role: dict):
+    def _assert_valid_role(role: dict) -> None:
         if not isinstance(role, dict):
             raise TypeError('Roles must be of type dict: %s' % role)
 

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

@@ -6,7 +6,7 @@ TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY'
 TEST_ROLE_ID = 654321012345678912
 
 
-def create_role(id: int, name: str, managed=False):
+def create_role(id: int, name: str, managed=False) -> dict:
     return {
         'id': int(id),
         'name': str(name),
@@ -21,8 +21,10 @@ def create_matched_role(role, created=False) -> tuple:
 ROLE_ALPHA = create_role(1, 'alpha')
 ROLE_BRAVO = create_role(2, 'bravo')
 ROLE_CHARLIE = create_role(3, 'charlie')
+ROLE_CHARLIE_2 = create_role(4, 'Charlie')  # Discord roles are case sensitive
 ROLE_MIKE = create_role(13, 'mike', True)
 
+
 ALL_ROLES = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE]
 
 

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

@@ -1,6 +1,14 @@
 from unittest import TestCase
 
-from . import ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ALL_ROLES, create_role
+from . import (
+    ROLE_ALPHA,
+    ROLE_BRAVO,
+    ROLE_CHARLIE,
+    ROLE_CHARLIE_2,
+    ROLE_MIKE,
+    ALL_ROLES,
+    create_role
+)
 from .. import DiscordRoles
 
 
@@ -143,6 +151,16 @@ class TestSubset(TestCase):
         expected = {1, 2, 3, 13}
         self.assertSetEqual(roles.ids(), expected)
 
+    def test_should_return_role_names_only(self):
+        # given
+        all_roles = DiscordRoles([
+            ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_CHARLIE_2
+        ])
+        # when
+        roles = all_roles.subset(role_names={"bravo", "charlie"})
+        # then
+        self.assertSetEqual(roles.ids(), {2, 3, 4})
+
 
 class TestHasRoles(TestCase):
 

allianceauth/services/modules/discord/models.py

@@ -6,11 +6,12 @@ from django.contrib.auth.models import User
 from django.db import models
 from django.utils.translation import gettext_lazy
 
+from allianceauth.groupmanagement.models import ReservedGroupName
 from allianceauth.notifications import notify
 
 from . import __title__
 from .app_settings import DISCORD_GUILD_ID
-from .discord_client import DiscordApiBackoff, DiscordRoles
+from .discord_client import DiscordApiBackoff, DiscordClient, DiscordRoles
 from .discord_client.helpers import match_or_create_roles_from_names
 from .managers import DiscordUserManager
 from .utils import LoggerAddTag
@@ -109,11 +110,16 @@ class DiscordUser(models.Model):
         - False on error or raises exception
         """
         client = DiscordUser.objects._bot_client()
+        member_roles = self._determine_member_roles(client)
+        if member_roles is None:
+            return None
+        return self._update_roles_if_needed(client, state_name, member_roles)
+
+    def _determine_member_roles(self, client: DiscordClient) -> DiscordRoles:
+        """Determine the roles of the current member / user."""
         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
-
+            return None  # User is no longer a member
         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:
@@ -128,10 +134,13 @@ class DiscordUser(models.Model):
                             set(member_info['roles']).difference(guild_roles.ids())
                         )
                     )
-            member_roles = guild_roles.subset(member_info['roles'])
-        else:
-            raise RuntimeError('member_info from %s is not valid' % self.user)
+            return guild_roles.subset(member_info['roles'])
+        raise RuntimeError('member_info from %s is not valid' % self.user)
 
+    def _update_roles_if_needed(
+        self, client: DiscordClient, state_name: str, member_roles: DiscordRoles
+    ) -> bool:
+        """Update the roles of this member/user if needed."""
         requested_roles = match_or_create_roles_from_names(
             client=client,
             guild_id=DISCORD_GUILD_ID,
@@ -143,10 +152,13 @@ class DiscordUser(models.Model):
             'Requested roles for user %s: %s', self.user, requested_roles.ids()
         )
         logger.debug('Current roles user %s: %s', self.user, member_roles.ids())
+        reserved_role_names = ReservedGroupName.objects.values_list("name", flat=True)
+        member_roles_reserved = member_roles.subset(role_names=reserved_role_names)
         member_roles_managed = member_roles.subset(managed_only=True)
-        if requested_roles != member_roles.difference(member_roles_managed):
+        member_roles_persistent = member_roles_managed.union(member_roles_reserved)
+        if requested_roles != member_roles.difference(member_roles_persistent):
             logger.debug('Need to update roles for user %s', self.user)
-            new_roles = requested_roles.union(member_roles_managed)
+            new_roles = requested_roles.union(member_roles_persistent)
             success = client.modify_guild_member(
                 guild_id=DISCORD_GUILD_ID,
                 user_id=self.uid,
@@ -157,10 +169,8 @@ class DiscordUser(models.Model):
             else:
                 logger.warning('Failed to update roles for %s', self.user)
             return success
-
-        else:
-            logger.info('No need to update roles for user %s', self.user)
-            return True
+        logger.info('No need to update roles for user %s', self.user)
+        return True
 
     def update_username(self) -> bool:
         """Updates the username incl. the discriminator
@@ -171,7 +181,6 @@ class DiscordUser(models.Model):
         - 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:

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

@@ -9,6 +9,7 @@ from ..discord_client.tests import (    # noqa
     ROLE_ALPHA,
     ROLE_BRAVO,
     ROLE_CHARLIE,
+    ROLE_CHARLIE_2,
     ROLE_MIKE,
     ALL_ROLES,
     create_user_info

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

@@ -5,6 +5,7 @@ from requests.exceptions import HTTPError
 from django.test import TestCase
 
 from allianceauth.tests.auth_utils import AuthUtils
+from allianceauth.groupmanagement.models import ReservedGroupName
 
 from . import (
     TEST_USER_NAME,
@@ -15,7 +16,8 @@ from . import (
     ROLE_ALPHA,
     ROLE_BRAVO,
     ROLE_CHARLIE,
-    ROLE_MIKE
+    ROLE_CHARLIE_2,
+    ROLE_MIKE,
 )
 from ..discord_client import DiscordClient, DiscordApiBackoff
 from ..discord_client.tests import create_matched_role
@@ -294,25 +296,33 @@ class TestUpdateGroups(TestCase):
         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(
+    def test_should_update_and_preserve_managed_and_reserved_roles(
         self,
         mock_user_group_names,
         mock_DiscordClient
     ):
-        roles_current = [1, 13]
+        # given
+        roles_current = [1, 3, 4, 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.guild_roles.return_value = [
+            ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_CHARLIE_2
+        ]
+        mock_DiscordClient.return_value.guild_member.return_value = {
+            'roles': roles_current
+        }
         mock_DiscordClient.return_value.modify_guild_member.return_value = True
-
+        ReservedGroupName.objects.create(
+            name="charlie", reason="dummy", created_by="xyz"
+        )
+        # when
         result = self.discord_user.update_groups()
+        # then
         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})
+        self.assertEqual(set(kwargs['role_ids']), {1, 2, 3, 4, 13})
 
     def test_dont_update_if_not_needed(
         self,