Enable group state restrictions to work with internal groups

allianceauth/groupmanagement/signals.py

@@ -1,16 +1,22 @@
-from allianceauth.authentication.signals import state_changed
-from .managers import GroupManager
-from .models import Group
-from django.dispatch import receiver
 import logging
+from django.dispatch import receiver
+from allianceauth.authentication.signals import state_changed
+
+
 logger = logging.getLogger(__name__)
 
+
 @receiver(state_changed)
 def check_groups_on_state_change(sender, user, state, **kwargs):
-    logger.debug("Updating auth groups for {}".format(user))
-    visible_groups = GroupManager.get_joinable_groups(state)
-    visible_groups = visible_groups | Group.objects.select_related('authgroup').filter(authgroup__internal=True)
-    groups = user.groups.all()
-    for g in groups:
-        if g not in visible_groups:
-            user.groups.remove(g)
+    logger.debug(
+        "Checking group memberships for %s based on new state %s" % (user, state)
+    )        
+    state_groups = (
+        user.groups.select_related("authgroup").exclude(authgroup__states=None)
+    )
+    for group in state_groups:
+        if not group.authgroup.states.filter(id=state.id).exists():
+            logger.info(
+                "Removing user %s from group %s due to missing state" % (user, group)
+            )        
+            user.groups.remove(group)

allianceauth/groupmanagement/tests/test_signals.py

@@ -1,61 +1,100 @@
-from unittest import mock
-
 from django.test import TestCase
 from django.contrib.auth.models import User, Group
 
-from allianceauth.eveonline.models import EveCorporationInfo, EveAllianceInfo
+from allianceauth.eveonline.models import EveCorporationInfo
+from allianceauth.eveonline.autogroups.models import AutogroupsConfig
 from allianceauth.tests.auth_utils import AuthUtils
 
-from ..signals import check_groups_on_state_change
 
-
-class GroupManagementStateTestCase(TestCase):
+class TestCheckGroupsOnStateChange(TestCase):
 
     @classmethod
     def setUpTestData(cls):
         cls.user = AuthUtils.create_user('test')
-        AuthUtils.add_main_character(
-            cls.user, 'test character', '1', corp_id='2', corp_name='test_corp', corp_ticker='TEST', alliance_id='3', alliance_name='TEST'
+        cls.character = AuthUtils.add_main_character_2(
+            cls.user, 'test character', 1001, corp_id=2001, corp_name='test corp 1', corp_ticker='TEST'
         )
         cls.user.profile.refresh_from_db()        
-        cls.alliance = EveAllianceInfo.objects.create(
-            alliance_id='3', alliance_name='test alliance', alliance_ticker='TEST', executor_corp_id='2'
+        cls.corp_1 = EveCorporationInfo.objects.create(
+            corporation_id=2001, corporation_name='test corp 1', corporation_ticker='C1', member_count=1
         )
-        cls.corp = EveCorporationInfo.objects.create(
-            corporation_id='2', corporation_name='test corp', corporation_ticker='TEST', alliance=cls.alliance, member_count=1
+        cls.corp_2 = EveCorporationInfo.objects.create(
+            corporation_id=2002, corporation_name='test corp 2', corporation_ticker='C2', member_count=1
         )
-        cls.state_group = Group.objects.create(name='state_group')
-        cls.open_group = Group.objects.create(name='open_group')
-        cls.state = AuthUtils.create_state('test state', 500)
-        cls.state_group.authgroup.states.add(cls.state)
-        cls.state_group.authgroup.internal = False
-        cls.state_group.save()
+        cls.guest_state = AuthUtils.get_guest_state()
+        cls.test_state_1 = AuthUtils.create_state('test_state_1', 500)
+        cls.test_state_2 = AuthUtils.create_state('test_state_2', 600)
         
     def setUp(self):
         self.user.refresh_from_db()        
-        self.state.refresh_from_db()
 
     def _refresh_user(self):
         self.user = User.objects.get(pk=self.user.pk)
     
-    def _refresh_test_group(self):
-        self.state_group = Group.objects.get(pk=self.state_group.pk)
-
     def test_drop_state_group(self):
-        self.user.groups.add(self.open_group)
-        self.user.groups.add(self.state_group)     
-        self.assertEqual(self.user.profile.state.name, "Guest")
+        """
+        given user is member of: state group, normal group and auto group
+        when user looses state
+        then user is automatically kicked from state group
+        and remains member of normal group and auto group
+        """
+        # setup
+        state_group = Group.objects.create(name='state_group')        
+        state_group.authgroup.states.add(self.test_state_1)
+        state_group.authgroup.internal = False
+        state_group.save()
+        normal_group = Group.objects.create(name='normal_group')
+        normal_group.authgroup.internal = False
+        normal_group.save()
+        internal_group = Group.objects.create(name='internal_group')        
+        autogroup_config = AutogroupsConfig.objects.create(corp_groups=True)
+        autogroup_config.states.add(self.test_state_1)
+        autogroup_config.states.add(self.guest_state)
+        auto_group = autogroup_config.corp_managed_groups.first()
+        internal_state_group = Group.objects.create(name='internal_state_group')        
+        internal_state_group.authgroup.states.add(self.test_state_1)
+        self.test_state_1.member_corporations.add(self.corp_1)
+        self.user.groups.add(normal_group)
+        self.user.groups.add(internal_group)
+        self.user.groups.add(state_group)        
+        self.user.groups.add(internal_state_group)        
         
-        self.state.member_corporations.add(self.corp)
-        self._refresh_user()
-        self.assertEqual(self.user.profile.state, self.state)
-        groups = self.user.groups.all()
-        self.assertIn(self.state_group, groups) #keeps group
-        self.assertIn(self.open_group, groups) #public group unafected
+        # user changes state back to guest
+        self.test_state_1.member_corporations.clear()
         
-        self.state.member_corporations.clear()
+        # assert
         self._refresh_user()
-        self.assertEqual(self.user.profile.state.name, "Guest")
+        self.assertEqual(self.user.profile.state, self.guest_state)
         groups = self.user.groups.all()
-        self.assertNotIn(self.state_group, groups) #looses group
-        self.assertIn(self.open_group, groups) #public group unafected
+        self.assertNotIn(state_group, groups)  # looses state group
+        self.assertNotIn(internal_state_group, groups)  # looses state group
+        self.assertIn(normal_group, groups)  # normal group unafected
+        self.assertIn(internal_group, groups)  # internal group unafected
+        self.assertIn(auto_group, groups)  # auto group unafected
+
+    def test_change_to_other_state(self):
+        """
+        given a state group with 2 allowed states
+        when user changes from one state to the other
+        then user remains member of that group
+        """
+        # setup
+        state_group = Group.objects.create(name='state_group')        
+        state_group.authgroup.states.add(self.test_state_1)
+        state_group.authgroup.states.add(self.test_state_2)
+                
+        self.test_state_1.member_corporations.add(self.corp_1)
+        self.test_state_2.member_corporations.add(self.corp_2)
+        self.user.groups.add(state_group)
+        
+        # user changes state back to guest
+        self.character.corporation_id = 2002
+        self.character.corporation_name = "test corp 2"
+        self.character.save()
+        
+        # assert
+        self._refresh_user()
+        self.assertEqual(self.user.profile.state, self.test_state_2)
+        groups = self.user.groups.all()        
+        self.assertIn(state_group, groups)
+