From e1843fe1f1631f6ea1c6c3180e81617e88027c1b Mon Sep 17 00:00:00 2001 From: Erik Kalkoken Date: Sun, 9 May 2021 05:31:45 +0000 Subject: [PATCH] Improve authentication admin --- allianceauth/authentication/admin.py | 115 ++++++---------- allianceauth/authentication/tests/__init__.py | 3 +- .../authentication/tests/test_admin.py | 127 +++--------------- 3 files changed, 61 insertions(+), 184 deletions(-) diff --git a/allianceauth/authentication/admin.py b/allianceauth/authentication/admin.py index 69658f8f..5191981c 100644 --- a/allianceauth/authentication/admin.py +++ b/allianceauth/authentication/admin.py @@ -1,10 +1,8 @@ -from django.conf import settings - from django.contrib import admin from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.models import User as BaseUser, \ Permission as BasePermission, Group -from django.db.models import Q, F +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 @@ -24,11 +22,6 @@ from allianceauth.eveonline.tasks import update_character from .app_settings import AUTHENTICATION_ADMIN_USERS_MAX_GROUPS, \ AUTHENTICATION_ADMIN_USERS_MAX_CHARS -if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS: - _has_auto_groups = True -else: - _has_auto_groups = False - def make_service_hooks_update_groups_action(service): """ @@ -91,8 +84,7 @@ class UserProfileInline(admin.StackedInline): if request.user.is_superuser: query |= Q(userprofile__isnull=True) else: - query |= Q(character_ownership__user=obj) - qs = EveCharacter.objects.filter(query) + query |= Q(character_ownership__user=obj) formset = super().get_formset(request, obj=obj, **kwargs) def get_kwargs(self, index): @@ -121,6 +113,8 @@ def user_profile_pic(obj): ) else: return None + + user_profile_pic.short_description = '' @@ -152,6 +146,7 @@ def user_username(obj): user_obj.username, ) + user_username.short_description = 'user / main' user_username.admin_order_field = 'username' @@ -168,7 +163,8 @@ def user_main_organization(obj): else: corporation = user_obj.profile.main_character.corporation_name if user_obj.profile.main_character.alliance_id: - result = format_html('{}
{}', + result = format_html( + '{}
{}', corporation, user_obj.profile.main_character.alliance_name ) @@ -176,6 +172,7 @@ def user_main_organization(obj): result = corporation return result + user_main_organization.short_description = 'Corporation / Alliance (Main)' user_main_organization.admin_order_field = \ 'profile__main_character__corporation_name' @@ -205,13 +202,13 @@ class MainCorporationsFilter(admin.SimpleListFilter): return qs.all() else: if qs.model == User: - return qs\ - .filter(profile__main_character__corporation_id=\ - self.value()) + return qs.filter( + profile__main_character__corporation_id=self.value() + ) else: - return qs\ - .filter(user__profile__main_character__corporation_id=\ - self.value()) + return qs.filter( + user__profile__main_character__corporation_id=self.value() + ) class MainAllianceFilter(admin.SimpleListFilter): @@ -239,12 +236,11 @@ class MainAllianceFilter(admin.SimpleListFilter): return qs.all() else: if qs.model == User: - return qs\ - .filter(profile__main_character__alliance_id=self.value()) + return qs.filter(profile__main_character__alliance_id=self.value()) else: - return qs\ - .filter(user__profile__main_character__alliance_id=\ - self.value()) + return qs.filter( + user__profile__main_character__alliance_id=self.value() + ) def update_main_character_model(modeladmin, request, queryset): @@ -259,6 +255,7 @@ def update_main_character_model(modeladmin, request, queryset): 'Update from ESI started for {} characters'.format(tasks_count) ) + update_main_character_model.short_description = \ 'Update main character model from ESI' @@ -267,32 +264,16 @@ class UserAdmin(BaseUserAdmin): """Extending Django's UserAdmin model Behavior of groups and characters columns can be configured via settings - """ class Media: css = { "all": ("authentication/css/admin.css",) } - - class RealGroupsFilter(admin.SimpleListFilter): - """Custom filter to get groups w/o Autogroups""" - title = 'group' - parameter_name = 'group_id__exact' - - def lookups(self, request, model_admin): - qs = Group.objects.all().order_by(Lower('name')) - if _has_auto_groups: - qs = qs\ - .filter(managedalliancegroup__isnull=True)\ - .filter(managedcorpgroup__isnull=True) - return tuple([(x.pk, x.name) for x in qs]) - - def queryset(self, request, queryset): - if self.value() is None: - return queryset.all() - else: - return queryset.filter(groups__pk=self.value()) + + def get_queryset(self, request): + qs = super().get_queryset(request) + return qs.prefetch_related("character_ownerships__character", "groups") def get_actions(self, request): actions = super(BaseUserAdmin, self).get_actions(request) @@ -341,11 +322,9 @@ class UserAdmin(BaseUserAdmin): return result inlines = BaseUserAdmin.inlines + [UserProfileInline] - - ordering = ('username', ) - list_select_related = True - show_full_result_count = True - + ordering = ('username', ) + list_select_related = ('profile__state', 'profile__main_character') + show_full_result_count = True list_display = ( user_profile_pic, user_username, @@ -358,10 +337,9 @@ class UserAdmin(BaseUserAdmin): '_role' ) list_display_links = None - list_filter = ( 'profile__state', - RealGroupsFilter, + 'groups', MainCorporationsFilter, MainAllianceFilter, 'is_active', @@ -375,41 +353,25 @@ class UserAdmin(BaseUserAdmin): ) def _characters(self, obj): - my_characters = [ - x.character.character_name - for x in CharacterOwnership.objects\ - .filter(user=obj)\ - .order_by('character__character_name')\ - .select_related() - ] + character_ownerships = list(obj.character_ownerships.all()) + characters = [obj.character.character_name for obj in character_ownerships] return self._list_2_html_w_tooltips( - my_characters, + sorted(characters), AUTHENTICATION_ADMIN_USERS_MAX_CHARS ) _characters.short_description = 'characters' - def _state(self, obj): return obj.profile.state.name _state.short_description = 'state' _state.admin_order_field = 'profile__state' - def _groups(self, obj): - if not _has_auto_groups: - my_groups = [x.name for x in obj.groups.order_by('name')] - else: - my_groups = [ - x.name for x in obj.groups\ - .filter(managedalliancegroup__isnull=True)\ - .filter(managedcorpgroup__isnull=True)\ - .order_by('name') - ] - + 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 + my_groups, AUTHENTICATION_ADMIN_USERS_MAX_GROUPS ) _groups.short_description = 'groups' @@ -446,9 +408,14 @@ class StateAdmin(admin.ModelAdmin): list_select_related = True list_display = ('name', 'priority', '_user_count') + def get_queryset(self, request): + qs = super().get_queryset(request) + return qs.annotate(user_count=Count("userprofile__id")) + def _user_count(self, obj): - return obj.userprofile_set.all().count() + return obj.user_count _user_count.short_description = 'Users' + _user_count.admin_order_field = 'user_count' fieldsets = ( (None, { @@ -504,7 +471,8 @@ class BaseOwnershipAdmin(admin.ModelAdmin): "all": ("authentication/css/admin.css",) } - list_select_related = True + list_select_related = ( + 'user__profile__state', 'user__profile__main_character', 'character') list_display = ( user_profile_pic, user_username, @@ -542,6 +510,7 @@ class CharacterOwnershipAdmin(BaseOwnershipAdmin): class PermissionAdmin(admin.ModelAdmin): actions = None readonly_fields = [field.name for field in BasePermission._meta.fields] + search_fields = ('codename', ) list_display = ('admin_name', 'name', 'codename', 'content_type') list_filter = ('content_type__app_label',) diff --git a/allianceauth/authentication/tests/__init__.py b/allianceauth/authentication/tests/__init__.py index ba343c6d..7c31c2fe 100644 --- a/allianceauth/authentication/tests/__init__.py +++ b/allianceauth/authentication/tests/__init__.py @@ -10,9 +10,10 @@ def get_admin_change_view_url(obj: object) -> str: args=(obj.pk,) ) + def get_admin_search_url(ModelClass: type) -> str: """returns URL to search URL for model of given object""" return '{}{}/'.format( reverse('admin:app_list', args=(ModelClass._meta.app_label,)), ModelClass.__name__.lower() - ) \ No newline at end of file + ) diff --git a/allianceauth/authentication/tests/test_admin.py b/allianceauth/authentication/tests/test_admin.py index 10347afa..b43da12b 100644 --- a/allianceauth/authentication/tests/test_admin.py +++ b/allianceauth/authentication/tests/test_admin.py @@ -1,10 +1,8 @@ from urllib.parse import quote from unittest.mock import patch, MagicMock -from django.conf import settings -from django.contrib import admin from django.contrib.admin.sites import AdminSite -from django.contrib.auth.models import User as BaseUser, Group +from django.contrib.auth.models import Group from django.test import TestCase, RequestFactory, Client from allianceauth.authentication.models import ( @@ -18,8 +16,7 @@ from allianceauth.tests.auth_utils import AuthUtils from ..admin import ( BaseUserAdmin, - CharacterOwnershipAdmin, - PermissionAdmin, + CharacterOwnershipAdmin, StateAdmin, MainCorporationsFilter, MainAllianceFilter, @@ -35,11 +32,6 @@ from ..admin import ( ) from . import get_admin_change_view_url, get_admin_search_url -if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS: - _has_auto_groups = True - from allianceauth.eveonline.autogroups.models import AutogroupsConfig -else: - _has_auto_groups = False MODULE_PATH = 'allianceauth.authentication.admin' @@ -48,6 +40,7 @@ class MockRequest(object): def __init__(self, user=None): self.user = user + class TestCaseWithTestData(TestCase): @classmethod @@ -279,6 +272,7 @@ class TestStateAdmin(TestCaseWithTestData): expected = 200 self.assertEqual(response.status_code, expected) + class TestUserAdmin(TestCaseWithTestData): def setUp(self): @@ -287,24 +281,12 @@ class TestUserAdmin(TestCaseWithTestData): model=User, admin_site=AdminSite() ) self.character_1 = self.user_1.character_ownerships.first().character - - def _create_autogroups(self): - """create autogroups for corps and alliances""" - if _has_auto_groups: - autogroups_config = AutogroupsConfig( - corp_groups = True, - alliance_groups = True - ) - autogroups_config.save() - for state in State.objects.all(): - autogroups_config.states.add(state) - autogroups_config.update_corp_group_membership(self.user_1) - - # column rendering - + def test_user_profile_pic_u1(self): - expected = ('') + expected = ( + '' + ) self.assertEqual(user_profile_pic(self.user_1), expected) def test_user_profile_pic_u3(self): @@ -351,37 +333,17 @@ class TestUserAdmin(TestCaseWithTestData): result = self.modeladmin._characters(self.user_3) self.assertEqual(result, expected) - def test_groups_u1(self): - self._create_autogroups() + def test_groups_u1(self): expected = 'Group 1' result = self.modeladmin._groups(self.user_1) self.assertEqual(result, expected) - def test_groups_u2(self): - self._create_autogroups() + def test_groups_u2(self): expected = 'Group 2' result = self.modeladmin._groups(self.user_2) self.assertEqual(result, expected) - def test_groups_u3(self): - self._create_autogroups() - result = self.modeladmin._groups(self.user_3) - self.assertIsNone(result) - - @patch(MODULE_PATH + '._has_auto_groups', False) - def test_groups_u1_no_autogroups(self): - expected = 'Group 1' - result = self.modeladmin._groups(self.user_1) - self.assertEqual(result, expected) - - @patch(MODULE_PATH + '._has_auto_groups', False) - def test_groups_u2_no_autogroups(self): - expected = 'Group 2' - result = self.modeladmin._groups(self.user_2) - self.assertEqual(result, expected) - - @patch(MODULE_PATH + '._has_auto_groups', False) - def test_groups_u3_no_autogroups(self): + def test_groups_u3(self): result = self.modeladmin._groups(self.user_3) self.assertIsNone(result) @@ -413,8 +375,10 @@ class TestUserAdmin(TestCaseWithTestData): def test_list_2_html_w_tooltips_w_cutoff(self): items = ['one', 'two', 'three'] - expected = ('one, two, (...)') + expected = ( + 'one, two, (...)' + ) result = self.modeladmin._list_2_html_w_tooltips(items, 2) self.assertEqual(expected, result) @@ -439,63 +403,7 @@ class TestUserAdmin(TestCaseWithTestData): self.assertTrue(mock_message_user.called) # filters - - def test_filter_real_groups_with_autogroups(self): - - class UserAdminTest(BaseUserAdmin): - list_filter = (UserAdmin.RealGroupsFilter,) - - self._create_autogroups() - my_modeladmin = UserAdminTest(User, AdminSite()) - - # Make sure the lookups are correct - request = self.factory.get('/') - request.user = self.user_1 - changelist = my_modeladmin.get_changelist_instance(request) - filters = changelist.get_filters(request) - filterspec = filters[0][0] - expected = [ - (self.group_1.pk, self.group_1.name), - (self.group_2.pk, self.group_2.name), - ] - self.assertEqual(filterspec.lookup_choices, expected) - - # Make sure the correct queryset is returned - request = self.factory.get('/', {'group_id__exact': self.group_1.pk}) - request.user = self.user_1 - changelist = my_modeladmin.get_changelist_instance(request) - queryset = changelist.get_queryset(request) - expected = User.objects.filter(groups__in=[self.group_1]) - self.assertSetEqual(set(queryset), set(expected)) - - @patch(MODULE_PATH + '._has_auto_groups', False) - def test_filter_real_groups_no_autogroups(self): - - class UserAdminTest(BaseUserAdmin): - list_filter = (UserAdmin.RealGroupsFilter,) - - my_modeladmin = UserAdminTest(User, AdminSite()) - - # Make sure the lookups are correct - request = self.factory.get('/') - request.user = self.user_1 - changelist = my_modeladmin.get_changelist_instance(request) - filters = changelist.get_filters(request) - filterspec = filters[0][0] - expected = [ - (self.group_1.pk, self.group_1.name), - (self.group_2.pk, self.group_2.name), - ] - self.assertEqual(filterspec.lookup_choices, expected) - - # Make sure the correct queryset is returned - request = self.factory.get('/', {'group_id__exact': self.group_1.pk}) - request.user = self.user_1 - changelist = my_modeladmin.get_changelist_instance(request) - queryset = changelist.get_queryset(request) - expected = User.objects.filter(groups__in=[self.group_1]) - self.assertSetEqual(set(queryset), set(expected)) - + def test_filter_main_corporations(self): class UserAdminTest(BaseUserAdmin): @@ -603,7 +511,6 @@ class TestMakeServicesHooksActions(TestCaseWithTestData): def sync_nicknames_bulk(self, user): pass - def test_service_has_update_groups_only(self): service = self.MyServicesHookTypeA() mock_service = MagicMock(spec=service)