diff --git a/allianceauth/services/modules/discord/discord_client/app_settings.py b/allianceauth/services/modules/discord/discord_client/app_settings.py index 9eb5eb1a..668d6cf4 100644 --- a/allianceauth/services/modules/discord/discord_client/app_settings.py +++ b/allianceauth/services/modules/discord/discord_client/app_settings.py @@ -24,12 +24,12 @@ DISCORD_OAUTH_TOKEN_URL = clean_setting( # How long the Discord guild names retrieved from the server are # caches locally in milliseconds. DISCORD_GUILD_NAME_CACHE_MAX_AGE = clean_setting( - 'DISCORD_GUILD_NAME_CACHE_MAX_AGE', 3600 * 2 * 1000 + 'DISCORD_GUILD_NAME_CACHE_MAX_AGE', 3600 * 1 * 1000 ) # How long Discord roles retrieved from the server are caches locally in milliseconds. DISCORD_ROLES_CACHE_MAX_AGE = clean_setting( - 'DISCORD_ROLES_CACHE_MAX_AGE', 3600 * 2 * 1000 + 'DISCORD_ROLES_CACHE_MAX_AGE', 3600 * 1 * 1000 ) # Turns off creation of new roles. In case the rate limit for creating roles is diff --git a/allianceauth/services/modules/discord/discord_client/client.py b/allianceauth/services/modules/discord/discord_client/client.py index 701450aa..b5193f51 100644 --- a/allianceauth/services/modules/discord/discord_client/client.py +++ b/allianceauth/services/modules/discord/discord_client/client.py @@ -283,7 +283,10 @@ class DiscordClient: """ roles = list() guild_roles = DiscordRoles(self.guild_roles(guild_id)) - for role_name in role_names: + role_names_cleaned = { + DiscordRoles.sanitize_role_name(name) for name in role_names + } + for role_name in role_names_cleaned: role, created = self.match_or_create_role_from_name( guild_id=guild_id, role_name=DiscordRoles.sanitize_role_name(role_name), diff --git a/allianceauth/services/modules/discord/discord_client/tests/__init__.py b/allianceauth/services/modules/discord/discord_client/tests/__init__.py index 41d75108..bd354b57 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/__init__.py +++ b/allianceauth/services/modules/discord/discord_client/tests/__init__.py @@ -1,3 +1,11 @@ +TEST_GUILD_ID = 123456789012345678 +TEST_USER_ID = 198765432012345678 +TEST_USER_NAME = 'Peter Parker' +TEST_USER_DISCRIMINATOR = '1234' +TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY' +TEST_ROLE_ID = 654321012345678912 + + def create_role(id: int, name: str, managed=False): return { 'id': int(id), @@ -16,3 +24,15 @@ ROLE_CHARLIE = create_role(3, 'charlie') ROLE_MIKE = create_role(13, 'mike', True) ALL_ROLES = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE] + + +def create_user_info( + id: int = TEST_USER_ID, + username: str = TEST_USER_NAME, + discriminator: str = TEST_USER_DISCRIMINATOR +): + return { + 'id': str(id), + 'username': str(username[:32]), + 'discriminator': str(discriminator[:4]) + } diff --git a/allianceauth/services/modules/discord/discord_client/tests/test_client.py b/allianceauth/services/modules/discord/discord_client/tests/test_client.py index 84ef4121..8ae39fcf 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/test_client.py +++ b/allianceauth/services/modules/discord/discord_client/tests/test_client.py @@ -9,8 +9,22 @@ from requests.exceptions import HTTPError from allianceauth import __title__ as AUTH_TITLE, __url__, __version__ -from . import ROLE_ALPHA, ROLE_BRAVO, ALL_ROLES, create_role, create_matched_role -from ..client import DiscordClient, DURATION_CONTINGENCY, DEFAULT_BACKOFF_DELAY +from . import ( + TEST_GUILD_ID, + TEST_USER_ID, + TEST_USER_NAME, + TEST_BOT_TOKEN, + TEST_ROLE_ID, + ROLE_ALPHA, + ROLE_BRAVO, + ALL_ROLES, + create_role, + create_matched_role, + create_user_info +) +from ..client import ( + DiscordClient, DURATION_CONTINGENCY, DEFAULT_BACKOFF_DELAY, DiscordRoles +) from ..exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError from ...utils import set_logger_to_file @@ -20,13 +34,6 @@ logger = set_logger_to_file( MODULE_PATH = 'allianceauth.services.modules.discord.discord_client.client' API_BASE_URL = 'https://discordapp.com/api/' -TEST_GUILD_ID = 123456789012345678 -TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY' -TEST_USER_ID = 198765432012345678 -TEST_USER_NAME = 'John Doe' -TEST_ROLE_ID = 654321012345678912 - -TEST_ROUTE_KEY = 'abc123' TEST_RETRY_AFTER = 3000 @@ -104,7 +111,7 @@ class TestOtherMethods(TestCase): self.headers = DEFAULT_REQUEST_HEADERS def test_user_get_current(self, requests_mocker): - expected = {'id': "123456"} + expected = create_user_info() headers = { 'accept': 'application/json', 'authorization': 'Bearer accesstoken' @@ -140,10 +147,7 @@ class TestGuildRoles(TestCase): self.url = f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' def test_without_cache(self, requests_mocker): - expected = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] + expected = [ROLE_ALPHA, ROLE_BRAVO] my_mock_redis = MagicMock(**{ 'get.return_value': None, 'pttl.return_value': -1, @@ -159,10 +163,7 @@ class TestGuildRoles(TestCase): self.assertTrue(my_mock_redis.set.called) def test_return_from_cache_if_in_cache(self, requests_mocker): - expected = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] + expected = [ROLE_ALPHA, ROLE_BRAVO] my_mock_redis = MagicMock(**{ 'get.return_value': json.dumps(expected).encode('utf8') }) @@ -174,10 +175,7 @@ class TestGuildRoles(TestCase): def test_return_from_api_and_save_to_cache_if_not_in_cache( self, requests_mocker ): - expected = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] + expected = [ROLE_ALPHA, ROLE_BRAVO] my_mock_redis = MagicMock(**{ 'get.return_value': None, 'pttl.return_value': -1, @@ -237,7 +235,7 @@ class TestGuildMember(TestCase): self.headers = DEFAULT_REQUEST_HEADERS def test_return_guild_member_when_ok(self, requests_mocker): - expected = {'id': TEST_USER_ID, 'name': 'John Doe'} + expected = create_user_info() requests_mocker.get( f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}', request_headers=self.headers, @@ -902,7 +900,10 @@ class TestMatchGuildRolesToNames(TestCase): client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) expected = [create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)] - self.assertEqual(result, expected) + self.assertEqual( + DiscordRoles.create_from_matched_roles(result), + DiscordRoles.create_from_matched_roles(expected) + ) self.assertFalse(mock_guild_create_role.called) def test_return_roles_if_known_and_create_if_not_known( @@ -916,7 +917,10 @@ class TestMatchGuildRolesToNames(TestCase): result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) expected = \ [create_matched_role(ROLE_ALPHA), create_matched_role(new_role, True)] - self.assertEqual(result, expected) + self.assertEqual( + DiscordRoles.create_from_matched_roles(result), + DiscordRoles.create_from_matched_roles(expected) + ) self.assertTrue(mock_guild_create_role.called) @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', True) @@ -930,10 +934,44 @@ class TestMatchGuildRolesToNames(TestCase): client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) expected = [create_matched_role(ROLE_ALPHA)] - self.assertEqual(result, expected) + self.assertEqual( + DiscordRoles.create_from_matched_roles(result), + DiscordRoles.create_from_matched_roles(expected) + ) self.assertFalse(mock_guild_create_role.called) - + def test_consolidate_roles_of_same_name( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_names = ['alpha', 'bravo', 'alpha'] + mock_guild_get_roles.return_value = ALL_ROLES + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) + expected = [create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)] + self.assertEqual( + DiscordRoles.create_from_matched_roles(result), + DiscordRoles.create_from_matched_roles(expected) + ) + self.assertFalse(mock_guild_create_role.called) + + def test_consolidate_roles_of_same_name_after_sanitation( + self, mock_guild_get_roles, mock_guild_create_role, + ): + base_role_name = 'x' * 100 + new_role = create_role(77, base_role_name) + role_names = [base_role_name + '1', base_role_name + '2'] + mock_guild_get_roles.return_value = ALL_ROLES + [new_role] + mock_guild_create_role.return_value = new_role + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) + expected = [create_matched_role(new_role)] + self.assertEqual( + DiscordRoles.create_from_matched_roles(result), + DiscordRoles.create_from_matched_roles(expected) + ) + self.assertFalse(mock_guild_create_role.called) + + class TestApiRequestBasics(TestCase): def setUp(self): @@ -949,7 +987,7 @@ class TestApiRequestBasics(TestCase): @requests_mock.Mocker() class TestRateLimitMechanic(TestCase): - my_role = {'id': 1, 'name': 'alpha'} + my_role = ROLE_ALPHA @staticmethod def my_redis_pttl(name: str): @@ -1131,7 +1169,7 @@ class TestRateLimitMechanic(TestCase): @requests_mock.Mocker() class TestBackoffHandling(TestCase): - my_role = {'id': 1, 'name': 'alpha'} + my_role = ROLE_ALPHA def test_dont_raise_exception_when_no_global_backoff( self, mock_redis_decr_or_set, requests_mocker diff --git a/allianceauth/services/modules/discord/managers.py b/allianceauth/services/modules/discord/managers.py index f11b79c1..e1955e03 100644 --- a/allianceauth/services/modules/discord/managers.py +++ b/allianceauth/services/modules/discord/managers.py @@ -136,7 +136,9 @@ class DiscordUserManager(models.Manager): only checks locally, does not hit the API """ - return True if hasattr(user, self.model.USER_RELATED_NAME) else False + if not isinstance(user, User): + return False + return self.filter(user=user).select_related('user').exists() @classmethod def generate_bot_add_url(cls): diff --git a/allianceauth/services/modules/discord/models.py b/allianceauth/services/modules/discord/models.py index 6c280b3e..5aace010 100644 --- a/allianceauth/services/modules/discord/models.py +++ b/allianceauth/services/modules/discord/models.py @@ -184,7 +184,10 @@ class DiscordUser(models.Model): return success def delete_user( - self, notify_user: bool = False, is_rate_limited: bool = True + self, + notify_user: bool = False, + is_rate_limited: bool = True, + handle_api_exceptions: bool = False ) -> bool: """Deletes the Discount user both on the server and locally @@ -192,6 +195,8 @@ class DiscordUser(models.Model): - notify_user: When True will sent a notification to the user informing him about the deleting of his account - is_rate_limited: When False will disable default rate limiting (use with care) + - handle_api_exceptions: When True method will return False + when an API exception occurs Returns True when successful, otherwise False or raises exceptions Return None if user does no longer exist @@ -228,7 +233,10 @@ class DiscordUser(models.Model): return False except (HTTPError, ConnectionError, DiscordApiBackoff) as ex: - logger.exception( - 'Failed to remove user %s from Discord server: %s', self.user, ex - ) - return False + if handle_api_exceptions: + logger.exception( + 'Failed to remove user %s from Discord server: %s', self.user, ex + ) + return False + else: + raise ex diff --git a/allianceauth/services/modules/discord/tests/__init__.py b/allianceauth/services/modules/discord/tests/__init__.py index 6e307de0..6958ea48 100644 --- a/allianceauth/services/modules/discord/tests/__init__.py +++ b/allianceauth/services/modules/discord/tests/__init__.py @@ -1,21 +1,24 @@ from django.contrib.auth.models import Group from allianceauth.tests.auth_utils import AuthUtils -from ..discord_client.tests import create_role +from ..discord_client.tests import ( # noqa + TEST_GUILD_ID, + TEST_USER_ID, + TEST_USER_NAME, + TEST_USER_DISCRIMINATOR, + create_role, + ROLE_ALPHA, + ROLE_BRAVO, + ROLE_CHARLIE, + ROLE_MIKE, + create_user_info +) DEFAULT_AUTH_GROUP = 'Member' MODULE_PATH = 'allianceauth.services.modules.discord' -TEST_GUILD_ID = 123456789012345678 -TEST_USER_ID = 198765432012345678 -TEST_USER_NAME = 'Peter Parker' TEST_MAIN_NAME = 'Spiderman' TEST_MAIN_ID = 1005 -ROLE_ALPHA = create_role(1, 'alpha') -ROLE_BRAVO = create_role(2, 'bravo') -ROLE_CHARLIE = create_role(3, 'charlie') -ROLE_MIKE = create_role(13, 'mike', True) - def add_permissions_to_members(): permission = AuthUtils.get_permission_by_name('discord.access_discord') diff --git a/allianceauth/services/modules/discord/tests/test_integration.py b/allianceauth/services/modules/discord/tests/test_integration.py index 33acdc4a..57ec1949 100644 --- a/allianceauth/services/modules/discord/tests/test_integration.py +++ b/allianceauth/services/modules/discord/tests/test_integration.py @@ -6,10 +6,11 @@ Please note that these tests require Redis and will flush it """ from collections import namedtuple import logging -from unittest.mock import patch +from unittest.mock import patch, Mock from uuid import uuid1 from django_webtest import WebTest +from requests.exceptions import HTTPError import requests_mock from django.contrib.auth.models import Group, User @@ -23,7 +24,8 @@ from allianceauth.tests.auth_utils import AuthUtils from . import ( TEST_GUILD_ID, TEST_USER_NAME, - TEST_USER_ID, + TEST_USER_ID, + TEST_USER_DISCRIMINATOR, TEST_MAIN_NAME, TEST_MAIN_ID, MODULE_PATH, @@ -32,7 +34,8 @@ from . import ( ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, - create_role + create_role, + create_user_info ) from ..discord_client.app_settings import DISCORD_API_BASE_URL from ..models import DiscordUser @@ -43,6 +46,14 @@ ROLE_MEMBER = create_role(99, 'Member') # Putting all requests to Discord into objects so we can compare them better DiscordRequest = namedtuple('DiscordRequest', ['method', 'url']) +user_get_current_request = DiscordRequest( + method='GET', + url=f'{DISCORD_API_BASE_URL}users/@me' +) +guild_infos_request = DiscordRequest( + method='GET', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}' +) guild_roles_request = DiscordRequest( method='GET', url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' @@ -55,6 +66,10 @@ guild_member_request = DiscordRequest( method='GET', url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' ) +add_guild_member_request = DiscordRequest( + method='PUT', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' +) modify_guild_member_request = DiscordRequest( method='PATCH', url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' @@ -121,6 +136,60 @@ class TestServiceFeatures(TransactionTestCase): expected = [modify_guild_member_request, modify_guild_member_request] self.assertListEqual(requests_made, expected) + def test_name_of_main_changes_but_user_deleted(self, requests_mocker): + # modify_guild_member() + requests_mocker.patch( + modify_guild_member_request.url, status_code=404, json={'code': 10007} + ) + # remove_guild_member() + requests_mocker.delete(remove_guild_member_request.url, status_code=204) + + # changing nick to trigger signals + new_nick = f'Testnick {uuid1().hex}'[:32] + self.user.profile.main_character.character_name = new_nick + self.user.profile.main_character.save() + + # Need to have called modify_guild_member two times only + # Once for sync nickname + # Once for change of main character + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + expected = [ + modify_guild_member_request, + remove_guild_member_request, + ] + self.assertListEqual(requests_made, expected) + # self.assertFalse(DiscordUser.objects.user_has_account(self.user)) + + def test_name_of_main_changes_but_user_rate_limited( + self, requests_mocker + ): + # modify_guild_member() + requests_mocker.patch(modify_guild_member_request.url, status_code=204) + + # exhausting rate limit + client = DiscordUser.objects._bot_client() + client._redis.set( + name=client._KEY_GLOBAL_RATE_LIMIT_REMAINING, + value=0, + px=2000 + ) + + # changing nick to trigger signals + new_nick = f'Testnick {uuid1().hex}'[:32] + self.user.profile.main_character.character_name = new_nick + self.user.profile.main_character.save() + + # should not have called the API + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + expected = list() + self.assertListEqual(requests_made, expected) + def test_user_demoted_to_guest(self, requests_mocker): # remove_guild_member() requests_mocker.delete(remove_guild_member_request.url, status_code=204) @@ -139,7 +208,7 @@ class TestServiceFeatures(TransactionTestCase): requests_mocker.get( guild_member_request.url, json={ - 'user': {'id': str(TEST_USER_ID), 'username': TEST_MAIN_NAME}, + 'user': create_user_info(), 'roles': ['1', '13', '99'] } ) @@ -206,9 +275,13 @@ class TestServiceFeatures(TransactionTestCase): self.assertListEqual(requests_made, expected) -class TestServiceUserActivation(WebTest): +@patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID) +@patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID) +@requests_mock.Mocker() +class TestUserFeatures(WebTest): def setUp(self): + clear_cache() self.member = AuthUtils.create_member(TEST_USER_NAME) AuthUtils.add_main_character_2( self.member, @@ -217,15 +290,28 @@ class TestServiceUserActivation(WebTest): disconnect_signals=True ) add_permissions_to_members() - - @patch(MODULE_PATH + '.views.messages') - @patch(MODULE_PATH + '.models.DiscordUser.objects.add_user') + + @patch(MODULE_PATH + '.views.messages') @patch(MODULE_PATH + '.managers.OAuth2Session') - def test_user_activation( - self, mock_OAuth2Session, mock_add_user, mock_messages + def test_user_activation_normal( + self, requests_mocker, mock_OAuth2Session, mock_messages ): - authentication_code = 'auth_code' - mock_add_user.return_value = True + # user_get_current() + requests_mocker.get( + user_get_current_request.url, + json=create_user_info( + TEST_USER_ID, TEST_USER_NAME, TEST_USER_DISCRIMINATOR + ) + ) + # guild_roles() + requests_mocker.get( + guild_roles_request.url, + json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER] + ) + # add_guild_member() + requests_mocker.put(add_guild_member_request.url, status_code=201) + + authentication_code = 'auth_code' oauth_url = 'https://www.example.com/oauth' state = '' mock_OAuth2Session.return_value.authorization_url.return_value = \ @@ -246,9 +332,145 @@ class TestServiceUserActivation(WebTest): response = self.app.get( reverse('discord:callback'), params={'code': authentication_code} ) + + # user got a success message + self.assertTrue(mock_messages.success.called) + self.assertFalse(mock_messages.error.called) + + requests_made = list() + for r in requests_mocker.request_history: + obj = DiscordRequest(r.method, r.url) + requests_made.append(obj) + + expected = [ + user_get_current_request, guild_roles_request, add_guild_member_request + ] + self.assertListEqual(requests_made, expected) - # user was added to Discord - self.assertTrue(mock_add_user.called) + @patch(MODULE_PATH + '.views.messages') + @patch(MODULE_PATH + '.managers.OAuth2Session') + def test_user_activation_failed( + self, requests_mocker, mock_OAuth2Session, mock_messages + ): + # user_get_current() + requests_mocker.get( + user_get_current_request.url, + json=create_user_info( + TEST_USER_ID, TEST_USER_NAME, TEST_USER_DISCRIMINATOR + ) + ) + # guild_roles() + requests_mocker.get( + guild_roles_request.url, + json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER] + ) + # add_guild_member() + mock_exception = HTTPError('error') + mock_exception.response = Mock() + mock_exception.response.status_code = 503 + requests_mocker.put(add_guild_member_request.url, exc=mock_exception) + + authentication_code = 'auth_code' + oauth_url = 'https://www.example.com/oauth' + state = '' + mock_OAuth2Session.return_value.authorization_url.return_value = \ + oauth_url, state + + # login + self.app.set_user(self.member) + + # click activate on the service page + response = self.app.get(reverse('discord:activate')) + + # check we got a redirect to Discord OAuth + self.assertRedirects( + response, expected_url=oauth_url, fetch_redirect_response=False + ) + + # simulate Discord callback + response = self.app.get( + reverse('discord:callback'), params={'code': authentication_code} + ) + + # user got a success message + self.assertFalse(mock_messages.success.called) + self.assertTrue(mock_messages.error.called) + + requests_made = list() + for r in requests_mocker.request_history: + obj = DiscordRequest(r.method, r.url) + requests_made.append(obj) + + expected = [ + user_get_current_request, guild_roles_request, add_guild_member_request + ] + self.assertListEqual(requests_made, expected) + + @patch(MODULE_PATH + '.views.messages') + def test_user_deactivation_normal(self, requests_mocker, mock_messages): + # guild_infos() + requests_mocker.get( + guild_infos_request.url, json={'id': TEST_GUILD_ID, 'name': 'Test Guild'}) + + # remove_guild_member() + requests_mocker.delete(remove_guild_member_request.url, status_code=204) + + # user needs have an account + DiscordUser.objects.create(user=self.member, uid=TEST_USER_ID) + + # login + self.app.set_user(self.member) + + # click deactivate on the service page + response = self.app.get(reverse('discord:deactivate')) + + # check we got a redirect to service page + self.assertRedirects(response, expected_url=reverse('services:services')) # user got a success message self.assertTrue(mock_messages.success.called) + self.assertFalse(mock_messages.error.called) + + requests_made = list() + for r in requests_mocker.request_history: + obj = DiscordRequest(r.method, r.url) + requests_made.append(obj) + + expected = [remove_guild_member_request, guild_infos_request] + self.assertListEqual(requests_made, expected) + + @patch(MODULE_PATH + '.views.messages') + def test_user_deactivation_fails(self, requests_mocker, mock_messages): + # guild_infos() + requests_mocker.get( + guild_infos_request.url, json={'id': TEST_GUILD_ID, 'name': 'Test Guild'}) + + # remove_guild_member() + mock_exception = HTTPError('error') + mock_exception.response = Mock() + mock_exception.response.status_code = 503 + requests_mocker.delete(remove_guild_member_request.url, exc=mock_exception) + + # user needs have an account + DiscordUser.objects.create(user=self.member, uid=TEST_USER_ID) + + # login + self.app.set_user(self.member) + + # click deactivate on the service page + response = self.app.get(reverse('discord:deactivate')) + + # check we got a redirect to service page + self.assertRedirects(response, expected_url=reverse('services:services')) + + # user got a success message + self.assertFalse(mock_messages.success.called) + self.assertTrue(mock_messages.error.called) + + requests_made = list() + for r in requests_mocker.request_history: + obj = DiscordRequest(r.method, r.url) + requests_made.append(obj) + + expected = [remove_guild_member_request, guild_infos_request] + self.assertListEqual(requests_made, expected) diff --git a/allianceauth/services/modules/discord/tests/test_models.py b/allianceauth/services/modules/discord/tests/test_models.py index 268f6205..1ef85322 100644 --- a/allianceauth/services/modules/discord/tests/test_models.py +++ b/allianceauth/services/modules/discord/tests/test_models.py @@ -219,20 +219,44 @@ class TestDeleteUser(TestCase): ) self.assertTrue(mock_DiscordClient.return_value.remove_guild_member.called) self.assertFalse(mock_notify.called) - - def test_return_false_on_api_backoff(self, mock_DiscordClient, mock_notify): + + def test_raise_exception_on_api_backoff( + self, mock_DiscordClient, mock_notify + ): mock_DiscordClient.return_value.remove_guild_member.side_effect = \ DiscordApiBackoff(999) - result = self.discord_user.delete_user() + with self.assertRaises(DiscordApiBackoff): + self.discord_user.delete_user() + + def test_return_false_on_api_backoff_and_exception_handling_on( + self, mock_DiscordClient, mock_notify + ): + mock_DiscordClient.return_value.remove_guild_member.side_effect = \ + DiscordApiBackoff(999) + result = self.discord_user.delete_user(handle_api_exceptions=True) self.assertFalse(result) - def test_return_false_on_http_error(self, mock_DiscordClient, mock_notify): + def test_raise_exception_on_http_error( + self, mock_DiscordClient, mock_notify + ): mock_exception = HTTPError('error') mock_exception.response = Mock() mock_exception.response.status_code = 500 mock_DiscordClient.return_value.remove_guild_member.side_effect = \ mock_exception - result = self.discord_user.delete_user() + + with self.assertRaises(HTTPError): + self.discord_user.delete_user() + + def test_return_false_on_http_error_and_exception_handling_on( + self, mock_DiscordClient, mock_notify + ): + mock_exception = HTTPError('error') + mock_exception.response = Mock() + mock_exception.response.status_code = 500 + mock_DiscordClient.return_value.remove_guild_member.side_effect = \ + mock_exception + result = self.discord_user.delete_user(handle_api_exceptions=True) self.assertFalse(result) diff --git a/allianceauth/services/modules/discord/views.py b/allianceauth/services/modules/discord/views.py index 6e7c6501..efa290f8 100644 --- a/allianceauth/services/modules/discord/views.py +++ b/allianceauth/services/modules/discord/views.py @@ -23,7 +23,9 @@ ACCESS_PERM = 'discord.access_discord' @permission_required(ACCESS_PERM) def deactivate_discord(request): logger.debug("deactivate_discord called by user %s", request.user) - if request.user.discord.delete_user(is_rate_limited=False): + if request.user.discord.delete_user( + is_rate_limited=False, handle_api_exceptions=True + ): logger.info("Successfully deactivated discord for user %s", request.user) messages.success(request, _('Deactivated Discord account.')) else: @@ -40,7 +42,9 @@ def deactivate_discord(request): @permission_required(ACCESS_PERM) def reset_discord(request): logger.debug("reset_discord called by user %s", request.user) - if request.user.discord.delete_user(is_rate_limited=False): + if request.user.discord.delete_user( + is_rate_limited=False, handle_api_exceptions=True + ): logger.info( "Successfully deleted discord user for user %s - " "forwarding to discord activation.", diff --git a/docs/features/services/discord.md b/docs/features/services/discord.md index 76c61c86..1e8d7a6a 100644 --- a/docs/features/services/discord.md +++ b/docs/features/services/discord.md @@ -123,19 +123,20 @@ Name Description You can configure your Discord services with the following settings: ```eval_rst -============================== ============================================================================================= ======= -Name Description Default -============================== ============================================================================================= ======= -`DISCORD_APP_ID` Oauth client ID for the Discord Auth app `''` -`DISCORD_APP_SECRET` Oauth client secret for the Discord Auth app `''` -`DISCORD_BOT_TOKEN` Generated bot token for the Discord Auth app `''` -`DISCORD_CALLBACK_URL` Oauth callback URL `''` -`DISCORD_GUILD_ID` Discord ID of your Discord server `''` -`DISCORD_ROLES_CACHE_MAX_AGE` How long roles retrieved from the Discord server are cached locally in milliseconds `7200000` -`DISCORD_SYNC_NAMES` When set to True the nicknames of Discord users will be set to the user's main character name `False` -`DISCORD_TASKS_RETRY_PAUSE` Pause in seconds until next retry for tasks after an error occurred `60` -`DISCORD_TASKS_MAX_RETRIES` max retries of tasks after an error occurred `3` -============================== ============================================================================================= ======= +=================================== ============================================================================================= ======= +Name Description Default +=================================== ============================================================================================= ======= +`DISCORD_APP_ID` Oauth client ID for the Discord Auth app `''` +`DISCORD_APP_SECRET` Oauth client secret for the Discord Auth app `''` +`DISCORD_BOT_TOKEN` Generated bot token for the Discord Auth app `''` +`DISCORD_CALLBACK_URL` Oauth callback URL `''` +`DISCORD_GUILD_ID` Discord ID of your Discord server `''` +`DISCORD_GUILD_NAME_CACHE_MAX_AGE` How long the Discord server name is cached locally in milliseconds `3600000` +`DISCORD_ROLES_CACHE_MAX_AGE` How long roles retrieved from the Discord server are cached locally in milliseconds `3600000` +`DISCORD_SYNC_NAMES` When set to True the nicknames of Discord users will be set to the user's main character name `False` +`DISCORD_TASKS_RETRY_PAUSE` Pause in seconds until next retry for tasks after an error occurred `60` +`DISCORD_TASKS_MAX_RETRIES` max retries of tasks after an error occurred `3` +=================================== ============================================================================================= ======= ``` ## Troubleshooting