From 5bde9a6952a6d73fc069b0722b8c47946999cf1a Mon Sep 17 00:00:00 2001 From: Ariel Rin Date: Thu, 12 May 2022 18:54:22 +1000 Subject: [PATCH 01/10] Version Bump 2.12.0 --- allianceauth/__init__.py | 2 +- docker/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/allianceauth/__init__.py b/allianceauth/__init__.py index 6c5eb599..25d5865f 100644 --- a/allianceauth/__init__.py +++ b/allianceauth/__init__.py @@ -1,7 +1,7 @@ # This will make sure the app is always imported when # Django starts so that shared_task will use this app. -__version__ = '2.11.2' +__version__ = '2.12.0' __title__ = 'Alliance Auth' __url__ = 'https://gitlab.com/allianceauth/allianceauth' NAME = f'{__title__} v{__version__}' diff --git a/docker/Dockerfile b/docker/Dockerfile index 82d305c9..e7aa8f7f 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ FROM python:3.9-slim -ARG AUTH_VERSION=2.11.1 +ARG AUTH_VERSION=2.12.0 ARG AUTH_PACKAGE=allianceauth==${AUTH_VERSION} ENV VIRTUAL_ENV=/opt/venv ENV AUTH_USER=allianceauth From 37b9f5c88258d87d478d70c2abacde74ff1f9783 Mon Sep 17 00:00:00 2001 From: Ariel Rin Date: Thu, 12 May 2022 13:33:17 +0000 Subject: [PATCH 02/10] Merge branch 'fix-decimal_widthratio-template-tag' into 'v3.x' [FIX] Division by zero in decimal_widthratio template tag See merge request allianceauth/allianceauth!1419 (cherry picked from commit 4836559abe800e8d0f5048502b5cdbcd37d14f8b) 8dd07b97 [FIX] Devision by zero in decimal_widthratio template tag 17b06c88 Make it a string in accordance to the return value type --- allianceauth/templatetags/admin_status.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/allianceauth/templatetags/admin_status.py b/allianceauth/templatetags/admin_status.py index 73ee909b..b673e913 100644 --- a/allianceauth/templatetags/admin_status.py +++ b/allianceauth/templatetags/admin_status.py @@ -38,6 +38,9 @@ logger = logging.getLogger(__name__) @register.simple_tag() def decimal_widthratio(this_value, max_value, max_width) -> str: + if max_value == 0: + return str(0) + return str(round(this_value/max_value * max_width, 2)) From dd42b807f0a614e049867495ffa00d7bd93a72b0 Mon Sep 17 00:00:00 2001 From: Ariel Rin Date: Fri, 13 May 2022 00:19:45 +1000 Subject: [PATCH 03/10] Version Bump 2.12.1 --- allianceauth/__init__.py | 2 +- docker/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/allianceauth/__init__.py b/allianceauth/__init__.py index 25d5865f..ff092fe9 100644 --- a/allianceauth/__init__.py +++ b/allianceauth/__init__.py @@ -1,7 +1,7 @@ # This will make sure the app is always imported when # Django starts so that shared_task will use this app. -__version__ = '2.12.0' +__version__ = '2.12.1' __title__ = 'Alliance Auth' __url__ = 'https://gitlab.com/allianceauth/allianceauth' NAME = f'{__title__} v{__version__}' diff --git a/docker/Dockerfile b/docker/Dockerfile index e7aa8f7f..71fae64d 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ FROM python:3.9-slim -ARG AUTH_VERSION=2.12.0 +ARG AUTH_VERSION=2.12.1 ARG AUTH_PACKAGE=allianceauth==${AUTH_VERSION} ENV VIRTUAL_ENV=/opt/venv ENV AUTH_USER=allianceauth From bf1b4bb54986754475ea454600ca80d8daa334aa Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Mon, 6 Jun 2022 10:48:16 +0000 Subject: [PATCH 04/10] Fix: Broken docs generation on readthedocs.org --- .gitlab-ci.yml | 6 ++++++ .../development/dev_setup/aa-dev-setup-wsl-vsc-v2.md | 12 +++++++----- docs/requirements.txt | 4 +++- tox.ini | 10 +++++++++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index dea93d1d..6b8a81ac 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -168,6 +168,12 @@ test-3.11-all: path: coverage.xml allow_failure: true +test-docs: + <<: *only-default + image: python:3.9-bullseye + script: + - tox -e docs + deploy_production: stage: deploy image: python:3.10-bullseye diff --git a/docs/development/dev_setup/aa-dev-setup-wsl-vsc-v2.md b/docs/development/dev_setup/aa-dev-setup-wsl-vsc-v2.md index 4a74d90c..2ae86e90 100644 --- a/docs/development/dev_setup/aa-dev-setup-wsl-vsc-v2.md +++ b/docs/development/dev_setup/aa-dev-setup-wsl-vsc-v2.md @@ -150,12 +150,14 @@ sudo redis-server --daemonize yes ```eval_rst .. note:: - WSL does not have an init.d service, so it will not automatically start your services such as MySQL and Redis when you boot your Windows machine. For convenience we recommend putting the commands for starting these services in a bash script. Here is an example: :: + WSL does not have an init.d service, so it will not automatically start your services such as MySQL and Redis when you boot your Windows machine. For convenience we recommend putting the commands for starting these services in a bash script. Here is an example: - #/bin/bash - # start services for AA dev - sudo service mysql start - sudo redis-server --daemonize yes + :: + + #/bin/bash + # start services for AA dev + sudo service mysql start + sudo redis-server --daemonize yes In addition it is possible to configure Windows to automatically start WSL services, but that procedure goes beyond the scopes of this guide. ``` diff --git a/docs/requirements.txt b/docs/requirements.txt index b187b2e6..8b49539b 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -2,13 +2,15 @@ sphinx>=3.2.1,<4.0.0 sphinx_rtd_theme==0.5.0 recommonmark==0.6.0 +Jinja2<3.1 # Autodoc dependencies django>=3.2,<4.0.0 django-celery-beat>=2.0.0 +django-redis-cache django-bootstrap-form django-sortedm2m -django-esi>=3,<4 +django-esi>=3,<5 celery>5,<6 celery_once passlib diff --git a/tox.ini b/tox.ini index ea70747d..39896d2b 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,7 @@ [tox] skipsdist = true usedevelop = true -envlist = py{37,38,39,310,311}-{all,core} +envlist = py{37,38,39,310,311}-{all,core}, docs [testenv] setenv = @@ -21,3 +21,11 @@ commands = core: coverage run runtests.py allianceauth.authentication.tests.test_app_settings -v 2 --debug-mode all: coverage report -m all: coverage xml + +[testenv:docs] +description = invoke sphinx-build to build the HTML docs +basepython = python3.9 +deps = -r{toxinidir}/docs/requirements.txt +install_command = +commands = + sphinx-build -T -E -b html -d "{toxworkdir}/docs_doctree" -D language=en docs "{toxworkdir}/docs_out" {posargs} From 1e029af83adee84dfb3ae7e494ab9a9ee92ca439 Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Fri, 17 Jun 2022 11:58:05 +0000 Subject: [PATCH 05/10] Add example for template tags to docs --- docs/development/tech_docu/templatetags.rst | 24 +++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/development/tech_docu/templatetags.rst b/docs/development/tech_docu/templatetags.rst index 1437c311..7c8aa919 100644 --- a/docs/development/tech_docu/templatetags.rst +++ b/docs/development/tech_docu/templatetags.rst @@ -1,15 +1,27 @@ -============= -Template Tags -============= +======================= +Template tags & filters +======================= -The following template tags are available to be used by all apps. To use them just load the respeetive template tag in your template like so: +The following template tags and filters are available to be used by all apps. To use them just load them into your template like so: -.. code-block:: html +.. code-block:: html+django {% load evelinks %} + +Template Filters +================ + evelinks -======== +-------- + +Example for using an evelinks filter to render an alliance logo: + + +.. code-block:: html+django + + + .. automodule:: allianceauth.eveonline.templatetags.evelinks :members: From 919768c8bb3b5f1ab0cf084599c527eacaf82a29 Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Fri, 17 Jun 2022 11:58:45 +0000 Subject: [PATCH 06/10] Fix: Broken docs generation on readthedocs.org (2nd attempt) --- .../task_statistics/event_series.py | 49 ++++++++++++++++--- .../tests/test_event_series.py | 37 +++++++++++++- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/allianceauth/authentication/task_statistics/event_series.py b/allianceauth/authentication/task_statistics/event_series.py index f8591fbc..d1a222db 100644 --- a/allianceauth/authentication/task_statistics/event_series.py +++ b/allianceauth/authentication/task_statistics/event_series.py @@ -1,27 +1,62 @@ import datetime as dt -from typing import Optional, List +import logging +from typing import List, Optional -from redis import Redis from pytz import utc +from redis import Redis, RedisError from django.core.cache import cache +logger = logging.getLogger(__name__) + + +class _RedisStub: + """Stub of a Redis client. + + It's purpose is to prevent EventSeries objects from trying to access Redis + when it is not available. e.g. when the Sphinx docs are rendered by readthedocs.org. + """ + + def delete(self, *args, **kwargs): + pass + + def incr(self, *args, **kwargs): + return 0 + + def zadd(self, *args, **kwargs): + pass + + def zcount(self, *args, **kwargs): + pass + + def zrangebyscore(self, *args, **kwargs): + pass + class EventSeries: - """API for recording and analysing a series of events.""" + """API for recording and analyzing a series of events.""" _ROOT_KEY = "ALLIANCEAUTH_EVENT_SERIES" def __init__(self, key_id: str, redis: Redis = None) -> None: self._redis = cache.get_master_client() 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." + try: + if not self._redis.ping(): + raise RuntimeError() + except (AttributeError, RedisError, RuntimeError): + logger.exception( + "Failed to establish a connection with Redis. " + "This EventSeries object is disabled.", ) + 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" diff --git a/allianceauth/authentication/task_statistics/tests/test_event_series.py b/allianceauth/authentication/task_statistics/tests/test_event_series.py index 079c4721..30b61465 100644 --- a/allianceauth/authentication/task_statistics/tests/test_event_series.py +++ b/allianceauth/authentication/task_statistics/tests/test_event_series.py @@ -1,13 +1,48 @@ 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 +from allianceauth.authentication.task_statistics.event_series import ( + EventSeries, + _RedisStub, +) + +MODULE_PATH = "allianceauth.authentication.task_statistics.event_series" class TestEventSeries(TestCase): + def test_should_abort_without_redis_client(self): + # when + with patch(MODULE_PATH + ".cache.get_master_client") as mock: + mock.return_value = None + events = EventSeries("dummy") + # then + self.assertTrue(events._redis, _RedisStub) + self.assertTrue(events.is_disabled) + + def test_should_disable_itself_if_redis_not_available_1(self): + # when + with patch(MODULE_PATH + ".cache.get_master_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_disable_itself_if_redis_not_available_2(self): + # when + with patch(MODULE_PATH + ".cache.get_master_client") as mock_get_master_client: + mock_get_master_client.return_value.ping.return_value = False + events = EventSeries("dummy") + # then + self.assertIsInstance(events._redis, _RedisStub) + self.assertTrue(events.is_disabled) + def test_should_add_event(self): # given events = EventSeries("dummy") From 0b6af014fae3dc5cd0a6c4dc922452ce9785fe99 Mon Sep 17 00:00:00 2001 From: ErikKalkoken Date: Fri, 17 Jun 2022 21:49:18 +0200 Subject: [PATCH 07/10] Add automatic dark mode to docs --- docs/_static/css/rtd_dark.css | 114 ++++++++++++++++++++++++++++++++++ docs/conf.py | 3 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 docs/_static/css/rtd_dark.css diff --git a/docs/_static/css/rtd_dark.css b/docs/_static/css/rtd_dark.css new file mode 100644 index 00000000..0a8e9840 --- /dev/null +++ b/docs/_static/css/rtd_dark.css @@ -0,0 +1,114 @@ +/*! + * @name Readthedocs + * @namespace http://userstyles.org + * @description Styles the documentation pages hosted on Readthedocs.io + * @author Anthony Post + * @homepage https://userstyles.org/styles/142968 + * @version 0.20170529055029 + * + * Modified by Aloïs Dreyfus: 20200527-1037 + * Modified by Erik Kalkoken: 20220615 + */ + +@media (prefers-color-scheme: dark) { + a:visited { + color: #bf84d8; + } + + pre { + background-color: #2d2d2d !important; + } + + .wy-nav-content { + background: #3c3c3c; + color: aliceblue; + } + + .method dt, .class dt, .data dt, .attribute dt, .function dt, + .descclassname, .descname { + background-color: #525252 !important; + color: white !important; + } + + .toc-backref { + color: grey !important; + } + + code.literal { + background-color: #2d2d2d !important; + border: 1px solid #6d6d6d !important; + } + + .wy-nav-content-wrap { + background-color: rgba(0, 0, 0, 0.6) !important; + } + + .sidebar { + background-color: #191919 !important; + } + + .sidebar-title { + background-color: #2b2b2b !important; + } + + .xref, .py-meth { + color: #7ec3e6 !important; + } + + .admonition, .note { + background-color: #2d2d2d !important; + } + + .wy-side-nav-search { + background-color: inherit; + border-bottom: 1px solid #fcfcfc; + } + + .wy-table thead, .rst-content table.docutils thead, .rst-content table.field-list thead { + background-color: #b9b9b9; + } + + .wy-table thead th, .rst-content table.docutils thead th, .rst-content table.field-list thead th { + border: solid 2px #e1e4e5; + } + + .wy-table thead p, .rst-content table.docutils thead p, .rst-content table.field-list thead p { + margin: 0; + } + + .wy-table-odd td, .wy-table-striped tr:nth-child(2n-1) td, .rst-content table.docutils:not(.field-list) tr:nth-child(2n-1) td { + background-color: #343131; + } + + .highlight .m { + color: inherit + } + + /* Literal.Number */ + .highlight .nv { + color: #3a7ca8 + } + + /* Name.Variable */ + + body { + text-align: justify; + } + + .rst-content .section .admonition ul { + margin-bottom: 0; + } + + li.toctree-l1 { + margin-top: 5px; + margin-bottom: 5px; + } + + .wy-menu-vertical li code { + color: #E74C3C; + } + + .wy-menu-vertical .xref { + color: #2980B9 !important; + } +} diff --git a/docs/conf.py b/docs/conf.py index de813c7e..64cb80f0 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -60,7 +60,7 @@ master_doc = 'index' # General information about the project. project = 'Alliance Auth' -copyright = '2018-2020, Alliance Auth' +copyright = '2018-2022, Alliance Auth' author = 'R4stl1n' # The version info for the project you're documenting, acts as replacement for @@ -111,6 +111,7 @@ html_theme_options = { # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". html_static_path = ['_static'] +html_css_files = ["css/rtd_dark.css"] # -- Options for HTMLHelp output ------------------------------------------ From 84ad571aa486876ddd2066fbd112cf63d91c5a9f Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Sat, 18 Jun 2022 02:41:23 +0000 Subject: [PATCH 08/10] Fix: Service group updates broken when adding users to groups --- allianceauth/services/signals.py | 39 ++++--- allianceauth/services/tasks.py | 17 +++ allianceauth/services/tests/test_signals.py | 121 +++++++++++++++----- allianceauth/services/tests/test_tasks.py | 30 ++++- 4 files changed, 154 insertions(+), 53 deletions(-) diff --git a/allianceauth/services/signals.py b/allianceauth/services/signals.py index 34124a86..773f8ab6 100644 --- a/allianceauth/services/signals.py +++ b/allianceauth/services/signals.py @@ -1,4 +1,5 @@ import logging +from functools import partial from django.contrib.auth.models import User, Group, Permission from django.core.exceptions import ObjectDoesNotExist @@ -8,7 +9,7 @@ from django.db.models.signals import pre_delete from django.db.models.signals import pre_save from django.dispatch import receiver from .hooks import ServicesHook -from .tasks import disable_user +from .tasks import disable_user, update_groups_for_user from allianceauth.authentication.models import State, UserProfile from allianceauth.authentication.signals import state_changed @@ -19,21 +20,27 @@ logger = logging.getLogger(__name__) @receiver(m2m_changed, sender=User.groups.through) def m2m_changed_user_groups(sender, instance, action, *args, **kwargs): - logger.debug(f"Received m2m_changed from {instance} groups with action {action}") - - def trigger_service_group_update(): - logger.debug("Triggering service group update for %s" % instance) - # Iterate through Service hooks - for svc in ServicesHook.get_services(): - try: - svc.validate_user(instance) - svc.update_groups(instance) - except: - logger.exception(f'Exception running update_groups for services module {svc} on user {instance}') - - if instance.pk and (action == "post_add" or action == "post_remove" or action == "post_clear"): - logger.debug("Waiting for commit to trigger service group update for %s" % instance) - transaction.on_commit(trigger_service_group_update) + logger.debug( + "%s: Received m2m_changed from groups with action %s", instance, action + ) + if instance.pk and ( + action == "post_add" or action == "post_remove" or action == "post_clear" + ): + if isinstance(instance, User): + logger.debug( + "Waiting for commit to trigger service group update for %s", instance + ) + transaction.on_commit(partial(update_groups_for_user.delay, instance.pk)) + elif ( + isinstance(instance, Group) + and kwargs.get("model") is User + and "pk_set" in kwargs + ): + for user_pk in kwargs["pk_set"]: + logger.debug( + "%s: Waiting for commit to trigger service group update for user", user_pk + ) + transaction.on_commit(partial(update_groups_for_user.delay, user_pk)) @receiver(m2m_changed, sender=User.user_permissions.through) diff --git a/allianceauth/services/tasks.py b/allianceauth/services/tasks.py index cac663c6..58947577 100644 --- a/allianceauth/services/tasks.py +++ b/allianceauth/services/tasks.py @@ -47,3 +47,20 @@ def disable_user(user): for svc in ServicesHook.get_services(): if svc.service_active_for_user(user): svc.delete_user(user) + + +@shared_task +def update_groups_for_user(user_pk: int) -> None: + """Update groups for all services registered to a user.""" + user = User.objects.get(pk=user_pk) + logger.debug("%s: Triggering service group update for user", user) + for svc in ServicesHook.get_services(): + try: + svc.validate_user(user) + svc.update_groups(user) + except Exception: + logger.exception( + 'Exception running update_groups for services module %s on user %s', + svc, + user + ) diff --git a/allianceauth/services/tests/test_signals.py b/allianceauth/services/tests/test_signals.py index 7bd0c59e..e8cfdc57 100644 --- a/allianceauth/services/tests/test_signals.py +++ b/allianceauth/services/tests/test_signals.py @@ -1,7 +1,7 @@ from copy import deepcopy from unittest import mock -from django.test import TestCase +from django.test import override_settings, TestCase, TransactionTestCase from django.contrib.auth.models import Group, Permission from allianceauth.authentication.models import State @@ -9,6 +9,9 @@ from allianceauth.eveonline.models import EveCharacter from allianceauth.tests.auth_utils import AuthUtils +MODULE_PATH = 'allianceauth.services.signals' + + class ServicesSignalsTestCase(TestCase): def setUp(self): self.member = AuthUtils.create_user('auth_member', disconnect_signals=True) @@ -17,17 +20,12 @@ class ServicesSignalsTestCase(TestCase): ) self.none_user = AuthUtils.create_user('none_user', disconnect_signals=True) - @mock.patch('allianceauth.services.signals.transaction') - @mock.patch('allianceauth.services.signals.ServicesHook') - def test_m2m_changed_user_groups(self, services_hook, transaction): + @mock.patch(MODULE_PATH + '.transaction', spec=True) + @mock.patch(MODULE_PATH + '.update_groups_for_user', spec=True) + def test_m2m_changed_user_groups(self, update_groups_for_user, transaction): """ Test that update_groups hook function is called on user groups change """ - svc = mock.Mock() - svc.update_groups.return_value = None - svc.validate_user.return_value = None - - services_hook.get_services.return_value = [svc] # Overload transaction.on_commit so everything happens synchronously transaction.on_commit = lambda fn: fn() @@ -39,17 +37,11 @@ class ServicesSignalsTestCase(TestCase): self.member.save() # Assert - self.assertTrue(services_hook.get_services.called) + self.assertTrue(update_groups_for_user.delay.called) + args, _ = update_groups_for_user.delay.call_args + self.assertEqual(self.member.pk, args[0]) - self.assertTrue(svc.update_groups.called) - args, kwargs = svc.update_groups.call_args - self.assertEqual(self.member, args[0]) - - self.assertTrue(svc.validate_user.called) - args, kwargs = svc.validate_user.call_args - self.assertEqual(self.member, args[0]) - - @mock.patch('allianceauth.services.signals.disable_user') + @mock.patch(MODULE_PATH + '.disable_user') def test_pre_delete_user(self, disable_user): """ Test that disable_member is called when a user is deleted @@ -60,7 +52,7 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = disable_user.call_args self.assertEqual(self.none_user, args[0]) - @mock.patch('allianceauth.services.signals.disable_user') + @mock.patch(MODULE_PATH + '.disable_user') def test_pre_save_user_inactivation(self, disable_user): """ Test a user set inactive has disable_member called @@ -72,7 +64,7 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = disable_user.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.disable_user') + @mock.patch(MODULE_PATH + '.disable_user') def test_disable_services_on_loss_of_main_character(self, disable_user): """ Test a user set inactive has disable_member called @@ -84,8 +76,8 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = disable_user.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.transaction') - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.transaction') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_m2m_changed_group_permissions(self, services_hook, transaction): from django.contrib.contenttypes.models import ContentType svc = mock.Mock() @@ -116,8 +108,8 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = svc.validate_user.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.transaction') - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.transaction') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_m2m_changed_user_permissions(self, services_hook, transaction): from django.contrib.contenttypes.models import ContentType svc = mock.Mock() @@ -145,8 +137,8 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = svc.validate_user.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.transaction') - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.transaction') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_m2m_changed_user_state_permissions(self, services_hook, transaction): from django.contrib.contenttypes.models import ContentType svc = mock.Mock() @@ -180,7 +172,7 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = svc.validate_user.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_state_changed_services_validation_and_groups_update(self, services_hook): """Test a user changing state has service accounts validated and groups updated """ @@ -206,8 +198,7 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = svc.update_groups.call_args self.assertEqual(self.member, args[0]) - - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_state_changed_services_validation_and_groups_update_1(self, services_hook): """Test a user changing main has service accounts validated and sync updated """ @@ -238,7 +229,7 @@ class ServicesSignalsTestCase(TestCase): args, kwargs = svc.sync_nickname.call_args self.assertEqual(self.member, args[0]) - @mock.patch('allianceauth.services.signals.ServicesHook') + @mock.patch(MODULE_PATH + '.ServicesHook') def test_state_changed_services_validation_and_groups_update_2(self, services_hook): """Test a user changing main has service does not have accounts validated and sync updated if the new main is equal to the old main @@ -260,3 +251,71 @@ class ServicesSignalsTestCase(TestCase): self.assertFalse(services_hook.get_services.called) self.assertFalse(svc.validate_user.called) self.assertFalse(svc.sync_nickname.called) + + +@mock.patch( + "allianceauth.services.modules.mumble.auth_hooks.MumbleService.update_groups" +) +@override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True) +class TestUserGroupBulkUpdate(TransactionTestCase): + def test_should_run_user_service_check_when_group_added_to_user( + self, mock_update_groups + ): + # given + user = AuthUtils.create_user("Bruce Wayne") + AuthUtils.add_main_character_2(user, "Bruce Wayne", 1001) + group = Group.objects.create(name="Group") + mock_update_groups.reset_mock() + # when + user.groups.add(group) + # then + users_updated = {obj[0][0] for obj in mock_update_groups.call_args_list} + self.assertSetEqual(users_updated, {user}) + + def test_should_run_user_service_check_when_multiple_groups_are_added_to_user( + self, mock_update_groups + ): + # given + user = AuthUtils.create_user("Bruce Wayne") + AuthUtils.add_main_character_2(user, "Bruce Wayne", 1001) + group_1 = Group.objects.create(name="Group 1") + group_2 = Group.objects.create(name="Group 2") + mock_update_groups.reset_mock() + # when + user.groups.add(group_1, group_2) + # then + users_updated = {obj[0][0] for obj in mock_update_groups.call_args_list} + self.assertSetEqual(users_updated, {user}) + + def test_should_run_user_service_check_when_user_added_to_group( + self, mock_update_groups + ): + # given + user = AuthUtils.create_user("Bruce Wayne") + AuthUtils.add_main_character_2(user, "Bruce Wayne", 1001) + group = Group.objects.create(name="Group") + mock_update_groups.reset_mock() + # when + group.user_set.add(user) + # then + users_updated = {obj[0][0] for obj in mock_update_groups.call_args_list} + self.assertSetEqual(users_updated, {user}) + + def test_should_run_user_service_check_when_multiple_users_are_added_to_group( + self, mock_update_groups + ): + # given + user_1 = AuthUtils.create_user("Bruce Wayne") + AuthUtils.add_main_character_2(user_1, "Bruce Wayne", 1001) + user_2 = AuthUtils.create_user("Peter Parker") + AuthUtils.add_main_character_2(user_2, "Peter Parker", 1002) + user_3 = AuthUtils.create_user("Lex Luthor") + AuthUtils.add_main_character_2(user_3, "Lex Luthor", 1011) + group = Group.objects.create(name="Group") + user_1.groups.add(group) + mock_update_groups.reset_mock() + # when + group.user_set.add(user_2, user_3) + # then + users_updated = {obj[0][0] for obj in mock_update_groups.call_args_list} + self.assertSetEqual(users_updated, {user_2, user_3}) diff --git a/allianceauth/services/tests/test_tasks.py b/allianceauth/services/tests/test_tasks.py index 35d9329e..06257a1f 100644 --- a/allianceauth/services/tests/test_tasks.py +++ b/allianceauth/services/tests/test_tasks.py @@ -3,32 +3,50 @@ from unittest import mock from celery_once import AlreadyQueued from django.core.cache import cache -from django.test import TestCase +from django.test import override_settings, TestCase from allianceauth.tests.auth_utils import AuthUtils -from allianceauth.services.tasks import validate_services +from allianceauth.services.tasks import validate_services, update_groups_for_user from ..tasks import DjangoBackend +@override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True) class ServicesTasksTestCase(TestCase): def setUp(self): self.member = AuthUtils.create_user('auth_member') @mock.patch('allianceauth.services.tasks.ServicesHook') def test_validate_services(self, services_hook): + # given svc = mock.Mock() svc.validate_user.return_value = None - services_hook.get_services.return_value = [svc] - + # when validate_services.delay(self.member.pk) - + # then self.assertTrue(services_hook.get_services.called) self.assertTrue(svc.validate_user.called) - args, kwargs = svc.validate_user.call_args + args, _ = svc.validate_user.call_args self.assertEqual(self.member, args[0]) # Assert correct user is passed to service hook function + @mock.patch('allianceauth.services.tasks.ServicesHook') + def test_update_groups_for_user(self, services_hook): + # given + svc = mock.Mock() + svc.validate_user.return_value = None + services_hook.get_services.return_value = [svc] + # when + update_groups_for_user.delay(self.member.pk) + # then + self.assertTrue(services_hook.get_services.called) + self.assertTrue(svc.validate_user.called) + args, _ = svc.validate_user.call_args + self.assertEqual(self.member, args[0]) # Assert correct user + self.assertTrue(svc.update_groups.called) + args, _ = svc.update_groups.call_args + self.assertEqual(self.member, args[0]) # Assert correct user + class TestDjangoBackend(TestCase): From d815028c4dccb232a593475945fc0bc41a633b00 Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Sat, 18 Jun 2022 02:47:14 +0000 Subject: [PATCH 09/10] Fix: Changing group's state setting does not kick existing non-conforming group members --- allianceauth/groupmanagement/admin.py | 54 ++++-- allianceauth/groupmanagement/models.py | 9 + allianceauth/groupmanagement/tasks.py | 10 ++ .../groupmanagement/tests/test_admin.py | 170 ++++++++++++++---- .../groupmanagement/tests/test_models.py | 32 ++++ 5 files changed, 227 insertions(+), 48 deletions(-) create mode 100644 allianceauth/groupmanagement/tasks.py diff --git a/allianceauth/groupmanagement/admin.py b/allianceauth/groupmanagement/admin.py index 3657fc73..d51cf7d2 100644 --- a/allianceauth/groupmanagement/admin.py +++ b/allianceauth/groupmanagement/admin.py @@ -1,8 +1,8 @@ from django.apps import apps from django.contrib import admin -from django.contrib.auth.models import Group as BaseGroup -from django.contrib.auth.models import Permission, User -from django.db.models import Count + +from django.contrib.auth.models import Group as BaseGroup, Permission, User +from django.db.models import Count, Exists, OuterRef from django.db.models.functions import Lower from django.db.models.signals import ( m2m_changed, @@ -15,6 +15,7 @@ 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 @@ -106,14 +107,13 @@ class HasLeaderFilter(admin.SimpleListFilter): 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', @@ -129,31 +129,51 @@ class GroupAdmin(admin.ModelAdmin): def get_queryset(self, request): qs = super().get_queryset(request) - if _has_auto_groups: - qs = qs.prefetch_related('managedalliancegroup_set', 'managedcorpgroup_set') - qs = qs.prefetch_related('authgroup__group_leaders').select_related('authgroup') - qs = qs.annotate( - member_count=Count('user', distinct=True), + has_leader_qs = ( + AuthGroup.objects.filter(group=OuterRef('pk'), group_leaders__isnull=False) ) + 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") + @admin.display(description='Members', ordering='member_count') def _member_count(self, obj): return obj.member_count @admin.display(boolean=True) 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 def _properties(self, obj): properties = list() - if _has_auto_groups and ( - obj.managedalliancegroup_set.exists() - or obj.managedcorpgroup_set.exists() - ): + if _has_auto_groups and (obj.is_autogroup_corp or obj.is_autogroup_alliance): properties.append('Auto Group') elif obj.authgroup.internal: properties.append('Internal') @@ -183,6 +203,8 @@ 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): diff --git a/allianceauth/groupmanagement/models.py b/allianceauth/groupmanagement/models.py index 102f70a7..3ca6b8c6 100644 --- a/allianceauth/groupmanagement/models.py +++ b/allianceauth/groupmanagement/models.py @@ -189,6 +189,15 @@ 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. diff --git a/allianceauth/groupmanagement/tasks.py b/allianceauth/groupmanagement/tasks.py new file mode 100644 index 00000000..ae80f25f --- /dev/null +++ b/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() diff --git a/allianceauth/groupmanagement/tests/test_admin.py b/allianceauth/groupmanagement/tests/test_admin.py index 3b4a50c1..3873fe4c 100644 --- a/allianceauth/groupmanagement/tests/test_admin.py +++ b/allianceauth/groupmanagement/tests/test_admin.py @@ -6,7 +6,7 @@ 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 ( @@ -236,60 +236,104 @@ class TestGroupAdmin(TestCase): self.assertEqual(result, expected) def test_member_count(self): - expected = 1 - obj = self.modeladmin.get_queryset(MockRequest(user=self.user_1))\ - .get(pk=self.group_1.pk) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_1) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_2) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_3) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_4) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_5) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - result = self.modeladmin._properties(self.group_6) - self.assertListEqual(result, expected) + # given + request = MockRequest(user=self.user_1) + 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'] - my_group = Group.objects\ - .filter(managedcorpgroup__isnull=False)\ - .first() - result = self.modeladmin._properties(my_group) - self.assertListEqual(result, expected) + request = MockRequest(user=self.user_1) + queryset = self.modeladmin.get_queryset(request) + obj = queryset.filter(managedcorpgroup__isnull=False).first() + # when + result = self.modeladmin._properties(obj) + # 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 @@ -539,6 +583,68 @@ class TestGroupAdminChangeFormSuperuserExclusiveEdits(WebTest): 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): diff --git a/allianceauth/groupmanagement/tests/test_models.py b/allianceauth/groupmanagement/tests/test_models.py index dcf9a1e2..b9f60bba 100644 --- a/allianceauth/groupmanagement/tests/test_models.py +++ b/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: From 95b309c358e1fe411f368c0c91bd58c7820fbd98 Mon Sep 17 00:00:00 2001 From: Maestro-Zacht Date: Sat, 18 Jun 2022 02:53:11 +0000 Subject: [PATCH 10/10] fixed attribute error --- allianceauth/fleetactivitytracking/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/allianceauth/fleetactivitytracking/views.py b/allianceauth/fleetactivitytracking/views.py index 246842e8..d4e039d1 100644 --- a/allianceauth/fleetactivitytracking/views.py +++ b/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}")