Version Bump v3.1.1

Add `CSRF_TRUSTED_ORIGINS` to `local.py`

See merge request allianceauth/allianceauth!1446

.gitlab-ci.yml

@@ -5,16 +5,16 @@
     - merge_requests
 
 stages:
-- pre-commit
+  - pre-commit
-- gitlab
+  - gitlab
-- test
+  - test
-- deploy
+  - deploy
-- docker
+  - docker
 
 include:
-- template: Dependency-Scanning.gitlab-ci.yml
+  - template: Dependency-Scanning.gitlab-ci.yml
-- template: Security/SAST.gitlab-ci.yml
+  - template: Security/SAST.gitlab-ci.yml
-- template: Security/Secret-Detection.gitlab-ci.yml
+  - template: Security/Secret-Detection.gitlab-ci.yml
 
 before_script:
   - apt-get update && apt-get install redis-server -y
@@ -42,103 +42,150 @@ sast:
 dependency_scanning:
   stage: gitlab
   before_script:
-  - apt-get update && apt-get install redis-server libmariadb-dev -y
+    - apt-get update && apt-get install redis-server libmariadb-dev -y
-  - redis-server --daemonize yes
+    - redis-server --daemonize yes
-  - python -V
+    - python -V
-  - pip install wheel tox
+    - pip install wheel tox
+
+secret_detection:
+  stage: gitlab
+  before_script: []
 
 test-3.8-core:
   <<: *only-default
   image: python:3.8-bullseye
   script:
-  - tox -e py38-core
+    - tox -e py38-core
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.9-core:
   <<: *only-default
   image: python:3.9-bullseye
   script:
-  - tox -e py39-core
+    - tox -e py39-core
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.10-core:
   <<: *only-default
   image: python:3.10-bullseye
   script:
-  - tox -e py310-core
+    - tox -e py310-core
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.11-core:
   <<: *only-default
   image: python:3.11-rc-bullseye
   script:
-  - tox -e py311-core
+    - tox -e py311-core
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
   allow_failure: true
 
 test-3.8-all:
   <<: *only-default
   image: python:3.8-bullseye
   script:
-  - tox -e py38-all
+    - tox -e py38-all
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.9-all:
   <<: *only-default
   image: python:3.9-bullseye
   script:
-  - tox -e py39-all
+    - tox -e py39-all
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.10-all:
   <<: *only-default
   image: python:3.10-bullseye
   script:
-  - tox -e py310-all
+    - tox -e py310-all
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
 
 test-3.11-all:
   <<: *only-default
   image: python:3.11-rc-bullseye
   script:
-  - tox -e py311-all
+    - tox -e py311-all
   artifacts:
     when: always
     reports:
-      cobertura: coverage.xml
+      coverage_report:
+        coverage_format: cobertura
+        path: coverage.xml
   allow_failure: true
 
+build-test:
+  stage: test
+  image: python:3.10-bullseye
+
+  before_script:
+    - python -m pip install --upgrade pip
+    - python -m pip install --upgrade build
+    - python -m pip install --upgrade setuptools wheel
+
+  script:
+    - python -m build
+
+  artifacts:
+    when: always
+    name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
+    paths:
+      - dist/*
+    expire_in: 1 year
+
+test-docs:
+  <<: *only-default
+  image: python:3.10-bullseye
+  script:
+  - tox -e docs
+
 deploy_production:
   stage: deploy
   image: python:3.10-bullseye
 
   before_script:
-    - pip install twine wheel
+    - python -m pip install --upgrade pip
+    - python -m pip install --upgrade build
+    - python -m pip install --upgrade setuptools wheel twine
 
   script:
-    - python setup.py sdist bdist_wheel
+    - python -m build
-    - twine upload dist/*
+    - python -m twine upload dist/*
 
   rules:
     - if: $CI_COMMIT_TAG
@@ -166,6 +213,8 @@ build-image:
     docker image push --all-tags $CI_REGISTRY_IMAGE/auth
   rules:
     - if: $CI_COMMIT_TAG
+      when: delayed
+      start_in: 10 minutes
 
 build-image-dev:
   before_script: []

.pre-commit-config.yaml

@@ -5,7 +5,7 @@
 
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.3.0
     hooks:
       - id: check-case-conflict
       - id: check-json
@@ -25,10 +25,15 @@ repos:
     rev: 2.4.0
     hooks:
       - id: editorconfig-checker
-        exclude: ^(LICENSE|allianceauth\/static\/css\/themes\/bootstrap-locals.less|allianceauth\/eveonline\/swagger.json|(.*.po)|(.*.mo))
+        exclude: ^(LICENSE|allianceauth\/static\/allianceauth\/css\/themes\/bootstrap-locals.less|allianceauth\/eveonline\/swagger.json|(.*.po)|(.*.mo))
 
   - repo: https://github.com/asottile/pyupgrade
-    rev: v2.30.0
+    rev: v2.34.0
     hooks:
       - id: pyupgrade
         args: [ --py38-plus ]
+
+  - repo: https://github.com/asottile/setup-cfg-fmt
+    rev: v1.20.1
+    hooks:
+      - id: setup-cfg-fmt

.readthedocs.yml

@@ -5,19 +5,22 @@
 # Required
 version: 2
 
+# Set the version of Python and other tools you might need
+build:
+  os: ubuntu-20.04
+  apt_packages:
+    - redis
+  tools:
+    python: "3.8"
+
 # 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:
     - requirements: docs/requirements.txt

allianceauth/__init__.py

@@ -1,8 +1,7 @@
 # This will make sure the app is always imported when
 # Django starts so that shared_task will use this app.
 
-__version__ = '3.0.0a1'
+__version__ = '3.1.1'
 __title__ = 'Alliance Auth'
 __url__ = 'https://gitlab.com/allianceauth/allianceauth'
 NAME = f'{__title__} v{__version__}'
-default_app_config = 'allianceauth.apps.AllianceAuthConfig'

allianceauth/analytics/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.analytics.apps.AnalyticsConfig'

allianceauth/analytics/tests/test_integration.py

@@ -3,16 +3,17 @@ from urllib.parse import parse_qs
 
 import requests_mock
 
-from django.test import TestCase, override_settings
+from django.test import override_settings
 
 from allianceauth.analytics.tasks import ANALYTICS_URL
 from allianceauth.eveonline.tasks import update_character
 from allianceauth.tests.auth_utils import AuthUtils
+from allianceauth.utils.testing import NoSocketsTestCase
 
 
 @override_settings(CELERY_ALWAYS_EAGER=True)
 @requests_mock.mock()
-class TestAnalyticsForViews(TestCase):
+class TestAnalyticsForViews(NoSocketsTestCase):
     @override_settings(ANALYTICS_DISABLED=False)
     def test_should_run_analytics(self, requests_mocker):
         # given
@@ -40,7 +41,7 @@ class TestAnalyticsForViews(TestCase):
 
 @override_settings(CELERY_ALWAYS_EAGER=True)
 @requests_mock.mock()
-class TestAnalyticsForTasks(TestCase):
+class TestAnalyticsForTasks(NoSocketsTestCase):
     @override_settings(ANALYTICS_DISABLED=False)
     @patch("allianceauth.eveonline.models.EveCharacter.objects.update_character")
     def test_should_run_analytics_for_successful_task(

allianceauth/analytics/tests/test_tasks.py

@@ -1,12 +1,22 @@
+import requests_mock
+
+from django.test.utils import override_settings
+
 from allianceauth.analytics.tasks import (
     analytics_event,
     send_ga_tracking_celery_event,
     send_ga_tracking_web_view)
-from django.test.testcases import TestCase
+from allianceauth.utils.testing import NoSocketsTestCase
 
 
-class TestAnalyticsTasks(TestCase):
+GOOGLE_ANALYTICS_DEBUG_URL = 'https://www.google-analytics.com/debug/collect'
-    def test_analytics_event(self):
+
+
+@override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True)
+@requests_mock.Mocker()
+class TestAnalyticsTasks(NoSocketsTestCase):
+    def test_analytics_event(self, requests_mocker):
+        requests_mocker.register_uri('POST', GOOGLE_ANALYTICS_DEBUG_URL)
         analytics_event(
                         category='allianceauth.analytics',
                         action='send_tests',
@@ -14,15 +24,19 @@ class TestAnalyticsTasks(TestCase):
                         value=1,
                         event_type='Stats')
 
-    def test_send_ga_tracking_web_view_sent(self):
+    def test_send_ga_tracking_web_view_sent(self, requests_mocker):
-        # This test sends if the event SENDS to google
+        """This test sends if the event SENDS to google.
-        # Not if it was successful
+        Not if it was successful.
+        """
+        # given
+        requests_mocker.register_uri('POST', GOOGLE_ANALYTICS_DEBUG_URL)
         tracking_id = 'UA-186249766-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         page = '/index/'
         title = 'Hello World'
         locale = 'en'
         useragent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36"
+        # when
         response = send_ga_tracking_web_view(
                                             tracking_id,
                                             client_id,
@@ -30,15 +44,23 @@ class TestAnalyticsTasks(TestCase):
                                             title,
                                             locale,
                                             useragent)
+        # then
         self.assertEqual(response.status_code, 200)
 
-    def test_send_ga_tracking_web_view_success(self):
+    def test_send_ga_tracking_web_view_success(self, requests_mocker):
+        # given
+        requests_mocker.register_uri(
+            'POST',
+            GOOGLE_ANALYTICS_DEBUG_URL,
+            json={"hitParsingResult":[{'valid': True}]}
+        )
         tracking_id = 'UA-186249766-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         page = '/index/'
         title = 'Hello World'
         locale = 'en'
         useragent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36"
+        # when
         json_response = send_ga_tracking_web_view(
                                                 tracking_id,
                                                 client_id,
@@ -46,15 +68,42 @@ class TestAnalyticsTasks(TestCase):
                                                 title,
                                                 locale,
                                                 useragent).json()
+        # then
         self.assertTrue(json_response["hitParsingResult"][0]["valid"])
 
-    def test_send_ga_tracking_web_view_invalid_token(self):
+    def test_send_ga_tracking_web_view_invalid_token(self, requests_mocker):
+        # given
+        requests_mocker.register_uri(
+            'POST',
+            GOOGLE_ANALYTICS_DEBUG_URL,
+            json={
+                "hitParsingResult":[
+                    {
+                        'valid': False,
+                        'parserMessage': [
+                            {
+                                'messageType': 'INFO',
+                                'description': 'IP Address from this hit was anonymized to 1.132.110.0.',
+                                'messageCode': 'VALUE_MODIFIED'
+                            },
+                            {
+                                'messageType': 'ERROR',
+                                'description': "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.",
+                                'messageCode': 'VALUE_INVALID', 'parameter': 'tid'
+                            }
+                        ],
+                        'hit': '/debug/collect?v=1&tid=UA-IntentionallyBadTrackingID-2&cid=ab33e241fbf042b6aa77c7655a768af7&t=pageview&dp=/index/&dt=Hello World&ul=en&ua=Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36&aip=1&an=allianceauth&av=2.9.0a2'
+                    }
+                ]
+            }
+        )
         tracking_id = 'UA-IntentionallyBadTrackingID-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         page = '/index/'
         title = 'Hello World'
         locale = 'en'
         useragent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36"
+        # when
         json_response = send_ga_tracking_web_view(
                                             tracking_id,
                                             client_id,
@@ -62,18 +111,25 @@ class TestAnalyticsTasks(TestCase):
                                             title,
                                             locale,
                                             useragent).json()
+        # then
         self.assertFalse(json_response["hitParsingResult"][0]["valid"])
-        self.assertEqual(json_response["hitParsingResult"][0]["parserMessage"][1]["description"], "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.")
+        self.assertEqual(
+            json_response["hitParsingResult"][0]["parserMessage"][1]["description"],
+            "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details."
+        )
 
         # [{'valid': False, 'parserMessage': [{'messageType': 'INFO', 'description': 'IP Address from this hit was anonymized to 1.132.110.0.', 'messageCode': 'VALUE_MODIFIED'}, {'messageType': 'ERROR', 'description': "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.", 'messageCode': 'VALUE_INVALID', 'parameter': 'tid'}], 'hit': '/debug/collect?v=1&tid=UA-IntentionallyBadTrackingID-2&cid=ab33e241fbf042b6aa77c7655a768af7&t=pageview&dp=/index/&dt=Hello World&ul=en&ua=Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36&aip=1&an=allianceauth&av=2.9.0a2'}]
 
-    def test_send_ga_tracking_celery_event_sent(self):
+    def test_send_ga_tracking_celery_event_sent(self, requests_mocker):
+        # given
+        requests_mocker.register_uri('POST', GOOGLE_ANALYTICS_DEBUG_URL)
         tracking_id = 'UA-186249766-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         category = 'test'
         action = 'test'
         label = 'test'
         value = '1'
+        # when
         response = send_ga_tracking_celery_event(
                                                 tracking_id,
                                                 client_id,
@@ -81,15 +137,23 @@ class TestAnalyticsTasks(TestCase):
                                                 action,
                                                 label,
                                                 value)
+        # then
         self.assertEqual(response.status_code, 200)
 
-    def test_send_ga_tracking_celery_event_success(self):
+    def test_send_ga_tracking_celery_event_success(self, requests_mocker):
+        # given
+        requests_mocker.register_uri(
+            'POST',
+            GOOGLE_ANALYTICS_DEBUG_URL,
+            json={"hitParsingResult":[{'valid': True}]}
+        )
         tracking_id = 'UA-186249766-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         category = 'test'
         action = 'test'
         label = 'test'
         value = '1'
+        # when
         json_response = send_ga_tracking_celery_event(
                                                     tracking_id,
                                                     client_id,
@@ -97,15 +161,42 @@ class TestAnalyticsTasks(TestCase):
                                                     action,
                                                     label,
                                                     value).json()
+        # then
         self.assertTrue(json_response["hitParsingResult"][0]["valid"])
 
-    def test_send_ga_tracking_celery_event_invalid_token(self):
+    def test_send_ga_tracking_celery_event_invalid_token(self, requests_mocker):
+        # given
+        requests_mocker.register_uri(
+            'POST',
+            GOOGLE_ANALYTICS_DEBUG_URL,
+            json={
+                "hitParsingResult":[
+                    {
+                        'valid': False,
+                        'parserMessage': [
+                            {
+                                'messageType': 'INFO',
+                                'description': 'IP Address from this hit was anonymized to 1.132.110.0.',
+                                'messageCode': 'VALUE_MODIFIED'
+                            },
+                            {
+                                'messageType': 'ERROR',
+                                'description': "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.",
+                                'messageCode': 'VALUE_INVALID', 'parameter': 'tid'
+                            }
+                        ],
+                        'hit': '/debug/collect?v=1&tid=UA-IntentionallyBadTrackingID-2&cid=ab33e241fbf042b6aa77c7655a768af7&t=pageview&dp=/index/&dt=Hello World&ul=en&ua=Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36&aip=1&an=allianceauth&av=2.9.0a2'
+                    }
+                ]
+            }
+        )
         tracking_id = 'UA-IntentionallyBadTrackingID-2'
         client_id = 'ab33e241fbf042b6aa77c7655a768af7'
         category = 'test'
         action = 'test'
         label = 'test'
         value = '1'
+        # when
         json_response = send_ga_tracking_celery_event(
                                                     tracking_id,
                                                     client_id,
@@ -113,7 +204,9 @@ class TestAnalyticsTasks(TestCase):
                                                     action,
                                                     label,
                                                     value).json()
+        # then
         self.assertFalse(json_response["hitParsingResult"][0]["valid"])
-        self.assertEqual(json_response["hitParsingResult"][0]["parserMessage"][1]["description"], "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.")
+        self.assertEqual(
-
+            json_response["hitParsingResult"][0]["parserMessage"][1]["description"],
-        # [{'valid': False, 'parserMessage': [{'messageType': 'INFO', 'description': 'IP Address from this hit was anonymized to 1.132.110.0.', 'messageCode': 'VALUE_MODIFIED'}, {'messageType': 'ERROR', 'description': "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details.", 'messageCode': 'VALUE_INVALID', 'parameter': 'tid'}], 'hit': '/debug/collect?v=1&tid=UA-IntentionallyBadTrackingID-2&cid=ab33e241fbf042b6aa77c7655a768af7&t=pageview&dp=/index/&dt=Hello World&ul=en&ua=Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36&aip=1&an=allianceauth&av=2.9.0a2'}]
+            "The value provided for parameter 'tid' is invalid. Please see http://goo.gl/a8d4RP#tid for details."
+        )

allianceauth/authentication/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.authentication.apps.AuthenticationConfig'

allianceauth/authentication/admin.py

@@ -1,30 +1,44 @@
 from django.contrib import admin
 from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
-from django.contrib.auth.models import User as BaseUser, \
+from django.contrib.auth.models import Group
-    Permission as BasePermission, Group
+from django.contrib.auth.models import Permission as BasePermission
+from django.contrib.auth.models import User as BaseUser
 from django.db.models import Count, Q
-from allianceauth.services.hooks import ServicesHook
-from django.db.models.signals import pre_save, post_save, pre_delete, \
-    post_delete, m2m_changed
 from django.db.models.functions import Lower
+from django.db.models.signals import (
+    m2m_changed,
+    post_delete,
+    post_save,
+    pre_delete,
+    pre_save
+)
 from django.dispatch import receiver
-from django.forms import ModelForm
-from django.utils.html import format_html
 from django.urls import reverse
+from django.utils.html import format_html
 from django.utils.text import slugify
 
 from allianceauth.authentication.models import (
-    State,
-    get_guest_state,
     CharacterOwnership,
+    OwnershipRecord,
+    State,
     UserProfile,
-    OwnershipRecord)
+    get_guest_state
-from allianceauth.hooks import get_hooks
+)
-from allianceauth.eveonline.models import EveCharacter, EveCorporationInfo,\
+from allianceauth.eveonline.models import (
-    EveAllianceInfo, EveFactionInfo
+    EveAllianceInfo,
+    EveCharacter,
+    EveCorporationInfo,
+    EveFactionInfo
+)
 from allianceauth.eveonline.tasks import update_character
-from .app_settings import AUTHENTICATION_ADMIN_USERS_MAX_GROUPS, \
+from allianceauth.hooks import get_hooks
-    AUTHENTICATION_ADMIN_USERS_MAX_CHARS
+from allianceauth.services.hooks import ServicesHook
+
+from .app_settings import (
+    AUTHENTICATION_ADMIN_USERS_MAX_CHARS,
+    AUTHENTICATION_ADMIN_USERS_MAX_GROUPS
+)
+from .forms import UserChangeForm, UserProfileForm
 
 
 def make_service_hooks_update_groups_action(service):
@@ -63,19 +77,10 @@ def make_service_hooks_sync_nickname_action(service):
     return sync_nickname
 
 
-class QuerysetModelForm(ModelForm):
-    # allows specifying FK querysets through kwarg
-    def __init__(self, querysets=None, *args, **kwargs):
-        querysets = querysets or {}
-        super().__init__(*args, **kwargs)
-        for field, qs in querysets.items():
-            self.fields[field].queryset = qs
-
-
 class UserProfileInline(admin.StackedInline):
     model = UserProfile
     readonly_fields = ('state',)
-    form = QuerysetModelForm
+    form = UserProfileForm
     verbose_name = ''
     verbose_name_plural = 'Profile'
 
@@ -103,6 +108,7 @@ class UserProfileInline(admin.StackedInline):
         return False
 
 
+@admin.display(description="")
 def user_profile_pic(obj):
     """profile pic column data for user objects
 
@@ -115,13 +121,10 @@ def user_profile_pic(obj):
             '<img src="{}" class="img-circle">',
             user_obj.profile.main_character.portrait_url(size=32)
         )
-    else:
+    return None
-        return None
-
-
-user_profile_pic.short_description = ''
 
 
+@admin.display(description="user / main", ordering="username")
 def user_username(obj):
     """user column data for user objects
 
@@ -143,18 +146,17 @@ def user_username(obj):
             user_obj.username,
             user_obj.profile.main_character.character_name
         )
-    else:
+    return format_html(
-        return format_html(
+        '<strong><a href="{}">{}</a></strong>',
-            '<strong><a href="{}">{}</a></strong>',
+        link,
-            link,
+        user_obj.username,
-            user_obj.username,
+    )
-        )
-
-
-user_username.short_description = 'user / main'
-user_username.admin_order_field = 'username'
 
 
+@admin.display(
+    description="Corporation / Alliance (Main)",
+    ordering="profile__main_character__corporation_name"
+)
 def user_main_organization(obj):
     """main organization column data for user objects
 
@@ -163,21 +165,15 @@ def user_main_organization(obj):
     """
     user_obj = obj.user if hasattr(obj, 'user') else obj
     if not user_obj.profile.main_character:
-        result = ''
+        return ''
-    else:
+    result = user_obj.profile.main_character.corporation_name
-        result = user_obj.profile.main_character.corporation_name
+    if user_obj.profile.main_character.alliance_id:
-        if user_obj.profile.main_character.alliance_id:
+        result += f'<br>{user_obj.profile.main_character.alliance_name}'
-            result += f'<br>{user_obj.profile.main_character.alliance_name}'
+    elif user_obj.profile.main_character.faction_name:
-        elif user_obj.profile.main_character.faction_name:
+        result += f'<br>{user_obj.profile.main_character.faction_name}'
-            result += f'<br>{user_obj.profile.main_character.faction_name}'
     return format_html(result)
 
 
-user_main_organization.short_description = 'Corporation / Alliance (Main)'
-user_main_organization.admin_order_field = \
-    'profile__main_character__corporation_name'
-
-
 class MainCorporationsFilter(admin.SimpleListFilter):
     """Custom filter to filter on corporations from mains only
 
@@ -200,15 +196,13 @@ class MainCorporationsFilter(admin.SimpleListFilter):
     def queryset(self, request, qs):
         if self.value() is None:
             return qs.all()
-        else:
+        if qs.model == User:
-            if qs.model == User:
+            return qs.filter(
-                return qs.filter(
+                profile__main_character__corporation_id=self.value()
-                    profile__main_character__corporation_id=self.value()
+            )
-                )
+        return qs.filter(
-            else:
+            user__profile__main_character__corporation_id=self.value()
-                return qs.filter(
+        )
-                    user__profile__main_character__corporation_id=self.value()
-                )
 
 
 class MainAllianceFilter(admin.SimpleListFilter):
@@ -221,12 +215,14 @@ class MainAllianceFilter(admin.SimpleListFilter):
     parameter_name = 'main_alliance_id__exact'
 
     def lookups(self, request, model_admin):
-        qs = EveCharacter.objects\
+        qs = (
-            .exclude(alliance_id=None)\
+            EveCharacter.objects
-            .exclude(userprofile=None)\
+            .exclude(alliance_id=None)
-            .values('alliance_id', 'alliance_name')\
+            .exclude(userprofile=None)
-            .distinct()\
+            .values('alliance_id', 'alliance_name')
+            .distinct()
             .order_by(Lower('alliance_name'))
+        )
         return tuple(
             (x['alliance_id'], x['alliance_name']) for x in qs
         )
@@ -234,13 +230,11 @@ class MainAllianceFilter(admin.SimpleListFilter):
     def queryset(self, request, qs):
         if self.value() is None:
             return qs.all()
-        else:
+        if qs.model == User:
-            if qs.model == User:
+            return qs.filter(profile__main_character__alliance_id=self.value())
-                return qs.filter(profile__main_character__alliance_id=self.value())
+        return qs.filter(
-            else:
+            user__profile__main_character__alliance_id=self.value()
-                return qs.filter(
+        )
-                    user__profile__main_character__alliance_id=self.value()
-                )
 
 
 class MainFactionFilter(admin.SimpleListFilter):
@@ -253,12 +247,14 @@ class MainFactionFilter(admin.SimpleListFilter):
     parameter_name = 'main_faction_id__exact'
 
     def lookups(self, request, model_admin):
-        qs = EveCharacter.objects\
+        qs = (
-            .exclude(faction_id=None)\
+            EveCharacter.objects
-            .exclude(userprofile=None)\
+            .exclude(faction_id=None)
-            .values('faction_id', 'faction_name')\
+            .exclude(userprofile=None)
-            .distinct()\
+            .values('faction_id', 'faction_name')
+            .distinct()
             .order_by(Lower('faction_name'))
+        )
         return tuple(
             (x['faction_id'], x['faction_name']) for x in qs
         )
@@ -266,15 +262,14 @@ class MainFactionFilter(admin.SimpleListFilter):
     def queryset(self, request, qs):
         if self.value() is None:
             return qs.all()
-        else:
+        if qs.model == User:
-            if qs.model == User:
+            return qs.filter(profile__main_character__faction_id=self.value())
-                return qs.filter(profile__main_character__faction_id=self.value())
+        return qs.filter(
-            else:
+            user__profile__main_character__faction_id=self.value()
-                return qs.filter(
+        )
-                    user__profile__main_character__faction_id=self.value()
-                )
 
 
+@admin.display(description="Update main character model from ESI")
 def update_main_character_model(modeladmin, request, queryset):
     tasks_count = 0
     for obj in queryset:
@@ -283,33 +278,72 @@ def update_main_character_model(modeladmin, request, queryset):
             tasks_count += 1
 
     modeladmin.message_user(
-        request,
+        request, f'Update from ESI started for {tasks_count} characters'
-        f'Update from ESI started for {tasks_count} characters'
     )
 
 
-update_main_character_model.short_description = \
-    'Update main character model from ESI'
-
-
 class UserAdmin(BaseUserAdmin):
     """Extending Django's UserAdmin model
 
     Behavior of groups and characters columns can be configured via settings
     """
 
+    inlines = BaseUserAdmin.inlines + [UserProfileInline]
+    ordering = ('username', )
+    list_select_related = ('profile__state', 'profile__main_character')
+    show_full_result_count = True
+    list_display = (
+        user_profile_pic,
+        user_username,
+        '_state',
+        '_groups',
+        user_main_organization,
+        '_characters',
+        'is_active',
+        'date_joined',
+        '_role'
+    )
+    list_display_links = None
+    list_filter = (
+        'profile__state',
+        'groups',
+        MainCorporationsFilter,
+        MainAllianceFilter,
+        MainFactionFilter,
+        'is_active',
+        'date_joined',
+        'is_staff',
+        'is_superuser'
+    )
+    search_fields = ('username', 'character_ownerships__character__character_name')
+    readonly_fields = ('date_joined', 'last_login')
+    filter_horizontal = ('groups', 'user_permissions',)
+    form = UserChangeForm
+
     class Media:
         css = {
-            "all": ("authentication/css/admin.css",)
+            "all": ("allianceauth/authentication/css/admin.css",)
         }
 
     def get_queryset(self, request):
         qs = super().get_queryset(request)
         return qs.prefetch_related("character_ownerships__character", "groups")
 
-    def get_actions(self, request):
+    def get_form(self, request, obj=None, **kwargs):
-        actions = super(BaseUserAdmin, self).get_actions(request)
+        """Inject current request into change form object."""
 
+        MyForm = super().get_form(request, obj, **kwargs)
+        if obj:
+            class MyFormInjected(MyForm):
+                def __new__(cls, *args, **kwargs):
+                    kwargs['request'] = request
+                    return MyForm(*args, **kwargs)
+
+            return MyFormInjected
+        return MyForm
+
+    def get_actions(self, request):
+        actions = super().get_actions(request)
         actions[update_main_character_model.__name__] = (
             update_main_character_model,
             update_main_character_model.__name__,
@@ -353,39 +387,6 @@ class UserAdmin(BaseUserAdmin):
             )
         return result
 
-    inlines = BaseUserAdmin.inlines + [UserProfileInline]
-    ordering = ('username', )
-    list_select_related = ('profile__state', 'profile__main_character')
-    show_full_result_count = True
-    list_display = (
-        user_profile_pic,
-        user_username,
-        '_state',
-        '_groups',
-        user_main_organization,
-        '_characters',
-        'is_active',
-        'date_joined',
-        '_role'
-    )
-    list_display_links = None
-    list_filter = (
-        'profile__state',
-        'groups',
-        MainCorporationsFilter,
-        MainAllianceFilter,
-        MainFactionFilter,
-        'is_active',
-        'date_joined',
-        'is_staff',
-        'is_superuser'
-    )
-    search_fields = (
-        'username',
-        'character_ownerships__character__character_name'
-    )
-    readonly_fields = ('date_joined', 'last_login')
-
     def _characters(self, obj):
         character_ownerships = list(obj.character_ownerships.all())
         characters = [obj.character.character_name for obj in character_ownerships]
@@ -394,22 +395,16 @@ class UserAdmin(BaseUserAdmin):
             AUTHENTICATION_ADMIN_USERS_MAX_CHARS
         )
 
-    _characters.short_description = 'characters'
+    @admin.display(ordering="profile__state")
-
     def _state(self, obj):
         return obj.profile.state.name
 
-    _state.short_description = 'state'
-    _state.admin_order_field = 'profile__state'
-
     def _groups(self, obj):
         my_groups = sorted(group.name for group in list(obj.groups.all()))
         return self._list_2_html_w_tooltips(
             my_groups, AUTHENTICATION_ADMIN_USERS_MAX_GROUPS
         )
 
-    _groups.short_description = 'groups'
-
     def _role(self, obj):
         if obj.is_superuser:
             role = 'Superuser'
@@ -419,8 +414,6 @@ class UserAdmin(BaseUserAdmin):
             role = 'User'
         return role
 
-    _role.short_description = 'role'
-
     def has_change_permission(self, request, obj=None):
         return request.user.has_perm('auth.change_user')
 
@@ -442,9 +435,16 @@ class UserAdmin(BaseUserAdmin):
             if obj_state:
                 matching_groups_qs = Group.objects.filter(authgroup__states=obj_state)
                 groups_qs = groups_qs | matching_groups_qs
-            kwargs["queryset"] = groups_qs.order_by(Lower('name'))
+            kwargs["queryset"] = groups_qs.order_by(Lower("name"))
         return super().formfield_for_manytomany(db_field, request, **kwargs)
 
+    def get_readonly_fields(self, request, obj=None):
+        if obj and not request.user.is_superuser:
+            return self.readonly_fields + (
+                "is_staff", "is_superuser", "user_permissions"
+            )
+        return self.readonly_fields
+
 
 @admin.register(State)
 class StateAdmin(admin.ModelAdmin):
@@ -455,10 +455,9 @@ class StateAdmin(admin.ModelAdmin):
         qs = super().get_queryset(request)
         return qs.annotate(user_count=Count("userprofile__id"))
 
+    @admin.display(description="Users", ordering="user_count")
     def _user_count(self, obj):
         return obj.user_count
-    _user_count.short_description = 'Users'
-    _user_count.admin_order_field = 'user_count'
 
     fieldsets = (
         (None, {
@@ -514,13 +513,13 @@ class StateAdmin(admin.ModelAdmin):
             )
         return super().get_fieldsets(request, obj=obj)
 
+    def get_readonly_fields(self, request, obj=None):
+        if not request.user.is_superuser:
+            return self.readonly_fields + ("permissions",)
+        return self.readonly_fields
+
 
 class BaseOwnershipAdmin(admin.ModelAdmin):
-    class Media:
-        css = {
-            "all": ("authentication/css/admin.css",)
-        }
-
     list_select_related = (
         'user__profile__state', 'user__profile__main_character', 'character')
     list_display = (
@@ -541,6 +540,11 @@ class BaseOwnershipAdmin(admin.ModelAdmin):
         MainAllianceFilter,
     )
 
+    class Media:
+        css = {
+            "all": ("allianceauth/authentication/css/admin.css",)
+        }
+
     def get_readonly_fields(self, request, obj=None):
         if obj and obj.pk:
             return 'owner_hash', 'character'

allianceauth/authentication/forms.py

@@ -1,8 +1,66 @@
 from django import forms
+from django.contrib.auth.forms import UserChangeForm as BaseUserChangeForm
+from django.contrib.auth.models import Group
+from django.core.exceptions import ValidationError
+from django.forms import ModelForm
 from django.utils.translation import gettext_lazy as _
+
 from allianceauth.authentication.models import User
+
+
 class RegistrationForm(forms.Form):
     email = forms.EmailField(label=_('Email'), max_length=254, required=True)
 
     class _meta:
         model = User
+
+
+class UserProfileForm(ModelForm):
+    """Allows specifying FK querysets through kwarg"""
+
+    def __init__(self, querysets=None, *args, **kwargs):
+        querysets = querysets or {}
+        super().__init__(*args, **kwargs)
+        for field, qs in querysets.items():
+            self.fields[field].queryset = qs
+
+
+class UserChangeForm(BaseUserChangeForm):
+    """Add custom cleaning to UserChangeForm"""
+
+    def __init__(self, *args, **kwargs):
+        self.request = kwargs.pop("request")  # Inject current request into form object
+        super().__init__(*args, **kwargs)
+
+    def clean(self):
+        cleaned_data = super().clean()
+        if not self.request.user.is_superuser:
+            if self.instance:
+                current_restricted = set(
+                    self.instance.groups.filter(
+                        authgroup__restricted=True
+                    ).values_list("pk", flat=True)
+                )
+            else:
+                current_restricted = set()
+            new_restricted = set(
+                cleaned_data["groups"].filter(
+                    authgroup__restricted=True
+                ).values_list("pk", flat=True)
+            )
+            if current_restricted != new_restricted:
+                restricted_removed = current_restricted - new_restricted
+                restricted_added = new_restricted - current_restricted
+                restricted_changed = restricted_removed | restricted_added
+                restricted_names_qs = Group.objects.filter(
+                    pk__in=restricted_changed
+                ).values_list("name", flat=True)
+                restricted_names = ",".join(list(restricted_names_qs))
+                raise ValidationError(
+                    {
+                        "groups": _(
+                            "You are not allowed to add or remove these "
+                            "restricted groups: %s" % restricted_names
+                        )
+                    }
+                )

allianceauth/authentication/task_statistics/counters.py

@@ -0,0 +1,40 @@
+from collections import namedtuple
+import datetime as dt
+
+from .event_series import EventSeries
+
+
+"""Global series for counting task events."""
+succeeded_tasks = EventSeries("SUCCEEDED_TASKS")
+retried_tasks = EventSeries("RETRIED_TASKS")
+failed_tasks = EventSeries("FAILED_TASKS")
+
+
+_TaskCounts = namedtuple(
+    "_TaskCounts", ["succeeded", "retried", "failed", "total", "earliest_task", "hours"]
+)
+
+
+def dashboard_results(hours: int) -> _TaskCounts:
+    """Counts of all task events within the given timeframe."""
+
+    def earliest_if_exists(events: EventSeries, earliest: dt.datetime) -> list:
+        my_earliest = events.first_event(earliest=earliest)
+        return [my_earliest] if my_earliest else []
+
+    earliest = dt.datetime.utcnow() - dt.timedelta(hours=hours)
+    earliest_events = list()
+    succeeded_count = succeeded_tasks.count(earliest=earliest)
+    earliest_events += earliest_if_exists(succeeded_tasks, earliest)
+    retried_count = retried_tasks.count(earliest=earliest)
+    earliest_events += earliest_if_exists(retried_tasks, earliest)
+    failed_count = failed_tasks.count(earliest=earliest)
+    earliest_events += earliest_if_exists(failed_tasks, earliest)
+    return _TaskCounts(
+        succeeded=succeeded_count,
+        retried=retried_count,
+        failed=failed_count,
+        total=succeeded_count + retried_count + failed_count,
+        earliest_task=min(earliest_events) if earliest_events else None,
+        hours=hours,
+    )

allianceauth/authentication/task_statistics/event_series.py

@@ -1,74 +1,69 @@
 import datetime as dt
-from collections import namedtuple
+import logging
-from typing import Optional, List
+from typing import List, Optional
 
-from redis import Redis
 from pytz import utc
+from redis import Redis, RedisError
 
-from django_redis import get_redis_connection
+from allianceauth.utils.cache import get_redis_client
 
-_TaskCounts = namedtuple(
+logger = logging.getLogger(__name__)
-    "_TaskCounts", ["succeeded", "retried", "failed", "total", "earliest_task", "hours"]
-)
 
 
-def dashboard_results(hours: int) -> _TaskCounts:
+class _RedisStub:
-    """Counts of all task events within the given timeframe."""
+    """Stub of a Redis client.
-    def earliest_if_exists(events: EventSeries, earliest: dt.datetime) -> list:
-        my_earliest = events.first_event(earliest=earliest)
-        return [my_earliest] if my_earliest else []
 
-    earliest = dt.datetime.utcnow() - dt.timedelta(hours=hours)
+    It's purpose is to prevent EventSeries objects from trying to access Redis
-    earliest_events = list()
+    when it is not available. e.g. when the Sphinx docs are rendered by readthedocs.org.
-    succeeded = SucceededTaskSeries()
+    """
-    succeeded_count = succeeded.count(earliest=earliest)
+
-    earliest_events += earliest_if_exists(succeeded, earliest)
+    def delete(self, *args, **kwargs):
-    retried = RetriedTaskSeries()
+        pass
-    retried_count = retried.count(earliest=earliest)
+
-    earliest_events += earliest_if_exists(retried, earliest)
+    def incr(self, *args, **kwargs):
-    failed = FailedTaskSeries()
+        return 0
-    failed_count = failed.count(earliest=earliest)
+
-    earliest_events += earliest_if_exists(failed, earliest)
+    def zadd(self, *args, **kwargs):
-    return _TaskCounts(
+        pass
-        succeeded=succeeded_count,
+
-        retried=retried_count,
+    def zcount(self, *args, **kwargs):
-        failed=failed_count,
+        pass
-        total=succeeded_count + retried_count + failed_count,
+
-        earliest_task=min(earliest_events) if earliest_events else None,
+    def zrangebyscore(self, *args, **kwargs):
-        hours=hours,
+        pass
-    )
 
 
 class EventSeries:
-    """Base class for recording and analysing a series of events.
+    """API for recording and analyzing a series of events."""
 
-    This class must be inherited from and the child class must define KEY_ID.
+    _ROOT_KEY = "ALLIANCEAUTH_EVENT_SERIES"
-    """
 
-    _ROOT_KEY = "ALLIANCEAUTH_TASK_SERIES"
+    def __init__(self, key_id: str, redis: Redis = None) -> None:
-
+        self._redis = get_redis_client() if not redis else redis
-    def __init__(
+        try:
-        self,
+            if not self._redis.ping():
-        redis: Redis = None,
+                raise RuntimeError()
-    ) -> None:
+        except (AttributeError, RedisError, RuntimeError):
-        if type(self) == EventSeries:
+            logger.exception(
-            raise TypeError("Can not instantiate base class.")
+                "Failed to establish a connection with Redis. "
-        if not hasattr(self, "KEY_ID"):
+                "This EventSeries object is disabled.",
-            raise ValueError("KEY_ID not defined")
-        self._redis = get_redis_connection("default") if not redis else redis
-        if not isinstance(self._redis, Redis):
-            raise TypeError(
-                "This class requires a Redis client, but none was provided "
-                "and the default Django cache backend is not Redis either."
             )
+            self._redis = _RedisStub()
+        self._key_id = str(key_id)
+        self.clear()
+
+    @property
+    def is_disabled(self):
+        """True when this object is disabled, e.g. Redis was not available at startup."""
+        return isinstance(self._redis, _RedisStub)
 
     @property
     def _key_counter(self):
-        return f"{self._ROOT_KEY}_{self.KEY_ID}_COUNTER"
+        return f"{self._ROOT_KEY}_{self._key_id}_COUNTER"
 
     @property
     def _key_sorted_set(self):
-        return f"{self._ROOT_KEY}_{self.KEY_ID}_SORTED_SET"
+        return f"{self._ROOT_KEY}_{self._key_id}_SORTED_SET"
 
     def add(self, event_time: dt.datetime = None) -> None:
         """Add event.
@@ -133,21 +128,3 @@ class EventSeries:
     @staticmethod
     def _cast_scores_to_dt(score) -> dt.datetime:
         return dt.datetime.fromtimestamp(float(score), tz=utc)
-
-
-class SucceededTaskSeries(EventSeries):
-    """A task has succeeded."""
-
-    KEY_ID = "SUCCEEDED"
-
-
-class RetriedTaskSeries(EventSeries):
-    """A task has been retried."""
-
-    KEY_ID = "RETRIED"
-
-
-class FailedTaskSeries(EventSeries):
-    """A task has failed."""
-
-    KEY_ID = "FAILED"

allianceauth/authentication/task_statistics/signals.py

@@ -1,15 +1,21 @@
-from celery.signals import task_failure, task_retry, task_success, worker_ready
+from celery.signals import (
+    task_failure,
+    task_internal_error,
+    task_retry,
+    task_success,
+    worker_ready
+)
 
 from django.conf import settings
 
-from .event_series import FailedTaskSeries, RetriedTaskSeries, SucceededTaskSeries
+from .counters import failed_tasks, retried_tasks, succeeded_tasks
 
 
 def reset_counters():
     """Reset all counters for the celery status."""
-    SucceededTaskSeries().clear()
+    succeeded_tasks.clear()
-    FailedTaskSeries().clear()
+    failed_tasks.clear()
-    RetriedTaskSeries().clear()
+    retried_tasks.clear()
 
 
 def is_enabled() -> bool:
@@ -27,16 +33,22 @@ def reset_counters_when_celery_restarted(*args, **kwargs):
 @task_success.connect
 def record_task_succeeded(*args, **kwargs):
     if is_enabled():
-        SucceededTaskSeries().add()
+        succeeded_tasks.add()
 
 
 @task_retry.connect
 def record_task_retried(*args, **kwargs):
     if is_enabled():
-        RetriedTaskSeries().add()
+        retried_tasks.add()
 
 
 @task_failure.connect
 def record_task_failed(*args, **kwargs):
     if is_enabled():
-        FailedTaskSeries().add()
+        failed_tasks.add()
+
+
+@task_internal_error.connect
+def record_task_internal_error(*args, **kwargs):
+    if is_enabled():
+        failed_tasks.add()

allianceauth/authentication/task_statistics/tests/test_counters.py

@@ -0,0 +1,51 @@
+import datetime as dt
+
+from django.test import TestCase
+from django.utils.timezone import now
+
+from allianceauth.authentication.task_statistics.counters import (
+    dashboard_results,
+    succeeded_tasks,
+    retried_tasks,
+    failed_tasks,
+)
+
+
+class TestDashboardResults(TestCase):
+    def test_should_return_counts_for_given_timeframe_only(self):
+        # given
+        earliest_task = now() - dt.timedelta(minutes=15)
+        succeeded_tasks.clear()
+        succeeded_tasks.add(now() - dt.timedelta(hours=1, seconds=1))
+        succeeded_tasks.add(earliest_task)
+        succeeded_tasks.add()
+        succeeded_tasks.add()
+        retried_tasks.clear()
+        retried_tasks.add(now() - dt.timedelta(hours=1, seconds=1))
+        retried_tasks.add(now() - dt.timedelta(seconds=30))
+        retried_tasks.add()
+        failed_tasks.clear()
+        failed_tasks.add(now() - dt.timedelta(hours=1, seconds=1))
+        failed_tasks.add()
+        # when
+        results = dashboard_results(hours=1)
+        # then
+        self.assertEqual(results.succeeded, 3)
+        self.assertEqual(results.retried, 2)
+        self.assertEqual(results.failed, 1)
+        self.assertEqual(results.total, 6)
+        self.assertEqual(results.earliest_task, earliest_task)
+
+    def test_should_work_with_no_data(self):
+        # given
+        succeeded_tasks.clear()
+        retried_tasks.clear()
+        failed_tasks.clear()
+        # when
+        results = dashboard_results(hours=1)
+        # then
+        self.assertEqual(results.succeeded, 0)
+        self.assertEqual(results.retried, 0)
+        self.assertEqual(results.failed, 0)
+        self.assertEqual(results.total, 0)
+        self.assertIsNone(results.earliest_task)

allianceauth/authentication/task_statistics/tests/test_event_series.py

@@ -1,49 +1,51 @@
 import datetime as dt
+from unittest.mock import patch
 
 from pytz import utc
+from redis import RedisError
+
 from django.test import TestCase
 from django.utils.timezone import now
 
 from allianceauth.authentication.task_statistics.event_series import (
     EventSeries,
-    FailedTaskSeries,
+    _RedisStub,
-    RetriedTaskSeries,
-    SucceededTaskSeries,
-    dashboard_results,
 )
 
+MODULE_PATH = "allianceauth.authentication.task_statistics.event_series"
+
 
 class TestEventSeries(TestCase):
-    """Testing EventSeries class."""
+    def test_should_abort_without_redis_client(self):
-
-    class IncompleteEvents(EventSeries):
-        """Child class without KEY ID"""
-
-    class MyEventSeries(EventSeries):
-        KEY_ID = "TEST"
-
-    def test_should_create_object(self):
         # when
-        events = self.MyEventSeries()
+        with patch(MODULE_PATH + ".get_redis_client") as mock:
+            mock.return_value = None
+            events = EventSeries("dummy")
         # then
-        self.assertIsInstance(events, self.MyEventSeries)
+        self.assertTrue(events._redis, _RedisStub)
+        self.assertTrue(events.is_disabled)
 
-    def test_should_abort_when_redis_client_invalid(self):
+    def test_should_disable_itself_if_redis_not_available_1(self):
-        with self.assertRaises(TypeError):
+        # when
-            self.MyEventSeries(redis="invalid")
+        with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client:
+            mock_get_master_client.return_value.ping.side_effect = RedisError
+            events = EventSeries("dummy")
+        # then
+        self.assertIsInstance(events._redis, _RedisStub)
+        self.assertTrue(events.is_disabled)
 
-    def test_should_not_allow_instantiation_of_base_class(self):
+    def test_should_disable_itself_if_redis_not_available_2(self):
-        with self.assertRaises(TypeError):
+        # when
-            EventSeries()
+        with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client:
-
+            mock_get_master_client.return_value.ping.return_value = False
-    def test_should_not_allow_creating_child_class_without_key_id(self):
+            events = EventSeries("dummy")
-        with self.assertRaises(ValueError):
+        # then
-            self.IncompleteEvents()
+        self.assertIsInstance(events._redis, _RedisStub)
+        self.assertTrue(events.is_disabled)
 
     def test_should_add_event(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         # when
         events.add()
         # then
@@ -53,8 +55,7 @@ class TestEventSeries(TestCase):
 
     def test_should_add_event_with_specified_time(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         my_time = dt.datetime(2021, 11, 1, 12, 15, tzinfo=utc)
         # when
         events.add(my_time)
@@ -65,8 +66,7 @@ class TestEventSeries(TestCase):
 
     def test_should_count_events(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add()
         events.add()
         # when
@@ -76,8 +76,7 @@ class TestEventSeries(TestCase):
 
     def test_should_count_zero(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         # when
         result = events.count()
         # then
@@ -85,8 +84,7 @@ class TestEventSeries(TestCase):
 
     def test_should_count_events_within_timeframe_1(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add(dt.datetime(2021, 12, 1, 12, 0, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 10, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 15, tzinfo=utc))
@@ -101,8 +99,7 @@ class TestEventSeries(TestCase):
 
     def test_should_count_events_within_timeframe_2(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add(dt.datetime(2021, 12, 1, 12, 0, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 10, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 15, tzinfo=utc))
@@ -114,8 +111,7 @@ class TestEventSeries(TestCase):
 
     def test_should_count_events_within_timeframe_3(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add(dt.datetime(2021, 12, 1, 12, 0, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 10, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 15, tzinfo=utc))
@@ -127,8 +123,7 @@ class TestEventSeries(TestCase):
 
     def test_should_clear_events(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add()
         events.add()
         # when
@@ -138,8 +133,7 @@ class TestEventSeries(TestCase):
 
     def test_should_return_date_of_first_event(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add(dt.datetime(2021, 12, 1, 12, 0, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 10, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 15, tzinfo=utc))
@@ -151,8 +145,7 @@ class TestEventSeries(TestCase):
 
     def test_should_return_date_of_first_event_with_range(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add(dt.datetime(2021, 12, 1, 12, 0, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 10, tzinfo=utc))
         events.add(dt.datetime(2021, 12, 1, 12, 15, tzinfo=utc))
@@ -166,57 +159,10 @@ class TestEventSeries(TestCase):
 
     def test_should_return_all_events(self):
         # given
-        events = self.MyEventSeries()
+        events = EventSeries("dummy")
-        events.clear()
         events.add()
         events.add()
         # when
         results = events.all()
         # then
         self.assertEqual(len(results), 2)
-
-
-class TestDashboardResults(TestCase):
-    def test_should_return_counts_for_given_timeframe_only(self):
-        # given
-        earliest_task = now() - dt.timedelta(minutes=15)
-        succeeded = SucceededTaskSeries()
-        succeeded.clear()
-        succeeded.add(now() - dt.timedelta(hours=1, seconds=1))
-        succeeded.add(earliest_task)
-        succeeded.add()
-        succeeded.add()
-        retried = RetriedTaskSeries()
-        retried.clear()
-        retried.add(now() - dt.timedelta(hours=1, seconds=1))
-        retried.add(now() - dt.timedelta(seconds=30))
-        retried.add()
-        failed = FailedTaskSeries()
-        failed.clear()
-        failed.add(now() - dt.timedelta(hours=1, seconds=1))
-        failed.add()
-        # when
-        results = dashboard_results(hours=1)
-        # then
-        self.assertEqual(results.succeeded, 3)
-        self.assertEqual(results.retried, 2)
-        self.assertEqual(results.failed, 1)
-        self.assertEqual(results.total, 6)
-        self.assertEqual(results.earliest_task, earliest_task)
-
-    def test_should_work_with_no_data(self):
-        # given
-        succeeded = SucceededTaskSeries()
-        succeeded.clear()
-        retried = RetriedTaskSeries()
-        retried.clear()
-        failed = FailedTaskSeries()
-        failed.clear()
-        # when
-        results = dashboard_results(hours=1)
-        # then
-        self.assertEqual(results.succeeded, 0)
-        self.assertEqual(results.retried, 0)
-        self.assertEqual(results.failed, 0)
-        self.assertEqual(results.total, 0)
-        self.assertIsNone(results.earliest_task)

allianceauth/authentication/task_statistics/tests/test_signals.py

@@ -4,10 +4,10 @@ from celery.exceptions import Retry
 
 from django.test import TestCase, override_settings
 
-from allianceauth.authentication.task_statistics.event_series import (
+from allianceauth.authentication.task_statistics.counters import (
-    FailedTaskSeries,
+    failed_tasks,
-    RetriedTaskSeries,
+    retried_tasks,
-    SucceededTaskSeries,
+    succeeded_tasks,
 )
 from allianceauth.authentication.task_statistics.signals import (
     reset_counters,
@@ -17,15 +17,16 @@ from allianceauth.eveonline.tasks import update_character
 
 
 @override_settings(
-    CELERY_ALWAYS_EAGER=True, ALLIANCEAUTH_DASHBOARD_TASK_STATISTICS_DISABLED=False
+    CELERY_ALWAYS_EAGER=True,ALLIANCEAUTH_DASHBOARD_TASK_STATISTICS_DISABLED=False
 )
 class TestTaskSignals(TestCase):
     fixtures = ["disable_analytics"]
 
     def test_should_record_successful_task(self):
         # given
-        events = SucceededTaskSeries()
+        succeeded_tasks.clear()
-        events.clear()
+        retried_tasks.clear()
+        failed_tasks.clear()
         # when
         with patch(
             "allianceauth.eveonline.tasks.EveCharacter.objects.update_character"
@@ -33,12 +34,15 @@ class TestTaskSignals(TestCase):
             mock_update.return_value = None
             update_character.delay(1)
         # then
-        self.assertEqual(events.count(), 1)
+        self.assertEqual(succeeded_tasks.count(), 1)
+        self.assertEqual(retried_tasks.count(), 0)
+        self.assertEqual(failed_tasks.count(), 0)
 
     def test_should_record_retried_task(self):
         # given
-        events = RetriedTaskSeries()
+        succeeded_tasks.clear()
-        events.clear()
+        retried_tasks.clear()
+        failed_tasks.clear()
         # when
         with patch(
             "allianceauth.eveonline.tasks.EveCharacter.objects.update_character"
@@ -46,12 +50,15 @@ class TestTaskSignals(TestCase):
             mock_update.side_effect = Retry
             update_character.delay(1)
         # then
-        self.assertEqual(events.count(), 1)
+        self.assertEqual(succeeded_tasks.count(), 0)
+        self.assertEqual(failed_tasks.count(), 0)
+        self.assertEqual(retried_tasks.count(), 1)
 
     def test_should_record_failed_task(self):
         # given
-        events = FailedTaskSeries()
+        succeeded_tasks.clear()
-        events.clear()
+        retried_tasks.clear()
+        failed_tasks.clear()
         # when
         with patch(
             "allianceauth.eveonline.tasks.EveCharacter.objects.update_character"
@@ -59,28 +66,21 @@ class TestTaskSignals(TestCase):
             mock_update.side_effect = RuntimeError
             update_character.delay(1)
         # then
-        self.assertEqual(events.count(), 1)
+        self.assertEqual(succeeded_tasks.count(), 0)
+        self.assertEqual(retried_tasks.count(), 0)
+        self.assertEqual(failed_tasks.count(), 1)
 
-
-@override_settings(ALLIANCEAUTH_DASHBOARD_TASK_STATISTICS_DISABLED=False)
-class TestResetCounters(TestCase):
     def test_should_reset_counters(self):
         # given
-        succeeded = SucceededTaskSeries()
+        succeeded_tasks.add()
-        succeeded.clear()
+        retried_tasks.add()
-        succeeded.add()
+        failed_tasks.add()
-        retried = RetriedTaskSeries()
-        retried.clear()
-        retried.add()
-        failed = FailedTaskSeries()
-        failed.clear()
-        failed.add()
         # when
         reset_counters()
         # then
-        self.assertEqual(succeeded.count(), 0)
+        self.assertEqual(succeeded_tasks.count(), 0)
-        self.assertEqual(retried.count(), 0)
+        self.assertEqual(retried_tasks.count(), 0)
-        self.assertEqual(failed.count(), 0)
+        self.assertEqual(failed_tasks.count(), 0)
 
 
 class TestIsEnabled(TestCase):

allianceauth/authentication/templates/authentication/dashboard.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Dashboard" %}{% endblock %}
@@ -28,7 +27,7 @@
                                         <table class="table">
                                             <tr>
                                                 <td class="text-center">
-                                                    <img class="ra-avatar"src="{{ main.portrait_url_128 }}">
+                                                    <img class="ra-avatar" src="{{ main.portrait_url_128 }}" alt="{{ main.character_name }}">
                                                 </td>
                                             </tr>
                                             <tr>
@@ -40,7 +39,7 @@
                                         <table class="table">
                                             <tr>
                                                 <td class="text-center">
-                                                    <img class="ra-avatar"src="{{ main.corporation_logo_url_128 }}">
+                                                    <img class="ra-avatar" src="{{ main.corporation_logo_url_128 }}" alt="{{ main.corporation_name }}">
                                                 </td>
                                             </tr>
                                             <tr>
@@ -53,7 +52,7 @@
                                             <table class="table">
                                                 <tr>
                                                     <td class="text-center">
-                                                        <img class="ra-avatar"src="{{ main.alliance_logo_url_128 }}">
+                                                        <img class="ra-avatar" src="{{ main.alliance_logo_url_128 }}" alt="{{ main.alliance_name }}">
                                                     </td>
                                                 </tr>
                                                 <tr>
@@ -64,7 +63,7 @@
                                             <table class="table">
                                                 <tr>
                                                     <td class="text-center">
-                                                        <img class="ra-avatar"src="{{ main.faction_logo_url_128 }}">
+                                                        <img class="ra-avatar" src="{{ main.faction_logo_url_128 }}" alt="{{ main.faction_name }}">
                                                     </td>
                                                 </tr>
                                                 <tr>
@@ -76,13 +75,13 @@
                                 </div>
                                 <div class="table visible-xs-block">
                                     <p>
-                                        <img class="ra-avatar" src="{{ main.portrait_url_64 }}">
+                                        <img class="ra-avatar" src="{{ main.portrait_url_64 }}" alt="{{ main.corporation_name }}">
-                                        <img class="ra-avatar" src="{{ main.corporation_logo_url_64 }}">
+                                        <img class="ra-avatar" src="{{ main.corporation_logo_url_64 }}" alt="{{ main.corporation_name }}">
                                         {% if main.alliance_id %}
-                                            <img class="ra-avatar" src="{{ main.alliance_logo_url_64 }}">
+                                            <img class="ra-avatar" src="{{ main.alliance_logo_url_64 }}" alt="{{ main.alliance_name }}">
                                         {% endif %}
                                         {% if main.faction_id %}
-                                            <img class="ra-avatar" src="{{ main.faction_logo_url_64 }}">
+                                            <img class="ra-avatar" src="{{ main.faction_logo_url_64 }}" alt="{{ main.faction_name }}">
                                         {% endif %}
                                     </p>
                                     <p>
@@ -122,7 +121,7 @@
                         <h3 class="panel-title">{% translate "Group Memberships" %}</h3>
                     </div>
                     <div class="panel-body">
-                        <div style="height: 240px;overflow:-moz-scrollbars-vertical;overflow-y:auto;">
+                        <div style="height: 240px;overflow-y:auto;">
                             <table class="table table-aa">
                                 {% for group in groups %}
                                     <tr>
@@ -155,7 +154,8 @@
                     <tbody>
                         {% for char in characters %}
                             <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 }}" alt="{{ char.character_name }}">
                                 </td>
                                 <td class="text-center">{{ char.character_name }}</td>
                                 <td class="text-center">{{ char.corporation_name }}</td>
@@ -169,7 +169,7 @@
                         {% for char in characters %}
                             <tr>
                                 <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 }}" alt="{{ char.character_name }}">
                                 </td>
                                 <td class="text-center" style="vertical-align: middle; width: 100%">
                                     <strong>{{ char.character_name }}</strong><br>

allianceauth/authentication/templates/public/base.html

@@ -7,7 +7,7 @@
         <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:image" content="{{ request.scheme }}://{{ request.get_host }}{% static 'allianceauth/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' %}
@@ -21,7 +21,7 @@
 
         <style>
             body {
-                background: url('{% static 'authentication/img/background.jpg' %}') no-repeat center center fixed;
+                background: url('{% static 'allianceauth/authentication/img/background.jpg' %}') no-repeat center center fixed;
                 -webkit-background-size: cover;
                 -moz-background-size: cover;
                 -o-background-size: cover;
@@ -47,7 +47,7 @@
         </style>
     </head>
     <body>
-        <div class="container" style="margin-top:150px">
+        <div class="container" style="margin-top:150px;">
             {% block content %}
             {% endblock %}
         </div>

allianceauth/authentication/templates/public/login.html

@@ -7,6 +7,6 @@
 
 {% block middle_box_content %}
     <a href="{% url 'auth_sso_login' %}{% if request.GET.next %}?next={{request.GET.next}}{%endif%}">
-        <img class="img-responsive center-block" src="{% static 'img/sso/EVE_SSO_Login_Buttons_Large_Black.png' %}" border=0>
+        <img class="img-responsive center-block" src="{% static 'allianceauth/authentication/img/sso/EVE_SSO_Login_Buttons_Large_Black.png' %}" alt="{% translate 'Login with Eve SSO' %}">
     </a>
 {% endblock %}

allianceauth/authentication/templates/public/middle_box.html

@@ -1,5 +1,4 @@
 {% extends 'public/base.html' %}
-{% load static %}
 {% block content %}
     <div class="col-md-4 col-md-offset-4">
         {% if messages %}

allianceauth/authentication/templates/public/register.html

@@ -1,6 +1,5 @@
 {% extends 'public/base.html' %}
 
-{% load static %}
 {% load bootstrap %}
 {% load i18n %}
 

allianceauth/authentication/templates/registration/password_reset_email.html

@@ -1,15 +0,0 @@
-{% load i18n %}{% autoescape off %}
-    {% blocktrans trimmed %}You're receiving this email because you requested a password reset for your
-        user account.{% endblocktrans %}
-
-    {% translate "Please go to the following page and choose a new password:" %}
-    {% block reset_link %}
-        {{domain}}{% url 'password_reset_confirm' uidb64=uid token=token %}
-    {% endblock %}
-    {% translate "Your username, in case you've forgotten:" %} {{ user.get_username }}
-
-    {% translate "Thanks for using our site!" %}
-
-    {% blocktrans %}Your IT Team{% endblocktrans %}
-
-{% endautoescape %}

allianceauth/authentication/templates/registration/registration_form.html

@@ -1,14 +0,0 @@
-{% extends 'public/middle_box.html' %}
-{% load bootstrap %}
-{% load i18n %}
-{% load static %}
-{% block page_title %}{% translate "Register" %}{% endblock %}
-{% block middle_box_content %}
-    <form class="form-signin" role="form" action="" method="POST">
-        {% csrf_token %}
-        {{ form|bootstrap }}
-        <br>
-        <button class="btn btn-lg btn-primary btn-block" type="submit">{% translate "Submit" %}</button>
-        <br>
-    </form>
-{% endblock %}

allianceauth/authentication/tests/test_admin.py

@@ -2,6 +2,8 @@ from bs4 import BeautifulSoup
 from urllib.parse import quote
 from unittest.mock import patch, MagicMock
 
+from django_webtest import WebTest
+
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import Group
 from django.test import TestCase, RequestFactory, Client
@@ -276,10 +278,10 @@ class TestOwnershipRecordAdmin(TestCaseWithTestData):
 class TestStateAdmin(TestCaseWithTestData):
     fixtures = ["disable_analytics"]
 
-    def setUp(self):
+    @classmethod
-        self.modeladmin = StateAdmin(
+    def setUpClass(cls) -> None:
-            model=User, admin_site=AdminSite()
+        super().setUpClass()
-        )
+        cls.modeladmin = StateAdmin(model=User, admin_site=AdminSite())
 
     def test_change_view_loads_normally(self):
         User.objects.create_superuser(
@@ -543,7 +545,74 @@ class TestUserAdmin(TestCaseWithTestData):
         self.assertEqual(response.status_code, expected)
 
 
+class TestStateAdminChangeFormSuperuserExclusiveEdits(WebTest):
+    @classmethod
+    def setUpClass(cls) -> None:
+        super().setUpClass()
+        cls.super_admin = User.objects.create_superuser("super_admin")
+        cls.staff_admin = User.objects.create_user("staff_admin")
+        cls.staff_admin.is_staff = True
+        cls.staff_admin.save()
+        cls.staff_admin = AuthUtils.add_permissions_to_user_by_name(
+            [
+                "authentication.add_state",
+                "authentication.change_state",
+                "authentication.view_state",
+            ],
+            cls.staff_admin
+        )
+        cls.superuser_exclusive_fields = ["permissions",]
+
+    def test_should_show_all_fields_to_superuser_for_add(self):
+        # given
+        self.app.set_user(self.super_admin)
+        page = self.app.get("/admin/authentication/state/add/")
+        # when
+        form = page.forms["state_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertIn(field, form.fields)
+
+    def test_should_not_show_all_fields_to_staff_admins_for_add(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        page = self.app.get("/admin/authentication/state/add/")
+        # when
+        form = page.forms["state_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertNotIn(field, form.fields)
+
+    def test_should_show_all_fields_to_superuser_for_change(self):
+        # given
+        self.app.set_user(self.super_admin)
+        state = AuthUtils.get_member_state()
+        page = self.app.get(f"/admin/authentication/state/{state.pk}/change/")
+        # when
+        form = page.forms["state_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertIn(field, form.fields)
+
+    def test_should_not_show_all_fields_to_staff_admin_for_change(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        state = AuthUtils.get_member_state()
+        page = self.app.get(f"/admin/authentication/state/{state.pk}/change/")
+        # when
+        form = page.forms["state_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertNotIn(field, form.fields)
+
+
 class TestUserAdminChangeForm(TestCase):
+    fixtures = ["disable_analytics"]
+
     @classmethod
     def setUpClass(cls) -> None:
         super().setUpClass()
@@ -552,7 +621,7 @@ class TestUserAdminChangeForm(TestCase):
     def test_should_show_groups_available_to_user_with_blue_state_only(self):
         # given
         superuser = User.objects.create_superuser("Super")
-        user = AuthUtils.create_user("Bruce Wayne")
+        user = AuthUtils.create_user("bruce_wayne")
         character = AuthUtils.add_main_character_2(
             user,
             name="Bruce Wayne",
@@ -579,6 +648,126 @@ class TestUserAdminChangeForm(TestCase):
         self.assertSetEqual(group_ids, {group_1.pk, group_2.pk})
 
 
+class TestUserAdminChangeFormSuperuserExclusiveEdits(WebTest):
+    fixtures = ["disable_analytics"]
+
+    @classmethod
+    def setUpClass(cls) -> None:
+        super().setUpClass()
+        cls.super_admin = User.objects.create_superuser("super_admin")
+        cls.staff_admin = User.objects.create_user("staff_admin")
+        cls.staff_admin.is_staff = True
+        cls.staff_admin.save()
+        cls.staff_admin = AuthUtils.add_permissions_to_user_by_name(
+            [
+                "auth.change_user",
+                "auth.view_user",
+                "authentication.change_user",
+                "authentication.change_userprofile",
+                "authentication.view_user"
+            ],
+            cls.staff_admin
+        )
+        cls.superuser_exclusive_fields = [
+            "is_staff", "is_superuser", "user_permissions"
+        ]
+
+    def setUp(self) -> None:
+        self.user = AuthUtils.create_user("bruce_wayne")
+
+    def test_should_show_all_fields_to_superuser_for_change(self):
+        # given
+        self.app.set_user(self.super_admin)
+
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        # when
+        form = page.forms["user_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertIn(field, form.fields)
+
+    def test_should_not_show_all_fields_to_staff_admin_for_change(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        # when
+        form = page.forms["user_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertNotIn(field, form.fields)
+
+    def test_should_allow_super_admin_to_add_restricted_group_to_user(self):
+        # given
+        self.app.set_user(self.super_admin)
+        group_restricted = Group.objects.create(name="restricted group")
+        group_restricted.authgroup.restricted = True
+        group_restricted.authgroup.save()
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        form = page.forms["user_form"]
+        # when
+        form["groups"].select_multiple(texts=["restricted group"])
+        response = form.submit("save")
+        # then
+        self.assertEqual(response.status_code, 302)
+        self.user.refresh_from_db()
+        self.assertIn(
+            "restricted group", self.user.groups.values_list("name", flat=True)
+        )
+
+    def test_should_not_allow_staff_admin_to_add_restricted_group_to_user(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        group_restricted = Group.objects.create(name="restricted group")
+        group_restricted.authgroup.restricted = True
+        group_restricted.authgroup.save()
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        form = page.forms["user_form"]
+        # when
+        form["groups"].select_multiple(texts=["restricted group"])
+        response = form.submit("save")
+        # then
+        self.assertEqual(response.status_code, 200)
+        self.assertIn(
+            "You are not allowed to add or remove these restricted groups",
+            response.text
+        )
+
+    def test_should_not_allow_staff_admin_to_remove_restricted_group_from_user(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        group_restricted = Group.objects.create(name="restricted group")
+        group_restricted.authgroup.restricted = True
+        group_restricted.authgroup.save()
+        self.user.groups.add(group_restricted)
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        form = page.forms["user_form"]
+        # when
+        form["groups"].select_multiple(texts=[])
+        response = form.submit("save")
+        # then
+        self.assertEqual(response.status_code, 200)
+        self.assertIn(
+            "You are not allowed to add or remove these restricted groups",
+            response.text
+        )
+
+    def test_should_allow_staff_admin_to_add_normal_group_to_user(self):
+        # given
+        self.app.set_user(self.super_admin)
+        Group.objects.create(name="normal group")
+        page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/")
+        form = page.forms["user_form"]
+        # when
+        form["groups"].select_multiple(texts=["normal group"])
+        response = form.submit("save")
+        # then
+        self.assertEqual(response.status_code, 302)
+        self.user.refresh_from_db()
+        self.assertIn("normal group", self.user.groups.values_list("name", flat=True))
+
+
 class TestMakeServicesHooksActions(TestCaseWithTestData):
 
     class MyServicesHookTypeA(ServicesHook):

allianceauth/corputils/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.corputils.apps.CorpUtilsConfig'

allianceauth/corputils/templates/corputils/corpstats.html

@@ -8,11 +8,11 @@
                 <table class="table">
                     <tr>
                         <td class="text-center col-lg-6{% if corpstats.corp.alliance %}{% else %}col-lg-offset-3{% endif %}">
-                                <img class="ra-avatar" src="{{ corpstats.corp.logo_url_64 }}">
+                                <img class="ra-avatar" src="{{ corpstats.corp.logo_url_64 }}" alt="{{ corpstats.corp.corporation_name }}">
                             </td>
                         {% if corpstats.corp.alliance %}
                             <td class="text-center col-lg-6">
-                                <img class="ra-avatar" src="{{ corpstats.corp.alliance.logo_url_64 }}">
+                                <img class="ra-avatar" src="{{ corpstats.corp.alliance.logo_url_64 }}" alt="{{ corpstats.corp.alliance.alliance_name }}">
                             </td>
                         {% endif %}
                     </tr>
@@ -59,7 +59,7 @@
                                                     <tr>
                                                         <td class="text-center" style="vertical-align:middle">
                                                             <div class="thumbnail" style="border: 0 none; box-shadow: none; background: transparent;">
-                                                                <img src="{{ main.main.portrait_url_64 }}" class="img-circle">
+                                                                <img src="{{ main.main.portrait_url_64 }}" class="img-circle" alt="{{ main.main }}">
                                                                 <div class="caption text-center">
                                                                     {{ main.main }}
                                                                 </div>
@@ -80,7 +80,7 @@
                                                                     <tr>
                                                                         <td class="text-center" style="width:5%">
                                                                             <div class="thumbnail" style="border: 0 none; box-shadow: none; background: transparent;">
-                                                                                <img src="{{ alt.portrait_url_32 }}" class="img-circle">
+                                                                                <img src="{{ alt.portrait_url_32 }}" class="img-circle" alt="{{ alt.character_name }}">
                                                                             </div>
                                                                         </td>
                                                                         <td class="text-center" style="width:30%">{{ alt.character_name }}</td>
@@ -119,7 +119,7 @@
                                             <tbody>
                                                 {% for member in members %}
                                                     <tr>
-                                                        <td><img src="{{ member.portrait_url }}" class="img-circle"></td>
+                                                        <td><img src="{{ member.portrait_url }}" class="img-circle" alt="{{ member }}"></td>
                                                         <td class="text-center">{{ member }}</td>
                                                         <td class="text-center">
                                                             <a href="https://zkillboard.com/character/{{ member.character_id }}/" class="label label-danger" target="_blank">{% translate "Killboard" %}</a>
@@ -131,7 +131,7 @@
                                                 {% endfor %}
                                                 {% for member in unregistered %}
                                                     <tr class="danger">
-                                                        <td><img src="{{ member.portrait_url }}" class="img-circle"></td>
+                                                        <td><img src="{{ member.portrait_url }}" class="img-circle" alt="{{ member.character_name }}"></td>
                                                         <td class="text-center">{{ member.character_name }}</td>
                                                         <td class="text-center">
                                                             <a href="https://zkillboard.com/character/{{ member.character_id }}/" class="label label-danger" target="_blank">{% translate "Killboard" %}</a>
@@ -160,7 +160,7 @@
                                             <tbody>
                                                 {% for member in unregistered %}
                                                     <tr class="danger">
-                                                        <td><img src="{{ member.portrait_url }}" class="img-circle"></td>
+                                                        <td><img src="{{ member.portrait_url }}" class="img-circle" alt="{{ member.character_name }}"></td>
                                                         <td class="text-center">{{ member.character_name }}</td>
                                                         <td class="text-center">
                                                             <a href="https://zkillboard.com/character/{{ member.character_id }}/" class="label label-danger" target="_blank">

allianceauth/corputils/templates/corputils/search.html

@@ -21,7 +21,7 @@
                 <tbody>
                     {% for result in results %}
                         <tr {% if not result.1.registered %}class="danger"{% endif %}>
-                            <td class="text-center"><img src="{{ result.1.portrait_url }}" class="img-circle"></td>
+                            <td class="text-center"><img src="{{ result.1.portrait_url }}" class="img-circle" alt="{{ result.1.character_name }}"></td>
                             <td class="text-center">{{ result.1.character_name }}</td>
                             <td class="text-center">{{ result.0.corp.corporation_name }}</td>
                             <td class="text-center"><a href="https://zkillboard.com/character/{{ result.1.character_id }}/" class="label label-danger" target="_blank">{% translate "Killboard" %}</a></td>

allianceauth/eveonline/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.eveonline.apps.EveonlineConfig'

allianceauth/eveonline/autogroups/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.eveonline.autogroups.apps.EveAutogroupsConfig'

allianceauth/fleetactivitytracking/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.fleetactivitytracking.apps.FatConfig'

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/characternotexisting.html

@@ -12,7 +12,7 @@
                     <div class="panel-heading">{{ character_name }}</div>
                     <div class="panel-body">
                         <div class="col-lg-2 col-sm-2">
-                            <img class="ra-avatar img-responsive" src="{{ character_portrait_url }}">
+                            <img class="ra-avatar img-responsive" src="{{ character_portrait_url }}" alt="{{ character_name }}">
                         </div>
                         <div class="col-lg-10 col-sm-2">
                             <div class="alert alert-danger" role="alert">{% translate "Character not registered!" %}</div>

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkformatter.html

@@ -1,6 +1,5 @@
 {% extends "allianceauth/base.html" %}
 {% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Create Fatlink" %}{% endblock page_title %}

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkmodify.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 {% block page_title %}{% translate "Fatlink view" %}{% endblock page_title %}
 

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Personal fatlink statistics" %}{% endblock page_title %}

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalstatisticsview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Personal fatlink statistics" %}{% endblock page_title %}

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkstatisticscorpview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Fatlink Corp Statistics" %}{% endblock page_title %}
@@ -29,7 +27,7 @@
             {% for memberStat in fatStats %}
             <tr>
                 <td>
-                    <img src="{{ memberStat.mainchar.portrait_url_32 }}" class="ra-avatar img-responsive">
+                    <img src="{{ memberStat.mainchar.portrait_url_32 }}" class="ra-avatar img-responsive" alt="{{ memberStat.mainchar.character_name }}">
                 </td>
                 <td class="text-center">{{ memberStat.mainchar.character_name }}</td>
                 <td class="text-center">{{ memberStat.n_chars }}</td>

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkstatisticsview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Fatlink statistics" %}{% endblock page_title %}
@@ -30,9 +28,9 @@
             {% for corpStat in fatStats %}
             <tr>
                 <td>
-                    <img src="{{ corpStat.corp.logo_url_32 }}" class="ra-avatar img-responsive">
+                    <img src="{{ corpStat.corp.logo_url_32 }}" class="ra-avatar img-responsive" alt="{{ corpStat.corp.corporation_name }}">
                 </td>
-                <td class="text-center"><a href="{% url 'fatlink:statistics_corp' corpStat.corp.corporation_id %}">[{{ corpStat.corp.corporation_ticker }}]</td>
+                <td class="text-center"><a href="{% url 'fatlink:statistics_corp' corpStat.corp.corporation_id %}">[{{ corpStat.corp.corporation_ticker }}]</a></td>
                 <td class="text-center">{{ corpStat.corp.corporation_name }}</td>
                 <td class="text-center">{{ corpStat.corp.member_count }}</td>
                 <td class="text-center">{{ corpStat.n_fats }}</td>

allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Fatlink view" %}{% endblock page_title %}

allianceauth/fleetactivitytracking/views.py

@@ -212,7 +212,14 @@ def fatlink_monthly_personal_statistics_view(request, year, month, char_id=None)
     start_of_previous_month = first_day_of_previous_month(year, month)
 
     if request.user.has_perm('auth.fleetactivitytracking_statistics') and char_id:
-        user = EveCharacter.objects.get(character_id=char_id).user
+        try:
+            user = EveCharacter.objects.get(character_id=char_id).character_ownership.user
+        except EveCharacter.DoesNotExist:
+            messages.error(request, _('Character does not exist'))
+            return redirect('fatlink:view')
+        except AttributeError:
+            messages.error(request, _('User does not exist'))
+            return redirect('fatlink:view')
     else:
         user = request.user
     logger.debug(f"Personal monthly statistics view for user {user} called by {request.user}")

allianceauth/groupmanagement/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.groupmanagement.apps.GroupManagementConfig'

allianceauth/groupmanagement/admin.py

@@ -1,19 +1,21 @@
-from django import forms
 from django.apps import apps
-from django.contrib.auth.models import Permission
 from django.contrib import admin
-from django.contrib.auth.models import Group as BaseGroup, User
-from django.core.exceptions import ValidationError
-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, \
-    post_delete, m2m_changed
-from django.dispatch import receiver
-from django.utils.timezone import now
-from django.utils.translation import gettext_lazy as _
 
-from .models import AuthGroup, ReservedGroupName
+from django.contrib.auth.models import Group as BaseGroup, Permission, User
-from .models import GroupRequest
+from django.db.models import Count, Exists, OuterRef
+from django.db.models.functions import Lower
+from django.db.models.signals import (
+    m2m_changed,
+    post_delete,
+    post_save,
+    pre_delete,
+    pre_save
+)
+from django.dispatch import receiver
+
+from .forms import GroupAdminForm, ReservedGroupNameAdminForm
+from .models import AuthGroup, GroupRequest, ReservedGroupName
+from .tasks import remove_users_not_matching_states_from_group
 
 if 'eve_autogroups' in apps.app_configs:
     _has_auto_groups = True
@@ -28,10 +30,12 @@ class AuthGroupInlineAdmin(admin.StackedInline):
         'description',
         'group_leaders',
         'group_leader_groups',
-        'states', 'internal',
+        'states',
+        'internal',
         'hidden',
         'open',
-        'public'
+        'public',
+        'restricted',
     )
     verbose_name_plural = 'Auth Settings'
     verbose_name = ''
@@ -50,6 +54,11 @@ class AuthGroupInlineAdmin(admin.StackedInline):
     def has_change_permission(self, request, obj=None):
         return request.user.has_perm('auth.change_group')
 
+    def get_readonly_fields(self, request, obj=None):
+        if not request.user.is_superuser:
+            return self.readonly_fields + ("restricted",)
+        return self.readonly_fields
+
 
 if _has_auto_groups:
     class IsAutoGroupFilter(admin.SimpleListFilter):
@@ -96,27 +105,15 @@ class HasLeaderFilter(admin.SimpleListFilter):
         return queryset
 
 
-class GroupAdminForm(forms.ModelForm):
-    def clean_name(self):
-        my_name = self.cleaned_data['name']
-        if ReservedGroupName.objects.filter(name__iexact=my_name).exists():
-            raise ValidationError(
-                _("This name has been reserved and can not be used for groups."),
-                code='reserved_name'
-            )
-        return my_name
-
-
 class GroupAdmin(admin.ModelAdmin):
     form = GroupAdminForm
-    list_select_related = ('authgroup',)
     ordering = ('name',)
     list_display = (
         'name',
         '_description',
         '_properties',
         '_member_count',
-        'has_leader'
+        'has_leader',
     )
     list_filter = [
         'authgroup__internal',
@@ -132,34 +129,51 @@ class GroupAdmin(admin.ModelAdmin):
 
     def get_queryset(self, request):
         qs = super().get_queryset(request)
-        if _has_auto_groups:
+        has_leader_qs = (
-            qs = qs.prefetch_related('managedalliancegroup_set', 'managedcorpgroup_set')
+            AuthGroup.objects.filter(group=OuterRef('pk'), group_leaders__isnull=False)
-        qs = qs.prefetch_related('authgroup__group_leaders').select_related('authgroup')
-        qs = qs.annotate(
-            member_count=Count('user', distinct=True),
         )
+        has_leader_groups_qs = (
+            AuthGroup.objects.filter(
+                group=OuterRef('pk'), group_leader_groups__isnull=False
+            )
+        )
+        qs = (
+            qs.select_related('authgroup')
+            .annotate(member_count=Count('user', distinct=True))
+            .annotate(has_leader=Exists(has_leader_qs))
+            .annotate(has_leader_groups=Exists(has_leader_groups_qs))
+        )
+        if _has_auto_groups:
+            is_autogroup_corp = (
+                Group.objects.filter(
+                    pk=OuterRef('pk'), managedcorpgroup__isnull=False
+                )
+            )
+            is_autogroup_alliance = (
+                Group.objects.filter(
+                    pk=OuterRef('pk'), managedalliancegroup__isnull=False
+                )
+            )
+            qs = (
+                qs.annotate(is_autogroup_corp=Exists(is_autogroup_corp))
+                .annotate(is_autogroup_alliance=Exists(is_autogroup_alliance))
+            )
         return qs
 
     def _description(self, obj):
         return obj.authgroup.description
 
+    @admin.display(description='Members', ordering='member_count')
     def _member_count(self, obj):
         return obj.member_count
 
-    _member_count.short_description = 'Members'
+    @admin.display(boolean=True)
-    _member_count.admin_order_field = 'member_count'
-
     def has_leader(self, obj):
-        return obj.authgroup.group_leaders.exists() or obj.authgroup.group_leader_groups.exists()
+        return obj.has_leader or obj.has_leader_groups
-
-    has_leader.boolean = True
 
     def _properties(self, obj):
         properties = list()
-        if _has_auto_groups and (
+        if _has_auto_groups and (obj.is_autogroup_corp or obj.is_autogroup_alliance):
-            obj.managedalliancegroup_set.exists()
-            or obj.managedcorpgroup_set.exists()
-        ):
             properties.append('Auto Group')
         elif obj.authgroup.internal:
             properties.append('Internal')
@@ -172,11 +186,10 @@ class GroupAdmin(admin.ModelAdmin):
                 properties.append('Public')
         if not properties:
             properties.append('Default')
-
+        if obj.authgroup.restricted:
+            properties.append('Restricted')
         return properties
 
-    _properties.short_description = "properties"
-
     filter_horizontal = ('permissions',)
     inlines = (AuthGroupInlineAdmin,)
 
@@ -190,8 +203,15 @@ class GroupAdmin(admin.ModelAdmin):
             ag_instance = inline_form.save(commit=False)
             ag_instance.group = form.instance
             ag_instance.save()
+            if ag_instance.states.exists():
+                remove_users_not_matching_states_from_group.delay(ag_instance.group.pk)
         formset.save()
 
+    def get_readonly_fields(self, request, obj=None):
+        if not request.user.is_superuser:
+            return self.readonly_fields + ("permissions",)
+        return self.readonly_fields
+
 
 class Group(BaseGroup):
     class Meta:
@@ -216,33 +236,10 @@ class GroupRequestAdmin(admin.ModelAdmin):
         'leave_request',
     )
 
+    @admin.display(boolean=True, description="is leave request")
     def _leave_request(self, obj) -> True:
         return obj.leave_request
 
-    _leave_request.short_description = 'is leave request'
-    _leave_request.boolean = True
-
-
-class ReservedGroupNameAdminForm(forms.ModelForm):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.fields['created_by'].initial = self.current_user.username
-        self.fields['created_at'].initial = _("(auto)")
-
-    created_by = forms.CharField(disabled=True)
-    created_at = forms.CharField(disabled=True)
-
-    def clean_name(self):
-        my_name = self.cleaned_data['name'].lower()
-        if Group.objects.filter(name__iexact=my_name).exists():
-            raise ValidationError(
-                _("There already exists a group with that name."), code='already_exists'
-            )
-        return my_name
-
-    def clean_created_at(self):
-        return now()
-
 
 @admin.register(ReservedGroupName)
 class ReservedGroupNameAdmin(admin.ModelAdmin):

allianceauth/groupmanagement/forms.py

@@ -0,0 +1,39 @@
+from django import forms
+from django.contrib.auth.models import Group
+from django.core.exceptions import ValidationError
+from django.utils.timezone import now
+from django.utils.translation import gettext_lazy as _
+
+from .models import ReservedGroupName
+
+
+class GroupAdminForm(forms.ModelForm):
+    def clean_name(self):
+        my_name = self.cleaned_data['name']
+        if ReservedGroupName.objects.filter(name__iexact=my_name).exists():
+            raise ValidationError(
+                _("This name has been reserved and can not be used for groups."),
+                code='reserved_name'
+            )
+        return my_name
+
+
+class ReservedGroupNameAdminForm(forms.ModelForm):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.fields['created_by'].initial = self.current_user.username
+        self.fields['created_at'].initial = _("(auto)")
+
+    created_by = forms.CharField(disabled=True)
+    created_at = forms.CharField(disabled=True)
+
+    def clean_name(self):
+        my_name = self.cleaned_data['name'].lower()
+        if Group.objects.filter(name__iexact=my_name).exists():
+            raise ValidationError(
+                _("There already exists a group with that name."), code='already_exists'
+            )
+        return my_name
+
+    def clean_created_at(self):
+        return now()

allianceauth/groupmanagement/migrations/0019_adding_restricted_to_groups.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.10 on 2022-04-08 19:30
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('groupmanagement', '0018_reservedgroupname'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='authgroup',
+            name='restricted',
+            field=models.BooleanField(default=False, help_text='Group is restricted. This means that adding or removing users for this group requires a superuser admin.'),
+        ),
+    ]

allianceauth/groupmanagement/models.py

@@ -13,6 +13,7 @@ from allianceauth.notifications import notify
 
 class GroupRequest(models.Model):
     """Request from a user for joining or leaving a group."""
+
     leave_request = models.BooleanField(default=0)
     user = models.ForeignKey(User, on_delete=models.CASCADE)
     group = models.ForeignKey(Group, on_delete=models.CASCADE)
@@ -44,6 +45,7 @@ class GroupRequest(models.Model):
 
 class RequestLog(models.Model):
     """Log entry about who joined and left a group and who approved it."""
+
     request_type = models.BooleanField(null=True)
     group = models.ForeignKey(Group, on_delete=models.CASCADE)
     request_info = models.CharField(max_length=254)
@@ -95,6 +97,7 @@ class AuthGroup(models.Model):
         Open - Users are automatically accepted into the group
         Not Open - Users requests must be approved before they are added to the group
     """
+
     group = models.OneToOneField(Group, on_delete=models.CASCADE, primary_key=True)
     internal = models.BooleanField(
         default=True,
@@ -126,6 +129,13 @@ class AuthGroup(models.Model):
             "are no longer authenticated."
         )
     )
+    restricted = models.BooleanField(
+        default=False,
+        help_text=_(
+            "Group is restricted. This means that adding or removing users "
+            "for this group requires a superuser admin."
+        )
+    )
     group_leaders = models.ManyToManyField(
         User,
         related_name='leads_groups',
@@ -179,12 +189,22 @@ class AuthGroup(models.Model):
             | User.objects.filter(groups__in=list(self.group_leader_groups.all()))
         )
 
+    def remove_users_not_matching_states(self):
+        """Remove users not matching defined states from related group."""
+        states_qs = self.states.all()
+        if states_qs.exists():
+            states = list(states_qs)
+            non_compliant_users = self.group.user_set.exclude(profile__state__in=states)
+            for user in non_compliant_users:
+                self.group.user_set.remove(user)
+
 
 class ReservedGroupName(models.Model):
     """Name that can not be used for groups.
 
     This enables AA to ignore groups on other services (e.g. Discord) with that name.
     """
+
     name = models.CharField(
         _('name'),
         max_length=150,

allianceauth/groupmanagement/tasks.py

@@ -0,0 +1,10 @@
+from celery import shared_task
+
+from django.contrib.auth.models import Group
+
+
+@shared_task
+def remove_users_not_matching_states_from_group(group_pk: int) -> None:
+    """Remove users not matching defined states from related group."""
+    group = Group.objects.get(pk=group_pk)
+    group.authgroup.remove_users_not_matching_states()

allianceauth/groupmanagement/templates/groupmanagement/audit.html

@@ -25,13 +25,15 @@
                     <div class="table-responsive">
                         <table class="table table-striped" id="log-entries">
                             <thead>
-                                <th scope="col">{% translate "Date/Time" %}</th>
+                                <tr>
-                                <th scope="col">{% translate "Requestor" %}</th>
+                                    <th scope="col">{% translate "Date/Time" %}</th>
-                                <th scope="col">{% translate "Character" %}</th>
+                                    <th scope="col">{% translate "Requestor" %}</th>
-                                <th scope="col">{% translate "Corporation" %}</th>
+                                    <th scope="col">{% translate "Character" %}</th>
-                                <th scope="col">{% translate "Type" %}</th>
+                                    <th scope="col">{% translate "Corporation" %}</th>
-                                <th scope="col">{% translate "Action" %}</th>
+                                    <th scope="col">{% translate "Type" %}</th>
-                                <th scope="col">{% translate "Actor" %}</th>
+                                    <th scope="col">{% translate "Action" %}</th>
+                                    <th scope="col">{% translate "Actor" %}</th>
+                                </tr>
                             </thead>
 
                             <tbody>
@@ -74,7 +76,7 @@
 {% block extra_javascript %}
     {% include 'bundles/datatables-js.html' %}
     {% include 'bundles/moment-js.html' with locale=True %}
-    <script type="application/javascript" src="{% static 'js/filterDropDown/filterDropDown.min.js' %}"></script>
+    {% include 'bundles/filterdropdown-js.html' %}
 {% endblock %}
 
 {% block extra_css %}

allianceauth/groupmanagement/templates/groupmanagement/groupmembers.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 {% load evelinks %}
 
@@ -37,7 +36,7 @@
                                 {% for member in members %}
                                     <tr>
                                         <td>
-                                            <img src="{{ member.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;">
+                                            <img src="{{ member.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;" alt="{{ member.main_char.character_name }}">
                                             {% if member.main_char %}
                                                 <a href="{{ member.main_char|evewho_character_url }}" target="_blank">
                                                     {{ member.main_char.character_name }}

allianceauth/groupmanagement/templates/groupmanagement/groupmembership.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Groups Membership" %}{% endblock page_title %}

allianceauth/groupmanagement/templates/groupmanagement/groups.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Available Groups" %}{% endblock page_title %}

allianceauth/groupmanagement/templates/groupmanagement/index.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 {% load evelinks %}
 
@@ -64,7 +63,7 @@
                                         {% for acceptrequest in acceptrequests %}
                                             <tr>
                                                 <td>
-                                                    <img src="{{ acceptrequest.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;">
+                                                    <img src="{{ acceptrequest.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;" alt="{{ acceptrequest.main_char.character_name }}">
                                                     {% if acceptrequest.main_char %}
                                                         <a href="{{ acceptrequest.main_char|evewho_character_url }}" target="_blank">
                                                             {{ acceptrequest.main_char.character_name }}
@@ -121,7 +120,7 @@
                                             {% for leaverequest in leaverequests %}
                                                 <tr>
                                                     <td>
-                                                        <img src="{{ leaverequest.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;">
+                                                        <img src="{{ leaverequest.main_char|character_portrait_url:32 }}" class="img-circle" style="margin-right: 1rem;" alt="{{ leaverequest.main_char.character_name }}">
                                                         {% if leaverequest.main_char %}
                                                             <a href="{{ leaverequest.main_char|evewho_character_url }}" target="_blank">
                                                                 {{ leaverequest.main_char.character_name }}

allianceauth/groupmanagement/templates/groupmanagement/menu.html

@@ -1,4 +1,3 @@
-{% load static %}
 {% load i18n %}
 {% load navactive %}
 

allianceauth/groupmanagement/tests/test_admin.py

@@ -1,17 +1,21 @@
 from unittest.mock import patch
 
+from django_webtest import WebTest
+
 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 django.test import TestCase, RequestFactory, Client, override_settings
 
 from allianceauth.authentication.models import CharacterOwnership, State
 from allianceauth.eveonline.models import (
     EveCharacter, EveCorporationInfo, EveAllianceInfo
 )
-from ..admin import HasLeaderFilter, GroupAdmin, Group
+from allianceauth.tests.auth_utils import AuthUtils
+
 from . import get_admin_change_view_url
+from ..admin import HasLeaderFilter, GroupAdmin, Group
 from ..models import ReservedGroupName
 
 
@@ -33,7 +37,6 @@ class MockRequest:
 
 
 class TestGroupAdmin(TestCase):
-
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
@@ -233,60 +236,104 @@ class TestGroupAdmin(TestCase):
         self.assertEqual(result, expected)
 
     def test_member_count(self):
-        expected = 1
+        # given
-        obj = self.modeladmin.get_queryset(MockRequest(user=self.user_1))\
+        request = MockRequest(user=self.user_1)
-            .get(pk=self.group_1.pk)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_1.pk)
+        # when
         result = self.modeladmin._member_count(obj)
-        self.assertEqual(result, expected)
+        # then
+        self.assertEqual(result, 1)
 
     def test_has_leader_user(self):
-        result = self.modeladmin.has_leader(self.group_1)
+        # given
+        request = MockRequest(user=self.user_1)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_1.pk)
+        # when
+        result = self.modeladmin.has_leader(obj)
+        # then
         self.assertTrue(result)
 
     def test_has_leader_group(self):
-        result = self.modeladmin.has_leader(self.group_2)
+        # given
+        request = MockRequest(user=self.user_1)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_2.pk)
+        # when
+        result = self.modeladmin.has_leader(obj)
+        # then
         self.assertTrue(result)
 
     def test_properties_1(self):
-        expected = ['Default']
+        # given
-        result = self.modeladmin._properties(self.group_1)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_1.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Default'])
 
     def test_properties_2(self):
-        expected = ['Internal']
+        # given
-        result = self.modeladmin._properties(self.group_2)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_2.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Internal'])
 
     def test_properties_3(self):
-        expected = ['Hidden']
+        # given
-        result = self.modeladmin._properties(self.group_3)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_3.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Hidden'])
 
     def test_properties_4(self):
-        expected = ['Open']
+        # given
-        result = self.modeladmin._properties(self.group_4)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_4.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Open'])
 
     def test_properties_5(self):
-        expected = ['Public']
+        # given
-        result = self.modeladmin._properties(self.group_5)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_5.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Public'])
 
     def test_properties_6(self):
-        expected = ['Hidden', 'Open', 'Public']
+        # given
-        result = self.modeladmin._properties(self.group_6)
+        request = MockRequest(user=self.user_1)
-        self.assertListEqual(result, expected)
+        obj = self.modeladmin.get_queryset(request).get(pk=self.group_6.pk)
+        # when
+        result = self.modeladmin._properties(obj)
+        self.assertListEqual(result, ['Hidden', 'Open', 'Public'])
 
     if _has_auto_groups:
         @patch(MODULE_PATH + '._has_auto_groups', True)
-        def test_properties_7(self):
+        def test_should_show_autogroup_for_corporation(self):
+            # given
             self._create_autogroups()
-            expected = ['Auto Group']
+            request = MockRequest(user=self.user_1)
-            my_group = Group.objects\
+            queryset = self.modeladmin.get_queryset(request)
-                .filter(managedcorpgroup__isnull=False)\
+            obj = queryset.filter(managedcorpgroup__isnull=False).first()
-                .first()
+            # when
-            result = self.modeladmin._properties(my_group)
+            result = self.modeladmin._properties(obj)
-            self.assertListEqual(result, expected)
+            # then
+            self.assertListEqual(result, ['Auto Group'])
+
+        @patch(MODULE_PATH + '._has_auto_groups', True)
+        def test_should_show_autogroup_for_alliance(self):
+            # given
+            self._create_autogroups()
+            request = MockRequest(user=self.user_1)
+            queryset = self.modeladmin.get_queryset(request)
+            obj = queryset.filter(managedalliancegroup__isnull=False).first()
+            # when
+            result = self.modeladmin._properties(obj)
+            # then
+            self.assertListEqual(result, ['Auto Group'])
 
     # actions
 
@@ -468,6 +515,136 @@ class TestGroupAdmin(TestCase):
         self.assertFalse(Group.objects.filter(name="new group").exists())
 
 
+class TestGroupAdminChangeFormSuperuserExclusiveEdits(WebTest):
+    @classmethod
+    def setUpClass(cls) -> None:
+        super().setUpClass()
+        cls.super_admin = User.objects.create_superuser("super_admin")
+        cls.staff_admin = User.objects.create_user("staff_admin")
+        cls.staff_admin.is_staff = True
+        cls.staff_admin.save()
+        cls.staff_admin = AuthUtils.add_permissions_to_user_by_name(
+            [
+                "auth.add_group",
+                "auth.change_group",
+                "auth.view_group",
+                "groupmanagement.add_group",
+                "groupmanagement.change_group",
+                "groupmanagement.view_group",
+            ],
+            cls.staff_admin
+        )
+        cls.superuser_exclusive_fields = ["permissions", "authgroup-0-restricted"]
+
+    def test_should_show_all_fields_to_superuser_for_add(self):
+        # given
+        self.app.set_user(self.super_admin)
+        page = self.app.get("/admin/groupmanagement/group/add/")
+        # when
+        form = page.forms["group_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertIn(field, form.fields)
+
+    def test_should_not_show_all_fields_to_staff_admins_for_add(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        page = self.app.get("/admin/groupmanagement/group/add/")
+        # when
+        form = page.forms["group_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertNotIn(field, form.fields)
+
+    def test_should_show_all_fields_to_superuser_for_change(self):
+        # given
+        self.app.set_user(self.super_admin)
+        group = Group.objects.create(name="Dummy group")
+        page = self.app.get(f"/admin/groupmanagement/group/{group.pk}/change/")
+        # when
+        form = page.forms["group_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertIn(field, form.fields)
+
+    def test_should_not_show_all_fields_to_staff_admin_for_change(self):
+        # given
+        self.app.set_user(self.staff_admin)
+        group = Group.objects.create(name="Dummy group")
+        page = self.app.get(f"/admin/groupmanagement/group/{group.pk}/change/")
+        # when
+        form = page.forms["group_form"]
+        # then
+        for field in self.superuser_exclusive_fields:
+            with self.subTest(field=field):
+                self.assertNotIn(field, form.fields)
+
+
+@override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True)
+class TestGroupAdmin2(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.superuser = User.objects.create_superuser("super")
+
+    def test_should_remove_users_from_state_groups(self):
+        # given
+        user_member = AuthUtils.create_user("Bruce Wayne")
+        character_member = AuthUtils.add_main_character_2(
+            user_member,
+            name="Bruce Wayne",
+            character_id=1001,
+            corp_id=2001,
+            corp_name="Wayne Technologies",
+        )
+        user_guest = AuthUtils.create_user("Lex Luthor")
+        AuthUtils.add_main_character_2(
+            user_guest,
+            name="Lex Luthor",
+            character_id=1011,
+            corp_id=2011,
+            corp_name="Luthor Corp",
+        )
+        member_state = AuthUtils.get_member_state()
+        member_state.member_characters.add(character_member)
+        user_member.refresh_from_db()
+        user_guest.refresh_from_db()
+        group = Group.objects.create(name="dummy")
+        user_member.groups.add(group)
+        user_guest.groups.add(group)
+        group.authgroup.states.add(member_state)
+        self.client.force_login(self.superuser)
+        # when
+        response = self.client.post(
+            f"/admin/groupmanagement/group/{group.pk}/change/",
+            data={
+                "name": f"{group.name}",
+                "authgroup-TOTAL_FORMS": "1",
+                "authgroup-INITIAL_FORMS": "1",
+                "authgroup-MIN_NUM_FORMS": "0",
+                "authgroup-MAX_NUM_FORMS": "1",
+                "authgroup-0-description": "",
+                "authgroup-0-states": f"{member_state.pk}",
+                "authgroup-0-internal": "on",
+                "authgroup-0-hidden": "on",
+                "authgroup-0-group": f"{group.pk}",
+                "authgroup-__prefix__-description": "",
+                "authgroup-__prefix__-internal": "on",
+                "authgroup-__prefix__-hidden": "on",
+                "authgroup-__prefix__-group": f"{group.pk}",
+                "_save": "Save"
+            }
+        )
+        # then
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, "/admin/groupmanagement/group/")
+        self.assertIn(group, user_member.groups.all())
+        self.assertNotIn(group, user_guest.groups.all())
+
+
 class TestReservedGroupNameAdmin(TestCase):
     @classmethod
     def setUpClass(cls):

allianceauth/groupmanagement/tests/test_models.py

@@ -232,6 +232,38 @@ class TestAuthGroup(TestCase):
         expected = 'Superheros'
         self.assertEqual(str(group.authgroup), expected)
 
+    def test_should_remove_guests_from_group_when_restricted_to_members_only(self):
+        # given
+        user_member = AuthUtils.create_user("Bruce Wayne")
+        character_member = AuthUtils.add_main_character_2(
+            user_member,
+            name="Bruce Wayne",
+            character_id=1001,
+            corp_id=2001,
+            corp_name="Wayne Technologies",
+        )
+        user_guest = AuthUtils.create_user("Lex Luthor")
+        AuthUtils.add_main_character_2(
+            user_guest,
+            name="Lex Luthor",
+            character_id=1011,
+            corp_id=2011,
+            corp_name="Luthor Corp",
+        )
+        member_state = AuthUtils.get_member_state()
+        member_state.member_characters.add(character_member)
+        user_member.refresh_from_db()
+        user_guest.refresh_from_db()
+        group = Group.objects.create(name="dummy")
+        user_member.groups.add(group)
+        user_guest.groups.add(group)
+        group.authgroup.states.add(member_state)
+        # when
+        group.authgroup.remove_users_not_matching_states()
+        # then
+        self.assertIn(group, user_member.groups.all())
+        self.assertNotIn(group, user_guest.groups.all())
+
 
 class TestAuthGroupRequestApprovers(TestCase):
     def setUp(self) -> None:

allianceauth/hrapplications/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.hrapplications.apps.HRApplicationsConfig'

allianceauth/hrapplications/templates/hrapplications/corpchoice.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Choose a Corp" %}{% endblock page_title %}

allianceauth/hrapplications/templates/hrapplications/create.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Apply To" %} {{ corp.corporation_name }}{% endblock page_title %}
@@ -16,11 +15,11 @@
                             <label class="control-label" for="id_{{ question.pk }}">{{ question.title }}</label>
                             <div class=" ">
                             {% if question.help_text %}
-                                <div cass="text-center">{{ question.help_text }}</div>
+                                <div class="text-center">{{ question.help_text }}</div>
                             {% endif %}
                             {% for choice in question.choices.all %}
-                                <input type={% if question.multi_select == False %}"radio"{% else %}"checkbox"{% endif %} name="{{ question.pk }}" id="id_{{ question.pk }}" value="{{ choice.choice_text }}">
+                                <input type={% if question.multi_select == False %}"radio"{% else %}"checkbox"{% endif %} name="{{ question.pk }}" id="id_{{ question.pk }}_choice_{{ forloop.counter }}" value="{{ choice.choice_text }}">
-                                <label for="choice{{ forloop.counter }}">{{ choice.choice_text }}</label><br>
+                                <label for="id_{{ question.pk }}_choice_{{ forloop.counter }}">{{ choice.choice_text }}</label><br>
                             {% empty %}
                                 <textarea class="form-control" cols="30" id="id_{{ question.pk }}" name="{{ question.pk }}" rows="4"></textarea>
                             {% endfor %}

allianceauth/hrapplications/templates/hrapplications/management.html

@@ -1,6 +1,5 @@
 {% extends "allianceauth/base.html" %}
 {% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "HR Application Management" %}{% endblock page_title %}
@@ -178,7 +177,7 @@
                         <h4 class="modal-title" id="myModalLabel">{% translate "Application Search" %}</h4>
                     </div>
                     <div class="modal-body">
-                        <form class="form-signin" role="form" action={% url 'hrapplications:search' %} method="POST">
+                        <form class="form-signin" role="form" action="{% url 'hrapplications:search' %}" method="POST">
                             {% csrf_token %}
                             {{ search_form|bootstrap }}
                             <br>

allianceauth/hrapplications/templates/hrapplications/searchview.html

@@ -1,6 +1,5 @@
 {% extends "allianceauth/base.html" %}
 {% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "HR Application Management" %}{% endblock page_title %}
@@ -64,7 +63,7 @@
                         <h4 class="modal-title" id="myModalLabel">{% translate "Application Search" %}</h4>
                     </div>
                     <div class="modal-body">
-                        <form class="form-signin" role="form" action={% url 'hrapplications:search' %} method="POST">
+                        <form class="form-signin" role="form" action="{% url 'hrapplications:search' %}" method="POST">
                             {% csrf_token %}
                             {{ search_form|bootstrap }}
                             <br>

allianceauth/hrapplications/templates/hrapplications/view.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load bootstrap %}
 {% load i18n %}
 
@@ -49,7 +48,7 @@
                             {% for char in app.characters %}
                                 <tr>
                                     <td class="text-center">
-                                        <img class="ra-avatar img-responsive img-circle" src="{{ char.portrait_url_32 }}">
+                                        <img class="ra-avatar img-responsive img-circle" src="{{ char.portrait_url_32 }}" alt="{{ char.character_name }}">
                                     </td>
                                     <td class="text-center">{{ char.character_name }}</td>
                                     <td class="text-center">{{ char.corporation_name }}</td>

allianceauth/hrapplications/views.py

@@ -219,7 +219,7 @@ def hr_application_search(request):
                 Q(user__character_ownerships__character__corporation_name__icontains=searchstring) |
                 Q(user__character_ownerships__character__alliance_name__icontains=searchstring) |
                 Q(user__username__icontains=searchstring)
-            )
+            ).distinct()
 
             context = {'applications': applications, 'search_form': HRApplicationSearchForm()}
 

allianceauth/locale/es/LC_MESSAGES/django.po

@@ -20,7 +20,7 @@ msgstr ""
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Language: es\n"
-"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"Plural-Forms: nplurals=3; plural=n == 1 ? 0 : n != 0 && n % 1000000 == 0 ? 1 : 2;\n"
 
 #: allianceauth/analytics/models.py:29
 msgid "Google Analytics Universal"
@@ -450,6 +450,7 @@ msgid "%(user)s has collected one link this month."
 msgid_plural "%(user)s has collected %(links)s links this month."
 msgstr[0] ""
 msgstr[1] ""
+msgstr[2] ""
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:28
 msgid "Times used"
@@ -461,6 +462,7 @@ msgid "%(user)s has created one link this month."
 msgid_plural "%(user)s has created %(links)s links this month."
 msgstr[0] ""
 msgstr[1] ""
+msgstr[2] ""
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:48
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkview.html:27
@@ -2141,6 +2143,7 @@ msgid "%(tasks)s task"
 msgid_plural "%(tasks)s tasks"
 msgstr[0] ""
 msgstr[1] ""
+msgstr[2] ""
 
 #: allianceauth/templates/allianceauth/night-toggle.html:6
 msgid "Night Mode"

allianceauth/locale/fr_FR/LC_MESSAGES/django.po

@@ -24,7 +24,7 @@ msgstr ""
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Language: fr_FR\n"
-"Plural-Forms: nplurals=2; plural=(n > 1);\n"
+"Plural-Forms: nplurals=3; plural=(n == 0 || n == 1) ? 0 : n != 0 && n % 1000000 == 0 ? 1 : 2;\n"
 
 #: allianceauth/analytics/models.py:29
 msgid "Google Analytics Universal"
@@ -460,6 +460,7 @@ msgid "%(user)s has collected one link this month."
 msgid_plural "%(user)s has collected %(links)s links this month."
 msgstr[0] "%(user)s a obtenu un lien ce mois."
 msgstr[1] "%(user)s a obtenu %(links)s liens ce mois."
+msgstr[2] "%(user)s a obtenu %(links)s liens ce mois."
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:28
 msgid "Times used"
@@ -471,6 +472,7 @@ msgid "%(user)s has created one link this month."
 msgid_plural "%(user)s has created %(links)s links this month."
 msgstr[0] "%(user)s a créé un lien ce mois."
 msgstr[1] "%(user)s a créé %(links)s liens ce mois."
+msgstr[2] "%(user)s a créé %(links)s liens ce mois."
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:48
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkview.html:27
@@ -2167,6 +2169,7 @@ msgid "%(tasks)s task"
 msgid_plural "%(tasks)s tasks"
 msgstr[0] ""
 msgstr[1] ""
+msgstr[2] ""
 
 #: allianceauth/templates/allianceauth/night-toggle.html:6
 msgid "Night Mode"

allianceauth/locale/it_IT/LC_MESSAGES/django.po

@@ -20,7 +20,7 @@ msgstr ""
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Language: it_IT\n"
-"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"Plural-Forms: nplurals=3; plural=n == 1 ? 0 : n != 0 && n % 1000000 == 0 ? 1 : 2;\n"
 
 #: allianceauth/analytics/models.py:29
 msgid "Google Analytics Universal"
@@ -460,6 +460,7 @@ msgid "%(user)s has collected one link this month."
 msgid_plural "%(user)s has collected %(links)s links this month."
 msgstr[0] "%(user)s ha ottenuto un link per questo mese."
 msgstr[1] "%(user)s ha ottenuto %(links)s links questo mese."
+msgstr[2] "%(user)s ha ottenuto %(links)s links questo mese."
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:28
 msgid "Times used"
@@ -471,6 +472,7 @@ msgid "%(user)s has created one link this month."
 msgid_plural "%(user)s has created %(links)s links this month."
 msgstr[0] "%(user)s ha creato un link questo mese."
 msgstr[1] "%(user)s ha creato %(links)s links questo mese."
+msgstr[2] "%(user)s ha creato %(links)s links questo mese."
 
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkpersonalmonthlystatisticsview.html:48
 #: allianceauth/fleetactivitytracking/templates/fleetactivitytracking/fatlinkview.html:27
@@ -2155,6 +2157,7 @@ msgid "%(tasks)s task"
 msgid_plural "%(tasks)s tasks"
 msgstr[0] ""
 msgstr[1] ""
+msgstr[2] ""
 
 #: allianceauth/templates/allianceauth/night-toggle.html:6
 msgid "Night Mode"

allianceauth/notifications/__init__.py

@@ -1,9 +1,3 @@
+from .core import notify  # noqa: F401
+
 default_app_config = 'allianceauth.notifications.apps.NotificationsConfig'
-
-
-def notify(
-    user: object, title: str, message: str = None, level: str = 'info'
-) -> None:
-    """Sends a new notification to user. Convenience function to manager pendant."""
-    from .models import Notification
-    Notification.objects.notify_user(user, title, message, level)

allianceauth/notifications/core.py

@@ -0,0 +1,33 @@
+class NotifyApiWrapper:
+    """Wrapper to create notify API."""
+
+    def __call__(self, *args, **kwargs):  # provide old API for backwards compatibility
+        return self._add_notification(*args, **kwargs)
+
+    def danger(self, user: object, title: str, message: str = None) -> None:
+        """Add danger notification for user."""
+        self._add_notification(user, title, message, level="danger")
+
+    def info(self, user: object, title: str, message: str = None) -> None:
+        """Add info notification for user."""
+        self._add_notification(user=user, title=title, message=message, level="info")
+
+    def success(self, user: object, title: str, message: str = None) -> None:
+        """Add success notification for user."""
+        self._add_notification(user, title, message, level="success")
+
+    def warning(self, user: object, title: str, message: str = None) -> None:
+        """Add warning notification for user."""
+        self._add_notification(user, title, message, level="warning")
+
+    def _add_notification(
+        self, user: object, title: str, message: str = None, level: str = "info"
+    ) -> None:
+        from .models import Notification
+
+        Notification.objects.notify_user(
+            user=user, title=title, message=message, level=level
+        )
+
+
+notify = NotifyApiWrapper()

allianceauth/notifications/templates/notifications/list.html

@@ -1,95 +1,37 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Notifications" %}{% endblock %}
 
 {% block content %}
-    <div class="col-lg-12">
+    <h1 class="page-header text-center">{% translate "Notifications" %}</h1>
-        <h1 class="page-header text-center">{% translate "Notifications" %}</h1>
+
-        <div class="col-lg-12 container" id="example">
+    <div class="panel panel-default">
-            <div class="row">
+
-                <div class="col-lg-12">
+        <div class="panel-heading clearfix">
-                    <div class="panel panel-default">
+            <ul class="nav nav-pills navbar-left">
-                        <div class="panel-heading">
+                <li class="active"><a data-toggle="tab" href="#unread">{% translate "Unread" %}<b>({{ unread|length }})</b></a></li>
-                            <ul class="nav nav-pills">
+                <li><a data-toggle="tab" href="#read">{% translate "Read" %} <b>({{ read|length }})</b></a></li>
-                                <li class="active"><a data-toggle="pill" href="#unread">{% translate "Unread" %}
+            </ul>
-                                    <b>({{ unread|length }})</b></a></li>
+            <div class="nav navbar-nav navbar-right" style="margin-right: 0;">
-                                <li><a data-toggle="pill" href="#read">{% translate "Read" %} <b>({{ read|length }})</b></a>
+                <a href="{% url 'notifications:mark_all_read' %}" class="btn btn-warning">{% translate "Mark All Read" %}</a>
-                                </li>
+                <a href="{% url 'notifications:delete_all_read' %}" class="btn btn-danger">{% translate "Delete All Read" %}</a>
-                                <div class="pull-right">
-                                    <a href="{% url 'notifications:mark_all_read' %}" class="btn btn-primary">{% translate "Mark All Read" %}</a>
-                                    <a href="{% url 'notifications:delete_all_read' %}" class="btn btn-danger">{% translate "Delete All Read" %}</a>
-                                </div>
-                            </ul>
-                        </div>
-                        <div class="panel-body">
-                            <div class="tab-content">
-                                <div id="unread" class="tab-pane fade in active">
-                                    <div class="table-responsive">
-                                        {% if unread %}
-                                            <table class="table table-condensed table-hover table-striped">
-                                                <tr>
-                                                    <th class="text-center">{% translate "Timestamp" %}</th>
-                                                    <th class="text-center">{% translate "Title" %}</th>
-                                                    <th class="text-center">{% translate "Action" %}</th>
-                                                </tr>
-                                                {% for notif in unread %}
-                                                    <tr class="{{ notif.level }}">
-                                                        <td class="text-center">{{ notif.timestamp }}</td>
-                                                        <td class="text-center">{{ notif.title }}</td>
-                                                        <td class="text-center">
-                                                            <a href="{% url 'notifications:view' notif.id %}" class="btn btn-success" title="View">
-                                                                <span class="glyphicon glyphicon-eye-open"></span>
-                                                            </a>
-                                                            <a href="{% url 'notifications:remove' notif.id %}" class="btn btn-danger" title="Remove">
-                                                                <span class="glyphicon glyphicon-remove"></span>
-                                                            </a>
-                                                        </td>
-                                                    </tr>
-                                                {% endfor %}
-                                            </table>
-                                        {% else %}
-                                            <div class="alert alert-warning text-center">{% translate "No unread notifications." %}</div>
-                                        {% endif %}
-                                    </div>
-                                </div>
-                                <div id="read" class="tab-pane fade">
-                                    <div class="panel-body">
-                                        <div class="table-responsive">
-                                            {% if read %}
-                                                <table class="table table-condensed table-hover table-striped">
-                                                    <tr>
-                                                        <th class="text-center">{% translate "Timestamp" %}</th>
-                                                        <th class="text-center">{% translate "Title" %}</th>
-                                                        <th class="text-center">{% translate "Action" %}</th>
-                                                    </tr>
-                                                    {% for notif in read %}
-                                                        <tr class="{{ notif.level }}">
-                                                            <td class="text-center">{{ notif.timestamp }}</td>
-                                                            <td class="text-center">{{ notif.title }}</td>
-                                                            <td class="text-center">
-                                                                <a href="{% url 'notifications:view' notif.id %}" class="btn btn-success" title="View">
-                                                                    <span class="glyphicon glyphicon-eye-open"></span>
-                                                                </a>
-                                                                <a href="{% url 'notifications:remove' notif.id %}" class="btn btn-danger" title="remove">
-                                                                    <span class="glyphicon glyphicon-remove"></span>
-                                                                </a>
-                                                            </td>
-                                                        </tr>
-                                                    {% endfor %}
-                                                </table>
-                                            {% else %}
-                                                <div class="alert alert-warning text-center">{% translate "No read notifications." %}</div>
-                                            {% endif %}
-                                        </div>
-                                    </div>
-                                </div>
-                            </div>
-                        </div>
-                    </div>
-                </div>
             </div>
         </div>
+
+        <div class="panel-body">
+            <div class="tab-content">
+
+                <div id="unread" class="tab-pane fade in active">
+                    {% include "notifications/list_partial.html" with notifications=unread %}
+                </div>
+
+                <div id="read" class="tab-pane fade">
+                    {% include "notifications/list_partial.html" with notifications=read %}
+                </div>
+
+            </div>
+        </div>
+
     </div>
 {% endblock %}

allianceauth/notifications/templates/notifications/list_partial.html

@@ -0,0 +1,29 @@
+{% load i18n %}
+
+{% if notifications %}
+    <div class="table-responsive">
+        <table class="table table-condensed table-hover table-striped">
+            <tr>
+                <th class="text-center">{% translate "Timestamp" %}</th>
+                <th class="text-center">{% translate "Title" %}</th>
+                <th class="text-center">{% translate "Action" %}</th>
+            </tr>
+            {% for notif in notifications %}
+                <tr class="{{ notif.level }}">
+                    <td class="text-center">{{ notif.timestamp }}</td>
+                    <td class="text-center">{{ notif.title }}</td>
+                    <td class="text-center">
+                        <a href="{% url 'notifications:view' notif.id %}" class="btn btn-primary" title="View">
+                            <span class="glyphicon glyphicon-eye-open"></span>
+                        </a>
+                        <a href="{% url 'notifications:remove' notif.id %}" class="btn btn-danger" title="Remove">
+                            <span class="glyphicon glyphicon-remove"></span>
+                        </a>
+                    </td>
+                </tr>
+            {% endfor %}
+        </table>
+    </div>
+{% else %}
+    <div class="alert alert-default text-center">{% translate "No notifications." %}</div>
+{% endif %}

allianceauth/notifications/templates/notifications/view.html

@@ -1,29 +1,25 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "View Notification" %}{% endblock page_title %}
 
 {% block content %}
+    <h1 class="page-header text-center">
+        {% translate "View Notification" %}
+        <div class="text-right">
+            <a href="{% url 'notifications:list' %}" class="btn btn-primary btn-lg">
+                <span class="glyphicon glyphicon-arrow-left"></span>
+            </a>
+        </div>
+    </h1>
 
-    <div class="col-lg-12">
+    <div class="row">
-        <h1 class="page-header text-center">
+        <div class="col-lg-12">
-            {% translate "View Notification" %}
+            <div class="panel panel-{{ notif.level }}">
-            <div class="text-right">
+                <div class="panel-heading">{{ notif.timestamp }} {{ notif.title }}</div>
-                <a href="{% url 'notifications:list' %}" class="btn btn-primary btn-lg">
+                <div class="panel-body"><pre>{{ notif.message }}</pre></div>
-                    <span class="glyphicon glyphicon-arrow-left"></span>
-                </a>
-            </div>
-        </h1>
-        <div class="col-lg-12 container">
-            <div class="row">
-                <div class="col-lg-12">
-                    <div class="panel panel-{{ notif.level }}">
-                        <div class="panel-heading">{{ notif.timestamp }} {{ notif.title }}</div>
-                        <div class="panel-body"><pre>{{ notif.message }}</pre></div>
-                    </div>
-                </div>
             </div>
         </div>
     </div>
+
 {% endblock %}

allianceauth/notifications/tests/test_core.py

@@ -0,0 +1,85 @@
+from django.test import TestCase
+
+from allianceauth.tests.auth_utils import AuthUtils
+
+from ..core import NotifyApiWrapper
+from ..models import Notification
+
+
+class TestUserNotificationCount(TestCase):
+    @classmethod
+    def setUpClass(cls) -> None:
+        super().setUpClass()
+        cls.user = AuthUtils.create_user("bruce_wayne")
+
+    def test_should_add_danger_notification(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify.danger(user=self.user, title="title", message="message")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.DANGER)
+
+    def test_should_add_info_notification(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify.info(user=self.user, title="title", message="message")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.INFO)
+
+    def test_should_add_success_notification(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify.success(user=self.user, title="title", message="message")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.SUCCESS)
+
+    def test_should_add_warning_notification(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify.warning(user=self.user, title="title", message="message")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.WARNING)
+
+    def test_should_add_info_notification_via_callable(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify(user=self.user, title="title", message="message")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.INFO)
+
+    def test_should_add_danger_notification_via_callable(self):
+        # given
+        notify = NotifyApiWrapper()
+        # when
+        notify(user=self.user, title="title", message="message", level="danger")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, Notification.Level.DANGER)

allianceauth/notifications/tests/test_init.py

@@ -4,11 +4,8 @@ from allianceauth.tests.auth_utils import AuthUtils
 from .. import notify
 from ..models import Notification
 
-MODULE_PATH = 'allianceauth.notifications'
-
 
 class TestUserNotificationCount(TestCase):
-
     @classmethod
     def setUpTestData(cls):
         cls.user = AuthUtils.create_user('magic_mike')
@@ -23,6 +20,18 @@ class TestUserNotificationCount(TestCase):
             alliance_name='RIDERS'
         )
 
-    def test_can_notify(self):
+    def test_can_notify_short(self):
-        notify(self.user, 'dummy')
+        # when
+        notify(self.user, "dummy")
+        # then
         self.assertEqual(Notification.objects.filter(user=self.user).count(), 1)
+
+    def test_can_notify_full(self):
+        # when
+        notify(user=self.user, title="title", message="message", level="danger")
+        # then
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, "title")
+        self.assertEqual(obj.message, "message")
+        self.assertEqual(obj.level, "danger")

allianceauth/optimer/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.optimer.apps.OptimerConfig'

allianceauth/optimer/templates/optimer/add.html

@@ -1,6 +1,5 @@
 {% extends "allianceauth/base.html" %}
 {% load bootstrap %}
-{% load static %}
 {% load i18n %}
 {% get_current_language as LANGUAGE_CODE %}
 

allianceauth/optimer/templates/optimer/management.html

@@ -1,5 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load static %}
 {% load i18n %}
 {% get_current_language as LANGUAGE_CODE %}
 
@@ -40,9 +39,9 @@
     </div>
 
     {% include 'bundles/moment-js.html' with locale=True %}
-    <script src="{% static 'js/timers.js' %}"></script>
+    {% include 'bundles/timers-js.html' %}
 
-    <script type="application/javascript">
+    <script>
         // Data
         let timers = [
         {% for op in optimer %}

allianceauth/optimer/templates/optimer/update.html

@@ -1,6 +1,5 @@
 {% extends "allianceauth/base.html" %}
 {% load bootstrap %}
-{% load static %}
 {% load i18n %}
 {% get_current_language as LANGUAGE_CODE %}
 

allianceauth/permissions_tool/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.permissions_tool.apps.PermissionsToolConfig'

allianceauth/permissions_tool/templates/permissions_tool/audit.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{{ permission.permission.codename }} - {% translate "Permissions Audit" %}{% endblock page_title %}
@@ -47,7 +45,7 @@
 
 {% block extra_javascript %}
     {% include 'bundles/datatables-js.html' %}
-    <script type="application/javascript" src="{% static 'js/filterDropDown/filterDropDown.min.js' %}"></script>
+    {% include 'bundles/filterdropdown-js.html' %}
 {% endblock %}
 
 {% block extra_css %}

allianceauth/permissions_tool/templates/permissions_tool/audit_row.html

@@ -5,7 +5,7 @@
         {{ type }}: {{ name }}
     </td>
     <td class="text-right">
-        <img src="{{ user.profile.main_character|character_portrait_url:32 }}" class="img-circle">
+        <img src="{{ user.profile.main_character|character_portrait_url:32 }}" class="img-circle" alt="{{ user.profile.main_character.character_name }}">
     </td>
     <td>
         <strong>{{ user }}<br></strong>

allianceauth/permissions_tool/templates/permissions_tool/overview.html

@@ -1,6 +1,4 @@
 {% extends "allianceauth/base.html" %}
-{% load bootstrap %}
-{% load static %}
 {% load i18n %}
 
 {% block page_title %}{% translate "Permissions Overview" %}{% endblock page_title %}
@@ -80,7 +78,7 @@
 
 {% block extra_javascript %}
     {% include 'bundles/datatables-js.html' %}
-    <script type="application/javascript" src="{% static 'js/filterDropDown/filterDropDown.min.js' %}"></script>
+    {% include 'bundles/filterdropdown-js.html' %}
 {% endblock %}
 
 {% block extra_css %}

allianceauth/project_template/project_name/settings/base.py

@@ -154,8 +154,6 @@ TIME_ZONE = 'UTC'
 
 USE_I18N = True
 
-USE_L10N = True
-
 USE_TZ = True
 
 # Static files (CSS, JavaScript, Images)

allianceauth/project_template/project_name/settings/local.py

@@ -13,6 +13,12 @@ STATIC_ROOT = "/var/www/{{ project_name }}/static/"
 # in page titles and the site header.
 SITE_NAME = '{{ project_name }}'
 
+# This is your websites URL, set it accordingly
+SITE_URL = "https://example.com"
+
+# Django security
+CSRF_TRUSTED_ORIGINS = [SITE_URL]
+
 # Change this to enable/disable debug mode, which displays
 # useful error messages but can leak sensitive data.
 DEBUG = False
@@ -39,15 +45,16 @@ DATABASES['default'] = {
 
 # Register an application at https://developers.eveonline.com for Authentication
 # & API Access and fill out these settings. Be sure to set the callback URL
-# to https://example.com/sso/callback substituting your domain for example.com
+# to https://example.com/sso/callback substituting your domain for example.com in
+# CCP's developer portal
 # Logging in to auth requires the publicData scope (can be overridden through the
 # LOGIN_TOKEN_SCOPES setting). Other apps may require more (see their docs).
 ESI_SSO_CLIENT_ID = ''
 ESI_SSO_CLIENT_SECRET = ''
-ESI_SSO_CALLBACK_URL = ''
+ESI_SSO_CALLBACK_URL = f"{SITE_URL}/sso/callback"
 ESI_USER_CONTACT_EMAIL = ''    # A server maintainer that CCP can contact in case of issues.
 
-# By default emails are validated before new users can log in.
+# By default, emails are validated before new users can log in.
 # It's recommended to use a free service like SparkPost or Elastic Email to send email.
 # https://www.sparkpost.com/docs/integrations/django/
 # https://elasticemail.com/resources/settings/smtp-api/

allianceauth/services/__init__.py

@@ -1 +0,0 @@
-default_app_config = 'allianceauth.services.apps.ServicesConfig'

allianceauth/services/admin.py

@@ -3,11 +3,11 @@ from django.contrib import admin
 
 from allianceauth import hooks
 from allianceauth.authentication.admin import (
+    MainAllianceFilter,
+    MainCorporationsFilter,
+    user_main_organization,
     user_profile_pic,
     user_username,
-    user_main_organization,
-    MainCorporationsFilter,
-    MainAllianceFilter
 )
 
 from .models import NameFormatConfig
@@ -17,7 +17,7 @@ class ServicesUserAdmin(admin.ModelAdmin):
     """Parent class for UserAdmin classes for all services"""
     class Media:
         css = {
-            "all": ("services/admin.css",)
+            "all": ("allianceauth/services/admin.css",)
         }
 
     search_fields = ('user__username',)
@@ -36,19 +36,18 @@ class ServicesUserAdmin(admin.ModelAdmin):
         MainAllianceFilter,
         'user__date_joined',
     )
+    list_select_related = (
+        'user', 'user__profile__main_character', 'user__profile__state'
+    )
 
+    @admin.display(ordering='user__profile__state__name')
     def _state(self, obj):
         return obj.user.profile.state.name
 
-    _state.short_description = 'state'
+    @admin.display(ordering='user__date_joined')
-    _state.admin_order_field = 'user__profile__state__name'
-
     def _date_joined(self, obj):
         return obj.user.date_joined
 
-    _date_joined.short_description = 'date joined'
-    _date_joined.admin_order_field = 'user__date_joined'
-
 
 class NameFormatConfigForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
@@ -62,6 +61,7 @@ class NameFormatConfigForm(forms.ModelForm):
         self.fields['service_name'] = forms.ChoiceField(choices=SERVICE_CHOICES)
 
 
+@admin.register(NameFormatConfig)
 class NameFormatConfigAdmin(admin.ModelAdmin):
     form = NameFormatConfigForm
     list_display = ('service_name', 'get_state_display_string')
@@ -69,6 +69,3 @@ class NameFormatConfigAdmin(admin.ModelAdmin):
     def get_state_display_string(self, obj):
         return ', '.join([state.name for state in obj.states.all()])
     get_state_display_string.short_description = 'States'
-
-
-admin.site.register(NameFormatConfig, NameFormatConfigAdmin)

allianceauth/services/modules/discord/__init__.py

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

allianceauth/services/modules/discord/admin.py

@@ -2,12 +2,11 @@ import logging
 
 from django.contrib import admin
 
-from . import __title__
 from ...admin import ServicesUserAdmin
+from . import __title__
 from .models import DiscordUser
 from .utils import LoggerAddTag
 
-
 logger = LoggerAddTag(logging.getLogger(__name__), __title__)
 
 
@@ -18,21 +17,16 @@ class DiscordUserAdmin(ServicesUserAdmin):
     list_filter = ServicesUserAdmin.list_filter + ('activated',)
     ordering = ('-activated',)
 
-    def _uid(self, obj):
-        return obj.uid
-
-    _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 ''
-
     def delete_queryset(self, request, queryset):
         for user in queryset:
             user.delete_user()
 
-    _username.short_description = 'Discord Username'
+    @admin.display(description='Discord ID (UID)', ordering='uid')
-    _username.admin_order_field = 'username'
+    def _uid(self, obj):
+        return obj.uid
+
+    @admin.display(description='Discord Username', ordering='username')
+    def _username(self, obj):
+        if obj.username and obj.discriminator:
+            return f'{obj.username}#{obj.discriminator}'
+        return ''

allianceauth/services/modules/discord/api.py

@@ -0,0 +1,37 @@
+"""Public interface for community apps who want to interact with the Discord server
+of the current Alliance Auth instance.
+
+Example
+=======
+
+Here is an example for using the api to fetch the current roles from the configured Discord server.
+
+.. code-block:: python
+
+    from allianceauth.services.modules.discord.api import create_bot_client, discord_guild_id
+
+    client = create_bot_client()  # create a new Discord client
+    guild_id = discord_guild_id()  # get the ID of the configured Discord server
+    roles = client.guild_roles(guild_id)  # fetch the roles from our Discord server
+
+.. seealso::
+    The docs for the client class can be found here: :py:class:`~allianceauth.services.modules.discord.discord_client.client.DiscordClient`
+"""
+
+from typing import Optional
+
+from .app_settings import DISCORD_GUILD_ID
+from .core import create_bot_client, group_to_role, server_name  # noqa
+from .discord_client.models import Role  # noqa
+from .models import DiscordUser  # noqa
+
+__all__ = ["create_bot_client", "group_to_role", "server_name", "DiscordUser", "Role"]
+
+
+def discord_guild_id() -> Optional[int]:
+    """Guild ID of configured Discord server.
+
+    Returns:
+        Guild ID or ``None`` if not configured
+    """
+    return int(DISCORD_GUILD_ID) if DISCORD_GUILD_ID else None

allianceauth/services/modules/discord/app_settings.py

@@ -2,16 +2,25 @@ from .utils import clean_setting
 
 
 DISCORD_APP_ID = clean_setting('DISCORD_APP_ID', '')
+"""App ID for the AA bot on Discord. Needs to be set."""
+
 DISCORD_APP_SECRET = clean_setting('DISCORD_APP_SECRET', '')
+"""App secret for the AA bot on Discord. Needs to be set."""
+
 DISCORD_BOT_TOKEN = clean_setting('DISCORD_BOT_TOKEN', '')
+"""Token used by the AA bot on Discord.  Needs to be set."""
+
 DISCORD_CALLBACK_URL = clean_setting('DISCORD_CALLBACK_URL', '')
+"""Callback URL for OAuth with Discord. Needs to be set."""
+
 DISCORD_GUILD_ID = clean_setting('DISCORD_GUILD_ID', '')
+"""ID of the Discord Server. Needs to be set."""
 
-# max retries of tasks after an error occurred
 DISCORD_TASKS_MAX_RETRIES = clean_setting('DISCORD_TASKS_MAX_RETRIES', 3)
+"""Max retries of tasks after an error occurred."""
 
-# 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)
+"""Pause in seconds until next retry for tasks after the API returned an error."""
 
-# automatically sync Discord users names to user's main character name when created
 DISCORD_SYNC_NAMES = clean_setting('DISCORD_SYNC_NAMES', False)
+"""Automatically sync Discord users names to user's main character name when created."""

allianceauth/services/modules/discord/auth_hooks.py

@@ -6,6 +6,7 @@ from django.template.loader import render_to_string
 from allianceauth import hooks
 from allianceauth.services.hooks import ServicesHook
 
+from .core import server_name, user_formatted_nick
 from .models import DiscordUser
 from .urls import urlpatterns
 from .utils import LoggerAddTag
@@ -53,7 +54,7 @@ class DiscordService(ServicesHook):
         return render_to_string(
             self.service_ctrl_template,
             {
-                'server_name': DiscordUser.objects.server_name(),
+                'server_name': server_name(),
                 'user_has_account': user_has_account,
                 'discord_username': discord_username
             },
@@ -73,7 +74,7 @@ class DiscordService(ServicesHook):
                     'user_pk': user.pk,
                     # since the new nickname is not yet in the DB we need to
                     # provide it manually to the task
-                    'nickname': DiscordUser.objects.user_formatted_nick(user)
+                    'nickname': user_formatted_nick(user)
                 },
                 priority=SINGLE_TASK_PRIORITY
             )

allianceauth/services/modules/discord/core.py

@@ -0,0 +1,129 @@
+"""Core functionality of the Discord service not directly related to models."""
+
+import logging
+from typing import List, Optional, Tuple
+
+from requests.exceptions import HTTPError
+
+from django.contrib.auth.models import Group, User
+
+from allianceauth.groupmanagement.models import ReservedGroupName
+from allianceauth.services.hooks import NameFormatter
+
+from . import __title__
+from .app_settings import DISCORD_BOT_TOKEN, DISCORD_GUILD_ID
+from .discord_client import DiscordClient, RolesSet, Role
+from .discord_client.exceptions import DiscordClientException
+from .utils import LoggerAddTag
+
+logger = LoggerAddTag(logging.getLogger(__name__), __title__)
+
+
+def create_bot_client(is_rate_limited: bool = True) -> DiscordClient:
+    """Create new bot client for accessing the configured Discord server.
+
+    Args:
+        is_rate_limited: Set to False to turn off rate limiting (use with care).
+
+    Return:
+        Discord client instance
+    """
+    return DiscordClient(DISCORD_BOT_TOKEN, is_rate_limited=is_rate_limited)
+
+
+def calculate_roles_for_user(
+    user: User,
+    client: DiscordClient,
+    discord_uid: int,
+    state_name: str = None,
+) -> Tuple[RolesSet, Optional[bool]]:
+    """Calculate current Discord roles for an Auth user.
+
+    Takes into account reserved groups and existing managed roles (e.g. nitro).
+
+    Returns:
+        - Discord roles, changed flag:
+            - True when roles have changed,
+            - False when they have not changed,
+            - None if user is not a member of the guild
+    """
+    roles_calculated = client.match_or_create_roles_from_names_2(
+        guild_id=DISCORD_GUILD_ID,
+        role_names=_user_group_names(user=user, state_name=state_name),
+    )
+    logger.debug("Calculated roles for user %s: %s", user, roles_calculated.ids())
+    roles_current = client.guild_member_roles(
+        guild_id=DISCORD_GUILD_ID, user_id=discord_uid
+    )
+    if roles_current is None:
+        logger.debug("User %s is not a member of the guild.", user)
+        return roles_calculated, None
+    logger.debug("Current roles user %s: %s", user, roles_current.ids())
+    reserved_role_names = ReservedGroupName.objects.values_list("name", flat=True)
+    roles_reserved = roles_current.subset(role_names=reserved_role_names)
+    roles_managed = roles_current.subset(managed_only=True)
+    roles_persistent = roles_managed.union(roles_reserved)
+    if roles_calculated == roles_current.difference(roles_persistent):
+        return roles_calculated, False
+    return roles_calculated.union(roles_persistent), True
+
+
+def _user_group_names(user: User, state_name: str = None) -> List[str]:
+    """Names of groups and state the given user is a member of."""
+    if not state_name:
+        state_name = user.profile.state.name
+    group_names = [group.name for group in user.groups.all()] + [state_name]
+    logger.debug("Group names for roles updates of user %s are: %s", user, group_names)
+    return group_names
+
+
+def user_formatted_nick(user: User) -> Optional[str]:
+    """Name of the given user's main character with name formatting applied.
+
+    Returns:
+        Name or ``None`` if user has no main.
+    """
+    from .auth_hooks import DiscordService
+
+    if user.profile.main_character:
+        return NameFormatter(DiscordService(), user).format_name()
+    return None
+
+
+def group_to_role(group: Group) -> Optional[Role]:
+    """Fetch the Discord role matching the given Django group by name.
+
+    Returns:
+        Discord role or None if no matching role exist
+    """
+    return default_bot_client.match_role_from_name(
+        guild_id=DISCORD_GUILD_ID, role_name=group.name
+    )
+
+
+def server_name(use_cache: bool = True) -> str:
+    """Fetches the name of the current Discord server.
+
+    Args:
+        use_cache: When set False will force an API call to get the server name
+
+    Returns:
+        Server name or an empty string if the name could not be retrieved
+    """
+    try:
+        server_name = default_bot_client.guild_name(
+            guild_id=DISCORD_GUILD_ID, use_cache=use_cache
+        )
+    except (HTTPError, DiscordClientException):
+        server_name = ""
+    except Exception:
+        logger.warning(
+            "Unexpected error when trying to retrieve the server name from Discord",
+            exc_info=True,
+        )
+        server_name = ""
+    return server_name
+
+
+# Default bot client to be used by modules of this package
+default_bot_client = create_bot_client()

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

@@ -1,3 +1,10 @@
-from .client import DiscordClient   # noqa
+from .app_settings import DISCORD_OAUTH_BASE_URL, DISCORD_OAUTH_TOKEN_URL  # noqa
-from .exceptions import DiscordApiBackoff  # noqa
+from .client import DiscordClient  # noqa
-from .helpers import DiscordRoles  # noqa
+from .exceptions import (  # noqa
+    DiscordApiBackoff,
+    DiscordClientException,
+    DiscordRateLimitExhausted,
+    DiscordTooManyRequestsError,
+)
+from .helpers import RolesSet  # noqa
+from .models import Guild, GuildMember, Role, User  # noqa

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

@@ -1,45 +1,56 @@
+"""Settings for the Discord client.
+
+To overwrite a default set the variable in your local Django settings, e.g:
+
+.. code:: python
+
+    DISCORD_GUILD_NAME_CACHE_MAX_AGE = 7200
+"""
+
 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://discord.com/api/'
 )
+"""Base URL for all API calls. Must end with /."""
 
-# Low level connecttimeout for requests to the Discord API in seconds
 DISCORD_API_TIMEOUT_CONNECT = clean_setting(
     'DISCORD_API_TIMEOUT', 5
 )
+"""Low level connect timeout for requests to the Discord API in seconds."""
 
-# Low level read timeout for requests to the Discord API in seconds
 DISCORD_API_TIMEOUT_READ = clean_setting(
     'DISCORD_API_TIMEOUT', 30
 )
+"""Low level read timeout for requests to the Discord API in seconds."""
 
-# Base authorization URL for Discord Oauth
 DISCORD_OAUTH_BASE_URL = clean_setting(
     'DISCORD_OAUTH_BASE_URL', 'https://discord.com/api/oauth2/authorize'
 )
+"""Base authorization URL for Discord Oauth."""
 
-# Base authorization URL for Discord Oauth
 DISCORD_OAUTH_TOKEN_URL = clean_setting(
     'DISCORD_OAUTH_TOKEN_URL', 'https://discord.com/api/oauth2/token'
 )
+"""Base authorization URL for Discord Oauth."""
 
-# How long the Discord guild names retrieved from the server are
-# caches locally in seconds.
 DISCORD_GUILD_NAME_CACHE_MAX_AGE = clean_setting(
     'DISCORD_GUILD_NAME_CACHE_MAX_AGE', 3600 * 24
 )
+"""How long the Discord guild names retrieved from the server
+are caches locally in seconds.
+"""
 
-# How long Discord roles retrieved from the server are caches locally in seconds.
 DISCORD_ROLES_CACHE_MAX_AGE = clean_setting(
     'DISCORD_ROLES_CACHE_MAX_AGE', 3600 * 1
 )
+"""How long Discord roles retrieved from the server are caches locally in seconds."""
 
-# 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
 )
+"""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.
+"""

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

@@ -1,32 +1,37 @@
-from hashlib import md5
+"""Client for interacting with the Discord API."""
+
 import json
 import logging
+from enum import IntEnum
+from hashlib import md5
+from http import HTTPStatus
 from time import sleep
+from typing import Iterable, List, Optional, Set, Tuple
 from urllib.parse import urljoin
 from uuid import uuid1
 
-from redis import Redis
 import requests
+from requests.exceptions import HTTPError
+from redis import Redis
 
-from django_redis import get_redis_connection
+from allianceauth.utils.cache import get_redis_client
 
-from allianceauth import __title__ as AUTH_TITLE, __url__, __version__
+from allianceauth import __title__ as AUTH_TITLE
+from allianceauth import __url__, __version__
 
 from .. import __title__
+from ..utils import LoggerAddTag
 from .app_settings import (
     DISCORD_API_BASE_URL,
     DISCORD_API_TIMEOUT_CONNECT,
     DISCORD_API_TIMEOUT_READ,
     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 .helpers import DiscordRoles
+from .helpers import RolesSet
-from ..utils import LoggerAddTag
+from .models import Guild, GuildMember, Role, User
-
 
 logger = LoggerAddTag(logging.getLogger(__name__), __title__)
 
@@ -58,8 +63,13 @@ MINIMUM_BLOCKING_WAIT = 50
 RATE_LIMIT_RETRIES = 1000
 
 
+class DiscordApiStatusCode(IntEnum):
+    """Status code returned from the Discord API."""
+    UNKNOWN_MEMBER = 10007  #:
+
+
 class DiscordClient:
-    """This class provides a web client for interacting with the Discord API
+    """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,
@@ -67,24 +77,30 @@ class DiscordClient:
 
     In addition the client support proper API backoff.
 
-    Synchronization of rate limit infos accross multiple processes
+    Synchronization of rate limit infos across multiple processes
     is implemented with Redis and thus requires Redis as Django cache backend.
 
-    All durations are in milliseconds.
+    The cache is shared across all clients and processes (also using Redis).
-    """
-    OAUTH_BASE_URL = DISCORD_OAUTH_BASE_URL
-    OAUTH_TOKEN_URL = DISCORD_OAUTH_TOKEN_URL
 
+    All durations are in milliseconds.
+
+    Most errors from the API will raise a requests.HTTPError.
+
+    Args:
+        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 turn off rate limiting (use with care).
+            If not specified will try to use the Redis instance
+            from the default Django cache backend.
+
+    Raises:
+        ValueError: No access token provided
+    """
     _KEY_GLOBAL_BACKOFF_UNTIL = 'DISCORD_GLOBAL_BACKOFF_UNTIL'
     _KEY_GLOBAL_RATE_LIMIT_REMAINING = 'DISCORD_GLOBAL_RATE_LIMIT_REMAINING'
     _KEYPREFIX_GUILD_NAME = 'DISCORD_GUILD_NAME'
     _KEYPREFIX_GUILD_ROLES = 'DISCORD_GUILD_ROLES'
     _KEYPREFIX_ROLE_NAME = 'DISCORD_ROLE_NAME'
-    _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,
@@ -92,18 +108,12 @@ class DiscordClient:
         redis: Redis = None,
         is_rate_limited: bool = True
     ) -> None:
-        """
+        if not access_token:
-        Params:
+            raise ValueError('You must provide an access token.')
-        - 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:
-            self._redis = get_redis_connection("default")
+            self._redis = get_redis_client()
             if not isinstance(self._redis, Redis):
                 raise RuntimeError(
                     'This class requires a Redis client, but none was provided '
@@ -131,19 +141,20 @@ class DiscordClient:
         self.__redis_script_set_longer = self._redis.register_script(lua_2)
 
     @property
-    def access_token(self):
+    def access_token(self) -> str:
+        """Discord access token."""
         return self._access_token
 
     @property
-    def is_rate_limited(self):
+    def is_rate_limited(self) -> bool:
+        """Wether this instance is rate limited."""
         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
+        """Decrease the key value if it exists and returns the result else set the key.
-        else sets the key
 
         Implemented as Lua script to ensure atomicity.
         """
@@ -152,7 +163,7 @@ class DiscordClient:
         )
 
     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
+        """Like set, but only goes through if either key doesn't exist
         or px would be extended.
 
         Implemented as Lua script to ensure atomicity.
@@ -163,111 +174,134 @@ class DiscordClient:
 
     # users
 
-    def current_user(self) -> dict:
+    def current_user(self) -> User:
-        """returns the user belonging to the current access_token"""
+        """Fetch 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()
+        return User.from_dict(r.json())
 
     # guild
 
-    def guild_infos(self, guild_id: int) -> dict:
+    def guild_infos(self, guild_id: int) -> Guild:
-        """Returns all basic infos about this guild"""
+        """Fetch all basic infos about this guild.
+
+        Args:
+            guild_id: Discord ID of the guild
+        """
         route = f"guilds/{guild_id}"
         r = self._api_request(method='get', route=route)
-        return r.json()
+        return Guild.from_dict(r.json())
 
     def guild_name(self, guild_id: int, use_cache: bool = True) -> str:
-        """returns the name of this guild (cached)
+        """Fetch the name of this guild (cached).
-        or an empty string if something went wrong
 
-        Params:
+        Args:
-        - guild_id: ID of current guild
+            guild_id: Discord ID of the guild
-        - use_cache: When set to False will force an API call to get the server name
+            use_cache: When set to False will force an API call to get the server name
+
+        Returns:
+            Name of the server or an empty string if something went wrong.
         """
         key_name = self._guild_name_cache_key(guild_id)
         if use_cache:
             guild_name = self._redis_decode(self._redis.get(key_name))
         else:
-            guild_name = None
+            guild_name = ""
         if not guild_name:
-            guild_infos = self.guild_infos(guild_id)
+            try:
-            if 'name' in guild_infos:
+                guild = self.guild_infos(guild_id)
-                guild_name = guild_infos['name']
+            except HTTPError:
-                self._redis.set(
+                guild_name = ""
-                    name=key_name,
-                    value=guild_name,
-                    ex=DISCORD_GUILD_NAME_CACHE_MAX_AGE
-                )
             else:
-                guild_name = ''
+                guild_name = guild.name
-
+                self._redis.set(
+                    name=key_name, value=guild_name, ex=DISCORD_GUILD_NAME_CACHE_MAX_AGE
+                )
         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"""
+        """Construct key for accessing role given by name in the role cache.
+
+        Args:
+            guild_id: Discord ID of the guild
+        """
         gen_key = DiscordClient._generate_hash(f'{guild_id}')
         return f'{cls._KEYPREFIX_GUILD_NAME}__{gen_key}'
 
     # guild roles
 
-    def guild_roles(self, guild_id: int, use_cache: bool = True) -> list:
+    def guild_roles(self, guild_id: int, use_cache: bool = True) -> Set[Role]:
-        """Returns the list of all roles for this guild
+        """Fetch all roles for this guild.
 
-        If use_cache is set to False it will always hit the API to retrieve
+        Args:
-        fresh data and update the cache
+            guild_id: Discord ID of the guild
+            use_cache: If is set to False it will always hit the API to retrieve
+                fresh data and update the cache.
+
+        Returns:
         """
         cache_key = self._guild_roles_cache_key(guild_id)
+        roles = None
         if use_cache:
             roles_raw = self._redis.get(name=cache_key)
             if roles_raw:
                 logger.debug('Returning roles for guild %s from cache', guild_id)
-                return json.loads(self._redis_decode(roles_raw))
+                roles = json.loads(self._redis_decode(roles_raw))
-            else:
+            logger.debug('No roles for guild %s in cache', guild_id)
-                logger.debug('No roles for guild %s in cache', guild_id)
+        if roles is None:
-
+            route = f"guilds/{guild_id}/roles"
-        route = f"guilds/{guild_id}/roles"
+            r = self._api_request(method='get', route=route)
-        r = self._api_request(method='get', route=route)
+            roles = r.json()
-        roles = r.json()
+            if not roles or not isinstance(roles, list):
-        if roles and isinstance(roles, list):
+                raise RuntimeError(
+                    f"Unexpected response when fetching roles from API: {roles}"
+                )
             self._redis.set(
                 name=cache_key,
                 value=json.dumps(roles),
                 ex=DISCORD_ROLES_CACHE_MAX_AGE
             )
-        return roles
+        return {Role.from_dict(role) for role in roles}
 
-    def create_guild_role(self, guild_id: int, role_name: str, **kwargs) -> dict:
+    def create_guild_role(
+        self, guild_id: int, role_name: str, **kwargs
+    ) -> Optional[Role]:
         """Create a new guild role with the given name.
+
         See official documentation for additional optional parameters.
 
         Note that Discord allows the creation of multiple roles with the same name,
         so to avoid duplicates it's important to check existing roles
         before creating new one
 
-        returns a new role dict on success
+        Args:
+            guild_id: Discord ID of the guild
+            role_name: Name of new role to create
+
+        Returns:
+            new role on success
         """
         route = f"guilds/{guild_id}/roles"
-        data = {'name': DiscordRoles.sanitize_role_name(role_name)}
+        data = {'name': Role.sanitize_name(role_name)}
         data.update(kwargs)
         r = self._api_request(method='post', route=route, data=data)
         role = r.json()
         if role:
             self._invalidate_guild_roles_cache(guild_id)
-        return role
+            return Role.from_dict(role)
+        return None
 
     def delete_guild_role(self, guild_id: int, role_id: int) -> bool:
-        """Deletes a guild role"""
+        """Delete a guild role."""
         route = f"guilds/{guild_id}/roles/{role_id}"
         r = self._api_request(method='delete', route=route)
         if r.status_code == 204:
             self._invalidate_guild_roles_cache(guild_id)
             return True
-        else:
+        return False
-            return False
 
     def _invalidate_guild_roles_cache(self, guild_id: int) -> None:
         cache_key = self._guild_roles_cache_key(guild_id)
@@ -276,67 +310,79 @@ class DiscordClient:
 
     @classmethod
     def _guild_roles_cache_key(cls, guild_id: int) -> str:
-        """Returns key for accessing cached roles for a guild"""
+        """Construct key for accessing cached roles for a guild.
+
+        Args:
+            guild_id: Discord ID of the guild
+        """
         gen_key = cls._generate_hash(f'{guild_id}')
         return f'{cls._KEYPREFIX_GUILD_ROLES}__{gen_key}'
 
-    def match_role_from_name(self, guild_id: int, role_name: str) -> dict:
+    def match_role_from_name(self, guild_id: int, role_name: str) -> Optional[Role]:
-        """returns Discord role matching the given name or an empty dict"""
+        """Fetch Discord role matching the given name (cached).
-        guild_roles = DiscordRoles(self.guild_roles(guild_id))
+
+        Args:
+            guild_id: Discord ID of the guild
+            role_name: Name of role
+
+        Returns:
+            Matching role or None if no match is found
+        """
+        guild_roles = RolesSet(self.guild_roles(guild_id))
         return guild_roles.role_by_name(role_name)
 
-    def match_or_create_roles_from_names(self, guild_id: int, role_names: list) -> list:
+    def match_or_create_roles_from_names(
-        """returns Discord roles matching the given names
+        self, guild_id: int, role_names: Iterable[str]
-
+    ) -> List[Tuple[Role, bool]]:
-        Returns as list of tuple of role and created flag
+        """Fetch or create Discord roles matching the given names (cached).
 
         Will try to match with existing roles names
         Non-existing roles will be created, then created flag will be True
 
-        Params:
+        Args:
-        - guild_id: ID of guild
+            guild_id: ID of guild
-        - role_names: list of name strings each defining a role
+            role_names: list of name strings each defining a role
+
+        Returns:
+            List of tuple of Role and created flag
         """
         roles = list()
-        guild_roles = DiscordRoles(self.guild_roles(guild_id))
+        guild_roles = RolesSet(self.guild_roles(guild_id))
-        role_names_cleaned = {
+        role_names_cleaned = {Role.sanitize_name(name) for name in role_names}
-            DiscordRoles.sanitize_role_name(name) for name in role_names
-        }
         for role_name in role_names_cleaned:
             role, created = self.match_or_create_role_from_name(
-                guild_id=guild_id,
+                guild_id=guild_id, role_name=role_name, guild_roles=guild_roles
-                role_name=DiscordRoles.sanitize_role_name(role_name),
-                guild_roles=guild_roles
             )
             if role:
                 roles.append((role, created))
             if created:
-                guild_roles = guild_roles.union(DiscordRoles([role]))
+                guild_roles = guild_roles.union(RolesSet([role]))
         return roles
 
     def match_or_create_role_from_name(
-        self, guild_id: int, role_name: str, guild_roles: DiscordRoles = None
+        self, guild_id: int, role_name: str, guild_roles: RolesSet = None
-    ) -> tuple:
+    ) -> Tuple[Role, bool]:
-        """returns Discord role matching the given name
+        """Fetch or create 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
 
-        Params:
+        Args:
-        - guild_id: ID of guild
+            guild_id: ID of guild
-        - role_name: strings defining name of a role
+            role_name: strings defining name of a role
-        - guild_roles: All known guild roles as DiscordRoles object.
+            guild_roles: All known guild roles as RolesSet object.
-        Helps to void redundant lookups of guild roles
+                Helps to void redundant lookups of guild roles
-        when this method is used multiple times.
+                when this method is used multiple times.
+
+        Returns:
+            Tuple of Role and created flag
         """
         if not isinstance(role_name, str):
             raise TypeError('role_name must be of type string')
 
         created = False
         if guild_roles is None:
-            guild_roles = DiscordRoles(self.guild_roles(guild_id))
+            guild_roles = RolesSet(self.guild_roles(guild_id))
         role = guild_roles.role_by_name(role_name)
         if not role:
             if not DISCORD_DISABLE_ROLE_CREATION:
@@ -345,9 +391,24 @@ class DiscordClient:
                 created = True
             else:
                 role = None
-
         return role, created
 
+    def match_or_create_roles_from_names_2(
+        self, guild_id: int, role_names: Iterable[str]
+    ) -> RolesSet:
+        """Fetch or create Discord role matching the given name.
+
+        Wrapper for ``match_or_create_role_from_name()``
+
+        Returns:
+            Roles as RolesSet object.
+        """
+        return RolesSet.create_from_matched_roles(
+            self.match_or_create_roles_from_names(
+                guild_id=guild_id, role_names=role_names
+            )
+        )
+
     # guild members
 
     def add_guild_member(
@@ -357,13 +418,13 @@ class DiscordClient:
         access_token: str,
         role_ids: list = None,
         nick: str = None
-    ) -> bool:
+    ) -> Optional[bool]:
-        """Adds a user to the guilds.
+        """Adds a user to the guild.
 
         Returns:
-        - True when a new user was added
+            - True when a new user was added
-        - None if the user already existed
+            - None if the user already existed
-        - False when something went wrong or raises exception
+            - False when something went wrong or raises exception
         """
         route = f"guilds/{guild_id}/members/{user_id}"
         data = {
@@ -371,42 +432,49 @@ class DiscordClient:
         }
         if role_ids:
             data['roles'] = self._sanitize_role_ids(role_ids)
-
         if nick:
-            data['nick'] = str(nick)[:self._NICK_MAX_CHARS]
+            data['nick'] = GuildMember.sanitize_nick(nick)
-
         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
-            return False
 
-    def guild_member(self, guild_id: int, user_id: int) -> dict:
+    def guild_member(self, guild_id: int, user_id: int) -> Optional[GuildMember]:
-        """returns the user info for a guild member
+        """Fetch info for a guild member.
 
-        or None if the user is not a member of the guild
+        Args:
+            guild_id: Discord ID of the guild
+            user_id: Discord ID of the user
+
+        Returns:
+            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()
-            r.raise_for_status()
+        return GuildMember.from_dict(r.json())
-            return r.json()
 
     def modify_guild_member(
-        self, guild_id: int, user_id: int, role_ids: list = None, nick: str = None
+        self, guild_id: int, user_id: int, role_ids: List[int] = None, nick: str = None
-    ) -> bool:
+    ) -> Optional[bool]:
-        """Modify attributes of a guild member.
+        """Set properties of a guild member.
+
+        Args:
+            guild_id: Discord ID of the guild
+            user_id: Discord ID of the user
+            roles_id: New list of role IDs (if provided)
+            nick: New nickname (if provided)
 
         Returns
-        - True when successful
+            - True when successful
-        - None if user is not a member of this guild
+            - None if user is not a member of this guild
-        - False otherwise
+            - False otherwise
         """
         if not role_ids and not nick:
             raise ValueError('Must specify role_ids or nick')
@@ -419,7 +487,7 @@ class DiscordClient:
             data['roles'] = self._sanitize_role_ids(role_ids)
 
         if nick:
-            data['nick'] = self._sanitize_nick(nick)
+            data['nick'] = GuildMember.sanitize_nick(nick)
 
         route = f"guilds/{guild_id}/members/{user_id}"
         r = self._api_request(
@@ -428,21 +496,22 @@ class DiscordClient:
         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()
-            r.raise_for_status()
-
         if r.status_code == 204:
             return True
-        else:
+        return False
-            return False
 
-    def remove_guild_member(self, guild_id: int, user_id: int) -> bool:
+    def remove_guild_member(self, guild_id: int, user_id: int) -> Optional[bool]:
-        """Remove a member from a guild
+        """Remove a member from a guild.
+
+        Args:
+            guild_id: Discord ID of the guild
+            user_id: Discord ID of the user
 
         Returns:
-        - True when successful
+            - True when successful
-        - None if member does not exist
+            - None if member does not exist
-        - False otherwise
+            - False otherwise
         """
         route = f"guilds/{guild_id}/members/{user_id}"
         r = self._api_request(
@@ -451,19 +520,16 @@ class DiscordClient:
         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()
-            r.raise_for_status()
-
         if r.status_code == 204:
             return True
-        else:
+        return False
-            return False
 
     # Guild member roles
 
     def add_guild_member_role(
         self, guild_id: int, user_id: int, role_id: int
-    ) -> bool:
+    ) -> Optional[bool]:
         """Adds a role to a guild member
 
         Returns:
@@ -476,43 +542,69 @@ class DiscordClient:
         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()
-            r.raise_for_status()
-
         if r.status_code == 204:
             return True
-        else:
+        return False
-            return False
 
     def remove_guild_member_role(
         self, guild_id: int, user_id: int, role_id: int
-    ) -> bool:
+    ) -> Optional[bool]:
-        """Removes a role to a guild member
+        """Remove a role to a guild member
+
+        Args:
+            guild_id: Discord ID of the guild
+            user_id: Discord ID of the user
+            role_id: Discord ID of role to be removed
 
         Returns:
-        - True when successful
+            - True when successful
-        - None if member does not exist
+            - None if member does not exist
-        - False otherwise
+            - 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()
-            r.raise_for_status()
-
         if r.status_code == 204:
             return True
-        else:
+        return False
-            return False
+
+    def guild_member_roles(self, guild_id: int, user_id: int) -> Optional[RolesSet]:
+        """Fetch the current guild roles of a guild member.
+
+        Args:
+        - guild_id: Discord guild ID
+        - user_id: Discord user ID
+
+        Returns:
+        - Member roles
+        - None if user is not a member of the guild
+        """
+        member_info = self.guild_member(guild_id=guild_id, user_id=user_id)
+        if member_info is None:
+            return None  # User is no longer a member
+        guild_roles = RolesSet(self.guild_roles(guild_id=guild_id))
+        logger.debug('Current guild roles: %s', guild_roles.ids())
+        if not guild_roles.has_roles(member_info.roles):
+            guild_roles = RolesSet(
+                self.guild_roles(guild_id=guild_id, use_cache=False)
+            )
+            if not guild_roles.has_roles(member_info.roles):
+                role_ids = set(member_info.roles).difference(guild_roles.ids())
+                raise RuntimeError(
+                    f'Discord user {user_id} has unknown roles: {role_ids}'
+                )
+        return guild_roles.subset(member_info.roles)
 
     @classmethod
     def _is_member_unknown_error(cls, r: requests.Response) -> bool:
         try:
             result = (
-                r.status_code == cls._HTTP_STATUS_CODE_NOT_FOUND
+                r.status_code == HTTPStatus.NOT_FOUND
-                and r.json()['code'] == cls._DISCORD_STATUS_CODE_UNKNOWN_MEMBER
+                and r.json()['code'] == DiscordApiStatusCode.UNKNOWN_MEMBER
             )
         except (ValueError, KeyError):
             result = False
@@ -529,7 +621,19 @@ class DiscordClient:
         authorization: str = None,
         raise_for_status: bool = True
     ) -> requests.Response:
-        """Core method for performing all API calls"""
+        """Core method for performing all API calls.
+
+        Args:
+            method: HTTP method of the request, e.g. "get"
+            route: Route in the Discord API, e.g. "users/@me"
+            data: Data to be send with the request
+            authorization: The authorization string to be used.
+                Will use the default bot token if not set.
+            raise_for_status: Whether a requests exception is to be raised when not ok
+
+        Returns:
+            The raw response from the API
+        """
         uid = uuid1().hex
 
         if not hasattr(requests, method):
@@ -577,7 +681,7 @@ class DiscordClient:
                 r.text
             )
 
-        if r.status_code == self._HTTP_STATUS_CODE_RATE_LIMITED:
+        if r.status_code == HTTPStatus.TOO_MANY_REQUESTS:
             self._handle_new_api_backoff(r, uid)
 
         self._report_rate_limit_from_api(r, uid)
@@ -588,9 +692,10 @@ class DiscordClient:
         return r
 
     def _handle_ongoing_api_backoff(self, uid: str) -> None:
-        """checks if api is currently on backoff
+        """Check if api is currently on backoff.
-        if on backoff: will do a blocking wait if it expires soon,
+
-        else raises exception
+        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:
@@ -610,8 +715,9 @@ class DiscordClient:
                 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
+        """Ensures that the rate limit is not exhausted.
-        if exhausted: will do a blocking wait if rate limit resets soon,
+
+        If exhausted: will do a blocking wait if rate limit resets soon,
         else raises exception
 
         returns requests remaining on success
@@ -654,10 +760,10 @@ class DiscordClient:
                 )
                 raise DiscordRateLimitExhausted(resets_in)
 
-        raise RuntimeError('Failed to handle rate limit after after too tries.')
+        raise RuntimeError('Failed to handle rate limit after after too many tries.')
 
     def _handle_new_api_backoff(self, r: requests.Response, uid: str) -> None:
-        """raises exception for new API backoff error"""
+        """Raise exception for new API backoff error."""
         response = r.json()
         if 'retry_after' in response:
             try:
@@ -679,8 +785,8 @@ class DiscordClient:
         )
         raise DiscordTooManyRequestsError(retry_after=retry_after)
 
-    def _report_rate_limit_from_api(self, r, uid):
+    def _report_rate_limit_from_api(self, r, uid) -> None:
-        """Tries to log the current rate limit reported from API"""
+        """Try to log the current rate limit reported from API."""
         if (
             logger.getEffectiveLevel() <= logging.DEBUG
             and 'x-ratelimit-limit' in r.headers
@@ -703,22 +809,17 @@ class DiscordClient:
 
     @staticmethod
     def _redis_decode(value: str) -> str:
-        """Decodes a string from Redis and passes through None and Booleans"""
+        """Decode 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
-            return value
 
     @staticmethod
     def _generate_hash(key: str) -> str:
+        """Generate hash key for given string."""
         return md5(key.encode('utf-8')).hexdigest()
 
     @staticmethod
-    def _sanitize_role_ids(role_ids: list) -> list:
+    def _sanitize_role_ids(role_ids: Iterable[int]) -> List[int]:
-        """make sure its a list of integers"""
+        """Sanitize a list of role IDs, i.e. make sure its a list of unique integers."""
-        return [int(role_id) for role_id in list(role_ids)]
+        return [int(role_id) for role_id in set(role_ids)]
-
-    @classmethod
-    def _sanitize_nick(cls, nick: str) -> str:
-        """shortens too long strings if necessary"""
-        return str(nick)[:cls._NICK_MAX_CHARS]