Merge branch 'fix_issue_1328' into 'master'

Fix: Changing group's state setting does not kick existing non-conforming group members

Closes #1328

See merge request allianceauth/allianceauth!1400
This commit is contained in:
Ariel Rin 2022-06-18 02:47:14 +00:00
commit cf3df3b715
5 changed files with 227 additions and 48 deletions

View File

@ -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)
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:
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),
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):

View File

@ -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.

View File

@ -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()

View File

@ -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):

View File

@ -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: