diff --git a/allianceauth/services/modules/discord/models.py b/allianceauth/services/modules/discord/models.py index 8e4c78eb..1f9c084f 100644 --- a/allianceauth/services/modules/discord/models.py +++ b/allianceauth/services/modules/discord/models.py @@ -141,28 +141,18 @@ class DiscordUser(models.Model): Returns: - True on success - None if user is no longer a member of the Discord server - - False on error or raises exception """ - user_info = default_bot_client.guild_member( + member_info = default_bot_client.guild_member( guild_id=DISCORD_GUILD_ID, user_id=self.uid ) - if user_info is None: - success = None - elif ( - user_info - and 'user' in user_info - and 'username' in user_info['user'] - and 'discriminator' in user_info['user'] - ): - self.username = user_info['user']['username'] - self.discriminator = user_info['user']['discriminator'] - self.save() - logger.info('Username for %s has been updated', self.user) - success = True - else: - logger.warning('Failed to update username for %s', self.user) - success = False - return success + if not member_info: + logger.warning('%s: User not a guild member', self.user) + return None + self.username = member_info.user.username + self.discriminator = member_info.user.discriminator + self.save() + logger.info('%s: Username has been updated', self.user) + return True def delete_user( self, diff --git a/allianceauth/services/modules/discord/tests/factories.py b/allianceauth/services/modules/discord/tests/factories.py new file mode 100644 index 00000000..5a414679 --- /dev/null +++ b/allianceauth/services/modules/discord/tests/factories.py @@ -0,0 +1,31 @@ +from django.utils.timezone import now + +from allianceauth.authentication.backends import StateBackend +from allianceauth.tests.auth_utils import AuthUtils + +from ..discord_client.tests.factories import ( + TEST_USER_DISCRIMINATOR, + TEST_USER_ID, + TEST_USER_NAME, +) +from ..models import DiscordUser + + +def create_user(**kwargs): + params = {"username": TEST_USER_NAME} + params.update(kwargs) + username = StateBackend.iterate_username(params["username"]) + user = AuthUtils.create_user(username) + return AuthUtils.add_permission_to_user_by_name("discord.access_discord", user) + + +def create_discord_user(user=None, **kwargs): + params = { + "user": user or create_user(), + "uid": TEST_USER_ID, + "username": TEST_USER_NAME, + "discriminator": TEST_USER_DISCRIMINATOR, + "activated": now(), + } + params.update(kwargs) + return DiscordUser.objects.create(**params) diff --git a/allianceauth/services/modules/discord/tests/test_integration.py b/allianceauth/services/modules/discord/tests/test_integration.py index c1d038b0..2d50db5e 100644 --- a/allianceauth/services/modules/discord/tests/test_integration.py +++ b/allianceauth/services/modules/discord/tests/test_integration.py @@ -14,8 +14,8 @@ import requests_mock from requests.exceptions import HTTPError from django.contrib.auth.models import Group, User -from django.shortcuts import reverse from django.test import TransactionTestCase, override_settings +from django.urls import reverse from django_webtest import WebTest from allianceauth.authentication.models import State @@ -42,6 +42,7 @@ from ..discord_client.tests.factories import ( ) from ..models import DiscordUser from . import MODULE_PATH, TEST_MAIN_ID, TEST_MAIN_NAME, add_permissions_to_members +from .factories import create_discord_user, create_user logger = logging.getLogger('allianceauth') @@ -349,6 +350,28 @@ class TestServiceFeatures(TransactionTestCase): self.assertTrue(DiscordUser.objects.user_has_account(self.user)) +@override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True) +@patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID) +@patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID) +@requests_mock.Mocker() +class TestTasks(NoSocketsTestCase): + def test_should_update_username(self, requests_mocker): + # given + user = create_user() + discord_user = create_discord_user(user) + discord_user_obj = create_discord_user_object() + data = create_discord_guild_member_object(user=discord_user_obj) + requests_mocker.get(guild_member_request.url, json=data) + # when + tasks.update_username.delay(user.pk) + # then + discord_user.refresh_from_db() + self.assertEqual(discord_user.username, discord_user_obj["username"]) + self.assertEqual( + discord_user.discriminator, discord_user_obj["discriminator"] + ) + + @override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True) @patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID) @patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID) diff --git a/allianceauth/services/modules/discord/tests/test_models.py b/allianceauth/services/modules/discord/tests/test_models.py index 4de9ff54..c7e73218 100644 --- a/allianceauth/services/modules/discord/tests/test_models.py +++ b/allianceauth/services/modules/discord/tests/test_models.py @@ -6,10 +6,17 @@ from allianceauth.tests.auth_utils import AuthUtils from allianceauth.utils.testing import NoSocketsTestCase from ..discord_client import DiscordApiBackoff, RolesSet -from ..discord_client.tests.factories import TEST_USER_ID, TEST_USER_NAME, create_role +from ..discord_client.tests.factories import ( + TEST_USER_ID, + TEST_USER_NAME, + create_guild_member, + create_role, +) +from ..discord_client.tests.factories import create_user as create_guild_user from ..models import DiscordUser from ..utils import set_logger_to_file from . import MODULE_PATH, TEST_MAIN_ID, TEST_MAIN_NAME +from .factories import create_discord_user, create_user logger = set_logger_to_file(MODULE_PATH + '.models', __file__) @@ -84,99 +91,41 @@ class TestUpdateNick(NoSocketsTestCase): self.assertTrue(mock_default_bot_client.modify_guild_member.called) -@patch(MODULE_PATH + '.models.default_bot_client', spec=True) +@patch(MODULE_PATH + '.models.default_bot_client.guild_member', spec=True) class TestUpdateUsername(NoSocketsTestCase): @classmethod def setUpClass(cls): super().setUpClass() - cls.user = AuthUtils.create_user(TEST_USER_NAME) + cls.user = create_user() - def setUp(self): - self.discord_user = DiscordUser.objects.create( - user=self.user, - uid=TEST_USER_ID, - username=TEST_MAIN_NAME, - discriminator='1234' - ) - - def test_can_update(self, mock_default_bot_client): + def test_can_update(self, mock_guild_member): # given + discord_user = create_discord_user(user=self.user) new_username = 'New name' new_discriminator = '9876' - user_info = { - 'user': { - 'id': str(TEST_USER_ID), - 'username': new_username, - 'discriminator': new_discriminator, - } - } - mock_default_bot_client.guild_member.return_value = user_info + guild_user = create_guild_user( + username='New name', discriminator=new_discriminator + ) + mock_guild_member.return_value = create_guild_member(user=guild_user) # when - result = self.discord_user.update_username() + result = discord_user.update_username() # then self.assertTrue(result) - self.assertTrue(mock_default_bot_client.guild_member.called) - self.discord_user.refresh_from_db() - self.assertEqual(self.discord_user.username, new_username) - self.assertEqual(self.discord_user.discriminator, new_discriminator) + self.assertTrue(mock_guild_member.called) + discord_user.refresh_from_db() + self.assertEqual(discord_user.username, new_username) + self.assertEqual(discord_user.discriminator, new_discriminator) - def test_return_none_if_user_no_longer_a_member(self, mock_default_bot_client): + def test_return_none_if_user_no_longer_a_member(self, mock_guild_member): # given - mock_default_bot_client.guild_member.return_value = None + discord_user = create_discord_user(user=self.user) + mock_guild_member.return_value = None # when - result = self.discord_user.update_username() + result = discord_user.update_username() # then self.assertIsNone(result) - self.assertTrue(mock_default_bot_client.guild_member.called) - - def test_return_false_if_api_returns_false(self, mock_default_bot_client): - # given - mock_default_bot_client.guild_member.return_value = False - # when - result = self.discord_user.update_username() - # then - self.assertFalse(result) - self.assertTrue(mock_default_bot_client.guild_member.called) - - def test_return_false_if_api_returns_corrput_data_1(self, mock_default_bot_client): - # given - mock_default_bot_client.guild_member.return_value = {'invalid': True} - # when - result = self.discord_user.update_username() - # then - self.assertFalse(result) - self.assertTrue(mock_default_bot_client.guild_member.called) - - def test_return_false_if_api_returns_corrput_data_2(self, mock_default_bot_client): - # given - user_info = { - 'user': { - 'id': str(TEST_USER_ID), - 'discriminator': '1234', - } - } - mock_default_bot_client.guild_member.return_value = user_info - # when - result = self.discord_user.update_username() - # then - self.assertFalse(result) - self.assertTrue(mock_default_bot_client.guild_member.called) - - def test_return_false_if_api_returns_corrput_data_3(self, mock_default_bot_client): - # given - user_info = { - 'user': { - 'id': str(TEST_USER_ID), - 'username': TEST_USER_NAME, - } - } - mock_default_bot_client.guild_member.return_value = user_info - # when - result = self.discord_user.update_username() - # then - self.assertFalse(result) - self.assertTrue(mock_default_bot_client.guild_member.called) + self.assertTrue(mock_guild_member.called) @patch(MODULE_PATH + '.models.notify', spec=True)