Merge branch 'redundant-sql' into 'master'

Use existing character variable on dashboard.

See merge request allianceauth/allianceauth!1202

.gitignore

@@ -69,3 +69,9 @@ celerybeat-schedule
 
 #gitlab configs
 .gitlab/
+
+#transifex
+.tx/
+
+#other
+.flake8

.gitlab-ci.yml

@@ -6,25 +6,35 @@ before_script:
 - python -V
 - pip install wheel tox
 
-test-3.5:
+test-3.6-core:
-  image: python:3.5-buster
-  script:  
-  - tox -e py35
-
-test-3.6:
   image: python:3.6-buster
   script:  
-  - tox -e py36
+  - tox -e py36-core
 
-test-3.7:
+test-3.7-core:
   image: python:3.7-buster
   script:  
-  - tox -e py37
+  - tox -e py37-core
 
-test-3.8:
+test-3.8-core:
   image: python:3.8-buster
   script:  
-  - tox -e py38
+  - tox -e py38-core
+
+test-3.6-all:
+  image: python:3.6-buster
+  script:  
+  - tox -e py36-all
+
+test-3.7-all:
+  image: python:3.7-buster
+  script:  
+  - tox -e py37-all
+
+test-3.8-all:
+  image: python:3.8-buster
+  script:  
+  - tox -e py38-all
 
 deploy_production:
   stage: deploy

.pylintrc

@@ -0,0 +1,14 @@
+[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
+

.readthedocs.yml

@@ -0,0 +1,27 @@
+# .readthedocs.yml
+# Read the Docs configuration file
+# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details
+
+# Required
+version: 2
+
+# Build documentation in the docs/ directory with Sphinx
+sphinx:
+  configuration: docs/conf.py
+
+# Build documentation with MkDocs
+#mkdocs:
+#  configuration: mkdocs.yml
+
+# Optionally build your docs in additional formats such as PDF and ePub
+formats: all
+
+# Optionally set the version of Python and requirements required to build your docs
+python:
+   version: 3.7
+   install:      
+      - method: pip
+        path: .        
+        extra_requirements:
+           - testing
+   system_packages: true

README.md

@@ -11,32 +11,34 @@
 
 An auth system for EVE Online to help in-game organizations manage online service access.
 
-## Contens
+## Content
 
 - [Overview](#overview)
 - [Documentation](http://allianceauth.rtfd.io)
 - [Support](#support)
 - [Release Notes](https://gitlab.com/allianceauth/allianceauth/-/releases)
-- [Devloper Team](#developer-team)
+- [Developer Team](#developer-team)
 - [Contributing](#contributing)
 
 ## Overview
 
-Alliance Auth (AA) is a web application that helps Eve Online organizations efficiently manage access to their applications and services.
+Alliance Auth (AA) is a web site that helps Eve Online organizations efficiently manage access to applications and services.
 
 Main features:
 
-- Automatically grants or revokes user access to external applications / services (e.g. Discord, Mumble) and web apps (e.g. SRP requests) based on the user's current membership to [in-game organizations](https://allianceauth.readthedocs.io/en/latest/features/states/) and [groups](https://allianceauth.readthedocs.io/en/latest/features/groups/)
+- Automatically grants or revokes user access to external services (e.g. Discord, Mumble) and web apps (e.g. SRP requests) based on the user's current membership to [in-game organizations](https://allianceauth.readthedocs.io/en/latest/features/core/states/) and [groups](https://allianceauth.readthedocs.io/en/latest/features/core/groups/)
 
 - Provides a central web site where users can directly access web apps (e.g. SRP requests, Fleet Schedule) and manage their access to external services and groups.
 
-- Includes a set of connectors (called ["services"](https://allianceauth.readthedocs.io/en/latest/installation/services/)) for integrating access management with many popular external applications / services like Discord, Mumble, Teamspeak 3, SMF and others
+- Includes a set of connectors (called ["services"](https://allianceauth.readthedocs.io/en/latest/features/services/)) for integrating access management with many popular external applications / services like Discord, Mumble, Teamspeak 3, SMF and others
 
-- Includes a set of web apps called ["plug-in apps"](https://allianceauth.readthedocs.io/en/latest/features/) which add many useful functions: fleet schedule, timer board, SRP request management, fleet activity tracker and character application management
+- Includes a set of web [apps](https://allianceauth.readthedocs.io/en/latest/features/apps/) which add many useful functions, e.g.: fleet schedule, timer board, SRP request management, fleet activity tracker
 
-- Can be easily extended with new services and plugin-apps. Many additional services and plugin-apps are provided by the community and can be found here: [Community Creations](https://gitlab.com/allianceauth/community-creations)
+- Can be easily extended with additional services and apps. Many are provided by the community and can be found here: [Community Creations](https://gitlab.com/allianceauth/community-creations)
 
-For further details about AA - including an installation guide and a full list of included services and plugin apps - please see the [offical documentation](http://allianceauth.rtfd.io).
+- Chinese :cn:, English :us:, German :de: and Spanish :es: localization
+
+For further details about AA - including an installation guide and a full list of included services and plugin apps - please see the [official documentation](http://allianceauth.rtfd.io).
 
 ## Screenshot
 

allianceauth/__init__.py

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

allianceauth/authentication/admin.py

@@ -18,14 +18,14 @@ from django.utils.text import slugify
 from allianceauth.authentication.models import State, get_guest_state,\
     CharacterOwnership, UserProfile, OwnershipRecord
 from allianceauth.hooks import get_hooks
-from allianceauth.eveonline.models import EveCharacter, EveCorporationInfo
+from allianceauth.eveonline.models import EveCharacter, EveCorporationInfo,\
+    EveAllianceInfo
 from allianceauth.eveonline.tasks import update_character
 from .app_settings import AUTHENTICATION_ADMIN_USERS_MAX_GROUPS, \
     AUTHENTICATION_ADMIN_USERS_MAX_CHARS
 
 if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS:
     _has_auto_groups = True    
-    from allianceauth.eveonline.autogroups.models import *
 else:
     _has_auto_groups = False
 
@@ -37,8 +37,11 @@ def make_service_hooks_update_groups_action(service):
     :return: fn to update services groups for the selected users
     """
     def update_service_groups(modeladmin, request, queryset):
-        for user in queryset:  # queryset filtering doesn't work here?
+        if hasattr(service, 'update_groups_bulk'):
-            service.update_groups(user)
+            service.update_groups_bulk(queryset)
+        else:
+            for user in queryset:  # queryset filtering doesn't work here?
+                service.update_groups(user)
 
     update_service_groups.__name__ = str('update_{}_groups'.format(slugify(service.name)))
     update_service_groups.short_description = "Sync groups for selected {} accounts".format(service.title)
@@ -52,8 +55,11 @@ def make_service_hooks_sync_nickname_action(service):
     :return: fn to sync nickname for the selected users
     """
     def sync_nickname(modeladmin, request, queryset):
-        for user in queryset:  # queryset filtering doesn't work here?
+        if hasattr(service, 'sync_nicknames_bulk'):
-            service.sync_nickname(user)
+            service.sync_nicknames_bulk(queryset)
+        else:
+            for user in queryset:  # queryset filtering doesn't work here?
+                service.sync_nickname(user)
 
     sync_nickname.__name__ = str('sync_{}_nickname'.format(slugify(service.name)))
     sync_nickname.short_description = "Sync nicknames for selected {} accounts".format(service.title)
@@ -241,6 +247,22 @@ class MainAllianceFilter(admin.SimpleListFilter):
                         self.value())
                 
 
+def update_main_character_model(modeladmin, request, queryset):    
+    tasks_count = 0
+    for obj in queryset:
+        if obj.profile.main_character:
+            update_character.delay(obj.profile.main_character.character_id)
+            tasks_count += 1
+
+    modeladmin.message_user(
+        request, 
+        'Update from ESI started for {} characters'.format(tasks_count)
+    )
+
+update_main_character_model.short_description = \
+    'Update main character model from ESI'
+
+
 class UserAdmin(BaseUserAdmin):
     """Extending Django's UserAdmin model
     
@@ -272,28 +294,13 @@ class UserAdmin(BaseUserAdmin):
             else:    
                 return queryset.filter(groups__pk=self.value())
 
-    def update_main_character_model(self, request, queryset):    
-        tasks_count = 0
-        for obj in queryset:
-            if obj.profile.main_character:
-                update_character.delay(obj.profile.main_character.character_id)
-                tasks_count += 1
-
-        self.message_user(
-            request, 
-            'Update from ESI started for {} characters'.format(tasks_count)
-        )
-
-    update_main_character_model.short_description = \
-        'Update main character model from ESI'
-
     def get_actions(self, request):
         actions = super(BaseUserAdmin, self).get_actions(request)
 
-        actions[self.update_main_character_model.__name__] = (
+        actions[update_main_character_model.__name__] = (
-            self.update_main_character_model, 
+            update_main_character_model, 
-            self.update_main_character_model.__name__, 
+            update_main_character_model.__name__, 
-            self.update_main_character_model.short_description
+            update_main_character_model.short_description
         )
 
         for hook in get_hooks('services_hook'):
@@ -505,7 +512,7 @@ class BaseOwnershipAdmin(admin.ModelAdmin):
         'character',
     )
     search_fields = (
-        'user__user', 
+        'user__username', 
         'character__character_name', 
         'character__corporation_name', 
         'character__alliance_name'

allianceauth/authentication/templates/authentication/dashboard.html

@@ -94,12 +94,12 @@
             <div class="col-sm-6 text-center">
                 <div class="panel panel-success" style="height:100%">
                     <div class="panel-heading">
-                        <h3 class="panel-title">{% trans "Groups" %}</h3>
+                        <h3 class="panel-title">{% trans "Group Memberships" %}</h3>
                     </div>
                     <div class="panel-body">
                         <div style="height: 240px;overflow:-moz-scrollbars-vertical;overflow-y:auto;">
                             <table class="table table-aa">
-                                {% for group in user.groups.all %}
+                                {% for group in groups %}
                                     <tr>
                                         <td>{{ group.name }}</td>
                                     </tr>
@@ -128,34 +128,30 @@
                         </tr>
                     </thead>
                     <tbody>
-                        {% for ownership in request.user.character_ownerships.all %}
+                        {% for char in characters %}                            
-                            {% with ownership.character as char %}
+                            <tr>
-                                <tr>
+                                <td class="text-center"><img class="ra-avatar img-circle" src="{{ char.portrait_url_32 }}">
-                                    <td class="text-center"><img class="ra-avatar img-circle" src="{{ char.portrait_url_32 }}">
+                                </td>
-                                    </td>
+                                <td class="text-center">{{ char.character_name }}</td>
-                                    <td class="text-center">{{ char.character_name }}</td>
+                                <td class="text-center">{{ char.corporation_name }}</td>
-                                    <td class="text-center">{{ char.corporation_name }}</td>
+                                <td class="text-center">{{ char.alliance_name }}</td>
-                                    <td class="text-center">{{ char.alliance_name }}</td>
+                            </tr>                        
-                                </tr>
-                            {% endwith %}
                         {% endfor %}
                     </tbody>
                 </table>
                 <table class="table table-aa visible-xs-block" style="width: 100%">
                     <tbody>
-                        {% for ownership in request.user.character_ownerships.all %}
+                        {% for char in characters %}                            
-                            {% with ownership.character as char %}
+                            <tr>
-                                <tr>
+                                <td class="text-center" style="vertical-align: middle">
-                                    <td class="text-center" style="vertical-align: middle">
+                                    <img class="ra-avatar img-circle" src="{{ char.portrait_url_32 }}">
-                                        <img class="ra-avatar img-circle" src="{{ char.portrait_url_32 }}">
+                                </td>
-                                    </td>
+                                <td class="text-center" style="vertical-align: middle; width: 100%">
-                                    <td class="text-center" style="vertical-align: middle; width: 100%">
+                                    <strong>{{ char.character_name }}</strong><br>
-                                        <strong>{{ char.character_name }}</strong><br>
+                                    {{ char.corporation_name }}<br>
-                                        {{ char.corporation_name }}<br>
+                                    {{ char.alliance_name|default:"" }}
-                                        {{ char.alliance_name|default:"" }}
+                                </td> 
-                                    </td> 
+                            </tr>
-                                </tr>
-                            {% endwith %}
                         {% endfor %}
                     </tbody>
                 </table>

allianceauth/authentication/templates/public/base.html

@@ -6,6 +6,10 @@
         <meta name="viewport" content="width=device-width, initial-scale=1">
         <meta name="description" content="">
         <meta name="author" content="">
+        <meta property="og:title" content="{{ SITE_NAME }}">
+        <meta property="og:image" content="{{ request.scheme }}://{{ request.get_host }}{% static 'icons/apple-touch-icon.png' %}">
+        <meta property="og:description" content="Alliance Auth - An auth system for EVE Online to help in-game organizations manage online service access.">
+
         {% include 'allianceauth/icons.html' %}
 
         <title>{% block title %}{{ SITE_NAME }}{% endblock %}</title>

allianceauth/authentication/templates/registration/password_reset_email.html

@@ -1,5 +1,5 @@
 {% load i18n %}{% autoescape off %}
-    {% blocktrans %}You're receiving this email because you requested a password reset for your
+    {% blocktrans trimmed %}You're receiving this email because you requested a password reset for your
         user account.{% endblocktrans %}
 
     {% trans "Please go to the following page and choose a new password:" %}

allianceauth/authentication/tests/__init__.py

@@ -0,0 +1,18 @@
+from django.urls import reverse
+
+
+def get_admin_change_view_url(obj: object) -> str:
+    """returns URL to admin change view for given object"""
+    return reverse(
+        'admin:{}_{}_change'.format(
+            obj._meta.app_label, type(obj).__name__.lower()
+        ),
+        args=(obj.pk,)
+    )
+
+def get_admin_search_url(ModelClass: type) -> str:
+    """returns URL to search URL for model of given object"""
+    return '{}{}/'.format(
+        reverse('admin:app_list', args=(ModelClass._meta.app_label,)),
+        ModelClass.__name__.lower()
+    )

allianceauth/authentication/tests/test_admin.py

@@ -1,43 +1,64 @@
-from unittest.mock import patch
+from urllib.parse import quote
+from unittest.mock import patch, MagicMock
 
-from django.test import TestCase, RequestFactory
+from django.conf import settings
 from django.contrib import admin
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import User as BaseUser, Group
+from django.test import TestCase, RequestFactory, Client
 
-from allianceauth.authentication.models import CharacterOwnership, State
+from allianceauth.authentication.models import (
-from allianceauth.eveonline.autogroups.models import AutogroupsConfig
+    CharacterOwnership, State, OwnershipRecord
+)
 from allianceauth.eveonline.models import (
     EveCharacter, EveCorporationInfo, EveAllianceInfo
 )
+from allianceauth.services.hooks import ServicesHook
+from allianceauth.tests.auth_utils import AuthUtils
 
 from ..admin import (
     BaseUserAdmin,
+    CharacterOwnershipAdmin,
+    PermissionAdmin,
+    StateAdmin,
     MainCorporationsFilter,
     MainAllianceFilter,
+    OwnershipRecordAdmin,
     User,
     UserAdmin,     
     user_main_organization,
     user_profile_pic, 
     user_username,
+    update_main_character_model,
+    make_service_hooks_update_groups_action,
+    make_service_hooks_sync_nickname_action
 )
+from . import get_admin_change_view_url, get_admin_search_url
 
+if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS:
+    _has_auto_groups = True
+    from allianceauth.eveonline.autogroups.models import AutogroupsConfig
+else:
+    _has_auto_groups = False
 
 MODULE_PATH = 'allianceauth.authentication.admin'
 
 
 class MockRequest(object):    
-    
     def __init__(self, user=None):
         self.user = user
 
-
+class TestCaseWithTestData(TestCase):
-class TestUserAdmin(TestCase):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
 
+        for MyModel in [
+            EveAllianceInfo, EveCorporationInfo, EveCharacter, Group, User
+        ]:
+            MyModel.objects.all().delete()
+        
         # groups
         cls.group_1 = Group.objects.create(
             name='Group 1'
@@ -47,7 +68,7 @@ class TestUserAdmin(TestCase):
         )
 
         # user 1 - corp and alliance, normal user
-        cls.character_1 = EveCharacter.objects.create(
+        character_1 = EveCharacter.objects.create(
             character_id='1001',
             character_name='Bruce Wayne',
             corporation_id='2001',
@@ -57,7 +78,7 @@ class TestUserAdmin(TestCase):
             alliance_name='Wayne Enterprises',
             alliance_ticker='WE',
         )
-        cls.character_1a = EveCharacter.objects.create(
+        character_1a = EveCharacter.objects.create(
             character_id='1002',
             character_name='Batman',
             corporation_id='2001',
@@ -81,26 +102,26 @@ class TestUserAdmin(TestCase):
             alliance=alliance
         )        
         cls.user_1 = User.objects.create_user(
-            cls.character_1.character_name.replace(' ', '_'),
+            character_1.character_name.replace(' ', '_'),
             'abc@example.com',
             'password'
         )
         CharacterOwnership.objects.create(
-            character=cls.character_1,
+            character=character_1,
-            owner_hash='x1' + cls.character_1.character_name,
+            owner_hash='x1' + character_1.character_name,
             user=cls.user_1
         )
         CharacterOwnership.objects.create(
-            character=cls.character_1a,
+            character=character_1a,
-            owner_hash='x1' + cls.character_1a.character_name,
+            owner_hash='x1' + character_1a.character_name,
             user=cls.user_1
         )
-        cls.user_1.profile.main_character = cls.character_1
+        cls.user_1.profile.main_character = character_1
         cls.user_1.profile.save()
         cls.user_1.groups.add(cls.group_1)        
 
         # user 2 - corp only, staff
-        cls.character_2 = EveCharacter.objects.create(
+        character_2 = EveCharacter.objects.create(
             character_id=1003,
             character_name='Clark Kent',
             corporation_id=2002,
@@ -116,23 +137,23 @@ class TestUserAdmin(TestCase):
             alliance=None
         )
         cls.user_2 = User.objects.create_user(
-            cls.character_2.character_name.replace(' ', '_'),
+            character_2.character_name.replace(' ', '_'),
             'abc@example.com',
             'password'
         )
         CharacterOwnership.objects.create(
-            character=cls.character_2,
+            character=character_2,
-            owner_hash='x1' + cls.character_2.character_name,
+            owner_hash='x1' + character_2.character_name,
             user=cls.user_2
         )
-        cls.user_2.profile.main_character = cls.character_2
+        cls.user_2.profile.main_character = character_2
         cls.user_2.profile.save()
         cls.user_2.groups.add(cls.group_2)
         cls.user_2.is_staff = True
         cls.user_2.save()
         
         # user 3 - no main, no group, superuser
-        cls.character_3 = EveCharacter.objects.create(
+        character_3 = EveCharacter.objects.create(
             character_id=1101,
             character_name='Lex Luthor',
             corporation_id=2101,
@@ -154,34 +175,130 @@ class TestUserAdmin(TestCase):
             executor_corp_id=''
         )
         cls.user_3 = User.objects.create_user(
-            cls.character_3.character_name.replace(' ', '_'),
+            character_3.character_name.replace(' ', '_'),
             'abc@example.com',
             'password'
         )
         CharacterOwnership.objects.create(
-            character=cls.character_3,
+            character=character_3,
-            owner_hash='x1' + cls.character_3.character_name,
+            owner_hash='x1' + character_3.character_name,
             user=cls.user_3
         )
         cls.user_3.is_superuser = True
         cls.user_3.save()
 
+
+def make_generic_search_request(ModelClass: type, search_term: str):
+    User.objects.create_superuser(
+        username='superuser', password='secret', email='admin@example.com'
+    )
+    c = Client()
+    c.login(username='superuser', password='secret')    
+    return c.get(
+        '%s?q=%s' % (get_admin_search_url(ModelClass), quote(search_term))
+    )    
+
+
+class TestCharacterOwnershipAdmin(TestCaseWithTestData):
+    
+    def setUp(self):
+        self.modeladmin = CharacterOwnershipAdmin(
+            model=User, admin_site=AdminSite()
+        )
+
+    def test_change_view_loads_normally(self):        
+        User.objects.create_superuser(
+            username='superuser', password='secret', email='admin@example.com'
+        )
+        c = Client()
+        c.login(username='superuser', password='secret')                
+        ownership = self.user_1.character_ownerships.first()
+        response = c.get(get_admin_change_view_url(ownership))
+        self.assertEqual(response.status_code, 200)
+
+    def test_search_works(self):
+        obj = CharacterOwnership.objects\
+            .filter(user=self.user_1)\
+            .first()
+        response = make_generic_search_request(type(obj), obj.user.username)
+        expected = 200
+        self.assertEqual(response.status_code, expected)
+
+
+class TestOwnershipRecordAdmin(TestCaseWithTestData):
+    
+    def setUp(self):
+        self.modeladmin = OwnershipRecordAdmin(
+            model=User, admin_site=AdminSite()
+        )
+
+    def test_change_view_loads_normally(self):        
+        User.objects.create_superuser(
+            username='superuser', password='secret', email='admin@example.com'
+        )
+        c = Client()
+        c.login(username='superuser', password='secret')                
+        ownership_record = OwnershipRecord.objects\
+            .filter(user=self.user_1)\
+            .first()
+        response = c.get(get_admin_change_view_url(ownership_record))
+        self.assertEqual(response.status_code, 200)
+
+    def test_search_works(self):
+        obj = OwnershipRecord.objects.first()
+        response = make_generic_search_request(type(obj), obj.user.username)
+        expected = 200
+        self.assertEqual(response.status_code, expected)
+
+
+class TestStateAdmin(TestCaseWithTestData):
+        
+    def setUp(self):
+        self.modeladmin = StateAdmin(
+            model=User, admin_site=AdminSite()
+        )
+
+    def test_change_view_loads_normally(self):        
+        User.objects.create_superuser(
+            username='superuser', password='secret', email='admin@example.com'
+        )
+        c = Client()
+        c.login(username='superuser', password='secret')                
+        
+        guest_state = AuthUtils.get_guest_state()
+        response = c.get(get_admin_change_view_url(guest_state))
+        self.assertEqual(response.status_code, 200)
+        
+        member_state = AuthUtils.get_member_state()
+        response = c.get(get_admin_change_view_url(member_state))
+        self.assertEqual(response.status_code, 200)
+
+    def test_search_works(self):
+        obj = State.objects.first()
+        response = make_generic_search_request(type(obj), obj.name)
+        expected = 200
+        self.assertEqual(response.status_code, expected)
+
+class TestUserAdmin(TestCaseWithTestData):
+
     def setUp(self):
         self.factory = RequestFactory()
         self.modeladmin = UserAdmin(
             model=User, admin_site=AdminSite()
         )
+        self.character_1 = self.user_1.character_ownerships.first().character
 
     def _create_autogroups(self):
         """create autogroups for corps and alliances"""
-        autogroups_config = AutogroupsConfig(
+        if _has_auto_groups:
-            corp_groups = True,
+            autogroups_config = AutogroupsConfig(
-            alliance_groups = True
+                corp_groups = True,
-        )
+                alliance_groups = True
-        autogroups_config.save()
+            )
-        for state in State.objects.all():
+            autogroups_config.save()
-            autogroups_config.states.add(state)        
+            for state in State.objects.all():
-        autogroups_config.update_corp_group_membership(self.user_1)        
+                autogroups_config.states.add(state)        
+            autogroups_config.update_corp_group_membership(self.user_1)        
     
     # column rendering
 
@@ -315,8 +432,8 @@ class TestUserAdmin(TestCase):
         self, mock_task, mock_message_user
     ):
         users_qs = User.objects.filter(pk__in=[self.user_1.pk, self.user_2.pk])
-        self.modeladmin.update_main_character_model(
+        update_main_character_model(
-            MockRequest(self.user_1), users_qs
+            self.modeladmin, MockRequest(self.user_1), users_qs
         )
         self.assertEqual(mock_task.delay.call_count, 2)
         self.assertTrue(mock_message_user.called)
@@ -437,3 +554,82 @@ class TestUserAdmin(TestCase):
         queryset = changelist.get_queryset(request)
         expected = [self.user_1]
         self.assertSetEqual(set(queryset), set(expected))
+
+    def test_change_view_loads_normally(self):
+        User.objects.create_superuser(
+            username='superuser', password='secret', email='admin@example.com'
+        )
+        c = Client()
+        c.login(username='superuser', password='secret')                
+        response = c.get(get_admin_change_view_url(self.user_1))
+        self.assertEqual(response.status_code, 200)
+
+    def test_search_works(self):
+        obj = User.objects.first()
+        response = make_generic_search_request(type(obj), obj.username)
+        expected = 200
+        self.assertEqual(response.status_code, expected)
+
+
+class TestMakeServicesHooksActions(TestCaseWithTestData):
+
+    class MyServicesHookTypeA(ServicesHook):
+
+        def __init__(self):
+            super().__init__()
+            self.name = 'My Service A'            
+        
+        def update_groups(self, user):
+            pass
+
+        def sync_nicknames(self, user):
+            pass
+
+    class MyServicesHookTypeB(ServicesHook):
+
+        def __init__(self):
+            super().__init__()
+            self.name = 'My Service B'
+        
+        def update_groups(self, user):
+            pass
+
+        def update_groups_bulk(self, user):
+            pass
+
+        def sync_nicknames(self, user):
+            pass
+
+        def sync_nicknames_bulk(self, user):
+            pass
+         
+
+    def test_service_has_update_groups_only(self):        
+        service = self.MyServicesHookTypeA()
+        mock_service = MagicMock(spec=service)      
+        action = make_service_hooks_update_groups_action(mock_service)
+        action(MagicMock(), MagicMock(), [self.user_1])
+        self.assertTrue(mock_service.update_groups.called)
+
+    def test_service_has_update_groups_bulk(self):        
+        service = self.MyServicesHookTypeB()
+        mock_service = MagicMock(spec=service)      
+        action = make_service_hooks_update_groups_action(mock_service)
+        action(MagicMock(), MagicMock(), [self.user_1])
+        self.assertFalse(mock_service.update_groups.called)
+        self.assertTrue(mock_service.update_groups_bulk.called)
+
+    def test_service_has_sync_nickname_only(self):        
+        service = self.MyServicesHookTypeA()
+        mock_service = MagicMock(spec=service)      
+        action = make_service_hooks_sync_nickname_action(mock_service)
+        action(MagicMock(), MagicMock(), [self.user_1])
+        self.assertTrue(mock_service.sync_nickname.called)
+
+    def test_service_has_sync_nicknames_bulk(self):        
+        service = self.MyServicesHookTypeB()
+        mock_service = MagicMock(spec=service)      
+        action = make_service_hooks_sync_nickname_action(mock_service)
+        action(MagicMock(), MagicMock(), [self.user_1])
+        self.assertFalse(mock_service.sync_nickname.called)
+        self.assertTrue(mock_service.sync_nicknames_bulk.called)

allianceauth/authentication/tests/test_app_settings.py

@@ -1,11 +1,11 @@
 from unittest.mock import Mock, patch
-
 from django.test import TestCase
 
 from .. import app_settings
 
 MODULE_PATH = 'allianceauth.authentication'
 
+
 class TestSetAppSetting(TestCase):
 
     @patch(MODULE_PATH + '.app_settings.settings')
@@ -17,7 +17,6 @@ class TestSetAppSetting(TestCase):
         )
         self.assertEqual(result, False)
 
-
     @patch(MODULE_PATH + '.app_settings.settings')
     def test_default_if_not_set_for_none(self, mock_settings):        
         mock_settings.TEST_SETTING_DUMMY = Mock(spec=None)
@@ -28,7 +27,6 @@ class TestSetAppSetting(TestCase):
         )
         self.assertEqual(result, None)
 
-
     @patch(MODULE_PATH + '.app_settings.settings')
     def test_true_stays_true(self, mock_settings):
         mock_settings.TEST_SETTING_DUMMY = True
@@ -56,7 +54,6 @@ class TestSetAppSetting(TestCase):
         )
         self.assertEqual(result, False)
 
-
     @patch(MODULE_PATH + '.app_settings.settings')
     def test_default_for_invalid_type_int(self, mock_settings):
         mock_settings.TEST_SETTING_DUMMY = 'invalid type'
@@ -95,7 +92,6 @@ class TestSetAppSetting(TestCase):
         )
         self.assertEqual(result, 50)
 
-    
     @patch(MODULE_PATH + '.app_settings.settings')
     def test_default_is_none_needs_required_type(self, mock_settings):
         mock_settings.TEST_SETTING_DUMMY = 'invalid type'
@@ -104,5 +100,3 @@ class TestSetAppSetting(TestCase):
                 'TEST_SETTING_DUMMY',             
                 default_value=None
             )
-        
-

allianceauth/authentication/urls.py

@@ -7,11 +7,21 @@ from . import views
 app_name = 'authentication'
 
 urlpatterns = [
-    url(r'^$', login_required(TemplateView.as_view(template_name='authentication/dashboard.html')),),
+    url(r'^$', views.index, name='index'),
-    url(r'^account/login/$', TemplateView.as_view(template_name='public/login.html'), name='login'),
+    url(
-    url(r'^account/characters/main/$', views.main_character_change, name='change_main_character'),
+        r'^account/login/$', 
-    url(r'^account/characters/add/$', views.add_character, name='add_character'),
+        TemplateView.as_view(template_name='public/login.html'), 
-    url(r'^help/$', login_required(TemplateView.as_view(template_name='allianceauth/help.html')), name='help'),
+        name='login'
-    url(r'^dashboard/$',
+    ),
-        login_required(TemplateView.as_view(template_name='authentication/dashboard.html')), name='dashboard'),
+    url(
+        r'^account/characters/main/$',
+        views.main_character_change,
+        name='change_main_character'
+    ),
+    url(
+        r'^account/characters/add/$', 
+        views.add_character, 
+        name='add_character'
+    ),   
+    url(r'^dashboard/$', views.dashboard, name='dashboard'),
 ]

allianceauth/authentication/views.py

@@ -7,20 +7,58 @@ from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.core import signing
 from django.urls import reverse
-from django.shortcuts import redirect
+from django.shortcuts import redirect, render
-from django.utils.translation import ugettext_lazy as _
+from django.utils.translation import gettext_lazy as _
+
+from allianceauth.eveonline.models import EveCharacter
 from esi.decorators import token_required
 from esi.models import Token
-from registration.backends.hmac.views import RegistrationView as BaseRegistrationView, \
+
-    ActivationView as BaseActivationView, REGISTRATION_SALT
+from registration.backends.hmac.views import (
+    RegistrationView as BaseRegistrationView, 
+    ActivationView as BaseActivationView, 
+    REGISTRATION_SALT
+)
 from registration.signals import user_registered
 
 from .models import CharacterOwnership
 from .forms import RegistrationForm
 
+if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS:
+    _has_auto_groups = True
+    from allianceauth.eveonline.autogroups.models import *
+else:
+    _has_auto_groups = False
+
+
 logger = logging.getLogger(__name__)
 
 
+@login_required
+def index(request):
+    return redirect('authentication:dashboard')
+
+
+@login_required
+def dashboard(request):
+    groups = request.user.groups.all()
+    if _has_auto_groups:
+        groups = groups\
+            .filter(managedalliancegroup__isnull=True)\
+            .filter(managedcorpgroup__isnull=True)
+    groups = groups.order_by('name')
+    characters = EveCharacter.objects\
+        .filter(character_ownership__user=request.user)\
+        .select_related()\
+        .order_by('character_name')
+        
+    context = {
+        'groups': groups,
+        'characters': characters
+    }
+    return render(request, 'authentication/dashboard.html', context)
+
+
 @login_required
 @token_required(scopes=settings.LOGIN_TOKEN_SCOPES)
 def main_character_change(request, token):
@@ -31,7 +69,10 @@ def main_character_change(request, token):
         if not CharacterOwnership.objects.filter(character__character_id=token.character_id).exists():
             co = CharacterOwnership.objects.create_by_token(token)
         else:
-            messages.error(request, 'Cannot change main character to %(char)s: character owned by a different account.' % ({'char': token.character_name}))
+            messages.error(
+                request, 
+                _('Cannot change main character to %(char)s: character owned by a different account.') % ({'char': token.character_name})
+            )
             co = None
     if co:
         request.user.profile.main_character = co.character

allianceauth/eveonline/tasks.py

@@ -7,6 +7,7 @@ from .models import EveCorporationInfo
 
 logger = logging.getLogger(__name__)
 
+TASK_PRIORITY = 7
 
 @shared_task
 def update_corp(corp_id):
@@ -27,11 +28,12 @@ def update_character(character_id):
 def run_model_update():
     # update existing corp models
     for corp in EveCorporationInfo.objects.all().values('corporation_id'):
-        update_corp.delay(corp['corporation_id'])
+        update_corp.apply_async(args=[corp['corporation_id']], priority=TASK_PRIORITY)
 
     # update existing alliance models
     for alliance in EveAllianceInfo.objects.all().values('alliance_id'):
-        update_alliance.delay(alliance['alliance_id'])
+        update_alliance.apply_async(args=[alliance['alliance_id']], priority=TASK_PRIORITY)
 
+    #update existing character models
     for character in EveCharacter.objects.all().values('character_id'):
-        update_character.delay(character['character_id'])
+        update_character.apply_async(args=[character['character_id']], priority=TASK_PRIORITY)

allianceauth/eveonline/tests/test_tasks.py

@@ -80,28 +80,28 @@ class TestTasks(TestCase):
             character_name='character.name',
             corporation_id='character.corp.id',
             corporation_name='character.corp.name',
-            corporation_ticker='character.corp.ticker',
+            corporation_ticker='c.c.t',  # max 5 chars 
             alliance_id='character.alliance.id',
             alliance_name='character.alliance.name',
         )
 
         run_model_update()
 
-        self.assertEqual(mock_update_corp.delay.call_count, 1)
+        self.assertEqual(mock_update_corp.apply_async.call_count, 1)
         self.assertEqual(
-            int(mock_update_corp.delay.call_args[0][0]), 
+            int(mock_update_corp.apply_async.call_args[1]['args'][0]), 
             2345
         )
         
-        self.assertEqual(mock_update_alliance.delay.call_count, 1)
+        self.assertEqual(mock_update_alliance.apply_async.call_count, 1)
         self.assertEqual(
-            int(mock_update_alliance.delay.call_args[0][0]), 
+            int(mock_update_alliance.apply_async.call_args[1]['args'][0]), 
             3456
         )
         
-        self.assertEqual(mock_update_character.delay.call_count, 1)
+        self.assertEqual(mock_update_character.apply_async.call_count, 1)
         self.assertEqual(
-            int(mock_update_character.delay.call_args[0][0]), 
+            int(mock_update_character.apply_async.call_args[1]['args'][0]), 
             1234
         )
 

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html

@@ -15,7 +15,13 @@
             </div>
             {% endif %}
         </h1>
-        <h2>{% blocktrans %}{{ user }} has collected {{ n_fats }} link{{ n_fats|pluralize }} this month.{% endblocktrans %}</h2>
+        <h2>
+            {% blocktrans count links=n_fats trimmed %}
+                {{ user }} has collected one link this month.
+            {% plural %}
+                {{ user }} has collected {{ links }} links this month.
+            {% endblocktrans %}
+        </h2>
         <table class="table table-responsive">
             <tr>
                 <th class="col-md-2 text-center">{% trans "Ship" %}</th>
@@ -29,7 +35,13 @@
             {% endfor %}
         </table>
         {%  if created_fats %}
-         <h2>{% blocktrans %}{{ user }} has created {{ n_created_fats }} link{{ n_created_fats|pluralize }} this month.{% endblocktrans %}</h2>
+        <h2>
+            {% blocktrans count links=n_created_fats trimmed  %}
+                {{ user }} has created one link this month.
+            {% plural %}
+                {{ user }} has created {{ links }} links this month.
+            {% endblocktrans %}
+        </h2>
         {% if created_fats %}
         <table class="table">
             <tr>

allianceauth/groupmanagement/admin.py

@@ -1,7 +1,7 @@
 from django.conf import settings
 
 from django.contrib import admin
-from django.contrib.auth.models import Group as BaseGroup
+from django.contrib.auth.models import Group as BaseGroup, User
 from django.db.models import Count
 from django.db.models.functions import Lower
 from django.db.models.signals import pre_save, post_save, pre_delete, \
@@ -14,7 +14,6 @@ from . import signals
 
 if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS:
     _has_auto_groups = True    
-    from allianceauth.eveonline.autogroups.models import *
 else:
     _has_auto_groups = False
 
@@ -22,19 +21,24 @@ else:
 class AuthGroupInlineAdmin(admin.StackedInline):
     model = AuthGroup
     filter_horizontal = ('group_leaders', 'group_leader_groups', 'states',)
-    fields = ('description', 'group_leaders', 'group_leader_groups', 'states', 'internal', 'hidden', 'open', 'public')
+    fields = (
+        'description', 
+        'group_leaders', 
+        'group_leader_groups', 
+        'states', 'internal', 
+        'hidden', 
+        'open', 
+        'public'
+    )
     verbose_name_plural = 'Auth Settings'
     verbose_name = ''
 
     def formfield_for_manytomany(self, db_field, request, **kwargs):
         """overriding this formfield to have sorted lists in the form"""
         if db_field.name == "group_leaders":
-            kwargs["queryset"] = User.objects\
+            kwargs["queryset"] = User.objects.order_by(Lower('username'))
-                .filter(profile__state__name='Member')\
-                .order_by(Lower('username'))
         elif db_field.name == "group_leader_groups":
-            kwargs["queryset"] = Group.objects\
+            kwargs["queryset"] = Group.objects.order_by(Lower('name'))
-                .order_by(Lower('name'))
         return super().formfield_for_manytomany(db_field, request, **kwargs)
 
     def has_add_permission(self, request):
@@ -103,14 +107,16 @@ class GroupAdmin(admin.ModelAdmin):
         '_member_count', 
         'has_leader'
     )
-    list_filter = (
+    list_filter = [
         'authgroup__internal', 
         'authgroup__hidden', 
         'authgroup__open', 
         'authgroup__public',        
-        IsAutoGroupFilter,
+    ]
-        HasLeaderFilter
+    if _has_auto_groups:
-    )
+        list_filter.append(IsAutoGroupFilter)
+    list_filter.append(HasLeaderFilter)
+
     search_fields = ('name', 'authgroup__description')
     
     def get_queryset(self, request):

allianceauth/groupmanagement/managers.py

@@ -1,27 +1,53 @@
-from django.contrib.auth.models import Group
+import logging
-from django.db.models import Q
+
+from django.contrib.auth.models import Group, User
+from django.db.models import Q, QuerySet
+
+from allianceauth.authentication.models import State
+
+
+logger = logging.getLogger(__name__)
 
 
 class GroupManager:
-    def __init__(self):
+    
-        pass
+    @classmethod
+    def get_joinable_groups_for_user(
+        cls, user: User, include_hidden = True
+    ) -> QuerySet:
+        """get groups a user could join incl. groups already joined"""
+        groups_qs = cls.get_joinable_groups(user.profile.state)
+
+        if not user.has_perm('groupmanagement.request_groups'):
+            groups_qs = groups_qs.filter(authgroup__public=True)
+        
+        if not include_hidden:
+            groups_qs = groups_qs.filter(authgroup__hidden=False)
+
+        return groups_qs
 
     @staticmethod
-    def get_joinable_groups(state):
+    def get_joinable_groups(state: State) -> QuerySet:
-        return Group.objects.select_related('authgroup').exclude(authgroup__internal=True)\
+        """get groups that can be joined by user with given state"""
+        return Group.objects\
+            .select_related('authgroup')\
+            .exclude(authgroup__internal=True)\
             .filter(Q(authgroup__states=state) | Q(authgroup__states=None))
 
     @staticmethod
-    def get_all_non_internal_groups():
+    def get_all_non_internal_groups() -> QuerySet:
-        return Group.objects.select_related('authgroup').exclude(authgroup__internal=True)
+        """get groups that are not internal"""
+        return Group.objects\
+            .select_related('authgroup')\
+            .exclude(authgroup__internal=True)
 
     @staticmethod
-    def get_group_leaders_groups(user):
+    def get_group_leaders_groups(user: User):
         return Group.objects.select_related('authgroup').filter(authgroup__group_leaders__in=[user]) | \
                Group.objects.select_related('authgroup').filter(authgroup__group_leader_groups__in=user.groups.all())
 
     @staticmethod
-    def joinable_group(group, state):
+    def joinable_group(group: Group, state: State) -> bool:
         """
         Check if a group is a user/state joinable group, i.e.
         not an internal group for Corp, Alliance, Members etc,
@@ -30,12 +56,15 @@ class GroupManager:
         :param state: allianceauth.authentication.State object
         :return: bool True if its joinable, False otherwise
         """
-        if len(group.authgroup.states.all()) != 0 and state not in group.authgroup.states.all():
+        if (len(group.authgroup.states.all()) != 0 
+            and state not in group.authgroup.states.all()
+        ):
             return False
-        return not group.authgroup.internal
+        else:
+            return not group.authgroup.internal
 
     @staticmethod
-    def check_internal_group(group):
+    def check_internal_group(group: Group) -> bool:
         """
         Check if a group is auditable, i.e not an internal group
         :param group: django.contrib.auth.models.Group object
@@ -44,20 +73,11 @@ class GroupManager:
         return not group.authgroup.internal
 
     @staticmethod
-    def check_internal_group(group):
+    def has_management_permission(user: User) -> bool:
-        """
-        Check if a group is auditable, i.e not an internal group
-        :param group: django.contrib.auth.models.Group object
-        :return: bool True if it is auditable, false otherwise
-        """
-        return not group.authgroup.internal
-
-    @staticmethod
-    def has_management_permission(user):
         return user.has_perm('auth.group_management')
 
     @classmethod
-    def can_manage_groups(cls, user):
+    def can_manage_groups(cls, user:User ) -> bool:
         """
         For use with user_passes_test decorator.
         Check if the user can manage groups. Either has the
@@ -71,7 +91,7 @@ class GroupManager:
         return False
 
     @classmethod
-    def can_manage_group(cls, user, group):
+    def can_manage_group(cls, user: User, group: Group) -> bool:
         """
         Check user has permission to manage the given group
         :param user: User object to test permission of
@@ -79,5 +99,5 @@ class GroupManager:
         :return: True if the user can manage the group
         """
         if user.is_authenticated:
-            return cls.has_management_permission(user) or user.leads_groups.filter(group=group).exists()
+            return cls.has_management_permission(user) or cls.get_group_leaders_groups(user).filter(pk=group.pk).exists()
         return False

allianceauth/groupmanagement/templates/groupmanagement/audit.html

@@ -15,7 +15,7 @@
             <div class="panel-body">            
                 <p> 
                     <a class="btn btn-default" href="{% url 'groupmanagement:membership' %}" role="button">
-                        Back
+                        {% trans "Back" %}
                     </a>
                 </p>              
                 {% if entries %}

allianceauth/groupmanagement/templates/groupmanagement/groupmembers.html

@@ -17,7 +17,7 @@
             <div class="panel-body">                
                 <p>                    
                     <a class="btn btn-default" href="{% url 'groupmanagement:membership' %}" role="button">
-                        Back
+                        {% trans "Back" %}
                     </a>
                 </p>                
                 {% if group.user_set %}                    

allianceauth/groupmanagement/templates/groupmanagement/groupmembership.html

@@ -11,7 +11,7 @@
         {% include 'groupmanagement/menu.html' %}
         <div class="panel panel-default">
             <div class="panel-heading">
-                Groups
+                {% trans "Groups" %}
             </div>
             <div class="panel-body">
                 {% if groups %}                    
@@ -53,6 +53,10 @@
                                             <a href="{% url "groupmanagement:audit_log" group.id %}" class="btn btn-info" title="{% trans "Audit Members" %}">
                                                 <i class="glyphicon glyphicon-list-alt"></i>
                                             </a>
+                                            <a id="clipboard-copy" data-clipboard-text="{{ request.scheme }}://{{request.get_host}}{% url 'groupmanagement:request_add' group.id %}" class="btn btn-warning" title="{% trans "Copy Direct Join Link" %}">
+                                                <i class="glyphicon glyphicon-copy"></i>
+                                            </a>
+
                                         </td>
                                     </tr>
                                 {% endfor %}
@@ -68,3 +72,9 @@
         </div>
     </div>
 {% endblock content %}
+{% block extra_javascript %}    
+<script src="https://cdnjs.cloudflare.com/ajax/libs/clipboard.js/2.0.4/clipboard.min.js"></script>
+<script>
+    new ClipboardJS('#clipboard-copy');
+</script>
+{% endblock %}

allianceauth/groupmanagement/tests/__init__.py

@@ -0,0 +1,11 @@
+from django.urls import reverse
+
+
+def get_admin_change_view_url(obj: object) -> str:
+    return reverse(
+        'admin:{}_{}_change'.format(
+            obj._meta.app_label,
+            type(obj).__name__.lower()
+        ),
+        args=(obj.pk,)
+    )

allianceauth/groupmanagement/tests/test_admin.py

@@ -1,22 +1,29 @@
 from unittest.mock import patch
 
-from django.test import TestCase, RequestFactory
+from django.conf import settings
 from django.contrib import admin
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import User
+from django.test import TestCase, RequestFactory, Client
 
 from allianceauth.authentication.models import CharacterOwnership, State
-from allianceauth.eveonline.autogroups.models import AutogroupsConfig
 from allianceauth.eveonline.models import (
     EveCharacter, EveCorporationInfo, EveAllianceInfo
 )
 
 from ..admin import (   
-   IsAutoGroupFilter,
    HasLeaderFilter,
    GroupAdmin,   
    Group
 )
+from . import get_admin_change_view_url
+
+if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS:
+    _has_auto_groups = True
+    from allianceauth.eveonline.autogroups.models import AutogroupsConfig
+    from ..admin import IsAutoGroupFilter
+else:
+    _has_auto_groups = False
 
 
 MODULE_PATH = 'allianceauth.groupmanagement.admin'
@@ -210,14 +217,15 @@ class TestGroupAdmin(TestCase):
 
     def _create_autogroups(self):
         """create autogroups for corps and alliances"""
-        autogroups_config = AutogroupsConfig(
+        if _has_auto_groups:
-            corp_groups = True,
+            autogroups_config = AutogroupsConfig(
-            alliance_groups = True
+                corp_groups = True,
-        )
+                alliance_groups = True
-        autogroups_config.save()
+            )
-        for state in State.objects.all():
+            autogroups_config.save()
-            autogroups_config.states.add(state)        
+            for state in State.objects.all():
-        autogroups_config.update_corp_group_membership(self.user_1)        
+                autogroups_config.states.add(state)
+            autogroups_config.update_corp_group_membership(self.user_1)
     
     # column rendering
 
@@ -267,70 +275,72 @@ class TestGroupAdmin(TestCase):
         result = self.modeladmin._properties(self.group_6)
         self.assertListEqual(result, expected)
 
-    @patch(MODULE_PATH + '._has_auto_groups', True)
+    if _has_auto_groups:
-    def test_properties_6(self):
+        @patch(MODULE_PATH + '._has_auto_groups', True)
-        self._create_autogroups()
+        def test_properties_6(self):
-        expected = ['Auto Group']
+            self._create_autogroups()
-        my_group = Group.objects\
+            expected = ['Auto Group']
-            .filter(managedcorpgroup__isnull=False)\
+            my_group = Group.objects\
-            .first()
+                .filter(managedcorpgroup__isnull=False)\
-        result = self.modeladmin._properties(my_group)
+                .first()
-        self.assertListEqual(result, expected)
+            result = self.modeladmin._properties(my_group)
+            self.assertListEqual(result, expected)
 
     # actions
    
     # filters
     
-    @patch(MODULE_PATH + '._has_auto_groups', True)
+    if _has_auto_groups:
-    def test_filter_is_auto_group(self):
+        @patch(MODULE_PATH + '._has_auto_groups', True)
+        def test_filter_is_auto_group(self):
             
-        class GroupAdminTest(admin.ModelAdmin): 
+            class GroupAdminTest(admin.ModelAdmin): 
-            list_filter = (IsAutoGroupFilter,)
+                list_filter = (IsAutoGroupFilter,)
                     
-        self._create_autogroups()        
+            self._create_autogroups()        
-        my_modeladmin = GroupAdminTest(Group, AdminSite())
+            my_modeladmin = GroupAdminTest(Group, AdminSite())
 
-        # Make sure the lookups are correct
+            # Make sure the lookups are correct
-        request = self.factory.get('/')
+            request = self.factory.get('/')
-        request.user = self.user_1
+            request.user = self.user_1
-        changelist = my_modeladmin.get_changelist_instance(request)
+            changelist = my_modeladmin.get_changelist_instance(request)
-        filters = changelist.get_filters(request)
+            filters = changelist.get_filters(request)
-        filterspec = filters[0][0]
+            filterspec = filters[0][0]
-        expected = [
+            expected = [
-            ('yes', 'Yes'),
+                ('yes', 'Yes'),
-            ('no', 'No'),
+                ('no', 'No'),
-        ]
+            ]
-        self.assertEqual(filterspec.lookup_choices, expected)
+            self.assertEqual(filterspec.lookup_choices, expected)
 
-        # Make sure the correct queryset is returned - no
+            # Make sure the correct queryset is returned - no
-        request = self.factory.get(
+            request = self.factory.get(
-            '/', {'is_auto_group__exact': 'no'}
+                '/', {'is_auto_group__exact': 'no'}
-        )
+            )
-        request.user = self.user_1
+            request.user = self.user_1
-        changelist = my_modeladmin.get_changelist_instance(request)
+            changelist = my_modeladmin.get_changelist_instance(request)
-        queryset = changelist.get_queryset(request)
+            queryset = changelist.get_queryset(request)
-        expected = [
+            expected = [
-            self.group_1,
+                self.group_1,
-            self.group_2,
+                self.group_2,
-            self.group_3,
+                self.group_3,
-            self.group_4,
+                self.group_4,
-            self.group_5,
+                self.group_5,
-            self.group_6
+                self.group_6
-        ]
+            ]
-        self.assertSetEqual(set(queryset), set(expected))
+            self.assertSetEqual(set(queryset), set(expected))
 
-        # Make sure the correct queryset is returned - yes
+            # Make sure the correct queryset is returned - yes
-        request = self.factory.get(
+            request = self.factory.get(
-            '/', {'is_auto_group__exact': 'yes'}
+                '/', {'is_auto_group__exact': 'yes'}
-        )
+            )
-        request.user = self.user_1
+            request.user = self.user_1
-        changelist = my_modeladmin.get_changelist_instance(request)
+            changelist = my_modeladmin.get_changelist_instance(request)
-        queryset = changelist.get_queryset(request)
+            queryset = changelist.get_queryset(request)
-        expected = Group.objects.exclude(
+            expected = Group.objects.exclude(
-                    managedalliancegroup__isnull=True, 
+                        managedalliancegroup__isnull=True, 
-                    managedcorpgroup__isnull=True
+                        managedcorpgroup__isnull=True
-        )
+            )
-        self.assertSetEqual(set(queryset), set(expected))
+            self.assertSetEqual(set(queryset), set(expected))
          
     def test_filter_has_leader(self):
         
@@ -377,3 +387,11 @@ class TestGroupAdmin(TestCase):
         ]
         self.assertSetEqual(set(queryset), set(expected))
     
+    def test_change_view_loads_normally(self):
+        User.objects.create_superuser(
+            username='superuser', password='secret', email='admin@example.com'
+        )
+        c = Client()
+        c.login(username='superuser', password='secret')                
+        response = c.get(get_admin_change_view_url(self.group_1))
+        self.assertEqual(response.status_code, 200)

allianceauth/groupmanagement/tests/test_all.py

@@ -1,92 +0,0 @@
-from unittest import mock
-
-from django.test import TestCase
-from allianceauth.tests.auth_utils import AuthUtils
-from allianceauth.eveonline.models import EveCorporationInfo, EveAllianceInfo, EveCharacter
-from django.contrib.auth.models import User, Group
-from allianceauth.groupmanagement.managers import GroupManager
-from allianceauth.groupmanagement.signals import check_groups_on_state_change
-
-class GroupManagementVisibilityTestCase(TestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.user = AuthUtils.create_user('test')
-        AuthUtils.add_main_character(cls.user, 'test character', '1', corp_id='2', corp_name='test_corp', corp_ticker='TEST', alliance_id='3', alliance_name='TEST')
-        cls.user.profile.refresh_from_db()
-        cls.alliance = EveAllianceInfo.objects.create(alliance_id='3', alliance_name='test alliance', alliance_ticker='TEST', executor_corp_id='2')
-        cls.corp = EveCorporationInfo.objects.create(corporation_id='2', corporation_name='test corp', corporation_ticker='TEST', alliance=cls.alliance, member_count=1)
-        cls.group1 = Group.objects.create(name='group1')
-        cls.group2 = Group.objects.create(name='group2')
-        cls.group3 = Group.objects.create(name='group3')
-
-    def setUp(self):
-        self.user.refresh_from_db()
-
-    def _refresh_user(self):
-        self.user = User.objects.get(pk=self.user.pk)
-
-
-    def test_can_manage_group(self):
-        
-
-        self.group1.authgroup.group_leaders.add(self.user)
-        self.group2.authgroup.group_leader_groups.add(self.group1)
-        self._refresh_user()
-        groups = GroupManager.get_group_leaders_groups(self.user)
-
-        self.assertIn(self.group1, groups) #avail due to user
-        self.assertNotIn(self.group2, groups) #not avail due to group
-        self.assertNotIn(self.group3, groups) #not avail at all 
-
-        self.user.groups.add(self.group1)
-        self._refresh_user()
-        groups = GroupManager.get_group_leaders_groups(self.user)
-
-        self.assertIn(self.group1, groups) #avail due to user
-        self.assertIn(self.group2, groups) #avail due to group1
-        self.assertNotIn(self.group3, groups) #not avail at all 
-
-class GroupManagementStateTestCase(TestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.user = AuthUtils.create_user('test')
-        AuthUtils.add_main_character(cls.user, 'test character', '1', corp_id='2', corp_name='test_corp', corp_ticker='TEST', alliance_id='3', alliance_name='TEST')
-        cls.user.profile.refresh_from_db()
-        cls.alliance = EveAllianceInfo.objects.create(alliance_id='3', alliance_name='test alliance', alliance_ticker='TEST', executor_corp_id='2')
-        cls.corp = EveCorporationInfo.objects.create(corporation_id='2', corporation_name='test corp', corporation_ticker='TEST', alliance=cls.alliance, member_count=1)
-        cls.state_group = Group.objects.create(name='state_group')
-        cls.open_group = Group.objects.create(name='open_group')
-        cls.state = AuthUtils.create_state('test state', 500)
-        cls.state_group.authgroup.states.add(cls.state)
-        cls.state_group.authgroup.internal = False
-        cls.state_group.save()
-
-    def setUp(self):
-        self.user.refresh_from_db()
-        self.state.refresh_from_db()
-
-    def _refresh_user(self):
-        self.user = User.objects.get(pk=self.user.pk)
-
-    def _refresh_test_group(self):
-        self.state_group = Group.objects.get(pk=self.state_group.pk)
-
-    def test_drop_state_group(self):
-
-        self.user.groups.add(self.open_group)
-        self.user.groups.add(self.state_group)     
-        self.assertEqual(self.user.profile.state.name, "Guest")
-
-        self.state.member_corporations.add(self.corp)
-        self._refresh_user()
-        self.assertEqual(self.user.profile.state, self.state)
-        groups = self.user.groups.all()
-        self.assertIn(self.state_group, groups) #keeps group
-        self.assertIn(self.open_group, groups) #public group unafected
-
-        self.state.member_corporations.clear()
-        self._refresh_user()
-        self.assertEqual(self.user.profile.state.name, "Guest")
-        groups = self.user.groups.all()
-        self.assertNotIn(self.state_group, groups) #looses group
-        self.assertIn(self.open_group, groups) #public group unafected

allianceauth/groupmanagement/tests/test_managers.py

@@ -0,0 +1,337 @@
+from unittest.mock import Mock, patch
+
+from django.contrib.auth.models import Group, User
+from django.test import TestCase
+from django.urls import reverse
+
+from allianceauth.eveonline.models import EveCorporationInfo, EveAllianceInfo
+from allianceauth.tests.auth_utils import AuthUtils
+
+from ..models import AuthGroup
+from ..managers import GroupManager
+
+
+class MockUserNotAuthenticated():
+    def __init__(self):
+        self.is_authenticated = False
+
+class GroupManagementVisibilityTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = AuthUtils.create_user('test')
+        AuthUtils.add_main_character(
+            cls.user, 'test character', '1', corp_id='2', corp_name='test_corp', corp_ticker='TEST', alliance_id='3', alliance_name='TEST'
+        )
+        cls.user.profile.refresh_from_db()
+        cls.alliance = EveAllianceInfo.objects.create(alliance_id='3', alliance_name='test alliance', alliance_ticker='TEST', executor_corp_id='2')
+        cls.corp = EveCorporationInfo.objects.create(
+            corporation_id='2', corporation_name='test corp', corporation_ticker='TEST', alliance=cls.alliance, member_count=1
+        )
+        cls.group1 = Group.objects.create(name='group1')
+        cls.group2 = Group.objects.create(name='group2')
+        cls.group3 = Group.objects.create(name='group3')
+
+    def setUp(self):
+        self.user.refresh_from_db()
+
+    def _refresh_user(self):
+        self.user = User.objects.get(pk=self.user.pk)
+
+
+    def test_get_group_leaders_groups(self):
+        self.group1.authgroup.group_leaders.add(self.user)
+        self.group2.authgroup.group_leader_groups.add(self.group1)
+        self._refresh_user()
+        groups = GroupManager.get_group_leaders_groups(self.user)
+
+        self.assertIn(self.group1, groups) #avail due to user
+        self.assertNotIn(self.group2, groups) #not avail due to group
+        self.assertNotIn(self.group3, groups) #not avail at all 
+
+        self.user.groups.add(self.group1)
+        self._refresh_user()
+        groups = GroupManager.get_group_leaders_groups(self.user)
+
+
+    def test_can_manage_group(self):
+        self.group1.authgroup.group_leaders.add(self.user)
+        self.user.groups.add(self.group1)
+        self._refresh_user()
+
+        self.assertTrue(GroupManager.can_manage_group(self.user, self.group1))
+        self.assertFalse(GroupManager.can_manage_group(self.user, self.group2))
+        self.assertFalse(GroupManager.can_manage_group(self.user, self.group3))
+        
+        self.group2.authgroup.group_leader_groups.add(self.group1)
+        self.group1.authgroup.group_leaders.remove(self.user)
+        self._refresh_user()
+
+        self.assertFalse(GroupManager.can_manage_group(self.user, self.group1))
+        self.assertTrue(GroupManager.can_manage_group(self.user, self.group2))
+        self.assertFalse(GroupManager.can_manage_group(self.user, self.group3))
+
+
+class TestGroupManager(TestCase):
+        
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+
+        # group 1
+        cls.group_default = Group.objects.create(name='default')        
+        cls.group_default.authgroup.description = 'Default Group'
+        cls.group_default.authgroup.internal = False
+        cls.group_default.authgroup.hidden = False
+        cls.group_default.authgroup.save()
+
+        # group 2
+        cls.group_internal = Group.objects.create(name='internal')
+        cls.group_internal.authgroup.description = 'Internal Group'
+        cls.group_internal.authgroup.internal = True        
+        cls.group_internal.authgroup.save()
+
+        # group 3
+        cls.group_hidden = Group.objects.create(name='hidden')
+        cls.group_hidden.authgroup.description = 'Hidden Group'
+        cls.group_hidden.authgroup.internal = False
+        cls.group_hidden.authgroup.hidden = True
+        cls.group_hidden.authgroup.save()
+
+        # group 4
+        cls.group_open = Group.objects.create(name='open')
+        cls.group_open.authgroup.description = 'Open Group'
+        cls.group_open.authgroup.internal = False
+        cls.group_open.authgroup.hidden = False
+        cls.group_open.authgroup.open = True
+        cls.group_open.authgroup.save()
+
+        # group 5
+        cls.group_public_1 = Group.objects.create(name='public 1')
+        cls.group_public_1.authgroup.description = 'Public Group 1'
+        cls.group_public_1.authgroup.internal = False
+        cls.group_public_1.authgroup.hidden = False
+        cls.group_public_1.authgroup.public = True
+        cls.group_public_1.authgroup.save()
+
+        # group 6
+        cls.group_public_2 = Group.objects.create(name='public 2')
+        cls.group_public_2.authgroup.description = 'Public Group 2'
+        cls.group_public_2.authgroup.internal = False
+        cls.group_public_2.authgroup.hidden = True
+        cls.group_public_2.authgroup.open = True
+        cls.group_public_2.authgroup.public = True
+        cls.group_public_2.authgroup.save()
+
+        # group 7
+        cls.group_default_member = Group.objects.create(name='default members')
+        cls.group_default_member.authgroup.description = \
+            'Default Group for members only'
+        cls.group_default_member.authgroup.internal = False
+        cls.group_default_member.authgroup.hidden = False
+        cls.group_default_member.authgroup.open = False
+        cls.group_default_member.authgroup.public = False
+        cls.group_default_member.authgroup.states.add(
+            AuthUtils.get_member_state()
+        )
+        cls.group_default_member.authgroup.save()
+    
+    def setUp(self):
+        self.user = AuthUtils.create_user('Bruce Wayne')
+
+    def test_get_joinable_group_member(self):        
+        result = GroupManager.get_joinable_groups(
+            AuthUtils.get_member_state()
+        )
+        expected = {
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2,
+            self.group_default_member
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_get_joinable_group_guest(self):
+        result = GroupManager.get_joinable_groups(
+            AuthUtils.get_guest_state()
+        )
+        expected = {
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_joinable_group_member(self):
+        member_state = AuthUtils.get_member_state()
+        for x in [
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2,
+            self.group_default_member
+        ]:
+            self.assertTrue(GroupManager.joinable_group(x, member_state))
+
+        for x in [
+            self.group_internal,             
+        ]:
+            self.assertFalse(GroupManager.joinable_group(x, member_state))
+
+
+    def test_joinable_group_guest(self):
+        guest_state = AuthUtils.get_guest_state()
+        for x in [
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2
+        ]:
+            self.assertTrue(GroupManager.joinable_group(x, guest_state))
+
+        for x in [
+            self.group_internal,
+            self.group_default_member
+        ]:
+            self.assertFalse(GroupManager.joinable_group(x, guest_state))
+
+        
+    def test_get_all_non_internal_groups(self):
+        result = GroupManager.get_all_non_internal_groups()
+        expected = {
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2, 
+            self.group_default_member
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_check_internal_group(self):
+        self.assertTrue(
+            GroupManager.check_internal_group(self.group_default)
+        )
+        self.assertFalse(
+            GroupManager.check_internal_group(self.group_internal)
+        )
+
+    def test_get_joinable_groups_for_user_no_permission(self):
+        AuthUtils.assign_state(self.user, AuthUtils.get_guest_state())
+        result = GroupManager.get_joinable_groups_for_user(self.user)
+        expected= {self.group_public_1, self.group_public_2}
+        self.assertSetEqual(set(result), expected)
+
+    def test_get_joinable_groups_for_user_guest_w_permission_(self):
+        AuthUtils.assign_state(self.user, AuthUtils.get_guest_state())
+        AuthUtils.add_permission_to_user_by_name(
+            'groupmanagement.request_groups', self.user
+        )
+        result = GroupManager.get_joinable_groups_for_user(self.user)
+        expected = {
+            self.group_default, 
+            self.group_hidden, 
+            self.group_open, 
+            self.group_public_1, 
+            self.group_public_2
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_get_joinable_groups_for_user_member_w_permission(self):
+        AuthUtils.assign_state(self.user, AuthUtils.get_member_state(), True)
+        AuthUtils.add_permission_to_user_by_name(
+            'groupmanagement.request_groups', self.user
+        )
+        result = GroupManager.get_joinable_groups_for_user(self.user)
+        expected = {
+            self.group_default,
+            self.group_hidden,
+            self.group_open,
+            self.group_public_1,
+            self.group_public_2,
+            self.group_default_member
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_get_joinable_groups_for_user_member_w_permission_no_hidden(self):
+        AuthUtils.assign_state(self.user, AuthUtils.get_member_state(), True)
+        AuthUtils.add_permission_to_user_by_name(
+            'groupmanagement.request_groups', self.user
+        )
+        result = GroupManager.get_joinable_groups_for_user(
+            self.user, include_hidden=False
+        )
+        expected = {
+            self.group_default,
+            self.group_open, 
+            self.group_public_1,            
+            self.group_default_member
+        }
+        self.assertSetEqual(set(result), expected)
+
+    def test_has_management_permission(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        AuthUtils.add_permission_to_user_by_name(
+            'auth.group_management', user
+        )        
+        self.assertTrue(GroupManager.has_management_permission(user))
+
+    def test_can_manage_groups_no_perm_no_group(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        self.assertFalse(GroupManager.can_manage_groups(user))
+
+    def test_can_manage_groups_user_not_authenticated(self):        
+        user = MockUserNotAuthenticated()
+        self.assertFalse(GroupManager.can_manage_groups(user))
+
+    def test_can_manage_groups_has_perm(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        AuthUtils.add_permission_to_user_by_name(
+            'auth.group_management', user
+        )        
+        self.assertTrue(GroupManager.can_manage_groups(user))
+
+    def test_can_manage_groups_no_perm_leads_group(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        self.group_default.authgroup.group_leaders.add(user)
+        self.assertTrue(GroupManager.can_manage_groups(user))
+
+    def test_can_manage_group_no_perm_no_group(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        self.assertFalse(
+            GroupManager.can_manage_group(user, self.group_default)
+        )
+
+    def test_can_manage_group_has_perm(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        AuthUtils.add_permission_to_user_by_name(
+            'auth.group_management', user
+        )        
+        self.assertTrue(
+            GroupManager.can_manage_group(user, self.group_default)
+        )
+
+    def test_can_manage_group_no_perm_leads_correct_group(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        self.group_default.authgroup.group_leaders.add(user)
+        self.assertTrue(
+            GroupManager.can_manage_group(user, self.group_default)
+        )
+
+    def test_can_manage_group_no_perm_leads_other_group(self):        
+        user = AuthUtils.create_user('Clark Kent')
+        self.group_hidden.authgroup.group_leaders.add(user)
+        self.assertFalse(
+            GroupManager.can_manage_group(user, self.group_default)
+        )
+
+    def test_can_manage_group_user_not_authenticated(self):        
+        user = MockUserNotAuthenticated()
+        self.assertFalse(
+            GroupManager.can_manage_group(user, self.group_default)
+        )

allianceauth/groupmanagement/tests/test_models.py

@@ -0,0 +1,167 @@
+from unittest import mock
+
+from django.contrib.auth.models import User, Group
+from django.test import TestCase
+
+from allianceauth.tests.auth_utils import AuthUtils
+from allianceauth.eveonline.models import (
+    EveCorporationInfo, EveAllianceInfo, EveCharacter
+)
+
+from ..models import GroupRequest, RequestLog
+
+
+def create_testdata():    
+    # clear DB
+    User.objects.all().delete()
+    Group.objects.all().delete()
+    EveCharacter.objects.all().delete()
+    EveCorporationInfo.objects.all().delete()
+    EveAllianceInfo.objects.all().delete()
+
+    # group 1
+    group = Group.objects.create(name='Superheros')        
+    group.authgroup.description = 'Default Group'
+    group.authgroup.internal = False
+    group.authgroup.hidden = False
+    group.authgroup.save()
+    
+    # user 1        
+    user_1 = AuthUtils.create_user('Bruce Wayne')
+    AuthUtils.add_main_character_2(
+        user_1,
+        name='Bruce Wayne',
+        character_id=1001,
+        corp_id=2001,
+        corp_name='Wayne Technologies'
+    )        
+    user_1.groups.add(group)
+    group.authgroup.group_leaders.add(user_1)
+
+    # user 2
+    user_2 = AuthUtils.create_user('Clark Kent')
+    AuthUtils.add_main_character_2(
+        user_2,
+        name='Clark Kent',
+        character_id=1002,
+        corp_id=2002,
+        corp_name='Wayne Technologies'
+    )            
+    return group, user_1, user_2
+
+
+
+class TestGroupRequest(TestCase):
+    
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.group, cls.user_1, _ = create_testdata()
+
+    def test_main_char(self):
+        group_request = GroupRequest.objects.create(
+            status='Pending',
+            user=self.user_1,
+            group=self.group
+        )
+        expected = self.user_1.profile.main_character
+        self.assertEqual(group_request.main_char, expected)
+
+    def test_str(self):
+        group_request = GroupRequest.objects.create(
+            status='Pending',
+            user=self.user_1,
+            group=self.group
+        )
+        expected = 'Bruce Wayne:Superheros'
+        self.assertEqual(str(group_request), expected)
+
+
+class TestRequestLog(TestCase):
+    
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.group, cls.user_1, cls.user_2 = create_testdata()
+
+    def test_requestor(self):
+        request_log = RequestLog.objects.create(
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1
+        )
+        expected = 'Clark Kent'
+        self.assertEqual(request_log.requestor(), expected)
+
+    def test_type_to_str_removed(self):
+        request_log = RequestLog.objects.create(
+            request_type=None,
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1
+        )
+        expected = 'Removed'
+        self.assertEqual(request_log.type_to_str(), expected)
+
+    def test_type_to_str_leave(self):
+        request_log = RequestLog.objects.create(
+            request_type=True,
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1
+        )
+        expected = 'Leave'
+        self.assertEqual(request_log.type_to_str(), expected)
+
+    def test_type_to_str_join(self):
+        request_log = RequestLog.objects.create(
+            request_type=False,
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1
+        )
+        expected = 'Join'
+        self.assertEqual(request_log.type_to_str(), expected)
+
+    def test_action_to_str_accept(self):
+        request_log = RequestLog.objects.create(            
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1,
+            action = True
+        )
+        expected = 'Accept'
+        self.assertEqual(request_log.action_to_str(), expected)
+
+    def test_action_to_str_reject(self):
+        request_log = RequestLog.objects.create(            
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1,
+            action = False
+        )
+        expected = 'Reject'
+        self.assertEqual(request_log.action_to_str(), expected)
+
+    def test_req_char(self):
+        request_log = RequestLog.objects.create(            
+            group=self.group,
+            request_info='Clark Kent:Superheros',
+            request_actor=self.user_1,
+            action = False
+        )
+        expected = self.user_2.profile.main_character
+        self.assertEqual(request_log.req_char(), expected)
+
+
+class TestAuthGroup(TestCase):
+
+    def test_str(self):
+        group = Group.objects.create(name='Superheros')        
+        group.authgroup.description = 'Default Group'
+        group.authgroup.internal = False
+        group.authgroup.hidden = False
+        group.authgroup.save()
+
+        expected = 'Superheros'
+        self.assertEqual(str(group.authgroup), expected)

allianceauth/groupmanagement/tests/test_signals.py

@@ -0,0 +1,61 @@
+from unittest import mock
+
+from django.test import TestCase
+from django.contrib.auth.models import User, Group
+
+from allianceauth.eveonline.models import EveCorporationInfo, EveAllianceInfo
+from allianceauth.tests.auth_utils import AuthUtils
+
+from ..signals import check_groups_on_state_change
+
+
+class GroupManagementStateTestCase(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = AuthUtils.create_user('test')
+        AuthUtils.add_main_character(
+            cls.user, 'test character', '1', corp_id='2', corp_name='test_corp', corp_ticker='TEST', alliance_id='3', alliance_name='TEST'
+        )
+        cls.user.profile.refresh_from_db()
+        cls.alliance = EveAllianceInfo.objects.create(
+            alliance_id='3', alliance_name='test alliance', alliance_ticker='TEST', executor_corp_id='2'
+        )
+        cls.corp = EveCorporationInfo.objects.create(
+            corporation_id='2', corporation_name='test corp', corporation_ticker='TEST', alliance=cls.alliance, member_count=1
+        )
+        cls.state_group = Group.objects.create(name='state_group')
+        cls.open_group = Group.objects.create(name='open_group')
+        cls.state = AuthUtils.create_state('test state', 500)
+        cls.state_group.authgroup.states.add(cls.state)
+        cls.state_group.authgroup.internal = False
+        cls.state_group.save()
+
+    def setUp(self):
+        self.user.refresh_from_db()
+        self.state.refresh_from_db()
+
+    def _refresh_user(self):
+        self.user = User.objects.get(pk=self.user.pk)
+
+    def _refresh_test_group(self):
+        self.state_group = Group.objects.get(pk=self.state_group.pk)
+
+    def test_drop_state_group(self):
+        self.user.groups.add(self.open_group)
+        self.user.groups.add(self.state_group)     
+        self.assertEqual(self.user.profile.state.name, "Guest")
+
+        self.state.member_corporations.add(self.corp)
+        self._refresh_user()
+        self.assertEqual(self.user.profile.state, self.state)
+        groups = self.user.groups.all()
+        self.assertIn(self.state_group, groups) #keeps group
+        self.assertIn(self.open_group, groups) #public group unafected
+
+        self.state.member_corporations.clear()
+        self._refresh_user()
+        self.assertEqual(self.user.profile.state.name, "Guest")
+        groups = self.user.groups.all()
+        self.assertNotIn(self.state_group, groups) #looses group
+        self.assertIn(self.open_group, groups) #public group unafected

allianceauth/groupmanagement/tests/test_views.py

@@ -0,0 +1,22 @@
+from unittest.mock import Mock, patch
+
+from django.test import RequestFactory, TestCase
+from django.urls import reverse
+
+from allianceauth.tests.auth_utils import AuthUtils
+from esi.models import Token
+
+from .. import views
+
+
+class TestViews(TestCase):
+        
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.user = AuthUtils.create_user('Bruce Wayne')        
+
+    def test_groups_view_can_load(self):
+        request = self.factory.get(reverse('groupmanagement:groups'))
+        request.user = self.user
+        response = views.groups_view(request)
+        self.assertEqual(response.status_code, 200)        

allianceauth/groupmanagement/views.py

@@ -1,5 +1,6 @@
 import logging
 
+from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.decorators import user_passes_test
@@ -10,12 +11,12 @@ from django.db.models import Count
 from django.http import Http404
 from django.shortcuts import render, redirect, get_object_or_404
 from django.utils.translation import ugettext_lazy as _
-from .managers import GroupManager
-from .models import GroupRequest, RequestLog
 
 from allianceauth.notifications import notify
 
-from django.conf import settings
+from .managers import GroupManager
+from .models import GroupRequest, RequestLog
+
 
 logger = logging.getLogger(__name__)
 
@@ -33,7 +34,8 @@ def group_management(request):
         group_requests = base_group_query.all()
     else:
         # Group specific leader
-        group_requests = base_group_query.filter(group__authgroup__group_leaders__in=[request.user])
+        users__groups = GroupManager.get_group_leaders_groups(request.user)
+        group_requests = base_group_query.filter(group__in=users__groups)
 
     for grouprequest in group_requests:
         if grouprequest.leave_request:
@@ -234,7 +236,7 @@ def group_reject_request(request, group_request_id):
         raise p
     except:
         messages.error(request, _('An unhandled error occurred while processing the application from %(mainchar)s to %(group)s.') % {"mainchar": group_request.main_char, "group": group_request.group})
-        logger.exception("Unhandled exception occured while user %s attempting to reject group request id %s" % (
+        logger.exception("Unhandled exception occurred while user %s attempting to reject group request id %s" % (
             request.user, group_request_id))
         pass
 
@@ -268,9 +270,9 @@ def group_leave_accept_request(request, group_request_id):
                        (request.user, group_request_id))
         raise p
     except:
-        messages.error(request, _('An unhandled error occured while processing the application from %(mainchar)s to leave %(group)s.') % {
+        messages.error(request, _('An unhandled error occurred while processing the application from %(mainchar)s to leave %(group)s.') % {
             "mainchar": group_request.main_char, "group": group_request.group})
-        logger.exception("Unhandled exception occured while user %s attempting to accept group leave request id %s" % (
+        logger.exception("Unhandled exception occurred while user %s attempting to accept group leave request id %s" % (
             request.user, group_request_id))
         pass
 
@@ -302,9 +304,9 @@ def group_leave_reject_request(request, group_request_id):
                        (request.user, group_request_id))
         raise p
     except:
-        messages.error(request, _('An unhandled error occured while processing the application from %(mainchar)s to leave %(group)s.') % {
+        messages.error(request, _('An unhandled error occurred while processing the application from %(mainchar)s to leave %(group)s.') % {
             "mainchar": group_request.main_char, "group": group_request.group})
-        logger.exception("Unhandled exception occured while user %s attempting to reject group leave request id %s" % (
+        logger.exception("Unhandled exception occurred while user %s attempting to reject group leave request id %s" % (
             request.user, group_request_id))
         pass
 
@@ -314,24 +316,23 @@ def group_leave_reject_request(request, group_request_id):
 @login_required
 def groups_view(request):
     logger.debug("groups_view called by user %s" % request.user)
+    
+    groups_qs = GroupManager.get_joinable_groups_for_user(
+        request.user, include_hidden=False
+    )
+    groups_qs = groups_qs.order_by('name')
     groups = []
+    for group in groups_qs:        
+        group_request = GroupRequest.objects\
+            .filter(user=request.user)\
+            .filter(group=group)
+        groups.append({
+            'group': group, 
+            'request': group_request[0] if group_request else None
+        })
 
-    group_query = GroupManager.get_joinable_groups(request.user.profile.state)
+    context = {'groups': groups}
-
+    return render(request, 'groupmanagement/groups.html', context=context)
-    if not request.user.has_perm('groupmanagement.request_groups'):
-        # Filter down to public groups only for non-members
-        group_query = group_query.filter(authgroup__public=True)
-        logger.debug("Not a member, only public groups will be available")
-
-    for group in group_query:
-        # Exclude hidden
-        if not group.authgroup.hidden:
-            group_request = GroupRequest.objects.filter(user=request.user).filter(group=group)
-
-            groups.append({'group': group, 'request': group_request[0] if group_request else None})
-
-    render_items = {'groups': groups}
-    return render(request, 'groupmanagement/groups.html', context=render_items)
 
 
 @login_required
@@ -348,13 +349,13 @@ def group_request_add(request, group_id):
         # User is already a member of this group.
         logger.warning("User %s attempted to join group id %s but they are already a member." %
                        (request.user, group_id))
-        messages.warning(request, "You are already a member of that group.")
+        messages.warning(request, _("You are already a member of that group."))
         return redirect('groupmanagement:groups')
     if not request.user.has_perm('groupmanagement.request_groups') and not group.authgroup.public:
         # Does not have the required permission, trying to join a non-public group
         logger.warning("User %s attempted to join group id %s but it is not a public group" %
                        (request.user, group_id))
-        messages.warning(request, "You cannot join that group")
+        messages.warning(request, _("You cannot join that group"))
         return redirect('groupmanagement:groups')
     if group.authgroup.open:
         logger.info("%s joining %s as is an open group" % (request.user, group))
@@ -363,7 +364,7 @@ def group_request_add(request, group_id):
     req = GroupRequest.objects.filter(user=request.user, group=group)
     if len(req) > 0:
         logger.info("%s attempted to join %s but already has an open application" % (request.user, group))
-        messages.warning(request, "You already have a pending application for that group.")
+        messages.warning(request, _("You already have a pending application for that group."))
         return redirect("groupmanagement:groups")
     grouprequest = GroupRequest()
     grouprequest.status = _('Pending')
@@ -397,7 +398,7 @@ def group_request_leave(request, group_id):
     req = GroupRequest.objects.filter(user=request.user, group=group)
     if len(req) > 0:
         logger.info("%s attempted to leave %s but already has an pending leave request." % (request.user, group))
-        messages.warning(request, "You already have a pending leave request for that group.")
+        messages.warning(request, _("You already have a pending leave request for that group."))
         return redirect("groupmanagement:groups")
     if getattr(settings, 'AUTO_LEAVE', False):
         logger.info("%s leaving joinable group %s due to auto_leave" % (request.user, group))

allianceauth/project_template/project_name/celery.py

@@ -11,6 +11,15 @@ app = Celery('{{ project_name }}')
 # Using a string here means the worker don't have to serialize
 # the configuration object to child processes.
 app.config_from_object('django.conf:settings')
+
+# setup priorities ( 0 Highest, 9 Lowest )
+app.conf.broker_transport_options = {
+    'priority_steps': list(range(10)),  # setup que to have 10 steps
+    'queue_order_strategy': 'priority',  # setup que to use prio sorting
+}
+app.conf.task_default_priority = 5  # anything called with the task.delay() will be given normal priority (5)
+app.conf.worker_prefetch_multiplier = 1  # only prefetch single tasks at a time on the workers so that prio tasks happen
+
 app.conf.ONCE = {
     'backend': 'allianceauth.services.tasks.DjangoBackend',
     'settings': {}

allianceauth/project_template/project_name/settings/base.py

@@ -83,6 +83,9 @@ LANGUAGES = (
     ('en', ugettext('English')),
     ('de', ugettext('German')),
     ('es', ugettext('Spanish')),
+    ('zh-hans', ugettext('Chinese Simplified')),
+    ('ru', ugettext('Russian')),
+    ('ko', ugettext('Korean')),
 )
 
 TEMPLATES = [
@@ -217,6 +220,14 @@ LOGGING = {
             'maxBytes': 1024 * 1024 * 5,  # edit this line to change max log file size
             'backupCount': 5,  # edit this line to change number of log backups
         },
+        'extension_file': {
+            'level': 'DEBUG',
+            'class': 'logging.handlers.RotatingFileHandler',
+            'filename': os.path.join(BASE_DIR, 'log/extensions.log'),
+            'formatter': 'verbose',
+            'maxBytes': 1024 * 1024 * 5,  # edit this line to change max log file size
+            'backupCount': 5,  # edit this line to change number of log backups
+        },
         'console': {
             'level': 'DEBUG',  # edit this line to change logging level to console
             'class': 'logging.StreamHandler',
@@ -233,6 +244,10 @@ LOGGING = {
             'handlers': ['log_file', 'console', 'notifications'],
             'level': 'DEBUG',
         },
+        'extensions': {
+            'handlers': ['extension_file', 'console'],
+            'level': 'DEBUG',
+        },
         'django': {
             'handlers': ['log_file', 'console'],
             'level': 'ERROR',

allianceauth/project_template/project_name/urls.py

@@ -4,3 +4,8 @@ from allianceauth import urls
 urlpatterns = [
     url(r'', include(urls)),
 ]
+
+handler500 = 'allianceauth.views.Generic500Redirect'
+handler404 = 'allianceauth.views.Generic404Redirect'
+handler403 = 'allianceauth.views.Generic403Redirect'
+handler400 = 'allianceauth.views.Generic400Redirect'

allianceauth/services/abstract.py

@@ -17,6 +17,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMix
 from django.db import models, IntegrityError
 from django.core.exceptions import ObjectDoesNotExist
 from django.shortcuts import render, Http404, redirect
+from django.utils.translation import gettext_lazy as _
 
 from .forms import ServicePasswordModelForm
 
@@ -68,7 +69,7 @@ class BaseCreatePasswordServiceAccountView(BaseServiceView, ServiceCredentialsVi
         try:
             svc_obj = self.model.objects.create(user=request.user)
         except IntegrityError:
-            messages.error(request, "That service account already exists")
+            messages.error(request, _("That service account already exists"))
             return redirect(self.index_redirect)
 
         return render(request, self.template_name,
@@ -100,7 +101,7 @@ class BaseSetPasswordServiceAccountView(ServicesCRUDMixin, BaseServiceView, Upda
     def post(self, request, *args, **kwargs):
         result = super(BaseSetPasswordServiceAccountView, self).post(request, *args, **kwargs)
         if self.get_form().is_valid():
-            messages.success(request, "Successfully set your {} password".format(self.service_name))
+            messages.success(request, _("Successfully set your {} password".format(self.service_name)))
         return result
 
 

allianceauth/services/admin.py

@@ -1,14 +1,14 @@
 from django import forms
 from django.contrib import admin
-from django.db.models.functions import Lower
-from django.urls import reverse
-from django.utils.html import format_html
 
 from allianceauth import hooks
-from allianceauth.eveonline.models import EveCharacter
+from allianceauth.authentication.admin import (
-from allianceauth.authentication.admin import user_profile_pic, \
+    user_profile_pic, 
-    user_username, user_main_organization, MainCorporationsFilter,\
+    user_username, 
+    user_main_organization, 
+    MainCorporationsFilter,
     MainAllianceFilter
+)
 
 from .models import NameFormatConfig
 
@@ -20,24 +20,29 @@ class ServicesUserAdmin(admin.ModelAdmin):
             "all": ("services/admin.css",)
         }
 
-    search_fields = (
+    search_fields = ('user__username',)
-        'user__username', 
+    ordering = ('user__username',)
-        'uid'
-    )
-    ordering = ('user__username', )
     list_select_related = True              
     list_display = (
         user_profile_pic,
         user_username,
+        '_state',
         user_main_organization, 
         '_date_joined'
     )
     list_filter = (        
+        'user__profile__state',
         MainCorporationsFilter,        
         MainAllianceFilter,
-        'user__date_joined'
+        'user__date_joined',
     )
 
+    def _state(self, obj):
+        return obj.user.profile.state.name
+
+    _state.short_description = 'state'
+    _state.admin_order_field = 'user__profile__state__name'
+
     def _date_joined(self, obj):
         return obj.user.date_joined
     
@@ -48,7 +53,8 @@ class ServicesUserAdmin(admin.ModelAdmin):
 class NameFormatConfigForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super(NameFormatConfigForm, self).__init__(*args, **kwargs)
-        SERVICE_CHOICES = [(s.name, s.name) for h in hooks.get_hooks('services_hook') for s in [h()]]
+        SERVICE_CHOICES = \
+            [(s.name, s.name) for h in hooks.get_hooks('services_hook') for s in [h()]]
         if self.instance.id:
             current_choice = (self.instance.service_name, self.instance.service_name)
             if current_choice not in SERVICE_CHOICES:

allianceauth/services/auth_hooks.py

@@ -1,4 +1,6 @@
+from django.utils.translation import gettext_lazy as _
 from allianceauth import hooks
+
 from .hooks import MenuItemHook
 from .hooks import ServicesHook
 
@@ -6,7 +8,7 @@ from .hooks import ServicesHook
 class Services(MenuItemHook):
     def __init__(self):
         MenuItemHook.__init__(self,
-                              'Services',
+                              _('Services'),
                               'fa fa-cogs fa-fw',
                               'services:services', 100)
 

allianceauth/services/hooks.py

@@ -9,6 +9,29 @@ from allianceauth.hooks import get_hooks
 from .models import NameFormatConfig
 
 
+def get_extension_logger(name):
+    """
+    Takes the name of a plugin/extension and generates a child logger of the extensions logger
+    to be used by the extension to log events to the extensions logger.
+
+    The logging level is decided by whether or not DEBUG is set to true in the project settings. If
+    DEBUG is set to false, then the logging level is set to INFO.
+
+    :param: name: the name of the extension doing the logging
+    :return: an extensions child logger
+    """
+    import logging
+    from django.conf import settings
+
+    logger = logging.getLogger('extensions.' + name)
+    logger.name = name
+    logger.level = logging.INFO
+    if settings.DEBUG:
+        logger.level = logging.DEBUG
+
+    return logger
+
+
 class ServicesHook:
     """
     Abstract base class for creating a compatible services

allianceauth/services/migrations/0001_initial.py

@@ -1,38 +0,0 @@
-# -*- coding: utf-8 -*-
-# Generated by Django 1.10.1 on 2016-09-05 21:40
-from __future__ import unicode_literals
-
-from django.conf import settings
-from django.db import migrations, models
-import django.db.models.deletion
-
-
-class Migration(migrations.Migration):
-
-    initial = True
-
-    dependencies = [
-        ('auth', '0008_alter_user_username_max_length'),
-        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
-    ]
-
-    operations = [
-        migrations.CreateModel(
-            name='DiscordAuthToken',
-            fields=[
-                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('email', models.CharField(max_length=254, unique=True)),
-                ('token', models.CharField(max_length=254)),
-                ('user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
-            ],
-        ),
-        migrations.CreateModel(
-            name='GroupCache',
-            fields=[
-                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('created', models.DateTimeField(auto_now_add=True)),
-                ('groups', models.TextField(default={})),
-                ('service', models.CharField(choices=[(b'discourse', b'discourse'), (b'discord', b'discord')], max_length=254, unique=True)),
-            ],
-        ),
-    ]

allianceauth/services/migrations/0002_auto_20161016_0135.py

@@ -1,22 +0,0 @@
-# -*- coding: utf-8 -*-
-# Generated by Django 1.10.1 on 2016-10-16 01:35
-from __future__ import unicode_literals
-
-from django.db import migrations
-
-
-class Migration(migrations.Migration):
-
-    dependencies = [
-        ('services', '0001_initial'),
-    ]
-
-    operations = [
-        migrations.RemoveField(
-            model_name='discordauthtoken',
-            name='user',
-        ),
-        migrations.DeleteModel(
-            name='DiscordAuthToken',
-        ),
-    ]

allianceauth/services/migrations/0003_delete_groupcache.py

@@ -1,18 +0,0 @@
-# -*- coding: utf-8 -*-
-# Generated by Django 1.10.5 on 2017-09-02 06:07
-from __future__ import unicode_literals
-
-from django.db import migrations
-
-
-class Migration(migrations.Migration):
-
-    dependencies = [
-        ('services', '0002_auto_20161016_0135'),
-    ]
-
-    operations = [
-        migrations.DeleteModel(
-            name='GroupCache',
-        ),
-    ]

allianceauth/services/migrations/0003_remove_broken_link.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.10 on 2020-03-21 13:11
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('services', '0002_nameformatter'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='nameformatconfig',
+            name='format',
+            field=models.CharField(help_text='For information on constructing name formats please see the official documentation, topic "Services Name Formats".', max_length=100),
+        ),
+    ]

allianceauth/services/models.py

@@ -4,14 +4,30 @@ from allianceauth.authentication.models import State
 
 
 class NameFormatConfig(models.Model):
-    service_name = models.CharField(max_length=100, blank=False, null=False)
+    service_name = models.CharField(max_length=100, blank=False)
-    default_to_username = models.BooleanField(default=True, help_text="If a user has no main_character, "
+    default_to_username = models.BooleanField(
-                                                                      "default to using their Auth username instead.")
+        default=True, 
-    format = models.CharField(max_length=100, blank=False, null=False,
+        help_text=
-                              help_text='For information on constructing name formats, please see the '
+            'If a user has no main_character, '
-                                        '<a href="https://allianceauth.readthedocs.io/en/latest/features/nameformats">'
+            'default to using their Auth username instead.'
-                                        'name format documentation</a>')
+    )
-    states = models.ManyToManyField(State, help_text="States to apply this format to. You should only have one "
+    format = models.CharField(
-                                                     "formatter for each state for each service.")
+        max_length=100,
+        blank=False,
+        help_text=
+            'For information on constructing name formats '
+            'please see the official documentation, '
+            'topic "Services Name Formats".'
+    )
+    states = models.ManyToManyField(
+        State, 
+        help_text=
+            "States to apply this format to. You should only have one "
+            "formatter for each state for each service."        
+    )
 
+    def __str__(self):
+        return '%s: %s' % (
+            self.service_name, ', '.join([str(x) for x in self.states.all()])
+        )
 

allianceauth/services/modules/discord/__init__.py

@@ -1 +1,3 @@
-default_app_config = 'allianceauth.services.modules.discord.apps.DiscordServiceConfig'
+default_app_config = 'allianceauth.services.modules.discord.apps.DiscordServiceConfig'  # noqa
+
+__title__ = 'Discord Service'

allianceauth/services/modules/discord/admin.py

@@ -1,14 +1,22 @@
+import logging
+
 from django.contrib import admin
 
-from .models import DiscordUser
+from . import __title__
 from ...admin import ServicesUserAdmin
+from .models import DiscordUser
+from .utils import LoggerAddTag
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
 
 
 @admin.register(DiscordUser)
 class DiscordUserAdmin(ServicesUserAdmin):            
-    list_display = ServicesUserAdmin.list_display + (        
+    search_fields = ServicesUserAdmin.search_fields + ('uid', 'username')
-        '_uid',        
+    list_display = ServicesUserAdmin.list_display + ('activated', '_username', '_uid')
-    )    
+    list_filter = ServicesUserAdmin.list_filter + ('activated',)
+    ordering = ('-activated',)
    
     def _uid(self, obj):
         return obj.uid
@@ -16,3 +24,11 @@ class DiscordUserAdmin(ServicesUserAdmin):
     _uid.short_description = 'Discord ID (UID)'
     _uid.admin_order_field = 'uid'
 
+    def _username(self, obj):
+        if obj.username and obj.discriminator:
+            return f'{obj.username}#{obj.discriminator}'
+        else:
+            return ''
+    
+    _username.short_description = 'Discord Username'
+    _username.admin_order_field = 'username'

allianceauth/services/modules/discord/app_settings.py

@@ -0,0 +1,17 @@
+from .utils import clean_setting
+
+
+DISCORD_APP_ID = clean_setting('DISCORD_APP_ID', '')
+DISCORD_APP_SECRET = clean_setting('DISCORD_APP_SECRET', '')
+DISCORD_BOT_TOKEN = clean_setting('DISCORD_BOT_TOKEN', '')
+DISCORD_CALLBACK_URL = clean_setting('DISCORD_CALLBACK_URL', '')
+DISCORD_GUILD_ID = clean_setting('DISCORD_GUILD_ID', '')
+
+# max retries of tasks after an error occurred
+DISCORD_TASKS_MAX_RETRIES = clean_setting('DISCORD_TASKS_MAX_RETRIES', 3)
+
+# Pause in seconds until next retry for tasks after the API returned an error
+DISCORD_TASKS_RETRY_PAUSE = clean_setting('DISCORD_TASKS_RETRY_PAUSE', 60)
+
+# automatically sync Discord users names to user's main character name when created
+DISCORD_SYNC_NAMES = clean_setting('DISCORD_SYNC_NAMES', False)

allianceauth/services/modules/discord/auth_hooks.py

@@ -1,17 +1,26 @@
 import logging
 
+from django.contrib.auth.models import User 
 from django.template.loader import render_to_string
-from django.conf import settings
 
 from allianceauth import hooks
 from allianceauth.services.hooks import ServicesHook
-from .tasks import DiscordTasks
-from .urls import urlpatterns
 
-logger = logging.getLogger(__name__)
+from .models import DiscordUser
+from .urls import urlpatterns
+from .utils import LoggerAddTag
+from . import tasks, __title__
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
+
+# Default priority for single tasks like update group and sync nickname
+SINGLE_TASK_PRIORITY = 3
 
 
 class DiscordService(ServicesHook):
+    """Service for managing a Discord server with Auth"""
+
     def __init__(self):
         ServicesHook.__init__(self)
         self.urlpatterns = urlpatterns
@@ -20,36 +29,85 @@ class DiscordService(ServicesHook):
         self.access_perm = 'discord.access_discord'
         self.name_format = '{character_name}'
 
-    def delete_user(self, user, notify_user=False):
+    def delete_user(self, user: User, notify_user: bool = False) -> None:
-        logger.debug('Deleting user %s %s account' % (user, self.name))
+        if self.user_has_account(user):
-        return DiscordTasks.delete_user(user, notify_user=notify_user)
+            logger.debug('Deleting user %s %s account', user, self.name)
+            tasks.delete_user.apply_async(
+                kwargs={'user_pk': user.pk}, priority=SINGLE_TASK_PRIORITY
+            )        
     
-    def update_groups(self, user):
+    def render_services_ctrl(self, request):                
-        logger.debug('Processing %s groups for %s' % (self.name, user))
+        if self.user_has_account(request.user):
-        if DiscordTasks.has_account(user):
+            user_has_account = True
-            DiscordTasks.update_groups.delay(user.pk)
+            username = request.user.discord.username
+            discriminator = request.user.discord.discriminator
+            if username and discriminator:
+                discord_username = f'{username}#{discriminator}'
+            else:
+                discord_username = ''
+        else:
+            discord_username = ''
+            user_has_account = False
 
-    def validate_user(self, user):
+        return render_to_string(
-        logger.debug('Validating user %s %s account' % (user, self.name))
+            self.service_ctrl_template, 
-        if DiscordTasks.has_account(user) and not self.service_active_for_user(user):
+            {
-            self.delete_user(user, notify_user=True)
+                'server_name': DiscordUser.objects.server_name(),
-
+                'user_has_account': user_has_account,
-    def sync_nickname(self, user):
+                'discord_username': discord_username
-        logger.debug('Syncing %s nickname for user %s' % (self.name, user))
+            }, 
-        DiscordTasks.update_nickname.delay(user.pk)
+            request=request
-
+        )
-    def update_all_groups(self):
-        logger.debug('Update all %s groups called' % self.name)
-        DiscordTasks.update_all_groups.delay()
 
     def service_active_for_user(self, user):
         return user.has_perm(self.access_perm)
 
-    def render_services_ctrl(self, request):
+    def sync_nickname(self, user):
-        return render_to_string(self.service_ctrl_template, {
+        logger.debug('Syncing %s nickname for user %s', self.name, user)
-            'discord_uid': request.user.discord.uid if DiscordTasks.has_account(request.user) else None,
+        if self.user_has_account(user):
-            'DISCORD_SERVER_ID': getattr(settings, 'DISCORD_GUILD_ID', ''),
+            tasks.update_nickname.apply_async(
-        }, request=request)
+                kwargs={'user_pk': user.pk}, priority=SINGLE_TASK_PRIORITY
+            )
+
+    def sync_nicknames_bulk(self, users: list):
+        """Sync nickname for a list of users in bulk. 
+        Preferred over sync_nickname(), because it will not break the rate limit
+        """
+        logger.debug(
+            'Syncing %s nicknames in bulk for %d users', self.name, len(users)
+        )
+        user_pks = [user.pk for user in users]
+        tasks.update_nicknames_bulk.delay(user_pks)
+
+    def update_all_groups(self):
+        logger.debug('Update all %s groups called', self.name)
+        tasks.update_all_groups.delay()
+
+    def update_groups(self, user):        
+        logger.debug('Processing %s groups for %s', self.name, user)
+        if self.user_has_account(user):            
+            tasks.update_groups.apply_async(
+                kwargs={'user_pk': user.pk}, priority=SINGLE_TASK_PRIORITY
+            )
+
+    def update_groups_bulk(self, users: list):
+        """Updates groups for a list of users in bulk. 
+        Preferred over update_groups(), because it will not break the rate limit
+        """
+        logger.debug(
+            'Processing %s groups in bulk for %d users', self.name, len(users)
+        )
+        user_pks = [user.pk for user in users]
+        tasks.update_groups_bulk.delay(user_pks)
+    
+    @staticmethod
+    def user_has_account(user: User) -> bool:
+        return DiscordUser.objects.user_has_account(user)
+
+    def validate_user(self, user):
+        logger.debug('Validating user %s %s account', user, self.name)
+        if self.user_has_account(user) and not self.service_active_for_user(user):
+            self.delete_user(user, notify_user=True)
 
 
 @hooks.register('services_hook')

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

@@ -0,0 +1,2 @@
+from .client import DiscordClient   # noqa
+from .exceptions import DiscordApiBackoff  # noqa

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

@@ -0,0 +1,40 @@
+from ..utils import clean_setting
+
+
+# Base URL for all API calls. Must end with /.
+DISCORD_API_BASE_URL = clean_setting(
+    'DISCORD_API_BASE_URL', 'https://discordapp.com/api/'
+)
+
+# Low level timeout for requests to the Discord API in ms
+DISCORD_API_TIMEOUT = clean_setting(
+    'DISCORD_API_TIMEOUT', 5000
+)
+
+# Base authorization URL for Discord Oauth
+DISCORD_OAUTH_BASE_URL = clean_setting(
+    'DISCORD_OAUTH_BASE_URL', 'https://discordapp.com/api/oauth2/authorize'
+)
+
+# Base authorization URL for Discord Oauth
+DISCORD_OAUTH_TOKEN_URL = clean_setting(
+    'DISCORD_OAUTH_TOKEN_URL', 'https://discordapp.com/api/oauth2/token'
+)
+
+# 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
+)
+
+# 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
+)
+
+# Turns off creation of new roles. In case the rate limit for creating roles is
+# exhausted, this setting allows the Discord service to continue to function 
+# and wait out the reset. Rate limit is about 250 per 48 hrs.
+DISCORD_DISABLE_ROLE_CREATION = clean_setting(
+    'DISCORD_DISABLE_ROLE_CREATION', False
+)

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

@@ -0,0 +1,690 @@
+from hashlib import md5
+import logging
+from time import sleep
+from urllib.parse import urljoin
+from uuid import uuid1
+
+from redis import Redis
+import requests
+
+from django.core.cache import caches
+
+from allianceauth import __title__ as AUTH_TITLE, __url__, __version__
+
+from .. import __title__
+from .app_settings import (
+    DISCORD_API_BASE_URL,
+    DISCORD_API_TIMEOUT, 
+    DISCORD_DISABLE_ROLE_CREATION,
+    DISCORD_GUILD_NAME_CACHE_MAX_AGE,
+    DISCORD_OAUTH_BASE_URL, 
+    DISCORD_OAUTH_TOKEN_URL,    
+    DISCORD_ROLES_CACHE_MAX_AGE,    
+)
+from .exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError
+from ..utils import LoggerAddTag
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
+
+# max requests that can be executed until reset
+RATE_LIMIT_MAX_REQUESTS = 5        
+
+# Time until remaining requests are reset
+RATE_LIMIT_RESETS_AFTER = 5000
+
+# Delay used for API backoff in case no info returned from API on 429s
+DEFAULT_BACKOFF_DELAY = 5000
+
+# additional duration to compensate for potential clock discrepancies 
+# with the Discord server
+DURATION_CONTINGENCY = 500
+
+# Client will do a blocking wait rather than throwing a backoff exception if the
+# time until next reset is below this threshold
+WAIT_THRESHOLD = 250
+
+# If the rate limit resets soon we will wait it out and then retry to
+# either get a remaining request from our cached counter 
+# or again wait out a short reset time and retry again.
+# This could happen several times within a high concurrency situation, 
+# but must fail after x tries to avoid an infinite loop
+RATE_LIMIT_RETRIES = 1000
+
+
+class DiscordClient:
+    """This class provides a web client for interacting with the Discord API
+    
+    The client has rate limiting that supports concurrency.
+    This means it is able to ensure the API rate limit is not violated, 
+    even when used concurrently, e.g. with multiple parallel celery tasks.
+
+    In addition the client support proper API backoff.
+
+    Synchronization of rate limit infos accross multiple processes 
+    is implemented with Redis and thus requires Redis as Django cache backend.
+
+    All durations are in milliseconds.
+    """
+    OAUTH_BASE_URL = DISCORD_OAUTH_BASE_URL
+    OAUTH_TOKEN_URL = DISCORD_OAUTH_TOKEN_URL
+
+    _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_ROLE_NAME = 'DISCORD_ROLE_NAME'    
+    _ROLE_NAME_MAX_CHARS = 100
+    _NICK_MAX_CHARS = 32
+    
+    _HTTP_STATUS_CODE_NOT_FOUND = 404
+    _HTTP_STATUS_CODE_RATE_LIMITED = 429
+    _DISCORD_STATUS_CODE_UNKNOWN_MEMBER = 10007
+
+    def __init__(
+        self, 
+        access_token: str, 
+        redis: Redis = None, 
+        is_rate_limited: bool = True
+    ) -> None:
+        """        
+        Params:
+        - access_token: Discord access token used to authenticate all calls to the API
+        - redis: Redis instance to be used. 
+        - is_rate_limited: Set to False to run of rate limiting (use with care)
+        If not specified will try to use the Redis instance 
+        from the default Django cache backend.
+        """
+        self._access_token = str(access_token)
+        self._is_rate_limited = bool(is_rate_limited)
+        if not redis:
+            default_cache = caches['default']
+            self._redis = default_cache.get_master_client()
+            if not isinstance(self._redis, Redis):
+                raise RuntimeError(
+                    'This class requires a Redis client, but none was provided '
+                    'and the default Django cache backend is not Redis either.'
+                )
+        else:
+            self._redis = redis
+
+        lua_1 = """
+            if redis.call("exists", KEYS[1]) == 0 then
+                redis.call("set", KEYS[1], ARGV[1], 'px', ARGV[2])
+            end    
+            return redis.call("decr", KEYS[1])
+        """
+        self.__redis_script_decr_or_set = self._redis.register_script(lua_1)
+
+        lua_2 = """
+            local current_px = tonumber(redis.call("pttl", KEYS[1]))
+            if current_px < tonumber(ARGV[2]) then
+                return redis.call("set", KEYS[1], ARGV[1], 'px', ARGV[2])
+            else
+                return nil
+            end
+        """
+        self.__redis_script_set_longer = self._redis.register_script(lua_2)
+
+    @property
+    def access_token(self):
+        return self._access_token
+
+    @property
+    def is_rate_limited(self):
+        return self._is_rate_limited
+    
+    def __repr__(self):
+        return f'{type(self).__name__}(access_token=...{self.access_token[-5:]})'
+
+    def _redis_decr_or_set(self, name: str, value: str, px: int) -> bool:
+        """decreases the key value if it exists and returns the result
+        else sets the key
+        
+        Implemented as Lua script to ensure atomicity.
+        """
+        return self.__redis_script_decr_or_set(
+            keys=[str(name)], args=[str(value), int(px)]
+        )
+    
+    def _redis_set_if_longer(self, name: str, value: str, px: int) -> bool:
+        """like set, but only goes through if either key doesn't exist 
+        or px would be extended. 
+        
+        Implemented as Lua script to ensure atomicity.
+        """
+        return self.__redis_script_set_longer(
+            keys=[str(name)], args=[str(value), int(px)]
+        )
+
+    # users
+    
+    def current_user(self) -> dict:
+        """returns the user belonging to the current access_token"""
+        authorization = f'Bearer {self.access_token}'
+        r = self._api_request(
+            method='get', route='users/@me', authorization=authorization
+        )
+        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()
+
+    def guild_infos(self, guild_id: int) -> dict:
+        """Returns all basic infos about this guild"""
+        route = f"guilds/{guild_id}"
+        r = self._api_request(method='get', route=route)
+        return r.json()
+
+    def guild_name(self, guild_id: int) -> str:
+        """returns the name of this guild (cached) 
+        or an empty string if something went wrong
+        """
+        key_name = self._guild_name_cache_key(guild_id)
+        guild_name = self._redis_decode(self._redis.get(key_name))
+        if not guild_name:
+            guild_infos = self.guild_infos(guild_id)
+            if 'name' in guild_infos:
+                guild_name = guild_infos['name']
+                self._redis.set(
+                    name=key_name, 
+                    value=guild_name, 
+                    px=DISCORD_GUILD_NAME_CACHE_MAX_AGE
+                )
+            else:
+                guild_name = ''
+
+        return guild_name
+
+    @classmethod
+    def _guild_name_cache_key(cls, guild_id: int) -> str:        
+        """Returns key for accessing role given by name in the role cache"""
+        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"""
+        route = f"guilds/{guild_id}/roles"
+        r = self._api_request(method='get', route=route)
+        return r.json()
+
+    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:
+            return True
+        else:
+            return False
+
+    # guild role cache
+
+    def match_guild_roles_to_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
+        """
+        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)
+            )
+            if role:
+                roles.append((role, created))            
+        return roles
+
+    def match_guild_role_to_name(self, guild_id: int, role_name: str) -> 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    
+        """
+        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)
+
+            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)}
+    
+    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}'
+
+    # guild members
+
+    def add_guild_member(
+        self, 
+        guild_id: int, 
+        user_id: int, 
+        access_token: str, 
+        role_ids: list = None, 
+        nick: str = None
+    ) -> bool:           
+        """Adds a user to the guilds.
+        
+        Returns:
+        - True when a new user was added
+        - None if the user already existed
+        - False when something went wrong or raises exception
+        """
+        route = f"guilds/{guild_id}/members/{user_id}"
+        data = {
+            'access_token': str(access_token)
+        }
+        if role_ids:            
+            data['roles'] = self._sanitize_role_ids(role_ids)
+
+        if nick:
+            data['nick'] = str(nick)[:self._NICK_MAX_CHARS]
+
+        r = self._api_request(method='put', route=route, data=data)        
+        r.raise_for_status()
+        if r.status_code == 201:
+            return True
+        elif r.status_code == 204:
+            return None
+        else:
+            return False
+        
+    def guild_member(self, guild_id: int, user_id: int) -> dict:
+        """returns the user info for a guild member
+        
+        or None if the user is not a member of the guild
+        """
+        route = f'guilds/{guild_id}/members/{user_id}'
+        r = self._api_request(method='get', route=route, raise_for_status=False)
+        if self._is_member_unknown_error(r):
+            logger.warning("Discord user ID %s could not be found on server.", user_id)
+            return None
+        else:
+            r.raise_for_status()
+            return r.json()
+
+    def modify_guild_member(
+        self, guild_id: int, user_id: int, role_ids: list = None, nick: str = None
+    ) -> bool:
+        """Modify attributes of a guild member.
+
+        Returns
+        - True when successful
+        - None if user is not a member of this guild
+        - False otherwise
+        """
+        if not role_ids and not nick:
+            raise ValueError('Must specify role_ids or nick')
+        
+        if role_ids and not isinstance(role_ids, list):
+            raise TypeError('role_ids must be a list type')
+
+        data = dict()
+        if role_ids:            
+            data['roles'] = self._sanitize_role_ids(role_ids)
+        
+        if nick:
+            data['nick'] = self._sanitize_nick(nick)
+
+        route = f"guilds/{guild_id}/members/{user_id}"
+        r = self._api_request(
+            method='patch', route=route, data=data, raise_for_status=False
+        )
+        if self._is_member_unknown_error(r):
+            logger.warning('User ID %s is not a member of this guild', user_id)
+            return None
+        else:
+            r.raise_for_status()
+        
+        if r.status_code == 204:
+            return True
+        else:
+            return False
+
+    def remove_guild_member(self, guild_id: int, user_id: int) -> bool:
+        """Remove a member from a guild
+        
+        Returns:
+        - True when successful
+        - None if member does not exist
+        - False otherwise
+        """
+        route = f"guilds/{guild_id}/members/{user_id}"
+        r = self._api_request(
+            method='delete', route=route, raise_for_status=False
+        )        
+        if self._is_member_unknown_error(r):
+            logger.warning('User ID %s is not a member of this guild', user_id)
+            return None
+        else:
+            r.raise_for_status()
+
+        if r.status_code == 204:
+            return True
+        else:
+            return False
+
+    # Guild member roles
+    
+    def add_guild_member_role(
+        self, guild_id: int, user_id: int, role_id: int
+    ) -> bool:        
+        """Adds a role to a guild member
+        
+        Returns:
+        - True when successful
+        - None if member does not exist
+        - False otherwise
+        """
+        route = f"guilds/{guild_id}/members/{user_id}/roles/{role_id}"
+        r = self._api_request(method='put', route=route, raise_for_status=False)
+        if self._is_member_unknown_error(r):
+            logger.warning('User ID %s is not a member of this guild', user_id)
+            return None
+        else:
+            r.raise_for_status()
+        
+        if r.status_code == 204:
+            return True
+        else:
+            return False
+
+    def remove_guild_member_role(
+        self, guild_id: int, user_id: int, role_id: int
+    ) -> bool:
+        """Removes a role to a guild member
+        
+        Returns:
+        - True when successful
+        - None if member does not exist
+        - False otherwise
+        """
+        route = f"guilds/{guild_id}/members/{user_id}/roles/{role_id}"
+        r = self._api_request(method='delete', route=route, raise_for_status=False)
+        if self._is_member_unknown_error(r):
+            logger.warning('User ID %s is not a member of this guild', user_id)
+            return None
+        else:
+            r.raise_for_status()
+
+        if r.status_code == 204:
+            return True
+        else:
+            return False
+
+    @classmethod
+    def _is_member_unknown_error(cls, r: requests.Response) -> bool:
+        try:
+            result = (
+                r.status_code == cls._HTTP_STATUS_CODE_NOT_FOUND
+                and r.json()['code'] == cls._DISCORD_STATUS_CODE_UNKNOWN_MEMBER
+            )
+        except (ValueError, KeyError):
+            result = False
+        
+        return result
+
+    # Internal methods
+
+    def _api_request(
+        self, 
+        method: str, 
+        route: str, 
+        data: dict = None, 
+        authorization: str = None,
+        raise_for_status: bool = True
+    ) -> requests.Response:
+        """Core method for performing all API calls"""
+        uid = uuid1().hex
+        
+        if not hasattr(requests, method):
+            raise ValueError('Invalid method: %s' % method)
+
+        if not authorization:
+            authorization = f'Bot {self.access_token}'
+                
+        self._handle_ongoing_api_backoff(uid)        
+        if self.is_rate_limited:
+            self._ensure_rate_limed_not_exhausted(uid)        
+        headers = {
+            'User-Agent': f'{AUTH_TITLE} ({__url__}, {__version__})',
+            'accept': 'application/json',
+            'X-RateLimit-Precision': 'millisecond',
+            'authorization': str(authorization)
+        }
+        if data:
+            headers['content-type'] = 'application/json'
+
+        url = urljoin(DISCORD_API_BASE_URL, route)
+        args = {
+            'url': url,
+            'headers': headers,
+            'timeout': DISCORD_API_TIMEOUT / 1000
+        }
+        if data:
+            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)
+        r = getattr(requests, method)(**args)
+        logger.debug(
+            '%s: returned status code %d with headers:\n%s', 
+            uid, 
+            r.status_code, 
+            r.headers
+        )
+        logger.debug('%s: response:\n%s', uid, r.text)
+        if not r.ok:
+            logger.warning(
+                '%s: Discord API returned error code %d and this response: %s',
+                uid, 
+                r.status_code,
+                r.text
+            )
+
+        if r.status_code == self._HTTP_STATUS_CODE_RATE_LIMITED:
+            self._handle_new_api_backoff(r, uid)
+
+        self._report_rate_limit_from_api(r, uid)
+                
+        if raise_for_status:
+            r.raise_for_status()
+        
+        return r
+
+    def _handle_ongoing_api_backoff(self, uid: str) -> None:        
+        """checks if api is currently on backoff
+        if on backoff: will do a blocking wait if it expires soon, 
+        else raises exception
+        """
+        global_backoff_duration = self._redis.pttl(self._KEY_GLOBAL_BACKOFF_UNTIL)
+        if global_backoff_duration > 0:
+            if global_backoff_duration < WAIT_THRESHOLD:
+                logger.info(
+                    '%s: Global API backoff still ongoing for %s ms. Waiting.',
+                    uid,
+                    global_backoff_duration
+                )
+                sleep(global_backoff_duration / 1000)
+            else:
+                logger.info(
+                    '%s: Global API backoff still ongoing for %s ms. Re-raising.',
+                    uid,
+                    global_backoff_duration
+                )
+                raise DiscordTooManyRequestsError(retry_after=global_backoff_duration)
+
+    def _ensure_rate_limed_not_exhausted(self, uid: str) -> int:
+        """ensures that the rate limit is not exhausted 
+        if exhausted: will do a blocking wait if rate limit resets soon, 
+        else raises exception
+
+        returns requests remaining on success
+        """
+        for _ in range(RATE_LIMIT_RETRIES):
+            requests_remaining = self._redis_decr_or_set(
+                name=self._KEY_GLOBAL_RATE_LIMIT_REMAINING, 
+                value=RATE_LIMIT_MAX_REQUESTS, 
+                px=RATE_LIMIT_RESETS_AFTER + DURATION_CONTINGENCY
+            )            
+            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',                 
+                    uid,                
+                    requests_remaining + 1,
+                    resets_in
+                )
+                return requests_remaining
+
+            elif resets_in < WAIT_THRESHOLD:
+                sleep(resets_in / 1000)
+                logger.debug(
+                    '%s: No requests remaining until reset in %d ms. '
+                    'Waiting for reset.',
+                    uid, 
+                    resets_in
+                )
+                continue
+
+            else:                
+                logger.debug(
+                    '%s: No requests remaining until reset in %d ms. '
+                    'Raising exception.',
+                    uid, 
+                    resets_in
+                )
+                raise DiscordRateLimitExhausted(resets_in)
+
+        raise RuntimeError('Failed to handle rate limit after after too tries.')
+                    
+    def _handle_new_api_backoff(self, r: requests.Response, uid: str) -> None:
+        """raises exception for new API backoff error"""
+        response = r.json()
+        if 'retry_after' in response:
+            try:
+                retry_after = \
+                    int(response['retry_after']) + DURATION_CONTINGENCY
+            except ValueError:
+                retry_after = DEFAULT_BACKOFF_DELAY
+        else:
+            retry_after = DEFAULT_BACKOFF_DELAY
+        self._redis_set_if_longer(
+            name=self._KEY_GLOBAL_BACKOFF_UNTIL, 
+            value='GLOBAL_API_BACKOFF',
+            px=retry_after
+        )
+        logger.warning(
+            "%s: Rate limit violated. Need to back off for at least %d ms",
+            uid,
+            retry_after
+        )
+        raise DiscordTooManyRequestsError(retry_after=retry_after)
+
+    def _report_rate_limit_from_api(self, r, uid):
+        """Tries to log the current rate limit reported from API"""
+        if (
+            logger.getEffectiveLevel() <= logging.DEBUG
+            and 'x-ratelimit-limit' in r.headers 
+            and 'x-ratelimit-remaining' in r.headers 
+            and 'x-ratelimit-reset-after' in r.headers
+        ):
+            try:
+                limit = int(r.headers['x-ratelimit-limit'])
+                remaining = int(r.headers['x-ratelimit-remaining'])
+                reset_after = float(r.headers['x-ratelimit-reset-after']) * 1000
+                if remaining + 1 == limit:
+                    logger.debug(
+                        '%s: Rate limit reported from API: %d requests per %s ms',
+                        uid,
+                        limit,
+                        reset_after
+                    )
+            except ValueError:
+                pass
+       
+    @staticmethod
+    def _redis_decode(value: str) -> str:
+        """Decodes a string from Redis and passes through None and Booleans"""
+        if value is not None and not isinstance(value, bool):
+            return value.decode('utf-8')
+        else:
+            return value
+
+    @staticmethod
+    def _generate_hash(key: str) -> str:
+        return md5(key.encode('utf-8')).hexdigest()
+
+    @staticmethod
+    def _sanitize_role_ids(role_ids: list) -> list:
+        """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"""
+        return str(nick)[:cls._NICK_MAX_CHARS]

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

@@ -0,0 +1,33 @@
+import math
+
+
+class DiscordClientException(Exception):
+    """Base Exception for the Discord client"""
+   
+
+class DiscordApiBackoff(DiscordClientException):
+    """Exception signaling we need to backoff from sending requests to the API for now
+    """
+    
+    def __init__(self, retry_after: int):
+        """
+        :param retry_after: int time to retry after in milliseconds
+        """
+        super().__init__()
+        self.retry_after = int(retry_after)        
+
+    @property
+    def retry_after_seconds(self):
+        return math.ceil(self.retry_after / 1000)
+
+
+class DiscordRateLimitExhausted(DiscordApiBackoff):
+    """Exception signaling that the total number of requests allowed under the 
+    current rate limit have been exhausted and weed to wait until next reset.
+    """
+   
+
+class DiscordTooManyRequestsError(DiscordApiBackoff):
+    """API has responded with a 429 Too Many Requests Error. 
+    Need to backoff for now.
+    """    

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

@@ -0,0 +1,85 @@
+"""This is script is for concurrency testing the Discord client with a Discord server.
+
+It will run multiple requests against Discord with multiple workers in parallel.
+The results can be analysed in a special log file.
+
+This script is design to be run manually as unit test, e.g. by running the following:
+
+python manage.py test 
+allianceauth.services.modules.discord.discord_client.tests.piloting_concurrency
+
+To make it work please set the below mentioned environment variables for your server.
+Since this may cause lots of 429s we'd recommend NOT to use your 
+alliance Discord server for this.
+"""
+
+import os
+from random import random
+import threading
+from time import sleep
+from django.test import TestCase
+
+from .. import DiscordClient, DiscordApiBackoff
+
+from ...utils import set_logger_to_file
+
+logger = set_logger_to_file(
+    'allianceauth.services.modules.discord.discord_client.client', __file__
+)
+
+# Make sure to set these environnement variables for your Discord server and user
+DISCORD_GUILD_ID = os.environ['DISCORD_GUILD_ID']
+DISCORD_BOT_TOKEN = os.environ['DISCORD_BOT_TOKEN']
+DISCORD_USER_ID = os.environ['DISCORD_USER_ID']
+NICK = 'Dummy'
+
+# Configure these settings to adjust the load profile
+NUMBER_OF_WORKERS = 5
+NUMBER_OF_RUNS = 10
+
+# max seconds a worker waits before starting a new run
+# set to near 0 for max load preassure
+MAX_JITTER_PER_RUN_SECS = 1.0
+
+
+def worker(num: int):
+    """worker function"""
+    worker_info = 'worker %d' % num
+    logger.info('%s: started', worker_info)
+    client = DiscordClient(DISCORD_BOT_TOKEN)
+    try:
+        runs = 0
+        while runs < NUMBER_OF_RUNS:            
+            run_info = '%s: run %d' % (worker_info, runs + 1)
+            my_jitter_secs = random() * MAX_JITTER_PER_RUN_SECS
+            logger.info('%s - waiting %s secs', run_info, f'{my_jitter_secs:.3f}')
+            sleep(my_jitter_secs)
+            logger.info('%s - started', run_info)
+            try:
+                client.modify_guild_member(
+                    DISCORD_GUILD_ID, DISCORD_USER_ID, nick=NICK
+                )
+                runs += 1
+            except DiscordApiBackoff as bo:
+                message = '%s - waiting out API backoff for %d ms' % (
+                    run_info, bo.retry_after
+                )
+                logger.info(message)
+                print()
+                print(message)
+                sleep(bo.retry_after / 1000)                
+    
+    except Exception as ex:
+        logger.exception('%s: Processing aborted: %s', worker_info, ex)
+        
+    logger.info('%s: finished', worker_info)
+    return
+
+
+class TestMulti(TestCase):
+    
+    def test_multi(self):        
+        logger.info('Starting multi test')
+        for num in range(NUMBER_OF_WORKERS):
+            x = threading.Thread(target=worker, args=(num + 1,))
+            x.start()

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

@@ -0,0 +1,130 @@
+"""This script is for functional testing of the Discord client with a Discord server
+
+It will run single requests of the various functions to validate 
+that they actually work - excluding those that require Oauth, or does not work 
+with a bot token. The results can be also seen in a special log file.
+
+This script is design to be run manually as unit test, e.g. by running the following:
+
+python manage.py test
+allianceauth.services.modules.discord.discord_self.client.tests.piloting_functionality
+
+To make it work please set the below mentioned environment variables for your server.
+Since this may cause lots of 429s we'd recommend NOT to use your 
+alliance Discord server for this.
+"""
+
+from uuid import uuid1
+import os
+from unittest import TestCase
+from time import sleep
+
+from .. import DiscordClient
+from ...utils import set_logger_to_file
+
+logger = set_logger_to_file(
+    'allianceauth.services.modules.discord.discord_self.client.client', __file__
+)
+
+# Make sure to set these environnement variables for your Discord server and user
+DISCORD_GUILD_ID = os.environ['DISCORD_GUILD_ID']
+DISCORD_BOT_TOKEN = os.environ['DISCORD_BOT_TOKEN']
+DISCORD_USER_ID = os.environ['DISCORD_USER_ID']
+
+RATE_LIMIT_DELAY_SECS = 1
+
+
+class TestDiscordApiLive(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        logger.info('Live demo of the Discord API Client')
+        cls.client = DiscordClient(DISCORD_BOT_TOKEN)
+
+    def test_run_other_features(self):
+        """runs features that have not been run in any of the other tests"""
+        self.client.guild_infos(DISCORD_GUILD_ID)
+        sleep(RATE_LIMIT_DELAY_SECS)
+        
+        self.client.guild_name(DISCORD_GUILD_ID)
+        sleep(RATE_LIMIT_DELAY_SECS)
+
+        self.client.match_guild_role_to_name(DISCORD_GUILD_ID, 'Testrole')
+        sleep(RATE_LIMIT_DELAY_SECS)
+
+        self.client.match_guild_roles_to_names(
+            DISCORD_GUILD_ID, ['Testrole A', 'Testrole B']
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+        
+    def test_create_and_remove_roles(self):        
+        # get base        
+        logger.info('guild_roles')
+        expected = {role['id'] for role in self.client.guild_roles(DISCORD_GUILD_ID)}
+
+        # add role
+        role_name = 'my test role 12345678'
+        logger.info('create_guild_role')
+        new_role = self.client.create_guild_role(
+            guild_id=DISCORD_GUILD_ID, role_name=role_name
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+        self.assertEqual(new_role['name'], role_name)        
+        
+        # remove role again
+        logger.info('delete_guild_role')
+        self.client.delete_guild_role(
+            guild_id=DISCORD_GUILD_ID, role_id=new_role['id']
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+
+        # verify it worked
+        logger.info('guild_roles')
+        role_ids = {role['id'] for role in self.client.guild_roles(DISCORD_GUILD_ID)}
+        sleep(RATE_LIMIT_DELAY_SECS)
+        self.assertSetEqual(role_ids, expected)
+
+    def test_change_member_nick(self):
+        # set new nick for user
+        logger.info('modify_guild_member')
+        new_nick = f'Testnick {uuid1().hex}'[:32]
+        self.assertTrue(
+            self.client.modify_guild_member(
+                guild_id=DISCORD_GUILD_ID, user_id=DISCORD_USER_ID, nick=new_nick
+            )
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+
+        # verify it is saved
+        logger.info('guild_member')
+        user = self.client.guild_member(DISCORD_GUILD_ID, DISCORD_USER_ID)
+        sleep(RATE_LIMIT_DELAY_SECS)
+        self.assertEqual(user['nick'], new_nick)
+
+    def test_member_add_remove_roles(self):        
+        # create new guild role
+        logger.info('create_guild_role')
+        new_role = self.client.create_guild_role(
+            guild_id=DISCORD_GUILD_ID, role_name='Special role 98765'
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+        new_role_id = new_role['id']
+        
+        # add to member
+        logger.info('add_guild_member_role')
+        self.assertTrue(
+            self.client.add_guild_member_role(
+                guild_id=DISCORD_GUILD_ID, user_id=DISCORD_USER_ID, role_id=new_role_id
+            )
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)
+        
+        # remove again
+        logger.info('remove_guild_member_role')
+        self.assertTrue(
+            self.client.remove_guild_member_role(
+                guild_id=DISCORD_GUILD_ID, user_id=DISCORD_USER_ID, role_id=new_role_id
+            )
+        )
+        sleep(RATE_LIMIT_DELAY_SECS)

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

@@ -0,0 +1,47 @@
+"""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/rate_limits.md

@@ -0,0 +1,26 @@
+# Discord rate limits
+
+The following table shows the rate limit as reported from the API for different routes.
+
+method | limit | reset | rate / s | bucket
+-- | -- | -- | -- | --
+add_guild_member | 10 | 10,000 | 1 | self
+create_guild_role | 250 | 180,000,000 | 0.001 | self
+delete_guild_role | g | g | g | g
+guild_member | 5 | 1,000 | 5 | self
+guild_roles | g | g | g | g
+add_guild_member_role | 10 | 10,000 | 1 | B1
+remove_guild_member_role | 10 | 10,000 | 1 | B1
+modify_guild_member | 10 | 10,000 | 1 | self
+remove_guild_member | 5 | 1,000 | 5 | self
+current_user | g | g | g | g
+
+Legend:
+
+- g: global rate limit. API does not provide any rate limit infos for those routes.
+
+- reset: Values in milliseconds.
+
+- bucket: "self" means the rate limit is only counted for that route, Bx means the same rate limit is counted for multiple routes.
+
+- Data was collected on 2020-MAY-07 and is subject to change.

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

@@ -0,0 +1,33 @@
+from unittest import TestCase
+
+from ..exceptions import (
+    DiscordApiBackoff, 
+    DiscordClientException, 
+    DiscordRateLimitExhausted,
+    DiscordTooManyRequestsError
+)
+
+
+class TestExceptions(TestCase):
+
+    def test_DiscordApiException(self):
+        with self.assertRaises(DiscordClientException):
+            raise DiscordClientException()
+
+    def test_DiscordApiBackoff_raise(self):
+        with self.assertRaises(DiscordApiBackoff):
+            raise DiscordApiBackoff(999)
+
+    def test_DiscordApiBackoff_retry_after_seconds(self):
+        retry_after = 999
+        ex = DiscordApiBackoff(retry_after)
+        self.assertEqual(ex.retry_after, retry_after)
+        self.assertEqual(ex.retry_after_seconds, 1)
+
+    def test_DiscordRateLimitedExhausted_raise(self):
+        with self.assertRaises(DiscordRateLimitExhausted):
+            raise DiscordRateLimitExhausted(999)
+
+    def test_DiscordApiBackoffError_raise(self):
+        with self.assertRaises(DiscordTooManyRequestsError):
+            raise DiscordTooManyRequestsError(999)

allianceauth/services/modules/discord/manager.py

@@ -1,333 +0,0 @@
-import requests
-import math
-from django.conf import settings
-from requests_oauthlib import OAuth2Session
-from functools import wraps
-import logging
-import datetime
-import time
-from django.core.cache import cache
-from hashlib import md5
-
-logger = logging.getLogger(__name__)
-
-DISCORD_URL = "https://discordapp.com/api"
-
-AUTH_URL = "https://discordapp.com/api/oauth2/authorize"
-TOKEN_URL = "https://discordapp.com/api/oauth2/token"
-
-"""
-Previously all we asked for was permission to kick members, manage roles, and manage nicknames.
-Users have reported weird unauthorized errors we don't understand. So now we ask for full server admin.
-It's almost fixed the problem.
-"""
-# kick members, manage roles, manage nicknames, create instant invite
-# BOT_PERMISSIONS = 0x00000002 + 0x10000000 + 0x08000000 + 0x00000001
-BOT_PERMISSIONS = 0x00000008
-
-# get user ID, accept invite
-SCOPES = [
-    'identify',
-    'guilds.join',
-]
-
-GROUP_CACHE_MAX_AGE = getattr(settings, 'DISCORD_GROUP_CACHE_MAX_AGE', 2 * 60 * 60)  # 2 hours default
-
-
-class DiscordApiException(Exception):
-    def __init__(self):
-        super(Exception, self).__init__()
-
-
-class DiscordApiTooBusy(DiscordApiException):
-    def __init__(self):
-        super(DiscordApiException, self).__init__()
-        self.message = "The Discord API is too busy to process this request now, please try again later."
-
-
-class DiscordApiBackoff(DiscordApiException):
-    def __init__(self, retry_after, global_ratelimit):
-        """
-        :param retry_after: int time to retry after in milliseconds
-        :param global_ratelimit: bool Is the API under a global backoff
-        """
-        super(DiscordApiException, self).__init__()
-        self.retry_after = retry_after
-        self.global_ratelimit = global_ratelimit
-
-    @property
-    def retry_after_seconds(self):
-        return math.ceil(self.retry_after / 1000)
-
-
-cache_time_format = '%Y-%m-%d %H:%M:%S.%f'
-
-
-def api_backoff(func):
-    """
-    Decorator, Handles HTTP 429 "Too Many Requests" messages from the Discord API
-    If blocking=True is specified, this function will block and retry
-    the function up to max_retries=n times, or 3 if retries is not specified.
-    If the API call still recieves a backoff timer this function will raise
-    a <DiscordApiTooBusy> exception.
-    If the caller chooses blocking=False, the decorator will raise a DiscordApiBackoff
-    exception and the caller can choose to retry after the given timespan available in
-    the retry_after property in seconds.
-    """
-
-    class PerformBackoff(Exception):
-        def __init__(self, retry_after, retry_datetime, global_ratelimit):
-            super(Exception, self).__init__()
-            self.retry_after = int(retry_after)
-            self.retry_datetime = retry_datetime
-            self.global_ratelimit = global_ratelimit
-
-    @wraps(func)
-    def decorated(*args, **kwargs):
-        blocking = kwargs.get('blocking', False)
-        retries = kwargs.get('max_retries', 3)
-
-        # Strip our parameters
-        if 'max_retries' in kwargs:
-            del kwargs['max_retries']
-        if 'blocking' in kwargs:
-            del kwargs['blocking']
-
-        cache_key = 'DISCORD_BACKOFF_' + func.__name__
-        cache_global_key = 'DISCORD_BACKOFF_GLOBAL'
-
-        while retries > 0:
-            try:
-                try:
-                    # Check global backoff first, then route backoff
-                    existing_global_backoff = cache.get(cache_global_key)
-                    existing_backoff = existing_global_backoff or cache.get(cache_key)
-                    if existing_backoff:
-                        backoff_timer = datetime.datetime.strptime(existing_backoff, cache_time_format)
-                        if backoff_timer > datetime.datetime.utcnow():
-                            backoff_seconds = (backoff_timer - datetime.datetime.utcnow()).total_seconds()
-                            logger.debug("Still under backoff for %s seconds, backing off" % backoff_seconds)
-                            # Still under backoff
-                            raise PerformBackoff(
-                                retry_after=backoff_seconds,
-                                retry_datetime=backoff_timer,
-                                global_ratelimit=bool(existing_global_backoff)
-                            )
-                    logger.debug("Calling API calling function")
-                    return func(*args, **kwargs)
-                except requests.HTTPError as e:
-                    if e.response.status_code == 429:
-                        try:
-                            retry_after = int(e.response.headers['Retry-After'])
-                        except (TypeError, KeyError):
-                            # Pick some random time
-                            retry_after = 5000
-
-                        logger.info("Received backoff from API of %s seconds, handling" % retry_after)
-                        # Store value in redis
-                        backoff_until = (datetime.datetime.utcnow() +
-                                         datetime.timedelta(milliseconds=retry_after))
-                        global_backoff = bool(e.response.headers.get('X-RateLimit-Global', False))
-                        if global_backoff:
-                            logger.info("Global backoff!!")
-                            cache.set(cache_global_key, backoff_until.strftime(cache_time_format), retry_after)
-                        else:
-                            cache.set(cache_key, backoff_until.strftime(cache_time_format), retry_after)
-                        raise PerformBackoff(retry_after=retry_after, retry_datetime=backoff_until,
-                                             global_ratelimit=global_backoff)
-                    else:
-                        # Not 429, re-raise
-                        raise e
-            except PerformBackoff as bo:
-                # Sleep if we're blocking
-                if blocking:
-                    logger.info("Blocking Back off from API calls for %s seconds" % bo.retry_after)
-                    time.sleep((10 if bo.retry_after > 10 else bo.retry_after) / 1000)
-                else:
-                    # Otherwise raise exception and let caller handle the backoff
-                    raise DiscordApiBackoff(retry_after=bo.retry_after, global_ratelimit=bo.global_ratelimit)
-            finally:
-                retries -= 1
-        if retries == 0:
-            raise DiscordApiTooBusy()
-    return decorated
-
-
-class DiscordOAuthManager:
-    def __init__(self):
-        pass
-
-    @staticmethod
-    def _sanitize_name(name):
-        return name[:32]
-
-    @staticmethod
-    def _sanitize_group_name(name):
-        return name[:100]
-
-    @staticmethod
-    def generate_bot_add_url():
-        return AUTH_URL + '?client_id=' + settings.DISCORD_APP_ID + '&scope=bot&permissions=' + str(BOT_PERMISSIONS)
-
-    @staticmethod
-    def generate_oauth_redirect_url():
-        oauth = OAuth2Session(settings.DISCORD_APP_ID, redirect_uri=settings.DISCORD_CALLBACK_URL, scope=SCOPES)
-        url, state = oauth.authorization_url(AUTH_URL)
-        return url
-
-    @staticmethod
-    def _process_callback_code(code):
-        oauth = OAuth2Session(settings.DISCORD_APP_ID, redirect_uri=settings.DISCORD_CALLBACK_URL)
-        token = oauth.fetch_token(TOKEN_URL, client_secret=settings.DISCORD_APP_SECRET, code=code)
-        return token
-
-    @staticmethod
-    def add_user(code, groups, nickname=None):
-        try:
-            token = DiscordOAuthManager._process_callback_code(code)['access_token']
-            logger.debug("Received token from OAuth")
-
-            custom_headers = {'accept': 'application/json', 'authorization': 'Bearer ' + token}
-            path = DISCORD_URL + "/users/@me"
-            r = requests.get(path, headers=custom_headers)
-            logger.debug("Got status code %s after retrieving Discord profile" % r.status_code)
-            r.raise_for_status()
-
-            user_id = r.json()['id']
-
-            path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
-            group_ids = [DiscordOAuthManager._group_name_to_id(DiscordOAuthManager._sanitize_group_name(g)) for g in
-                         groups]
-            data = {
-                'roles': group_ids,
-                'access_token': token,
-            }
-            if nickname:
-                data['nick'] = DiscordOAuthManager._sanitize_name(nickname)
-            custom_headers['authorization'] = 'Bot ' + settings.DISCORD_BOT_TOKEN
-            r = requests.put(path, headers=custom_headers, json=data)
-            logger.debug("Got status code %s after joining Discord server" % r.status_code)
-            r.raise_for_status()
-
-            logger.info("Added Discord user ID %s to server." % user_id)
-            return user_id
-        except:
-            logger.exception("Failed to add Discord user")
-            return None
-
-    @staticmethod
-    @api_backoff
-    def update_nickname(user_id, nickname):
-        nickname = DiscordOAuthManager._sanitize_name(nickname)
-        custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        data = {'nick': nickname}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
-        r = requests.patch(path, headers=custom_headers, json=data)
-        logger.debug("Got status code %s after setting nickname for Discord user ID %s (%s)" % (
-            r.status_code, user_id, nickname))
-        if r.status_code == 404:
-            logger.warn("Discord user ID %s could not be found in server." % user_id)
-            return True
-        r.raise_for_status()
-        return True
-
-    @staticmethod
-    def delete_user(user_id):
-        try:
-            custom_headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-            path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
-            r = requests.delete(path, headers=custom_headers)
-            logger.debug("Got status code %s after removing Discord user ID %s" % (r.status_code, user_id))
-            if r.status_code == 404:
-                logger.warn("Discord user ID %s already left the server." % user_id)
-                return True
-            r.raise_for_status()
-            return True
-        except:
-            logger.exception("Failed to remove Discord user ID %s" % user_id)
-            return False
-
-    @staticmethod
-    def _get_groups():
-        custom_headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/roles"
-        r = requests.get(path, headers=custom_headers)
-        logger.debug("Got status code %s after retrieving Discord roles" % r.status_code)
-        r.raise_for_status()
-        return r.json()
-
-    @staticmethod
-    def _generate_cache_role_key(name):
-        return 'DISCORD_ROLE_NAME__%s' % md5(str(name).encode('utf-8')).hexdigest()
-
-    @staticmethod
-    def _group_name_to_id(name):
-        name = DiscordOAuthManager._sanitize_group_name(name)
-
-        def get_or_make_role():
-            groups = DiscordOAuthManager._get_groups()
-            for g in groups:
-                if g['name'] == name:
-                    return g['id']
-            return DiscordOAuthManager._create_group(name)['id']
-        return cache.get_or_set(DiscordOAuthManager._generate_cache_role_key(name), get_or_make_role, GROUP_CACHE_MAX_AGE)
-
-    @staticmethod
-    def __generate_role(name, **kwargs):
-        custom_headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/roles"
-        data = {'name': name}
-        data.update(kwargs)
-        r = requests.post(path, headers=custom_headers, json=data)
-        logger.debug("Received status code %s after generating new role." % r.status_code)
-        r.raise_for_status()
-        return r.json()
-
-    @staticmethod
-    def __edit_role(role_id, **kwargs):
-        custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/roles/" + str(role_id)
-        r = requests.patch(path, headers=custom_headers, json=kwargs)
-        logger.debug("Received status code %s after editing role id %s" % (r.status_code, role_id))
-        r.raise_for_status()
-        return r.json()
-
-    @staticmethod
-    def _create_group(name):
-        return DiscordOAuthManager.__generate_role(name)
-
-    @staticmethod
-    def _get_user(user_id):
-        custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
-        r = requests.get(path, headers=custom_headers)
-        r.raise_for_status()
-        return r.json()
-
-    @staticmethod
-    def _get_user_roles(user_id):
-        user = DiscordOAuthManager._get_user(user_id)
-        return user['roles']
-
-    @staticmethod
-    def _modify_user_role(user_id, role_id, method):
-        custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id) + "/roles/" + str(
-            role_id)
-        r = getattr(requests, method)(path, headers=custom_headers)
-        r.raise_for_status()
-        logger.debug("%s role %s for user %s" % (method, role_id, user_id))
-
-    @staticmethod
-    @api_backoff
-    def update_groups(user_id, groups):
-        group_ids = [DiscordOAuthManager._group_name_to_id(DiscordOAuthManager._sanitize_group_name(g)) for g in groups]
-        user_group_ids = DiscordOAuthManager._get_user_roles(user_id)
-        for g in group_ids:
-            if g not in user_group_ids:
-                DiscordOAuthManager._modify_user_role(user_id, g, 'put')
-                time.sleep(1)  # we're gonna be hammering the API here
-        for g in user_group_ids:
-            if g not in group_ids:
-                DiscordOAuthManager._modify_user_role(user_id, g, 'delete')
-                time.sleep(1)

allianceauth/services/modules/discord/managers.py

@@ -0,0 +1,175 @@
+import logging
+from urllib.parse import urlencode
+
+from requests_oauthlib import OAuth2Session
+from requests.exceptions import HTTPError
+
+from django.contrib.auth.models import User
+from django.db import models
+from django.utils.timezone import now
+
+from allianceauth.services.hooks import NameFormatter
+
+from . import __title__
+from .app_settings import (
+    DISCORD_APP_ID,
+    DISCORD_APP_SECRET,
+    DISCORD_BOT_TOKEN,
+    DISCORD_CALLBACK_URL,
+    DISCORD_GUILD_ID,    
+    DISCORD_SYNC_NAMES
+)
+from .discord_client import DiscordClient, DiscordApiBackoff
+from .utils import LoggerAddTag
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
+
+
+class DiscordUserManager(models.Manager):
+    """Manager for DiscordUser"""
+
+    # full server admin
+    BOT_PERMISSIONS = 0x00000008
+
+    # get user ID, accept invite
+    SCOPES = [
+        'identify',
+        'guilds.join',
+    ]
+
+    def add_user(
+        self, 
+        user: User, 
+        authorization_code: str, 
+        is_rate_limited: bool = True
+    ) -> bool:
+        """adds a new Discord user
+
+        Params:
+        - user: Auth user to join
+        - authorization_code: authorization code returns from oauth
+        - is_rate_limited: When False will disable default rate limiting (use with care)
+        
+        Returns: True on success, else False or raises exception
+        """
+        try:        
+            nickname = self.user_formatted_nick(user) if DISCORD_SYNC_NAMES else None
+            group_names = self.user_group_names(user)
+            access_token = self._exchange_auth_code_for_token(authorization_code)
+            user_client = DiscordClient(access_token, is_rate_limited=is_rate_limited)
+            discord_user = user_client.current_user()
+            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
+                )
+            else:
+                role_ids = None
+                        
+            created = bot_client.add_guild_member(
+                guild_id=DISCORD_GUILD_ID, 
+                user_id=user_id,
+                access_token=access_token,
+                role_ids=role_ids,
+                nick=nickname
+            )
+            if created is not False:
+                if created is None:
+                    logger.debug(
+                        "User %s with Discord ID %s is already a member.",
+                        user,
+                        user_id,
+                    )
+                self.update_or_create(
+                    user=user, 
+                    defaults={
+                        'uid': user_id, 
+                        'username': discord_user['username'][:32], 
+                        'discriminator': discord_user['discriminator'][:4],
+                        'activated': now()
+                    }
+                )
+                logger.info(
+                    "Added user %s with Discord ID %s to Discord server", user, user_id
+                )
+                return True
+
+            else:
+                logger.warning(
+                    "Failed to add user %s with Discord ID %s to Discord server",
+                    user,
+                    user_id,
+                )
+                return False
+
+        except (HTTPError, ConnectionError, DiscordApiBackoff) as ex:
+            logger.exception(
+                'Failed to add user %s to Discord server: %s', user, ex
+            )
+            return False
+
+    @staticmethod
+    def user_formatted_nick(user: User) -> str:
+        """returns the name of the given users main character with name formatting
+        or None if user has no main
+        """
+        from .auth_hooks import DiscordService
+        
+        if user.profile.main_character:
+            return NameFormatter(DiscordService(), user).format_name()
+        else:
+            return None
+
+    @staticmethod
+    def user_group_names(user: User) -> list:
+        """returns list of group names plus state the given user is a member of"""
+        return [group.name for group in user.groups.all()] + [user.profile.state.name]
+    
+    def user_has_account(self, user: User) -> bool:
+        """Returns True if the user has an Discord account, else False
+        
+        only checks locally, does not hit the API
+        """
+        return True if hasattr(user, self.model.USER_RELATED_NAME) else False
+
+    @classmethod
+    def generate_bot_add_url(cls):
+        params = urlencode({
+            'client_id': DISCORD_APP_ID,
+            'scope': 'bot',
+            'permissions': str(cls.BOT_PERMISSIONS)
+
+        })
+        return f'{DiscordClient.OAUTH_BASE_URL}?{params}'        
+
+    @classmethod
+    def generate_oauth_redirect_url(cls):
+        oauth = OAuth2Session(
+            DISCORD_APP_ID, redirect_uri=DISCORD_CALLBACK_URL, scope=cls.SCOPES
+        )
+        url, state = oauth.authorization_url(DiscordClient.OAUTH_BASE_URL)
+        return url
+
+    @staticmethod
+    def _exchange_auth_code_for_token(authorization_code: str) -> str:
+        oauth = OAuth2Session(DISCORD_APP_ID, redirect_uri=DISCORD_CALLBACK_URL)
+        token = oauth.fetch_token(
+            DiscordClient.OAUTH_TOKEN_URL, 
+            client_secret=DISCORD_APP_SECRET, 
+            code=authorization_code
+        )
+        logger.debug("Received token from OAuth")
+        return token['access_token']
+    
+    @classmethod
+    def server_name(cls):
+        """returns the name of the Discord server"""        
+        return cls._bot_client().guild_name(DISCORD_GUILD_ID)
+
+    @staticmethod
+    def _bot_client(is_rate_limited: bool = True):
+        """returns a bot client for access to the Discord API"""
+        return DiscordClient(DISCORD_BOT_TOKEN, is_rate_limited=is_rate_limited)

allianceauth/services/modules/discord/migrations/0003_big_overhaul.py

@@ -0,0 +1,40 @@
+# Generated by Django 2.2.12 on 2020-05-10 19:59
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('discord', '0002_service_permissions'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='discorduser',
+            name='activated',
+            field=models.DateTimeField(blank=True, default=None, help_text='Date & time this service account was activated', null=True),
+        ),
+        migrations.AddField(
+            model_name='discorduser',
+            name='discriminator',
+            field=models.CharField(blank=True, default='', help_text="user's discriminator on Discord", max_length=4),
+        ),
+        migrations.AddField(
+            model_name='discorduser',
+            name='username',
+            field=models.CharField(blank=True, db_index=True, default='', help_text="user's username on Discord", max_length=32),
+        ),
+        migrations.AlterField(
+            model_name='discorduser',
+            name='uid',
+            field=models.BigIntegerField(db_index=True, help_text="user's ID on Discord"),
+        ),
+        migrations.AlterField(
+            model_name='discorduser',
+            name='user',
+            field=models.OneToOneField(help_text='Auth user owning this Discord account', on_delete=django.db.models.deletion.CASCADE, primary_key=True, related_name='discord', serialize=False, to=settings.AUTH_USER_MODEL),
+        ),
+    ]

allianceauth/services/modules/discord/models.py

@@ -1,18 +1,179 @@
+import logging
+
+from requests.exceptions import HTTPError
+
 from django.contrib.auth.models import User
 from django.db import models
+from django.utils.translation import gettext_lazy
+
+from allianceauth.notifications import notify
+
+from . import __title__
+from .app_settings import DISCORD_GUILD_ID
+from .discord_client import DiscordClient, DiscordApiBackoff
+from .managers import DiscordUserManager
+from .utils import LoggerAddTag
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
 
 
 class DiscordUser(models.Model):
-    user = models.OneToOneField(User,
-                                primary_key=True,
-                                on_delete=models.CASCADE,
-                                related_name='discord')
-    uid = models.CharField(max_length=254)
 
-    def __str__(self):
+    USER_RELATED_NAME = 'discord'
-        return "{} - {}".format(self.user.username, self.uid)
+    
+    user = models.OneToOneField(
+        User, 
+        primary_key=True, 
+        on_delete=models.CASCADE, 
+        related_name=USER_RELATED_NAME,
+        help_text='Auth user owning this Discord account'
+    )
+    uid = models.BigIntegerField(
+        db_index=True,
+        help_text='user\'s ID on Discord'
+    )
+    username = models.CharField(
+        max_length=32, 
+        default='', 
+        blank=True,
+        db_index=True,
+        help_text='user\'s username on Discord'
+    )
+    discriminator = models.CharField(
+        max_length=4, 
+        default='', 
+        blank=True, 
+        help_text='user\'s discriminator on Discord'
+    )
+    activated = models.DateTimeField(
+        default=None, 
+        null=True, 
+        blank=True,
+        help_text='Date & time this service account was activated'
+    )
+
+    objects = DiscordUserManager()
 
     class Meta:
         permissions = (
-            ("access_discord", u"Can access the Discord service"),
+            ("access_discord", "Can access the Discord service"),
         )
+
+    def __str__(self):
+        return f'{self.user.username} - {self.uid}'
+
+    def __repr__(self):
+        return f'{type(self).__name__}(user=\'{self.user}\', uid={self.uid})'
+
+    def update_nickname(self) -> bool:
+        """Update nickname with formatted name of main character
+                
+        Returns:
+        - True on success
+        - None if user is no longer a member of the Discord server
+        - False on error or raises exception
+        """        
+        requested_nick = DiscordUser.objects.user_formatted_nick(self.user)
+        if requested_nick:            
+            client = DiscordUser.objects._bot_client()            
+            success = client.modify_guild_member(
+                guild_id=DISCORD_GUILD_ID,
+                user_id=self.uid,
+                nick=requested_nick
+            )
+            if success:
+                logger.info('Nickname for %s has been updated', self.user)
+            else:
+                logger.warning('Failed to update nickname for %s', self.user)
+            return success
+         
+        else:
+            return False
+
+    def update_groups(self) -> bool:
+        """update groups for a user based on his current group memberships. 
+        Will add or remove roles of a user as needed.
+        
+        Returns:
+        - 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)
+        else:
+            logger.warning('Failed to update groups for %s', self.user)
+        return success
+
+    def delete_user(
+        self, notify_user: bool = False, is_rate_limited: bool = True
+    ) -> bool:
+        """Deletes the Discount user both on the server and locally
+
+        Params:
+        - 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)
+        
+        Returns True when successful, otherwise False or raises exceptions
+        Return None if user does no longer exist
+        """
+        try:
+            client = DiscordUser.objects._bot_client(is_rate_limited=is_rate_limited)
+            success = client.remove_guild_member(
+                guild_id=DISCORD_GUILD_ID, user_id=self.uid
+            )
+            if success is not False:
+                deleted_count, _ = self.delete()
+                if deleted_count > 0:
+                    if notify_user:
+                        notify(
+                            user=self.user, 
+                            title=gettext_lazy('Discord Account Disabled'), 
+                            message=gettext_lazy(
+                                'Your Discord account was disabeled automatically '
+                                'by Auth. If you think this was a mistake, '
+                                'please contact an admin.'
+                            ),
+                            level='warning'
+                        )
+                    logger.info('Account for user %s was deleted.', self.user)
+                    return True
+                else:
+                    logger.debug('Account for user %s was already deleted.', self.user)
+                    return None
+            
+            else:
+                logger.warning(
+                    'Failed to remove user %s from the Discord server', self.user
+                )
+                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
+            )
+        ]

allianceauth/services/modules/discord/tasks.py

@@ -1,148 +1,187 @@
 import logging
 
-from django.conf import settings
+from celery import shared_task, chain
-from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist
-from allianceauth.notifications import notify
-from celery import shared_task
 from requests.exceptions import HTTPError
-from allianceauth.services.hooks import NameFormatter
+
-from .manager import DiscordOAuthManager, DiscordApiBackoff
+from django.contrib.auth.models import User
-from .models import DiscordUser
+from django.db.models.query import QuerySet
+
 from allianceauth.services.tasks import QueueOnce
 
-logger = logging.getLogger(__name__)
+from . import __title__
+from .app_settings import (
+    DISCORD_TASKS_MAX_RETRIES, DISCORD_TASKS_RETRY_PAUSE, DISCORD_SYNC_NAMES
+)
+from .discord_client import DiscordApiBackoff
+from .models import DiscordUser
+from .utils import LoggerAddTag
 
 
-class DiscordTasks:
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
-    def __init__(self):
-        pass
 
-    @classmethod
+# task priority of bulk tasks
-    def add_user(cls, user, code):
+BULK_TASK_PRIORITY = 6
-        groups = DiscordTasks.get_groups(user)
-        nickname = None
-        if settings.DISCORD_SYNC_NAMES:
-            nickname = DiscordTasks.get_nickname(user)
-        user_id = DiscordOAuthManager.add_user(code, groups, nickname=nickname)
-        if user_id:
-            discord_user = DiscordUser()
-            discord_user.user = user
-            discord_user.uid = user_id
-            discord_user.save()
-            return True
-        return False
 
-    @classmethod
-    def delete_user(cls, user, notify_user=False):
-        if cls.has_account(user):
-            logger.debug("User %s has discord account %s. Deleting." % (user, user.discord.uid))
-            if DiscordOAuthManager.delete_user(user.discord.uid):
-                user.discord.delete()
-                if notify_user:
-                    notify(user, 'Discord Account Disabled', level='danger')
-                return True
-        return False
 
-    @classmethod
+@shared_task(
-    def has_account(cls, user):
+    bind=True, name='discord.update_groups', base=QueueOnce, max_retries=None
-        """
+)
-        Check if the user has an account (has a DiscordUser record)
+def update_groups(self, user_pk: int) -> None:
-        :param user: django.contrib.auth.models.User
+    """Update roles on Discord for given user according to his current groups
-        :return: bool
+    
-        """
+    Params:
+    - user_pk: PK of given user
+    """
+    _task_perform_user_action(self, user_pk, 'update_groups')
+
+
+@shared_task(
+    bind=True, name='discord.update_nickname', base=QueueOnce, max_retries=None
+)
+def update_nickname(self, user_pk: int) -> None:
+    """Set nickname on Discord for given user to his main character name
+    
+    Params:
+    - user_pk: PK of given user
+    """
+    _task_perform_user_action(self, user_pk, 'update_nickname')
+
+
+@shared_task(
+    bind=True, name='discord.delete_user', base=QueueOnce, max_retries=None
+)
+def delete_user(self, user_pk: int, notify_user: bool = False) -> None:
+    """Delete Discord user
+    
+    Params:
+    - user_pk: PK of given user
+    """
+    _task_perform_user_action(self, user_pk, 'delete_user', notify_user=notify_user)
+
+
+def _task_perform_user_action(self, user_pk: int, method: str, **kwargs) -> None:
+    """perform a user related action incl. managing all exceptions"""
+    logger.debug("Starting %s for user with pk %s", method, user_pk)
+    user = User.objects.get(pk=user_pk)    
+    if DiscordUser.objects.user_has_account(user):
+        logger.info("Running %s for user %s", method, user)
         try:
-            user.discord
+            success = getattr(user.discord, method)(**kwargs)
-        except ObjectDoesNotExist:
-            return False
-        else:
-            return True
         
-    @staticmethod
+        except DiscordApiBackoff as bo:
-    @shared_task(bind=True, name='discord.update_groups', base=QueueOnce)
+            logger.info(
-    def update_groups(self, pk):
+                "API back off for %s wth user %s due to %r, retrying in %s seconds",
-        user = User.objects.get(pk=pk)
+                method,
-        logger.debug("Updating discord groups for user %s" % user)
+                user,
-        if DiscordTasks.has_account(user):
+                bo,
-            groups = DiscordTasks.get_groups(user)
+                bo.retry_after_seconds
-            logger.debug("Updating user %s discord groups to %s" % (user, groups))
+            )
-            try:
+            raise self.retry(countdown=bo.retry_after_seconds)     
-                DiscordOAuthManager.update_groups(user.discord.uid, groups)
-            except DiscordApiBackoff as bo:
-                logger.info("Discord group sync API back off for %s, "
-                            "retrying in %s seconds" % (user, bo.retry_after_seconds))
-                raise self.retry(countdown=bo.retry_after_seconds)
-            except HTTPError as e:
-                if e.response.status_code == 404:
-                    try:
-                        if e.response.json()['code'] == 10007:
-                            # user has left the server
-                            DiscordTasks.delete_user(user)
-                            return
-                    finally:
-                        raise e
-            except Exception as e:
-                if self:
-                    logger.exception("Discord group sync failed for %s, retrying in 10 mins" % user)
-                    raise self.retry(countdown=60 * 10)
-                else:
-                    # Rethrow
-                    raise e
-            logger.debug("Updated user %s discord groups." % user)
-        else:
-            logger.debug("User does not have a discord account, skipping")
         
-    @staticmethod
+        except AttributeError:
-    @shared_task(name='discord.update_all_groups')
+            raise ValueError(f'{method} not a valid method for DiscordUser: %r')
-    def update_all_groups():
-        logger.debug("Updating ALL discord groups")
-        for discord_user in DiscordUser.objects.exclude(uid__exact=''):
-            DiscordTasks.update_groups.delay(discord_user.user.pk)
 
-    @staticmethod
+        except (HTTPError, ConnectionError):           
-    @shared_task(bind=True, name='discord.update_nickname', base=QueueOnce)
+            logger.warning(
-    def update_nickname(self, pk):
+                '%s failed for user %s, retrying in %d secs', 
-        user = User.objects.get(pk=pk)
+                method,
-        logger.debug("Updating discord nickname for user %s" % user)
+                user,                    
-        if DiscordTasks.has_account(user):
+                DISCORD_TASKS_RETRY_PAUSE,
-            if user.profile.main_character:
+                exc_info=True
-                character = user.profile.main_character
+            )
-                logger.debug("Updating user %s discord nickname to %s" % (user, character.character_name))
+            if self.request.retries < DISCORD_TASKS_MAX_RETRIES:
-                try:
+                raise self.retry(countdown=DISCORD_TASKS_RETRY_PAUSE)
-                    DiscordOAuthManager.update_nickname(user.discord.uid, DiscordTasks.get_nickname(user))
-                except DiscordApiBackoff as bo:
-                    logger.info("Discord nickname update API back off for %s, "
-                                "retrying in %s seconds" % (user, bo.retry_after_seconds))
-                    raise self.retry(countdown=bo.retry_after_seconds)
-                except Exception as e:
-                    if self:
-                        logger.exception("Discord nickname sync failed for %s, retrying in 10 mins" % user)
-                        raise self.retry(countdown=60 * 10)
-                    else:
-                        # Rethrow
-                        raise e
-                logger.debug("Updated user %s discord nickname." % user)
             else:                                            
-                logger.debug("User %s does not have a main character" % user)
+                logger.error(
+                    '%s failed for user %s after max retries',
+                    method,
+                    user,                    
+                    exc_info=True
+                )
+        except Exception:
+            logger.error(
+                '%s for %s failed due to unexpected exception',
+                method,
+                user,
+                exc_info=True
+            )            
+        
         else:
-            logger.debug("User %s does not have a discord account" % user)
+            if success is None and method != 'delete_user':
+                delete_user.delay(user.pk, notify_user=True)
 
-    @staticmethod
+    else:
-    @shared_task(name='discord.update_all_nicknames')
+        logger.debug(
-    def update_all_nicknames():
+            'User %s does not have a discord account, skipping %s', user, method
-        logger.debug("Updating ALL discord nicknames")
+        )
-        for discord_user in DiscordUser.objects.exclude(uid__exact=''):
-            DiscordTasks.update_nickname.delay(discord_user.user.pk)
 
-    @classmethod
-    def disable(cls):
-        DiscordUser.objects.all().delete()
 
-    @staticmethod
+@shared_task(name='discord.update_all_groups')
-    def get_nickname(user):
+def update_all_groups() -> None:
-        from .auth_hooks import DiscordService
+    """Update roles for all known users with a Discord account."""    
-        return NameFormatter(DiscordService(), user).format_name()
+    discord_users_qs = DiscordUser.objects.all()
+    _bulk_update_groups_for_users(discord_users_qs)
 
-    @staticmethod
+
-    def get_groups(user):
+@shared_task(name='discord.update_groups_bulk')
-        return [g.name for g in user.groups.all()] + [user.profile.state.name]
+def update_groups_bulk(user_pks: list) -> None:
+    """Update roles 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_groups_for_users(discord_users_qs)
+
+
+def _bulk_update_groups_for_users(discord_users_qs: QuerySet) -> None:
+    logger.info(
+        "Starting to bulk update discord roles for %d users", discord_users_qs.count()
+    )
+    update_groups_chain = list()
+    for discord_user in discord_users_qs:
+        update_groups_chain.append(update_groups.si(discord_user.user.pk))
+    
+    chain(update_groups_chain).apply_async(priority=BULK_TASK_PRIORITY)
+
+
+@shared_task(name='discord.update_all_nicknames')
+def update_all_nicknames() -> None:
+    """Update nicknames for all known users with a Discord account."""
+    discord_users_qs = DiscordUser.objects.all()
+    _bulk_update_nicknames_for_users(discord_users_qs)
+    
+
+@shared_task(name='discord.update_nicknames_bulk')
+def update_nicknames_bulk(user_pks: list) -> None:
+    """Update nicknames 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_nicknames_for_users(discord_users_qs)
+
+
+def _bulk_update_nicknames_for_users(discord_users_qs: QuerySet) -> None:
+    logger.info(
+        "Starting to bulk update discord nicknames for %d users", 
+        discord_users_qs.count()
+    )
+    update_nicknames_chain = list()
+    for discord_user in discord_users_qs:
+        update_nicknames_chain.append(update_nickname.si(discord_user.user.pk))
+    
+    chain(update_nicknames_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()
+    )
+    update_all_chain = list()
+    for discord_user in discord_users_qs:
+        update_all_chain.append(update_groups.si(discord_user.user.pk))
+        if DISCORD_SYNC_NAMES:
+            update_all_chain.append(update_nickname.si(discord_user.user.pk))
+        
+    chain(update_all_chain).apply_async(priority=BULK_TASK_PRIORITY)

allianceauth/services/modules/discord/templates/services/discord/discord_service_ctrl.html

@@ -3,10 +3,18 @@
 
 <tr>
     <td class="text-center">Discord</td>
-    <td class="text-center"></td>
-    <td class="text-center"><a href="https://discordapp.com/channels/{{ DISCORD_SERVER_ID }}/{{ DISCORD_SERVER_ID}}">https://discordapp.com</a></td>
     <td class="text-center">
-        {% if not discord_uid %}
+        {% if not user_has_account %}
+            (not activated)
+        {% else %}
+            {{discord_username}}
+        {% endif %}
+    </td>
+    <td class="text-center">
+        {{server_name}}
+    </td>
+    <td class="text-center">
+        {% if not user_has_account %}
             <a href="{% url 'discord:activate' %}" title="Activate" class="btn btn-warning">
                 <span class="glyphicon glyphicon-ok"></span>
             </a>
@@ -20,7 +28,9 @@
         {% 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' %}">{% trans "Link Discord Server" %}</a>
+                <a type="button" class="btn btn-success" href="{% url 'discord:add_bot' %}">
+                    {% trans "Link Discord Server" %}
+                </a>
             </div>
         {% endif %}
     </td>

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

@@ -1,10 +1,17 @@
-from django.contrib.auth.models import User, Group, Permission
+from django.contrib.auth.models import Group, Permission
 from allianceauth.tests.auth_utils import AuthUtils
 
 DEFAULT_AUTH_GROUP = 'Member'
 MODULE_PATH = 'allianceauth.services.modules.discord'
 
-def add_permissions():
+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')
     members = Group.objects.get_or_create(name=DEFAULT_AUTH_GROUP)[0]
     AuthUtils.add_permissions_to_groups([permission], [members])

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

@@ -1,9 +1,7 @@
-from unittest.mock import patch
-
 from django.test import TestCase, RequestFactory
-from django.contrib import admin
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import User
+from django.utils.timezone import now
 
 from allianceauth.authentication.models import CharacterOwnership
 from allianceauth.eveonline.models import (
@@ -18,18 +16,22 @@ from ....admin import (
     MainCorporationsFilter,
     MainAllianceFilter
 )
-from ..admin import (        
+from ..admin import DiscordUserAdmin
-    DiscordUser,
+from ..models import DiscordUser
-    DiscordUserAdmin    
-)
 
 
-class TestDiscordUserAdmin(TestCase):
+class TestDataMixin(TestCase):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
 
+        EveCharacter.objects.all().delete()
+        EveCorporationInfo.objects.all().delete()
+        EveAllianceInfo.objects.all().delete()
+        User.objects.all().delete()
+        DiscordUser.objects.all().delete()
+        
         # user 1 - corp and alliance, normal user
         cls.character_1 = EveCharacter.objects.create(
             character_id='1001',
@@ -83,7 +85,10 @@ class TestDiscordUserAdmin(TestCase):
         cls.user_1.profile.save()
         DiscordUser.objects.create(
             user=cls.user_1,
-            uid=1001
+            uid=1001,
+            username='Bruce Wayne',
+            discriminator='1234',
+            activated=now()
         )
 
         # user 2 - corp only, staff
@@ -156,18 +161,20 @@ class TestDiscordUserAdmin(TestCase):
             uid=1003
         )
         
-
     def setUp(self):
         self.factory = RequestFactory()
         self.modeladmin = DiscordUserAdmin(
             model=DiscordUser, admin_site=AdminSite()
         )    
     
-    # column rendering
+
+class TestColumnRendering(TestDataMixin, TestCase):
 
     def test_user_profile_pic_u1(self):
-        expected = ('<img src="https://images.evetech.net/characters/1001/'
+        expected = (
-            'portrait?size=32" class="img-circle">')        
+            '<img src="https://images.evetech.net/characters/1001/'
+            'portrait?size=32" class="img-circle">'
+        )
         self.assertEqual(user_profile_pic(self.user_1.discord), expected)
 
     def test_user_profile_pic_u3(self):
@@ -204,9 +211,26 @@ class TestDiscordUserAdmin(TestCase):
         result = user_main_organization(self.user_3.discord)
         self.assertEqual(result, expected)
 
+    def test_uid(self):
+        expected = 1001
+        result = self.modeladmin._uid(self.user_1.discord)
+        self.assertEqual(result, expected)
+
+    def test_username_when_defined(self):
+        expected = 'Bruce Wayne#1234'
+        result = self.modeladmin._username(self.user_1.discord)
+        self.assertEqual(result, expected)
+
+    def test_username_when_not_defined(self):
+        expected = ''
+        result = self.modeladmin._username(self.user_2.discord)
+        self.assertEqual(result, expected)
+
     # actions
 
-    # filters    
+
+class TestFilters(TestDataMixin, TestCase):
+    
     def test_filter_main_corporations(self):
         
         class DiscordUserAdminTest(ServicesUserAdmin): 
@@ -228,8 +252,7 @@ class TestDiscordUserAdmin(TestCase):
 
         # Make sure the correct queryset is returned
         request = self.factory.get(
-            '/', 
+            '/', {'main_corporation_id__exact': self.character_1.corporation_id}
-            {'main_corporation_id__exact': self.character_1.corporation_id}
         )
         request.user = self.user_1                
         changelist = my_modeladmin.get_changelist_instance(request)
@@ -257,12 +280,10 @@ class TestDiscordUserAdmin(TestCase):
 
         # Make sure the correct queryset is returned
         request = self.factory.get(
-            '/', 
+            '/', {'main_alliance_id__exact': self.character_1.alliance_id}
-            {'main_alliance_id__exact': self.character_1.alliance_id}
         )
         request.user = self.user_1                
         changelist = my_modeladmin.get_changelist_instance(request)
         queryset = changelist.get_queryset(request)
         expected = [self.user_1.discord]
         self.assertSetEqual(set(queryset), set(expected))
-    

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

@@ -0,0 +1,140 @@
+from unittest.mock import patch
+
+from django.test import TestCase, RequestFactory
+from django.test.utils import override_settings
+
+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 ..utils import set_logger_to_file
+
+
+logger = set_logger_to_file(MODULE_PATH + '.auth_hooks', __file__)
+
+
+@override_settings(CELERY_ALWAYS_EAGER=True)
+class TestDiscordService(TestCase):
+
+    def setUp(self):        
+        self.member = AuthUtils.create_member(TEST_USER_NAME)
+        DiscordUser.objects.create(
+            user=self.member, 
+            uid=TEST_USER_ID, 
+            username=TEST_USER_NAME,
+            discriminator='1234'
+        )
+        self.none_member = AuthUtils.create_user('Lex Luther')
+        self.service = DiscordService
+        add_permissions_to_members()
+        self.factory = RequestFactory()
+
+    def test_service_enabled(self):
+        service = self.service()        
+        self.assertTrue(service.service_active_for_user(self.member))
+        self.assertFalse(service.service_active_for_user(self.none_member))
+
+    @patch(MODULE_PATH + '.tasks.update_all_groups')
+    def test_update_all_groups(self, mock_update_all_groups):
+        service = self.service()
+        service.update_all_groups()        
+        self.assertTrue(mock_update_all_groups.delay.called)
+        
+    @patch(MODULE_PATH + '.tasks.update_groups_bulk')
+    def test_update_groups_bulk(self, mock_update_groups_bulk):
+        service = self.service()        
+        service.update_groups_bulk([self.member])
+        self.assertTrue(mock_update_groups_bulk.delay.called)        
+
+    @patch(MODULE_PATH + '.tasks.update_groups')
+    def test_update_groups_for_member(self, mock_update_groups):    
+        service = self.service()        
+        service.update_groups(self.member)
+        self.assertTrue(mock_update_groups.apply_async.called)        
+
+    @patch(MODULE_PATH + '.tasks.update_groups')
+    def test_update_groups_for_none_member(self, mock_update_groups):    
+        service = self.service()
+        service.update_groups(self.none_member)
+        self.assertFalse(mock_update_groups.apply_async.called)        
+
+    @patch(MODULE_PATH + '.models.notify')
+    @patch(MODULE_PATH + '.tasks.DiscordUser')
+    @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+    def test_validate_user(
+        self, mock_DiscordClient, mock_DiscordUser, mock_notify
+    ):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+        
+        # Test member is not deleted
+        service = self.service()
+        service.validate_user(self.member)
+        self.assertTrue(DiscordUser.objects.filter(user=self.member).exists())
+
+        # Test none member is deleted        
+        DiscordUser.objects.create(user=self.none_member, uid=TEST_USER_ID)
+        service.validate_user(self.none_member)        
+        self.assertFalse(DiscordUser.objects.filter(user=self.none_member).exists())
+
+    @patch(MODULE_PATH + '.tasks.update_nickname')
+    def test_sync_nickname(self, mock_update_nickname):
+        service = self.service()        
+        service.sync_nickname(self.member)
+        self.assertTrue(mock_update_nickname.apply_async.called)
+        
+    @patch(MODULE_PATH + '.tasks.update_nicknames_bulk')
+    def test_sync_nicknames_bulk(self, mock_update_nicknames_bulk):
+        service = self.service()        
+        service.sync_nicknames_bulk([self.member])        
+        self.assertTrue(mock_update_nicknames_bulk.delay.called)
+        
+    @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+    def test_delete_user_is_member(self, mock_DiscordClient):        
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+
+        service = self.service()        
+        service.delete_user(self.member)
+        
+        self.assertTrue(mock_DiscordClient.return_value.remove_guild_member.called)
+        self.assertFalse(DiscordUser.objects.filter(user=self.member).exists())
+
+    @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+    def test_delete_user_is_not_member(self, mock_DiscordClient):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+
+        service = self.service()        
+        service.delete_user(self.none_member)
+
+        self.assertFalse(mock_DiscordClient.return_value.remove_guild_member.called)
+        
+    @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+    def test_render_services_ctrl_with_username(self, mock_DiscordClient):
+        service = self.service()        
+        request = self.factory.get('/services/')
+        request.user = self.member
+
+        response = service.render_services_ctrl(request)
+        self.assertTemplateUsed(service.service_ctrl_template)
+        self.assertIn('/discord/reset/', response)
+        self.assertIn('/discord/deactivate/', response)
+
+        # Test register becomes available
+        self.member.discord.delete()
+        self.member.refresh_from_db()
+        request.user = self.member
+        response = service.render_services_ctrl(request)
+        self.assertIn('/discord/activate/', response)
+
+    @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+    def test_render_services_ctrl_wo_username(self, mock_DiscordClient):
+        my_member = AuthUtils.create_member('John Doe')
+        DiscordUser.objects.create(user=my_member, uid=111222333)
+        service = self.service()
+        request = self.factory.get('/services/')
+        request.user = my_member
+
+        response = service.render_services_ctrl(request)
+        self.assertTemplateUsed(service.service_ctrl_template)
+        self.assertIn('/discord/reset/', response)
+        self.assertIn('/discord/deactivate/', response)

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

@@ -1,127 +0,0 @@
-from unittest import mock
-
-from django.test import TestCase, RequestFactory
-from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist
-from allianceauth.tests.auth_utils import AuthUtils
-
-from ..auth_hooks import DiscordService
-from ..models import DiscordUser
-from ..tasks import DiscordTasks
-from ..manager import DiscordOAuthManager
-
-from . import DEFAULT_AUTH_GROUP, add_permissions, MODULE_PATH
-
-
-class DiscordHooksTestCase(TestCase):
-    def setUp(self):
-        self.member = 'member_user'
-        member = AuthUtils.create_member(self.member)
-        DiscordUser.objects.create(user=member, uid='12345')
-        self.none_user = 'none_user'
-        none_user = AuthUtils.create_user(self.none_user)
-        self.service = DiscordService
-        add_permissions()
-
-    def test_has_account(self):
-        member = User.objects.get(username=self.member)
-        none_user = User.objects.get(username=self.none_user)
-        self.assertTrue(DiscordTasks.has_account(member))
-        self.assertFalse(DiscordTasks.has_account(none_user))
-
-    def test_service_enabled(self):
-        service = self.service()
-        member = User.objects.get(username=self.member)
-        none_user = User.objects.get(username=self.none_user)
-
-        self.assertTrue(service.service_active_for_user(member))
-        self.assertFalse(service.service_active_for_user(none_user))
-
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
-    def test_update_all_groups(self, manager):
-        service = self.service()
-        service.update_all_groups()
-        # Check member and blue user have groups updated
-        self.assertTrue(manager.update_groups.called)
-        self.assertEqual(manager.update_groups.call_count, 1)
-
-    def test_update_groups(self):
-        # Check member has Member group updated
-        with mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager') as manager:
-            service = self.service()
-            member = User.objects.get(username=self.member)
-            AuthUtils.disconnect_signals()
-            service.update_groups(member)
-            self.assertTrue(manager.update_groups.called)
-            args, kwargs = manager.update_groups.call_args
-            user_id, groups = args
-            self.assertIn(DEFAULT_AUTH_GROUP, groups)
-            self.assertEqual(user_id, member.discord.uid)
-
-        # Check none user does not have groups updated
-        with mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager') as manager:
-            service = self.service()
-            none_user = User.objects.get(username=self.none_user)
-            service.update_groups(none_user)
-            self.assertFalse(manager.update_groups.called)
-
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
-    def test_validate_user(self, manager):
-        service = self.service()
-        # Test member is not deleted
-        member = User.objects.get(username=self.member)
-        service.validate_user(member)
-        self.assertTrue(member.discord)
-
-        # Test none user is deleted
-        none_user = User.objects.get(username=self.none_user)
-        DiscordUser.objects.create(user=none_user, uid='abc123')
-        service.validate_user(none_user)
-        self.assertTrue(manager.delete_user.called)
-        with self.assertRaises(ObjectDoesNotExist):
-            none_discord = User.objects.get(username=self.none_user).discord
-
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
-    def test_sync_nickname(self, manager):
-        service = self.service()
-        member = User.objects.get(username=self.member)
-        AuthUtils.add_main_character(member, 'test user', '12345', corp_ticker='AAUTH')
-
-        service.sync_nickname(member)
-
-        self.assertTrue(manager.update_nickname.called)
-        args, kwargs = manager.update_nickname.call_args
-        self.assertEqual(args[0], member.discord.uid)
-        self.assertEqual(args[1], 'test user')
-
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
-    def test_delete_user(self, manager):
-        member = User.objects.get(username=self.member)
-
-        service = self.service()
-        result = service.delete_user(member)
-
-        self.assertTrue(result)
-        self.assertTrue(manager.delete_user.called)
-        with self.assertRaises(ObjectDoesNotExist):
-            discord_user = User.objects.get(username=self.member).discord
-
-    def test_render_services_ctrl(self):
-        service = self.service()
-        member = User.objects.get(username=self.member)
-        request = RequestFactory().get('/services/')
-        request.user = member
-
-        response = service.render_services_ctrl(request)
-        self.assertTemplateUsed(service.service_ctrl_template)
-        self.assertIn('/discord/reset/', response)
-        self.assertIn('/discord/deactivate/', response)
-
-        # Test register becomes available
-        member.discord.delete()
-        member = User.objects.get(username=self.member)
-        request.user = member
-        response = service.render_services_ctrl(request)
-        self.assertIn('/discord/activate/', response)
-
-    # TODO: Test update nicknames

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

@@ -0,0 +1,62 @@
+from django_webtest import WebTest
+from unittest.mock import patch
+
+from django.shortcuts import reverse
+
+from allianceauth.tests.auth_utils import AuthUtils
+
+from . import (
+    add_permissions_to_members, 
+    MODULE_PATH, 
+    TEST_USER_NAME,    
+    TEST_MAIN_NAME,
+    TEST_MAIN_ID
+)
+
+
+class TestServiceUserActivation(WebTest):
+    
+    def setUp(self):
+        self.member = AuthUtils.create_member(TEST_USER_NAME)
+        AuthUtils.add_main_character_2(
+            self.member, 
+            TEST_MAIN_NAME, 
+            TEST_MAIN_ID,
+            disconnect_signals=True
+        )
+        add_permissions_to_members()
+    
+    @patch(MODULE_PATH + '.views.messages')
+    @patch(MODULE_PATH + '.models.DiscordUser.objects.add_user')
+    @patch(MODULE_PATH + '.managers.OAuth2Session')
+    def test_user_activation(
+        self, mock_OAuth2Session, mock_add_user, mock_messages
+    ): 
+        authentication_code = 'auth_code'
+        mock_add_user.return_value = True
+        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 was added to Discord
+        self.assertTrue(mock_add_user.called)
+
+        # user got a success message
+        self.assertTrue(mock_messages.success.called)

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

@@ -1,244 +1,356 @@
-import json
+from unittest.mock import patch, Mock
 import urllib
-import datetime
-import requests_mock
-from unittest import mock
 
+from requests.exceptions import HTTPError
+
+from django.contrib.auth.models import Group, User
 from django.test import TestCase
-from django.conf import settings
 
-from ..manager import DiscordOAuthManager
+from allianceauth.tests.auth_utils import AuthUtils
-from .. import manager
 
-from . import DEFAULT_AUTH_GROUP, add_permissions, MODULE_PATH
+from . import (
+    TEST_GUILD_ID, 
+    TEST_USER_NAME, 
+    TEST_USER_ID, 
+    TEST_MAIN_NAME, 
+    TEST_MAIN_ID, 
+    MODULE_PATH
+)
+from ..app_settings import (
+    DISCORD_APP_ID, 
+    DISCORD_APP_SECRET,     
+    DISCORD_CALLBACK_URL,    
+)
+from ..discord_client import DiscordClient, DiscordApiBackoff
+from ..models import DiscordUser
+from ..utils import set_logger_to_file
 
 
-class DiscordManagerTestCase(TestCase):
+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):
+        
     def setUp(self):
-        pass
+        self.user = AuthUtils.create_user(TEST_USER_NAME)
+        self.user_info = {
+            'id': TEST_USER_ID, 
+            'name': TEST_USER_NAME,
+            'username': TEST_USER_NAME,
+            'discriminator': '1234',
+        }
+        self.access_token = 'accesstoken'
     
-    def test__sanitize_group_name(self):
+    def test_can_create_user_no_roles_no_nick(
-        test_group_name = str(10**103)
+        self, 
-        group_name = DiscordOAuthManager._sanitize_group_name(test_group_name)
+        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 = 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.add_guild_member.return_value = True
         
-        self.assertEqual(group_name, test_group_name[:100])
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertTrue(result)
+        self.assertTrue(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.add_guild_member.call_args
+        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.assertIsNone(kwargs['role_ids'])
+        self.assertIsNone(kwargs['nick'])
 
-    def test_generate_Bot_add_url(self):
+    def test_can_create_user_with_roles_no_nick(
-        bot_add_url = DiscordOAuthManager.generate_bot_add_url()
+        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]
+        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_DiscordClient.return_value.current_user.return_value = self.user_info
+        mock_DiscordClient.return_value.add_guild_member.return_value = True
         
-        auth_url = manager.AUTH_URL
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
-        real_bot_add_url = '{}?client_id=appid&scope=bot&permissions={}'.format(auth_url, manager.BOT_PERMISSIONS)
+        self.assertTrue(result)
+        self.assertTrue(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.add_guild_member.call_args
+        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.assertIsNone(kwargs['nick'])
+
+    @patch(MODULE_PATH + '.managers.DISCORD_SYNC_NAMES', True)
+    def test_can_create_user_no_roles_with_nick(
+        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.add_guild_member.return_value = True
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertTrue(result)
+        self.assertTrue(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.add_guild_member.call_args
+        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.assertIsNone(kwargs['role_ids'])
+        self.assertEqual(kwargs['nick'], TEST_MAIN_NAME)
+
+    @patch(MODULE_PATH + '.managers.DISCORD_SYNC_NAMES', False)
+    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_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.add_guild_member.return_value = True
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertTrue(result)
+        self.assertTrue(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        args, kwargs = mock_DiscordClient.return_value.add_guild_member.call_args
+        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.assertIsNone(kwargs['role_ids'])
+        self.assertIsNone(kwargs['nick'])
+    
+    def test_can_activate_existing_guild_member(
+        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 = 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.add_guild_member.return_value = None
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertTrue(result)
+        self.assertTrue(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        
+    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_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.add_guild_member.return_value = False
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertFalse(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+
+    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_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.add_guild_member.side_effect = \
+            DiscordApiBackoff(999)
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertFalse(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+
+    def test_return_false_on_http_error(
+        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 = 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_exception = HTTPError('error')
+        mock_exception.response = Mock()
+        mock_exception.response.status_code = 500
+        mock_DiscordClient.return_value.add_guild_member.side_effect = mock_exception
+        
+        result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef')
+        self.assertFalse(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.add_guild_member.called)
+        
+
+class TestOauthHelpers(TestCase):
+        
+    @patch(MODULE_PATH + '.managers.DISCORD_APP_ID', '123456')    
+    def test_generate_bot_add_url(self):
+        bot_add_url = DiscordUser.objects.generate_bot_add_url()
+
+        auth_url = DiscordClient.OAUTH_BASE_URL
+        real_bot_add_url = (
+            f'{auth_url}?client_id=123456&scope=bot'
+            f'&permissions={DiscordUser.objects.BOT_PERMISSIONS}'
+        )
         self.assertEqual(bot_add_url, real_bot_add_url)
 
     def test_generate_oauth_redirect_url(self):
-        oauth_url = DiscordOAuthManager.generate_oauth_redirect_url()
+        oauth_url = DiscordUser.objects.generate_oauth_redirect_url()
 
-        self.assertIn(manager.AUTH_URL, oauth_url)
+        self.assertIn(DiscordClient.OAUTH_BASE_URL, oauth_url)
-        self.assertIn('+'.join(manager.SCOPES), oauth_url)
+        self.assertIn('+'.join(DiscordUser.objects.SCOPES), oauth_url)
-        self.assertIn(settings.DISCORD_APP_ID, oauth_url)
+        self.assertIn(DISCORD_APP_ID, oauth_url)
-        self.assertIn(urllib.parse.quote_plus(settings.DISCORD_CALLBACK_URL), oauth_url)
+        self.assertIn(urllib.parse.quote_plus(DISCORD_CALLBACK_URL), oauth_url)
 
-    @mock.patch(MODULE_PATH + '.manager.OAuth2Session')
+    @patch(MODULE_PATH + '.managers.OAuth2Session')
-    def test__process_callback_code(self, oauth):
+    def test_process_callback_code(self, oauth):
         instance = oauth.return_value
         instance.fetch_token.return_value = {'access_token': 'mywonderfultoken'}
 
-        token = DiscordOAuthManager._process_callback_code('12345')
+        token = DiscordUser.objects._exchange_auth_code_for_token('12345')
 
         self.assertTrue(oauth.called)
         args, kwargs = oauth.call_args
-        self.assertEqual(args[0], settings.DISCORD_APP_ID)
+        self.assertEqual(args[0], DISCORD_APP_ID)
-        self.assertEqual(kwargs['redirect_uri'], settings.DISCORD_CALLBACK_URL)
+        self.assertEqual(kwargs['redirect_uri'], DISCORD_CALLBACK_URL)
         self.assertTrue(instance.fetch_token.called)
         args, kwargs = instance.fetch_token.call_args
-        self.assertEqual(args[0], manager.TOKEN_URL)
+        self.assertEqual(args[0], DiscordClient.OAUTH_TOKEN_URL)
-        self.assertEqual(kwargs['client_secret'], settings.DISCORD_APP_SECRET)
+        self.assertEqual(kwargs['client_secret'], DISCORD_APP_SECRET)
         self.assertEqual(kwargs['code'], '12345')
-        self.assertEqual(token['access_token'], 'mywonderfultoken')
+        self.assertEqual(token, 'mywonderfultoken')
 
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._process_callback_code')
-    @requests_mock.Mocker()
-    def test_add_user(self, oauth_token, m):
-        # Arrange
-        oauth_token.return_value = {'access_token': 'accesstoken'}
 
-        headers = {'accept': 'application/json', 'authorization': 'Bearer accesstoken'}
+class TestUserFormattedNick(TestCase):
     
-        m.register_uri('GET',
+    def setUp(self):        
-                       manager.DISCORD_URL + "/users/@me",
+        self.user = AuthUtils.create_user(TEST_USER_NAME)
-                       request_headers=headers,
-                       text=json.dumps({'id': "123456"}))
     
-        headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
+    def test_return_nick_when_user_has_main(self):
+        AuthUtils.add_main_character_2(self.user, TEST_MAIN_NAME, TEST_MAIN_ID)
+        result = DiscordUser.objects.user_formatted_nick(self.user)
+        expected = TEST_MAIN_NAME
+        self.assertEqual(result, expected)
 
-        m.register_uri('PUT',
+    def test_return_none_if_user_has_no_main(self):        
-                       manager.DISCORD_URL + '/guilds/' + str(settings.DISCORD_GUILD_ID) + '/members/123456',
+        result = DiscordUser.objects.user_formatted_nick(self.user)        
-                       request_headers=headers,
+        self.assertIsNone(result)
-                       text='{}')
 
-        # Act
-        return_value = DiscordOAuthManager.add_user('abcdef', [])
 
-        # Assert
+class TestUserGroupNames(TestCase):
-        self.assertEqual(return_value, '123456')
-        self.assertEqual(m.call_count, 2)
 
-    @requests_mock.Mocker()
+    @classmethod
-    def test_delete_user(self, m):
+    def setUpClass(cls):
-        # Arrange
+        super().setUpClass()
-        headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
+        cls.group_1 = Group.objects.create(name='Group 1')
+        cls.group_2 = Group.objects.create(name='Group 2')
     
-        user_id = 12345
+    def setUp(self):
-        request_url = '{}/guilds/{}/members/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id)
+        self.user = AuthUtils.create_member(TEST_USER_NAME)        
-        m.register_uri('DELETE',
-                       request_url,
-                       request_headers=headers,
-                       text=json.dumps({}))
 
-        # Act
+    def test_return_groups_and_state_names_for_user(self):
-        result = DiscordOAuthManager.delete_user(user_id)
+        self.user.groups.add(self.group_1)
+        result = DiscordUser.objects.user_group_names(self.user)
+        expected = ['Group 1', 'Member']
+        self.assertSetEqual(set(result), set(expected))
     
-        # Assert
+    def test_return_state_only_if_user_has_no_groups(self):
-        self.assertTrue(result)
+        result = DiscordUser.objects.user_group_names(self.user)
+        expected = ['Member']
+        self.assertSetEqual(set(result), set(expected))
 
-        ###
-        # Test 404 (already deleted)
-        # Arrange
-        m.register_uri('DELETE',
-                       request_url,
-                       request_headers=headers,
-                       status_code=404)
 
-        # Act
+class TestUserHasAccount(TestCase):
-        result = DiscordOAuthManager.delete_user(user_id)
 
-        # Assert
+    @classmethod
-        self.assertTrue(result)
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_user(TEST_USER_NAME)
 
-        ###
+    def test_return_true_if_user_has_account(self):
-        # Test 500 (some random API error)
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
-        # Arrange
+        self.assertTrue(DiscordUser.objects.user_has_account(self.user))
-        m.register_uri('DELETE',
-                       request_url,
-                       request_headers=headers,
-                       status_code=500)
 
-        # Act
+    def test_return_false_if_user_has_no_account(self):
-        result = DiscordOAuthManager.delete_user(user_id)
+        self.assertFalse(DiscordUser.objects.user_has_account(self.user))
 
-        # Assert
+    def test_return_false_if_user_does_not_exist(self):        
-        self.assertFalse(result)
+        my_user = User(username='Dummy')
+        self.assertFalse(DiscordUser.objects.user_has_account(my_user))
 
-    @requests_mock.Mocker()
+    def test_return_false_if_not_called_with_user_object(self):                
-    def test_update_nickname(self, m):
+        self.assertFalse(DiscordUser.objects.user_has_account('abc'))
-        # Arrange
-        headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-
-        user_id = 12345
-        request_url = '{}/guilds/{}/members/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id)
-        m.patch(request_url,
-                request_headers=headers)
-
-        # Act
-        result = DiscordOAuthManager.update_nickname(user_id, 'somenick')
-
-        # Assert
-        self.assertTrue(result)
-
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_user_roles')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_groups')
-    @requests_mock.Mocker()
-    def test_update_groups(self, group_cache, user_roles, m):
-        # Arrange
-        groups = ['Member', 'Blue', 'SpecialGroup']
-
-        group_cache.return_value = [{'id': '111', 'name': 'Member'},
-                                    {'id': '222', 'name': 'Blue'},
-                                    {'id': '333', 'name': 'SpecialGroup'},
-                                    {'id': '444', 'name': 'NotYourGroup'}]
-        user_roles.return_value = ['444']
-
-        headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        user_id = 12345
-        user_request_url = '{}/guilds/{}/members/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id)
-        group_request_urls = ['{}/guilds/{}/members/{}/roles/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id, g['id']) for g in group_cache.return_value]
-
-        m.patch(user_request_url, request_headers=headers)
-        [m.put(url, request_headers=headers) for url in group_request_urls[:-1]]
-        m.delete(group_request_urls[-1], request_headers=headers)
-
-        # Act
-        DiscordOAuthManager.update_groups(user_id, groups)
-
-        # Assert
-        self.assertEqual(len(m.request_history), 4, 'Must be 4 HTTP calls made')
-
-    @mock.patch(MODULE_PATH + '.manager.cache')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_user_roles')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._group_name_to_id')
-    @requests_mock.Mocker()
-    def test_update_groups_backoff(self, name_to_id, user_groups, djcache, m):
-        # Arrange
-        groups = ['Member']
-        user_groups.return_value = []
-        name_to_id.return_value = '111'
-
-        headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        user_id = 12345
-        request_url = '{}/guilds/{}/members/{}/roles/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id, name_to_id.return_value)
-
-        djcache.get.return_value = None  # No existing backoffs in cache
-
-        m.put(request_url,
-              request_headers=headers,
-              headers={'Retry-After': '200000'},
-              status_code=429)
-
-        # Act & Assert
-        with self.assertRaises(manager.DiscordApiBackoff) as bo:
-            try:
-                DiscordOAuthManager.update_groups(user_id, groups, blocking=False)
-            except manager.DiscordApiBackoff as bo:
-                self.assertEqual(bo.retry_after, 200000, 'Retry-After time must be equal to Retry-After set in header')
-                self.assertFalse(bo.global_ratelimit, 'global_ratelimit must be False')
-                raise bo
-
-        self.assertTrue(djcache.set.called)
-        args, kwargs = djcache.set.call_args
-        self.assertEqual(args[0], 'DISCORD_BACKOFF_update_groups')
-        self.assertTrue(datetime.datetime.strptime(args[1], manager.cache_time_format) > datetime.datetime.now())
-
-    @mock.patch(MODULE_PATH + '.manager.cache')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_user_roles')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._group_name_to_id')
-    @requests_mock.Mocker()
-    def test_update_groups_global_backoff(self, name_to_id, user_groups, djcache, m):
-        # Arrange
-        groups = ['Member']
-        user_groups.return_value = []
-        name_to_id.return_value = '111'
-
-        headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        user_id = 12345
-        request_url = '{}/guilds/{}/members/{}/roles/{}'.format(manager.DISCORD_URL, settings.DISCORD_GUILD_ID, user_id, name_to_id.return_value)
-
-        djcache.get.return_value = None  # No existing backoffs in cache
-
-        m.put(request_url,
-              request_headers=headers,
-              headers={'Retry-After': '200000', 'X-RateLimit-Global': 'true'},
-              status_code=429)
-
-        # Act & Assert
-        with self.assertRaises(manager.DiscordApiBackoff) as bo:
-            try:
-                DiscordOAuthManager.update_groups(user_id, groups, blocking=False)
-            except manager.DiscordApiBackoff as bo:
-                self.assertEqual(bo.retry_after, 200000, 'Retry-After time must be equal to Retry-After set in header')
-                self.assertTrue(bo.global_ratelimit, 'global_ratelimit must be True')
-                raise bo
-
-        self.assertTrue(djcache.set.called)
-        args, kwargs = djcache.set.call_args
-        self.assertEqual(args[0], 'DISCORD_BACKOFF_GLOBAL')
-        self.assertTrue(datetime.datetime.strptime(args[1], manager.cache_time_format) > datetime.datetime.now())

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

@@ -0,0 +1,222 @@
+from unittest.mock import patch, Mock
+
+from requests.exceptions import HTTPError
+
+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 ..discord_client import DiscordClient, DiscordApiBackoff
+from ..models import DiscordUser
+from ..utils import set_logger_to_file
+
+
+logger = set_logger_to_file(MODULE_PATH + '.models', __file__)
+
+
+class TestBasicsAndHelpers(TestCase):
+
+    def test_str(self):
+        user = AuthUtils.create_user(TEST_USER_NAME)
+        discord_user = DiscordUser.objects.create(user=user, uid=TEST_USER_ID)
+        expected = 'Peter Parker - 198765432012345678'
+        self.assertEqual(str(discord_user), expected)
+
+    def test_repr(self):
+        user = AuthUtils.create_user(TEST_USER_NAME)
+        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): 
+        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)        
+        mock_DiscordClient.return_value.modify_guild_member.return_value = True
+        
+        result = self.discord_user.update_nickname()
+        self.assertTrue(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+
+    def test_dont_update_if_user_has_no_main(self, mock_DiscordClient):        
+        mock_DiscordClient.return_value.modify_guild_member.return_value = False
+        
+        result = self.discord_user.update_nickname()
+        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
+    ):
+        AuthUtils.add_main_character_2(self.user, TEST_MAIN_NAME, TEST_MAIN_ID)
+        mock_DiscordClient.return_value.modify_guild_member.return_value = None
+        
+        result = self.discord_user.update_nickname()
+        self.assertIsNone(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)        
+
+    def test_return_false_if_api_returns_false(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 = False
+        
+        result = self.discord_user.update_nickname()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_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)        
+        self.discord_user = DiscordUser.objects.create(
+            user=self.user, uid=TEST_USER_ID
+        )
+
+    def test_can_delete_user(self, mock_DiscordClient, mock_notify):        
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+        result = self.discord_user.delete_user()
+        self.assertTrue(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.remove_guild_member.called)
+        self.assertFalse(mock_notify.called)
+
+    def test_can_delete_user_and_notify_user(self, mock_DiscordClient, mock_notify):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+        result = self.discord_user.delete_user(notify_user=True)
+        self.assertTrue(result)
+        self.assertTrue(mock_notify.called)
+
+    def test_can_delete_user_when_member_is_unknown(
+        self, mock_DiscordClient, mock_notify
+    ): 
+        mock_DiscordClient.return_value.remove_guild_member.return_value = None
+        result = self.discord_user.delete_user()
+        self.assertTrue(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        self.assertTrue(mock_DiscordClient.return_value.remove_guild_member.called)
+        self.assertFalse(mock_notify.called)
+
+    def test_return_false_when_api_fails(self, mock_DiscordClient, mock_notify):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = False
+        result = self.discord_user.delete_user()
+        self.assertFalse(result)
+
+    def test_dont_notify_if_user_was_already_deleted_and_return_none(
+        self, mock_DiscordClient, mock_notify
+    ):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = None
+        DiscordUser.objects.get(pk=self.discord_user.pk).delete()
+        result = self.discord_user.delete_user()
+        self.assertIsNone(result)
+        self.assertFalse(
+            DiscordUser.objects.filter(user=self.user, uid=TEST_USER_ID).exists()
+        )
+        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):
+        mock_DiscordClient.return_value.remove_guild_member.side_effect = \
+            DiscordApiBackoff(999)
+        result = self.discord_user.delete_user()
+        self.assertFalse(result)
+
+    def test_return_false_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()
+        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):
+
+    def setUp(self): 
+        self.user = AuthUtils.create_user(TEST_USER_NAME)
+        self.discord_user = DiscordUser.objects.create(
+            user=self.user, uid=TEST_USER_ID
+        )
+        
+    def test_can_update(
+        self, 
+        mock_user_group_names, 
+        mock_guild_get_or_create_role_ids, 
+        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 = True
+        
+        result = self.discord_user.update_groups()
+        self.assertTrue(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)
+    
+    def test_return_none_if_user_no_longer_a_member(
+        self,
+        mock_user_group_names, 
+        mock_guild_get_or_create_role_ids, 
+        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
+                
+        result = self.discord_user.update_groups()
+        self.assertIsNone(result)
+        self.assertTrue(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_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 = False      
+        
+        result = self.discord_user.update_groups()
+        self.assertFalse(result)
+        self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called)

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

@@ -0,0 +1,310 @@
+from unittest.mock import MagicMock, patch
+
+from celery.exceptions import Retry
+from requests.exceptions import HTTPError
+
+from django.test import TestCase
+from django.contrib.auth.models import Group
+from django.test.utils import override_settings
+
+from allianceauth.tests.auth_utils import AuthUtils
+
+from . import TEST_USER_NAME, TEST_USER_ID, TEST_MAIN_NAME, TEST_MAIN_ID
+from ..models import DiscordUser
+from ..discord_client import DiscordApiBackoff
+from .. import tasks
+from ..utils import set_logger_to_file
+
+
+MODULE_PATH = 'allianceauth.services.modules.discord.tasks'
+logger = set_logger_to_file(MODULE_PATH, __file__)
+
+
+@patch(MODULE_PATH + '.DiscordUser.update_groups')
+class TestUpdateGroups(TestCase):
+    
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_member(TEST_USER_NAME)
+        cls.group_1 = Group.objects.create(name='Group 1')
+        cls.group_2 = Group.objects.create(name='Group 2')
+        cls.group_1.user_set.add(cls.user)
+        cls.group_2.user_set.add(cls.user)
+        
+    def test_can_update_groups(self, mock_update_groups):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        tasks.update_groups(self.user.pk)
+        self.assertTrue(mock_update_groups.called)
+
+    def test_no_action_if_user_has_no_discord_account(self, mock_update_groups):
+        tasks.update_groups(self.user.pk)
+        self.assertFalse(mock_update_groups.called)
+    
+    def test_retries_on_api_backoff(self, mock_update_groups):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        mock_exception = DiscordApiBackoff(999)
+        mock_update_groups.side_effect = mock_exception
+
+        with self.assertRaises(Retry):
+            tasks.update_groups(self.user.pk)
+
+    def test_retry_on_http_error_except_404(self, mock_update_groups):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        mock_exception = HTTPError('error')
+        mock_exception.response = MagicMock()
+        mock_exception.response.status_code = 500
+        mock_update_groups.side_effect = mock_exception
+
+        with self.assertRaises(Retry):
+            tasks.update_groups(self.user.pk)
+    
+    def test_retry_on_http_error_404_when_user_not_deleted(self, mock_update_groups):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        mock_exception = HTTPError('error')
+        mock_exception.response = MagicMock()
+        mock_exception.response.status_code = 404
+        mock_update_groups.side_effect = mock_exception
+
+        with self.assertRaises(Retry):
+            tasks.update_groups(self.user.pk)
+        
+    def test_retry_on_non_http_error(self, mock_update_groups):        
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        mock_update_groups.side_effect = ConnectionError
+
+        with self.assertRaises(Retry):
+            tasks.update_groups(self.user.pk)
+        
+    @patch(MODULE_PATH + '.DISCORD_TASKS_MAX_RETRIES', 3)
+    def test_log_error_if_retries_exhausted(self, mock_update_groups):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)        
+        mock_task = MagicMock(**{'request.retries': 3})
+        mock_update_groups.side_effect = ConnectionError
+        update_groups_inner = tasks.update_groups.__wrapped__.__func__
+                
+        update_groups_inner(mock_task, self.user.pk)
+                
+    @patch(MODULE_PATH + '.delete_user.delay')
+    def test_delete_user_if_user_is_no_longer_member_of_discord_server(
+        self, mock_delete_user, mock_update_groups
+    ):
+        mock_update_groups.return_value = None
+        
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
+        tasks.update_groups(self.user.pk)
+        self.assertTrue(mock_update_groups.called)
+        self.assertTrue(mock_delete_user.called)
+
+
+@patch(MODULE_PATH + '.DiscordUser.update_nickname')
+class TestUpdateNickname(TestCase):
+    
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_member(TEST_USER_NAME)
+        AuthUtils.add_main_character_2(
+            cls.user, 
+            TEST_MAIN_NAME, 
+            TEST_MAIN_ID, 
+            corp_id='2', 
+            corp_name='test_corp', 
+            corp_ticker='TEST',
+            disconnect_signals=True
+        )
+        cls.discord_user = DiscordUser.objects.create(user=cls.user, uid=TEST_USER_ID)
+
+    def test_can_update_nickname(self, mock_update_nickname):        
+        mock_update_nickname.return_value = True        
+        
+        tasks.update_nickname(self.user.pk)
+        self.assertTrue(mock_update_nickname.called)
+
+    def test_no_action_when_user_had_no_account(self, mock_update_nickname):
+        my_user = AuthUtils.create_user('Dummy User')
+        mock_update_nickname.return_value = False
+        
+        tasks.update_nickname(my_user.pk)
+        self.assertFalse(mock_update_nickname.called)
+        
+    def test_retries_on_api_backoff(self, mock_update_nickname):                
+        mock_exception = DiscordApiBackoff(999)
+        mock_update_nickname.side_effect = mock_exception
+        
+        with self.assertRaises(Retry):
+            tasks.update_nickname(self.user.pk)
+
+    def test_retries_on_general_exception(self, mock_update_nickname):        
+        mock_update_nickname.side_effect = ConnectionError
+        
+        with self.assertRaises(Retry):
+            tasks.update_nickname(self.user.pk)
+
+    @patch(MODULE_PATH + '.DISCORD_TASKS_MAX_RETRIES', 3)
+    def test_log_error_if_retries_exhausted(self, mock_update_nickname):
+        mock_task = MagicMock(**{'request.retries': 3})
+        mock_update_nickname.side_effect = ConnectionError
+        update_nickname_inner = tasks.update_nickname.__wrapped__.__func__
+                
+        update_nickname_inner(mock_task, self.user.pk)
+        
+
+@patch(MODULE_PATH + '.DiscordUser.delete_user')
+class TestDeleteUser(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_member('Peter Parker')    
+        cls.discord_user = DiscordUser.objects.create(user=cls.user, uid=TEST_USER_ID)
+
+    def test_can_delete_user(self, mock_delete_user):
+        mock_delete_user.return_value = True
+
+        tasks.delete_user(self.user.pk)
+        self.assertTrue(mock_delete_user.called)
+
+    def test_can_delete_user_with_notify(self, mock_delete_user):
+        mock_delete_user.return_value = True
+
+        tasks.delete_user(self.user.pk, notify_user=True)
+        self.assertTrue(mock_delete_user.called)
+        args, kwargs = mock_delete_user.call_args
+        self.assertTrue(kwargs['notify_user'])
+
+    @patch(MODULE_PATH + '.delete_user.delay')
+    def test_dont_retry_delete_user_if_user_is_no_longer_member_of_discord_server(
+        self, mock_delete_user_delay, mock_delete_user
+    ):
+        mock_delete_user.return_value = None
+        
+        tasks.delete_user(self.user.pk)
+        self.assertTrue(mock_delete_user.called)
+        self.assertFalse(mock_delete_user_delay.called)
+
+
+@patch(MODULE_PATH + '.DiscordUser.update_groups')
+class TestTaskPerformUserAction(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user = AuthUtils.create_member('Peter Parker')    
+        cls.discord_user = DiscordUser.objects.create(user=cls.user, uid=TEST_USER_ID)
+
+    def test_raise_value_error_on_unknown_method(self, mock_update_groups):
+        mock_task = MagicMock(**{'request.retries': 0})
+        
+        with self.assertRaises(ValueError):
+            tasks._task_perform_user_action(mock_task, self.user.pk, 'invalid_method')
+    
+    def test_catch_and_log_unexpected_exceptions(self, mock_update_groups):
+        mock_task = MagicMock(**{'request.retries': 0})
+        mock_update_groups.side_effect = RuntimeError
+
+        tasks._task_perform_user_action(mock_task, self.user.pk, 'update_groups')
+
+
+@override_settings(CELERY_ALWAYS_EAGER=True)
+class TestBulkTasks(TestCase):
+    
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.user_1 = AuthUtils.create_user('Peter Parker')
+        cls.user_2 = AuthUtils.create_user('Kara Danvers')
+        cls.user_3 = AuthUtils.create_user('Clark Kent')
+        DiscordUser.objects.all().delete()
+
+    @patch(MODULE_PATH + '.update_groups.si')
+    def test_can_update_groups_for_multiple_users(self, mock_update_groups):
+        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_groups_bulk(expected_pks)
+        self.assertEqual(mock_update_groups.call_count, 2)
+        current_pks = [args[0][0] for args in mock_update_groups.call_args_list]
+        
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
+    @patch(MODULE_PATH + '.update_groups.si')
+    def test_can_update_all_groups(self, mock_update_groups):
+        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_groups()
+        self.assertEqual(mock_update_groups.call_count, 3)
+        current_pks = [args[0][0] for args in mock_update_groups.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 + '.update_nickname.si')
+    def test_can_update_nicknames_for_multiple_users(self, 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)
+        DiscordUser.objects.create(user=self.user_3, uid=789)
+        expected_pks = [du_1.pk, du_2.pk]
+
+        tasks.update_nicknames_bulk(expected_pks)
+        self.assertEqual(mock_update_nickname.call_count, 2)
+        current_pks = [
+            args[0][0] for args in mock_update_nickname.call_args_list
+        ]        
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
+    @patch(MODULE_PATH + '.update_nickname.si')
+    def test_can_update_nicknames_for_all_users(self, 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')
+        du_3 = DiscordUser.objects.create(user=self.user_3, uid='789')
+
+        tasks.update_all_nicknames()
+        self.assertEqual(mock_update_nickname.call_count, 3)
+        current_pks = [
+            args[0][0] for args in mock_update_nickname.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')
+    def test_can_update_all_incl_nicknames(
+        self, 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)
+        du_3 = DiscordUser.objects.create(user=self.user_3, uid=789)
+
+        tasks.update_all()
+        self.assertEqual(mock_update_groups.si.call_count, 3)        
+        current_pks = [args[0][0] for args in mock_update_groups.si.call_args_list]
+        expected_pks = [du_1.pk, du_2.pk, du_3.pk]
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
+        self.assertEqual(mock_update_nickname.si.call_count, 3)
+        current_pks = [args[0][0] for args in mock_update_nickname.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')
+    def test_can_update_all_excl_nicknames(
+        self, 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)
+        du_3 = DiscordUser.objects.create(user=self.user_3, uid=789)
+
+        tasks.update_all()
+        self.assertEqual(mock_update_groups.si.call_count, 3)        
+        current_pks = [args[0][0] for args in mock_update_groups.si.call_args_list]
+        expected_pks = [du_1.pk, du_2.pk, du_3.pk]
+        self.assertSetEqual(set(current_pks), set(expected_pks))
+
+        self.assertEqual(mock_update_nickname.si.call_count, 0)

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

@@ -0,0 +1,102 @@
+from unittest.mock import Mock, patch
+from django.test import TestCase
+
+from ..utils import clean_setting
+
+MODULE_PATH = 'allianceauth.services.modules.discord.utils'
+
+
+class TestCleanSetting(TestCase):
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_if_not_set(self, mock_settings):        
+        mock_settings.TEST_SETTING_DUMMY = Mock(spec=None)
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            False,             
+        )
+        self.assertEqual(result, False)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_if_not_set_for_none(self, mock_settings):        
+        mock_settings.TEST_SETTING_DUMMY = Mock(spec=None)
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            None,
+            required_type=int
+        )
+        self.assertEqual(result, None)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_true_stays_true(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = True
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            False,         
+        )
+        self.assertEqual(result, True)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_false_stays_false(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = False
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            False
+        )
+        self.assertEqual(result, False)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_for_invalid_type_bool(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = 'invalid type'
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            False
+        )
+        self.assertEqual(result, False)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_for_invalid_type_int(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = 'invalid type'
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            50
+        )
+        self.assertEqual(result, 50)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_if_below_minimum_1(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = -5
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            default_value=50
+        )
+        self.assertEqual(result, 50)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_if_below_minimum_2(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = -50
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            default_value=50,
+            min_value=-10
+        )
+        self.assertEqual(result, 50)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_for_invalid_type_int_2(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = 1000
+        result = clean_setting(
+            'TEST_SETTING_DUMMY',             
+            default_value=50,
+            max_value=100
+        )
+        self.assertEqual(result, 50)
+
+    @patch(MODULE_PATH + '.settings')
+    def test_default_is_none_needs_required_type(self, mock_settings):
+        mock_settings.TEST_SETTING_DUMMY = 'invalid type'
+        with self.assertRaises(ValueError):
+            clean_setting(
+                'TEST_SETTING_DUMMY',             
+                default_value=None
+            )

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

@@ -1,66 +1,167 @@
-from django_webtest import WebTest
+from unittest.mock import patch
-from unittest import mock
 
-from django.test import TestCase
 from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist
+from django.test import TestCase, RequestFactory
-from django.conf import settings
+from django.urls import reverse
+
 from allianceauth.tests.auth_utils import AuthUtils
 
-from ..models import DiscordUser
+from . import MODULE_PATH, add_permissions_to_members, TEST_USER_NAME, TEST_USER_ID
-from ..manager import DiscordOAuthManager
+from ..models import DiscordUser, DiscordClient
-
+from ..utils import set_logger_to_file
-from . import DEFAULT_AUTH_GROUP, add_permissions, MODULE_PATH
+from ..views import (
+    discord_callback, 
+    reset_discord, 
+    deactivate_discord, 
+    discord_add_bot, 
+    activate_discord
+)
 
 
-class DiscordViewsTestCase(WebTest):
+logger = set_logger_to_file(MODULE_PATH + '.views', __file__)
+
+
+class SetupClassMixin(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.factory = RequestFactory()
+        cls.user = AuthUtils.create_member(TEST_USER_NAME)
+        add_permissions_to_members()
+        cls.services_url = reverse('services:services')
+
+
+class TestActivateDiscord(SetupClassMixin, TestCase):
+
+    @patch(MODULE_PATH + '.views.DiscordUser.objects.generate_oauth_redirect_url')
+    def test_redirects_to_correct_url(self, mock_generate_oauth_redirect_url):
+        expected_url = '/example.com/oauth/'
+        mock_generate_oauth_redirect_url.return_value = expected_url
+        request = self.factory.get(reverse('discord:activate'))
+        request.user = self.user
+        response = activate_discord(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, expected_url)
+
+
+@patch(MODULE_PATH + '.views.messages')
+@patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient)
+class TestDeactivateDiscord(SetupClassMixin, TestCase):
+    
     def setUp(self):
-        self.member = AuthUtils.create_member('auth_member')
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
-        AuthUtils.add_main_character(self.member, 'test character', '1234', '2345', 'test corp', 'testc')
-        add_permissions()
     
-    def login(self):
+    def test_when_successful_show_success_message(
-        self.app.set_user(self.member)
+        self, mock_DiscordClient, mock_messages
+    ):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
+        request = self.factory.get(reverse('discord:deactivate'))
+        request.user = self.user
+        response = deactivate_discord(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertTrue(mock_messages.success.called)
+        self.assertFalse(mock_messages.error.called)
 
-    @mock.patch(MODULE_PATH + '.views.DiscordOAuthManager')
+    def test_when_unsuccessful_show_error_message(
-    def test_activate(self, manager):
+        self, mock_DiscordClient, mock_messages
-        self.login()
+    ):
-        manager.generate_oauth_redirect_url.return_value = '/example.com/oauth/'
+        mock_DiscordClient.return_value.remove_guild_member.return_value = False
-        response = self.app.get('/discord/activate/', auto_follow=False)
+        request = self.factory.get(reverse('discord:deactivate'))
-        self.assertRedirects(response, expected_url='/example.com/oauth/', target_status_code=404)
+        request.user = self.user
+        response = deactivate_discord(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertFalse(mock_messages.success.called)
+        self.assertTrue(mock_messages.error.called)
 
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
-    def test_callback(self, manager):
-        self.login()
-        manager.add_user.return_value = '1234'
-        response = self.app.get('/discord/callback/', params={'code': '1234'})
 
-        self.member = User.objects.get(pk=self.member.pk)
+@patch(MODULE_PATH + '.views.messages')
+@patch(MODULE_PATH + '.managers.DiscordClient')
+class TestResetDiscord(SetupClassMixin, TestCase):
     
-        self.assertTrue(manager.add_user.called)
+    def setUp(self):
-        self.assertEqual(manager.update_nickname.called, settings.DISCORD_SYNC_NAMES)
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
-        self.assertEqual(self.member.discord.uid, '1234')
-        self.assertRedirects(response, expected_url='/services/', target_status_code=200)
 
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
+    def test_when_successful_redirect_to_activate(
-    def test_reset(self, manager):
+        self, mock_DiscordClient, mock_messages
-        self.login()
+    ):
-        DiscordUser.objects.create(user=self.member, uid='12345')
+        mock_DiscordClient.return_value.remove_guild_member.return_value = True
-        manager.delete_user.return_value = True
+        request = self.factory.get(reverse('discord:reset'))
+        request.user = self.user
+        response = reset_discord(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, reverse("discord:activate"))        
+        self.assertFalse(mock_messages.error.called)
 
-        response = self.app.get('/discord/reset/')
+    def test_when_unsuccessful_message_error_and_redirect_to_service(
+        self, mock_DiscordClient, mock_messages
+    ):
+        mock_DiscordClient.return_value.remove_guild_member.return_value = False
+        request = self.factory.get(reverse('discord:reset'))
+        request.user = self.user
+        response = reset_discord(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertTrue(mock_messages.error.called)
 
-        self.assertRedirects(response, expected_url='/discord/activate/', target_status_code=302)
 
-    @mock.patch(MODULE_PATH + '.tasks.DiscordOAuthManager')
+@patch(MODULE_PATH + '.views.messages')
-    def test_deactivate(self, manager):
+@patch(MODULE_PATH + '.views.DiscordUser.objects.add_user')
-        self.login()
+class TestDiscordCallback(SetupClassMixin, TestCase):
-        DiscordUser.objects.create(user=self.member, uid='12345')
-        manager.delete_user.return_value = True
     
-        response = self.app.get('/discord/deactivate/')
+    def setUp(self):
+        DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID)
 
-        self.assertTrue(manager.delete_user.called)
+    def test_success_message_when_ok(self, mock_add_user, mock_messages):
-        self.assertRedirects(response, expected_url='/services/', target_status_code=200)
+        mock_add_user.return_value = True
-        with self.assertRaises(ObjectDoesNotExist):
+        request = self.factory.get(
-            discord_user = User.objects.get(pk=self.member.pk).discord
+            reverse('discord:callback'), data={'code': '1234'}
+        )
+        request.user = self.user
+        response = discord_callback(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertTrue(mock_messages.success.called)
+        self.assertFalse(mock_messages.error.called)
+    
+    def test_handle_no_code(self, mock_add_user, mock_messages):
+        mock_add_user.return_value = True
+        request = self.factory.get(
+            reverse('discord:callback'), data={}
+        )
+        request.user = self.user
+        response = discord_callback(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertFalse(mock_messages.success.called)
+        self.assertTrue(mock_messages.error.called)
+    
+    def test_error_message_when_user_creation_failed(
+        self, mock_add_user, mock_messages
+    ):
+        mock_add_user.return_value = False
+        request = self.factory.get(
+            reverse('discord:callback'), data={'code': '1234'}
+        )
+        request.user = self.user
+        response = discord_callback(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, self.services_url)
+        self.assertFalse(mock_messages.success.called)
+        self.assertTrue(mock_messages.error.called)
+
+
+@patch(MODULE_PATH + '.views.DiscordUser.objects.generate_bot_add_url')
+class TestDiscordAddBot(TestCase):
+    
+    def test_add_bot(self, mock_generate_bot_add_url):
+        bot_url = 'https://www.example.com/bot'
+        mock_generate_bot_add_url.return_value = bot_url
+        my_user = User.objects.create_superuser('Lex Luthor', 'abc', 'def')
+        request = RequestFactory().get(reverse('discord:add_bot'))
+        request.user = my_user
+        response = discord_add_bot(request)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, bot_url)

allianceauth/services/modules/discord/utils.py

@@ -0,0 +1,89 @@
+import logging
+import os
+
+from django.conf import settings
+
+
+logger = logging.getLogger(__name__)
+
+
+class LoggerAddTag(logging.LoggerAdapter):
+    """add custom tag to a logger"""
+    def __init__(self, logger, prefix):
+        super(LoggerAddTag, self).__init__(logger, {})
+        self.prefix = prefix
+
+    def process(self, msg, kwargs):
+        return '[%s] %s' % (self.prefix, msg), kwargs
+        
+
+def clean_setting(
+    name: str,
+    default_value: object,
+    min_value: int = None,
+    max_value: int = None,
+    required_type: type = None
+):
+    """cleans the input for a custom setting
+
+    Will use `default_value` if settings does not exit or has the wrong type
+    or is outside define boundaries (for int only)
+
+    Need to define `required_type` if `default_value` is `None`
+
+    Will assume `min_value` of 0 for int (can be overriden)
+
+    Returns cleaned value for setting
+    """
+    if default_value is None and not required_type:
+        raise ValueError('You must specify a required_type for None defaults')
+
+    if not required_type:
+        required_type = type(default_value)
+
+    if min_value is None and required_type == int:
+        min_value = 0
+
+    if not hasattr(settings, name):
+        cleaned_value = default_value
+    else:
+        if (
+            isinstance(getattr(settings, name), required_type)
+            and (min_value is None or getattr(settings, name) >= min_value)
+            and (max_value is None or getattr(settings, name) <= max_value)
+        ):
+            cleaned_value = getattr(settings, name)
+        else:
+            logger.warning(
+                'You setting for %s it not valid. Please correct it. '
+                'Using default for now: %s',
+                name,
+                default_value                
+            )
+            cleaned_value = default_value
+    return cleaned_value
+
+
+def set_logger_to_file(logger_name: str, name: str) -> object:
+    """set logger for current module to log into a file. Useful for tests.
+    
+    Args:
+    - logger: current logger object
+    - name: name of current module, e.g. __file__
+    
+    Returns:
+    - amended logger
+    """
+    
+    # reconfigure logger so we get logging from tested module
+    f_format = logging.Formatter(
+        '%(asctime)s - %(levelname)s - %(module)s:%(funcName)s - %(message)s'
+    )
+    path = os.path.splitext(name)[0]
+    f_handler = logging.FileHandler('{}.log'.format(path), 'w+')
+    f_handler.setFormatter(f_format)
+    logger = logging.getLogger(logger_name)
+    logger.level = logging.DEBUG
+    logger.addHandler(f_handler)
+    logger.propagate = False
+    return logger

allianceauth/services/modules/discord/views.py

@@ -5,12 +5,16 @@ from django.contrib.auth.decorators import login_required
 from django.contrib.auth.decorators import permission_required
 from django.contrib.auth.decorators import user_passes_test
 from django.shortcuts import redirect
+from django.utils.translation import gettext_lazy as _
 
 from allianceauth.services.views import superuser_test
-from .manager import DiscordOAuthManager
-from .tasks import DiscordTasks
 
-logger = logging.getLogger(__name__)
+from . import __title__
+from .models import DiscordUser
+from .utils import LoggerAddTag
+
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
 
 ACCESS_PERM = 'discord.access_discord'
 
@@ -18,53 +22,94 @@ ACCESS_PERM = 'discord.access_discord'
 @login_required
 @permission_required(ACCESS_PERM)
 def deactivate_discord(request):
-    logger.debug("deactivate_discord called by user %s" % request.user)
+    logger.debug("deactivate_discord called by user %s", request.user)
-    if DiscordTasks.delete_user(request.user):
+    if request.user.discord.delete_user(is_rate_limited=False):
-        logger.info("Successfully deactivated discord for user %s" % request.user)
+        logger.info("Successfully deactivated discord for user %s", request.user)
-        messages.success(request, 'Deactivated Discord account.')
+        messages.success(request, _('Deactivated Discord account.'))
     else:
-        logger.error("Unsuccessful attempt to deactivate discord for user %s" % request.user)
+        logger.error(
-        messages.error(request, 'An error occurred while processing your Discord account.')
+            "Unsuccessful attempt to deactivate discord for user %s", request.user
+        )
+        messages.error(
+            request, _('An error occurred while processing your Discord account.')
+        )
     return redirect("services:services")
 
 
 @login_required
 @permission_required(ACCESS_PERM)
 def reset_discord(request):
-    logger.debug("reset_discord called by user %s" % request.user)
+    logger.debug("reset_discord called by user %s", request.user)
-    if DiscordTasks.delete_user(request.user):
+    if request.user.discord.delete_user(is_rate_limited=False):
-        logger.info("Successfully deleted discord user for user %s - forwarding to discord activation." % request.user)
+        logger.info(
+            "Successfully deleted discord user for user %s - "
+            "forwarding to discord activation.", 
+            request.user
+        )
         return redirect("discord:activate")
-    logger.error("Unsuccessful attempt to reset discord for user %s" % request.user)
+    
-    messages.error(request, 'An error occurred while processing your Discord account.')
+    logger.error(
+        "Unsuccessful attempt to reset discord for user %s", request.user
+    )
+    messages.error(
+        request, _('An error occurred while processing your Discord account.')
+    )
     return redirect("services:services")
 
 
 @login_required
 @permission_required(ACCESS_PERM)
 def activate_discord(request):
-    logger.debug("activate_discord called by user %s" % request.user)
+    logger.debug("activate_discord called by user %s", request.user)
-    return redirect(DiscordOAuthManager.generate_oauth_redirect_url())
+    return redirect(DiscordUser.objects.generate_oauth_redirect_url())
 
 
 @login_required
 @permission_required(ACCESS_PERM)
 def discord_callback(request):
-    logger.debug("Received Discord callback for activation of user %s" % request.user)
+    logger.debug(
-    code = request.GET.get('code', None)
+        "Received Discord callback for activation of user %s", request.user
-    if not code:
+    )
-        logger.warn("Did not receive OAuth code from callback of user %s" % request.user)
+    authorization_code = request.GET.get('code', None)    
-        return redirect("services:services")
+    if not authorization_code:
-    if DiscordTasks.add_user(request.user, code):
+        logger.warning(
-        logger.info("Successfully activated Discord for user %s" % request.user)
+            "Did not receive OAuth code from callback for user %s", request.user
-        messages.success(request, 'Activated Discord account.')
+        )
+        success = False
     else:
-        logger.error("Failed to activate Discord for user %s" % request.user)
+        if DiscordUser.objects.add_user(
-        messages.error(request, 'An error occurred while processing your Discord account.')
+            user=request.user, 
+            authorization_code=authorization_code, 
+            is_rate_limited=False
+        ):
+            logger.info(
+                "Successfully activated Discord account for user %s", request.user
+            )
+            success = True
+            
+        else:
+            logger.error(
+                "Failed to activate Discord account for user %s", request.user
+            )
+            success = False
+        
+    if success:
+        messages.success(
+            request, _('Your Discord account has been successfully activated.')
+        )
+    else:
+        messages.error(
+            request, 
+            _(
+                'An error occurred while trying to activate your Discord account. '
+                'Please try again.'
+            )
+        )
+    
     return redirect("services:services")
 
 
 @login_required
 @user_passes_test(superuser_test)
 def discord_add_bot(request):
-    return redirect(DiscordOAuthManager.generate_bot_add_url())
+    return redirect(DiscordUser.objects.generate_bot_add_url())

allianceauth/services/modules/discourse/manager.py

@@ -168,7 +168,7 @@ class DiscourseManager:
         for arg in kwargs:
             if arg not in endpoint['args']['required'] and arg not in endpoint['args']['optional'] and not silent:
                 logger.warn("Received unrecognized kwarg %s for endpoint %s" % (arg, endpoint))
-        r = getattr(requests, endpoint['method'])(settings.DISCOURSE_URL + endpoint['parsed_url'], params=params,
+        r = getattr(requests, endpoint['method'])(settings.DISCOURSE_URL + endpoint['parsed_url'], headers=params,
                                                   json=data)
         try:
             if 'errors' in r.json() and not silent:
@@ -185,6 +185,7 @@ class DiscourseManager:
                 r.raise_for_status()
             except requests.exceptions.HTTPError as e:
                 raise DiscourseError(endpoint, e.response.status_code)
+        logger.debug("Discourse API output:\n{}".format(out))  # this is spamy as hell remove before release
         return out
 
     @staticmethod

allianceauth/services/modules/discourse/views.py

@@ -2,6 +2,7 @@ from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
 from django.shortcuts import render, redirect
+from django.utils.translation import gettext_lazy as _
 
 from .manager import DiscourseManager
 from .tasks import DiscourseTasks
@@ -33,12 +34,12 @@ def discourse_sso(request):
 
     # Check if user has access
     if not request.user.has_perm(ACCESS_PERM):
-        messages.error(request, 'You are not authorized to access Discourse.')
+        messages.error(request, _('You are not authorized to access Discourse.'))
         logger.warning('User %s attempted to access Discourse but does not have permission.' % request.user)
         return redirect('authentication:dashboard')
 
     if not request.user.profile.main_character:
-        messages.error(request, "You must have a main character set to access Discourse.")
+        messages.error(request, _("You must have a main character set to access Discourse."))
         logger.warning('User %s attempted to access Discourse but does not have a main character.' % request.user)
         return redirect('authentication:characters')
 
@@ -48,7 +49,7 @@ def discourse_sso(request):
     signature = request.GET.get('sig')
 
     if None in [payload, signature]:
-        messages.error(request, 'No SSO payload or signature. Please contact support if this problem persists.')
+        messages.error(request, _('No SSO payload or signature. Please contact support if this problem persists.'))
         return redirect('authentication:dashboard')
 
     # Validate the payload
@@ -58,7 +59,7 @@ def discourse_sso(request):
         assert 'nonce' in decoded
         assert len(payload) > 0
     except AssertionError:
-        messages.error(request, 'Invalid payload. Please contact support if this problem persists.')
+        messages.error(request, _('Invalid payload. Please contact support if this problem persists.'))
         return redirect('authentication:dashboard')
 
     key = str(settings.DISCOURSE_SSO_SECRET).encode('utf-8')
@@ -66,7 +67,7 @@ def discourse_sso(request):
     this_signature = h.hexdigest()
 
     if this_signature != signature:
-        messages.error(request, 'Invalid payload. Please contact support if this problem persists.')
+        messages.error(request, _('Invalid payload. Please contact support if this problem persists.'))
         return redirect('authentication:dashboard')
 
     ## Build the return payload

allianceauth/services/modules/ips4/views.py

@@ -3,6 +3,7 @@ import logging
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required, permission_required
 from django.shortcuts import render, redirect
+from django.utils.translation import gettext_lazy as _
 
 from allianceauth.services.forms import ServicePasswordForm
 from .manager import Ips4Manager
@@ -27,7 +28,7 @@ def activate_ips4(request):
         logger.debug("Updated authserviceinfo for user %s with IPS4 credentials." % request.user)
         # update_ips4_groups.delay(request.user.pk)
         logger.info("Successfully activated IPS4 for user %s" % request.user)
-        messages.success(request, 'Activated IPSuite4 account.')
+        messages.success(request, _('Activated IPSuite4 account.'))
         credentials = {
             'username': result[0],
             'password': result[1],
@@ -36,7 +37,7 @@ def activate_ips4(request):
                       context={'credentials': credentials, 'service': 'IPSuite4'})
     else:
         logger.error("Unsuccessful attempt to activate IPS4 for user %s" % request.user)
-        messages.error(request, 'An error occurred while processing your IPSuite4 account.')
+        messages.error(request, _('An error occurred while processing your IPSuite4 account.'))
     return redirect("services:services")
 
 
@@ -49,7 +50,7 @@ def reset_ips4_password(request):
         # false we failed
         if result != "":
             logger.info("Successfully reset IPS4 password for user %s" % request.user)
-            messages.success(request, 'Reset IPSuite4 password.')
+            messages.success(request, _('Reset IPSuite4 password.'))
             credentials = {
                 'username': request.user.ips4.username,
                 'password': result,
@@ -58,7 +59,7 @@ def reset_ips4_password(request):
                           context={'credentials': credentials, 'service': 'IPSuite4'})
 
     logger.error("Unsuccessful attempt to reset IPS4 password for user %s" % request.user)
-    messages.error(request, 'An error occurred while processing your IPSuite4 account.')
+    messages.error(request, _('An error occurred while processing your IPSuite4 account.'))
     return redirect("services:services")
 
 
@@ -76,10 +77,10 @@ def set_ips4_password(request):
             result = Ips4Manager.update_custom_password(request.user.ips4.username, plain_password=password)
             if result != "":
                 logger.info("Successfully set IPS4 password for user %s" % request.user)
-                messages.success(request, 'Set IPSuite4 password.')
+                messages.success(request, _('Set IPSuite4 password.'))
             else:
                 logger.error("Failed to install custom IPS4 password for user %s" % request.user)
-                messages.error(request, 'An error occurred while processing your IPSuite4 account.')
+                messages.error(request, _('An error occurred while processing your IPSuite4 account.'))
             return redirect('services:services')
     else:
         logger.debug("Request is not type POST - providing empty form.")
@@ -96,9 +97,9 @@ def deactivate_ips4(request):
     logger.debug("deactivate_ips4 called by user %s" % request.user)
     if Ips4Tasks.delete_user(request.user):
         logger.info("Successfully deactivated IPS4 for user %s" % request.user)
-        messages.success(request, 'Deactivated IPSuite4 account.')
+        messages.success(request, _('Deactivated IPSuite4 account.'))
     else:
         logger.error("Unsuccessful attempt to deactivate IPS4 for user %s" % request.user)
-        messages.error(request, 'An error occurred while processing your IPSuite4 account.')
+        messages.error(request, _('An error occurred while processing your IPSuite4 account.'))
     return redirect("services:services")
 

allianceauth/services/modules/mumble/auth_hooks.py

@@ -40,6 +40,11 @@ class MumbleService(ServicesHook):
         if MumbleTasks.has_account(user):
             MumbleTasks.update_groups.delay(user.pk)
 
+    def sync_nickname(self, user):
+        logger.debug("Updating %s nickname for %s" % (self.name, user))
+        if MumbleTasks.has_account(user):
+            MumbleTasks.update_display_name.apply_async(args=[user.pk], countdown=5) # cooldown on this task to ensure DB clean when syncing
+            
     def validate_user(self, user):
         if MumbleTasks.has_account(user) and not self.service_active_for_user(user):
             self.delete_user(user, notify_user=True)

allianceauth/services/modules/mumble/migrations/0008_mumbleuser_display_name.py

@@ -0,0 +1,17 @@
+# Generated by Django 2.2.9 on 2020-03-16 07:49
+
+from django.db import migrations, models
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('mumble', '0007_not_null_user'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='mumbleuser',
+            name='display_name',
+            field=models.CharField(max_length=254, null=True),
+        )
+    ]

allianceauth/services/modules/mumble/migrations/0009_set_mumble_dissplay_names.py

@@ -0,0 +1,37 @@
+from django.db import migrations, models
+from ..auth_hooks import MumbleService
+from allianceauth.services.hooks import NameFormatter
+
+def fwd_func(apps, schema_editor):
+    MumbleUser = apps.get_model("mumble", "MumbleUser")
+    db_alias = schema_editor.connection.alias
+    all_users = MumbleUser.objects.using(db_alias).all()
+    for user in all_users:
+        display_name = NameFormatter(MumbleService(), user.user).format_name()
+        user.display_name = display_name
+        user.save()
+
+def rev_func(apps, schema_editor):
+    MumbleUser = apps.get_model("mumble", "MumbleUser")
+    db_alias = schema_editor.connection.alias
+    all_users = MumbleUser.objects.using(db_alias).all()
+    for user in all_users:
+        user.display_name = None
+        user.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('mumble', '0008_mumbleuser_display_name'),
+    ]
+
+    operations = [
+        migrations.RunPython(fwd_func, rev_func),
+        migrations.AlterField(
+            model_name='mumbleuser',
+            name='display_name',
+            field=models.CharField(max_length=254, unique=True),
+            preserve_default=False,
+        ),
+    ]

allianceauth/services/modules/mumble/models.py

@@ -15,10 +15,14 @@ class MumbleManager(models.Manager):
     HASH_FN = 'bcrypt-sha256'
 
     @staticmethod
-    def get_username(user):
+    def get_display_name(user):
         from .auth_hooks import MumbleService
         return NameFormatter(MumbleService(), user).format_name()
     
+    @staticmethod
+    def get_username(user):
+        return user.profile.main_character.character_name  # main character as the user.username may be incorect
+    
     @staticmethod
     def sanitise_username(username):
         return username.replace(" ", "_")
@@ -32,20 +36,26 @@ class MumbleManager(models.Manager):
         return bcrypt_sha256.encrypt(password.encode('utf-8'))
 
     def create(self, user):
-        username = self.get_username(user)
+        try:
-        logger.debug("Creating mumble user with username {}".format(username))
+            username = self.get_username(user)
-        username_clean = self.sanitise_username(username)
+            logger.debug("Creating mumble user with username {}".format(username))
-        password = self.generate_random_pass()
+            username_clean = self.sanitise_username(username)
-        pwhash = self.gen_pwhash(password)
+            display_name = self.get_display_name(user)
-        logger.debug("Proceeding with mumble user creation: clean username {}, pwhash starts with {}".format(
+            password = self.generate_random_pass()
-            username_clean, pwhash[0:5]))
+            pwhash = self.gen_pwhash(password)
-        logger.info("Creating mumble user {}".format(username_clean))
+            logger.debug("Proceeding with mumble user creation: clean username {}, pwhash starts with {}".format(
+                username_clean, pwhash[0:5]))
+            logger.info("Creating mumble user {}".format(username_clean))
 
-        result = super(MumbleManager, self).create(user=user, username=username_clean,
+            result = super(MumbleManager, self).create(user=user, username=username_clean,
-                                                   pwhash=pwhash, hashfn=self.HASH_FN)
+                                                    pwhash=pwhash, hashfn=self.HASH_FN,
-        result.update_groups()
+                                                    display_name=display_name)
-        result.credentials.update({'username': result.username, 'password': password})
+            result.update_groups()
-        return result
+            result.credentials.update({'username': result.username, 'password': password})
+            return result
+        except AttributeError:  # No Main or similar errors
+            return False
+        return False
 
     def user_exists(self, username):
         return self.filter(username=username).exists()
@@ -59,6 +69,8 @@ class MumbleUser(AbstractServiceModel):
 
     objects = MumbleManager()
 
+    display_name = models.CharField(max_length=254, unique=True)
+
     def __str__(self):
         return self.username
 
@@ -91,6 +103,12 @@ class MumbleUser(AbstractServiceModel):
         self.save()
         return True
 
+    def update_display_name(self):
+        logger.info("Updating mumble user {} display name".format(self.user))
+        self.display_name = MumbleManager.get_display_name(self.user)
+        self.save()
+        return True
+
     class Meta:
         permissions = (
             ("access_mumble", u"Can access the Mumble service"),

allianceauth/services/modules/mumble/tasks.py

@@ -45,9 +45,37 @@ class MumbleTasks:
             logger.debug("User %s does not have a mumble account, skipping" % user)
         return False
 
+    @staticmethod
+    @shared_task(bind=True, name="mumble.update_display_name", base=QueueOnce)
+    def update_display_name(self, pk):
+        user = User.objects.get(pk=pk)
+        logger.debug("Updating mumble groups for user %s" % user)
+        if MumbleTasks.has_account(user):
+            try:
+                if not user.mumble.update_display_name():
+                    raise Exception("Display Name Sync failed")
+                logger.debug("Updated user %s mumble display name." % user)
+                return True
+            except MumbleUser.DoesNotExist:
+                logger.info("Mumble display name sync failed for {}, user does not have a mumble account".format(user))
+            except:
+                logger.exception("Mumble display name sync failed for %s, retrying in 10 mins" % user)
+                raise self.retry(countdown=60 * 10)
+        else:
+            logger.debug("User %s does not have a mumble account, skipping" % user)
+        return False
+
     @staticmethod
     @shared_task(name="mumble.update_all_groups")
     def update_all_groups():
         logger.debug("Updating ALL mumble groups")
         for mumble_user in MumbleUser.objects.exclude(username__exact=''):
             MumbleTasks.update_groups.delay(mumble_user.user.pk)
+
+    @staticmethod
+    @shared_task(name="mumble.update_all_display_names")
+    def update_all_display_names():
+        logger.debug("Updating ALL mumble display names")
+        for mumble_user in MumbleUser.objects.exclude(username__exact=''):
+            MumbleTasks.update_display_name.delay(mumble_user.user.pk)
+

allianceauth/services/modules/mumble/tests.py

@@ -25,6 +25,9 @@ class MumbleHooksTestCase(TestCase):
     def setUp(self):
         self.member = 'member_user'
         member = AuthUtils.create_member(self.member)
+        AuthUtils.add_main_character(member, 'auth_member', '12345', corp_id='111', corp_name='Test Corporation',
+                                     corp_ticker='TESTR')
+        member = User.objects.get(pk=member.pk)
         MumbleUser.objects.create(user=member)
         self.none_user = 'none_user'
         none_user = AuthUtils.create_user(self.none_user)
@@ -122,23 +125,45 @@ class MumbleViewsTestCase(TestCase):
         self.member.save()
         AuthUtils.add_main_character(self.member, 'auth_member', '12345', corp_id='111', corp_name='Test Corporation',
                                      corp_ticker='TESTR')
+        self.member = User.objects.get(pk=self.member.pk)
         add_permissions()
 
     def login(self):
         self.client.force_login(self.member)
 
-    def test_activate(self):
+    def test_activate_update(self):
         self.login()
-        expected_username = '[TESTR]auth_member'
+        expected_username = 'auth_member'
+        expected_displayname = '[TESTR]auth_member'
         response = self.client.get(urls.reverse('mumble:activate'), follow=False)
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, expected_username)
+        # create
         mumble_user = MumbleUser.objects.get(user=self.member)
         self.assertEqual(mumble_user.username, expected_username)
+        self.assertTrue(MumbleUser.objects.user_exists(expected_username))
+        self.assertEqual(str(mumble_user), expected_username)
+        self.assertEqual(mumble_user.display_name, expected_displayname)
         self.assertTrue(mumble_user.pwhash)
         self.assertIn('Guest', mumble_user.groups)
         self.assertIn('Member', mumble_user.groups)
         self.assertIn(',', mumble_user.groups)
+        # test update
+        self.member.profile.main_character.character_name = "auth_member_updated"
+        self.member.profile.main_character.corporation_ticker = "TESTU"
+        self.member.profile.main_character.save()
+        mumble_user.update_display_name()
+        mumble_user = MumbleUser.objects.get(user=self.member)
+        expected_displayname = '[TESTU]auth_member_updated'
+        self.assertEqual(mumble_user.username, expected_username)
+        self.assertTrue(MumbleUser.objects.user_exists(expected_username))
+        self.assertEqual(str(mumble_user), expected_username)
+        self.assertEqual(mumble_user.display_name, expected_displayname)
+        self.assertTrue(mumble_user.pwhash)
+        self.assertIn('Guest', mumble_user.groups)
+        self.assertIn('Member', mumble_user.groups)
+        self.assertIn(',', mumble_user.groups)
+
 
     def test_deactivate_post(self):
         self.login()
@@ -171,7 +196,6 @@ class MumbleViewsTestCase(TestCase):
         self.assertTemplateUsed(response, 'services/service_credentials.html')
         self.assertContains(response, 'auth_member')
 
-
 class MumbleManagerTestCase(TestCase):
     def setUp(self):
         from .models import MumbleManager