From 8a27de5df8b292488e38b2bdef1ef7df38cb8b5a Mon Sep 17 00:00:00 2001 From: ErikKalkoken Date: Sun, 28 Jun 2020 01:45:51 +0200 Subject: [PATCH] Fix state change does not update groups --- .../services/modules/discord/auth_hooks.py | 4 ++- .../modules/discord/tests/test_integration.py | 30 +++++++++---------- allianceauth/services/signals.py | 13 +++----- allianceauth/services/tests/test_signals.py | 5 ++-- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/allianceauth/services/modules/discord/auth_hooks.py b/allianceauth/services/modules/discord/auth_hooks.py index 97119f46..b68b871c 100644 --- a/allianceauth/services/modules/discord/auth_hooks.py +++ b/allianceauth/services/modules/discord/auth_hooks.py @@ -60,7 +60,9 @@ class DiscordService(ServicesHook): ) def service_active_for_user(self, user): - return user.has_perm(self.access_perm) + has_perms = user.has_perm(self.access_perm) + logger.debug("User %s has service permission: %s", user, has_perms) + return has_perms def sync_nickname(self, user): logger.debug('Syncing %s nickname for user %s', self.name, user) diff --git a/allianceauth/services/modules/discord/tests/test_integration.py b/allianceauth/services/modules/discord/tests/test_integration.py index ca5715d2..3eda8cb7 100644 --- a/allianceauth/services/modules/discord/tests/test_integration.py +++ b/allianceauth/services/modules/discord/tests/test_integration.py @@ -118,6 +118,7 @@ class TestServiceFeatures(TransactionTestCase): self.blue_state = AuthUtils.create_state("Blue", 50) permission = AuthUtils.get_permission_by_name('discord.access_discord') self.member_state.permissions.add(permission) + self.blue_state.permissions.add(permission) # Test user self.user = AuthUtils.create_user(TEST_USER_NAME) @@ -237,38 +238,35 @@ class TestServiceFeatures(TransactionTestCase): DiscordRequest(r.method, r.url) for r in requests_mocker.request_history ] self.assertIn(remove_guild_member_request, requests_made) - - """ + def test_when_member_changes_to_blue_state_then_roles_are_updated_accordingly( self, requests_mocker - ): + ): # request mocks requests_mocker.get( guild_member_request.url, - json={'user': create_user_info(),'roles': ['1', '13', '99']} + json={'user': create_user_info(),'roles': ['13', '99']} ) requests_mocker.get( guild_roles_request.url, json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER, ROLE_BLUE] ) - requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE) + requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE) requests_mocker.patch(modify_guild_member_request.url, status_code=204) # demote user to blue state self.blue_state.member_characters.add(self.main) self.member_state.member_characters.remove(self.main) - # verify roles for user where updated - requests_made = [ - DiscordRequest(r.method, r.url) for r in requests_mocker.request_history - ] - expected = [ - guild_member_request, - guild_roles_request, - modify_guild_member_request - ] - self.assertListEqual(requests_made, expected) - """ + # verify roles for user where updated + roles_updated = False + for r in requests_mocker.request_history: + my_request = DiscordRequest(r.method, r.url) + if my_request == modify_guild_member_request and "roles" in r.json(): + roles_updated = True + self.assertSetEqual(set(r.json()["roles"]), {13, 98}) + + self.assertTrue(roles_updated) def test_adding_group_to_user_role_exists(self, requests_mocker): # guild_member() diff --git a/allianceauth/services/signals.py b/allianceauth/services/signals.py index 7ae8543d..66be3760 100644 --- a/allianceauth/services/signals.py +++ b/allianceauth/services/signals.py @@ -125,15 +125,10 @@ def m2m_changed_state_permissions(sender, instance, action, pk_set, *args, **kwa @receiver(state_changed, sender=UserProfile) def check_service_accounts_state_changed(sender, user, state, **kwargs): - logger.debug("Received state_changed from %s to state %s" % (user, state)) - service_perms = [svc.access_perm for svc in ServicesHook.get_services()] - state_perms = ["{}.{}".format(perm.natural_key()[1], perm.natural_key()[0]) for perm in state.permissions.all()] - for perm in service_perms: - if perm not in state_perms: - for svc in ServicesHook.get_services(): - if svc.access_perm == perm: - logger.debug("User %s new state %s does not have service %s permission. Checking account." % (user, state, svc)) - svc.validate_user(user) + logger.debug("Received state_changed from %s to state %s" % (user, state)) + for svc in ServicesHook.get_services(): + svc.validate_user(user) + svc.update_groups(user) @receiver(pre_delete, sender=User) diff --git a/allianceauth/services/tests/test_signals.py b/allianceauth/services/tests/test_signals.py index 04ff148f..4c1df12e 100644 --- a/allianceauth/services/tests/test_signals.py +++ b/allianceauth/services/tests/test_signals.py @@ -173,7 +173,7 @@ class ServicesSignalsTestCase(TestCase): self.assertEqual(self.member, args[0]) @mock.patch('allianceauth.services.signals.ServicesHook') - def test_state_changed_services_valudation(self, services_hook): + def test_state_changed_services_validation(self, services_hook): """ Test a user changing state has service accounts validated """ @@ -192,4 +192,5 @@ class ServicesSignalsTestCase(TestCase): self.assertTrue(svc.validate_user.called) args, kwargs = svc.validate_user.call_args - self.assertEqual(self.member, args[0]) \ No newline at end of file + self.assertEqual(self.member, args[0]) + \ No newline at end of file