diff options
| -rw-r--r-- | .gitignore | 1 | ||||
| -rw-r--r-- | bot/__main__.py | 2 | ||||
| -rw-r--r-- | bot/exts/backend/sync/_syncers.py | 98 | ||||
| -rw-r--r-- | bot/exts/fun/duck_pond.py | 4 | ||||
| -rw-r--r-- | bot/exts/moderation/dm_relay.py | 6 | ||||
| -rw-r--r-- | bot/exts/moderation/infraction/infractions.py | 28 | ||||
| -rw-r--r-- | bot/exts/moderation/infraction/superstarify.py | 5 | ||||
| -rw-r--r-- | bot/exts/moderation/verification.py | 10 | ||||
| -rw-r--r-- | bot/exts/utils/ping.py | 2 | ||||
| -rw-r--r-- | bot/utils/messages.py | 31 | ||||
| -rw-r--r-- | docker-compose.yml | 1 | ||||
| -rw-r--r-- | tests/bot/exts/backend/sync/test_users.py | 120 |
12 files changed, 211 insertions, 97 deletions
diff --git a/.gitignore b/.gitignore index fb3156ab1..2074887ad 100644 --- a/.gitignore +++ b/.gitignore @@ -110,6 +110,7 @@ ENV/ # Logfiles log.* +*.log.* # Custom user configuration config.yml diff --git a/bot/__main__.py b/bot/__main__.py index da042a5ed..367be1300 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -58,7 +58,7 @@ bot = Bot( redis_session=redis_session, loop=loop, command_prefix=when_mentioned_or(constants.Bot.prefix), - activity=discord.Game(name="Commands: !help"), + activity=discord.Game(name=f"Commands: {constants.Bot.prefix}help"), case_insensitive=True, max_messages=10_000, allowed_mentions=discord.AllowedMentions(everyone=False, roles=allowed_roles), diff --git a/bot/exts/backend/sync/_syncers.py b/bot/exts/backend/sync/_syncers.py index 3d4a09df3..38468c2b1 100644 --- a/bot/exts/backend/sync/_syncers.py +++ b/bot/exts/backend/sync/_syncers.py @@ -14,7 +14,6 @@ log = logging.getLogger(__name__) # These objects are declared as namedtuples because tuples are hashable, # something that we make use of when diffing site roles against guild roles. _Role = namedtuple('Role', ('id', 'name', 'colour', 'permissions', 'position')) -_User = namedtuple('User', ('id', 'name', 'discriminator', 'roles', 'in_guild')) _Diff = namedtuple('Diff', ('created', 'updated', 'deleted')) @@ -134,61 +133,76 @@ class UserSyncer(Syncer): async def _get_diff(self, guild: Guild) -> _Diff: """Return the difference of users between the cache of `guild` and the database.""" log.trace("Getting the diff for users.") - users = await self.bot.api_client.get('bot/users') - # Pack DB roles and guild roles into one common, hashable format. - # They're hashable so that they're easily comparable with sets later. - db_users = { - user_dict['id']: _User( - roles=tuple(sorted(user_dict.pop('roles'))), - **user_dict - ) - for user_dict in users - } - guild_users = { - member.id: _User( - id=member.id, - name=member.name, - discriminator=int(member.discriminator), - roles=tuple(sorted(role.id for role in member.roles)), - in_guild=True - ) - for member in guild.members - } + users_to_create = [] + users_to_update = [] + seen_guild_users = set() + + async for db_user in self._get_users(): + # Store user fields which are to be updated. + updated_fields = {} - users_to_create = set() - users_to_update = set() + def maybe_update(db_field: str, guild_value: t.Union[str, int]) -> None: + # Equalize DB user and guild user attributes. + if db_user[db_field] != guild_value: + updated_fields[db_field] = guild_value - for db_user in db_users.values(): - guild_user = guild_users.get(db_user.id) - if guild_user is not None: - if db_user != guild_user: - users_to_update.add(guild_user) + if guild_user := guild.get_member(db_user["id"]): + seen_guild_users.add(guild_user.id) - elif db_user.in_guild: + maybe_update("name", guild_user.name) + maybe_update("discriminator", int(guild_user.discriminator)) + maybe_update("in_guild", True) + + guild_roles = [role.id for role in guild_user.roles] + if set(db_user["roles"]) != set(guild_roles): + updated_fields["roles"] = guild_roles + + elif db_user["in_guild"]: # The user is known in the DB but not the guild, and the # DB currently specifies that the user is a member of the guild. # This means that the user has left since the last sync. # Update the `in_guild` attribute of the user on the site # to signify that the user left. - new_api_user = db_user._replace(in_guild=False) - users_to_update.add(new_api_user) - - new_user_ids = set(guild_users.keys()) - set(db_users.keys()) - for user_id in new_user_ids: - # The user is known on the guild but not on the API. This means - # that the user has joined since the last sync. Create it. - new_user = guild_users[user_id] - users_to_create.add(new_user) + updated_fields["in_guild"] = False + + if updated_fields: + updated_fields["id"] = db_user["id"] + users_to_update.append(updated_fields) + + for member in guild.members: + if member.id not in seen_guild_users: + # The user is known on the guild but not on the API. This means + # that the user has joined since the last sync. Create it. + new_user = { + "id": member.id, + "name": member.name, + "discriminator": int(member.discriminator), + "roles": [role.id for role in member.roles], + "in_guild": True + } + users_to_create.append(new_user) return _Diff(users_to_create, users_to_update, None) + async def _get_users(self) -> t.AsyncIterable: + """GET users from database.""" + query_params = { + "page": 1 + } + while query_params["page"]: + res = await self.bot.api_client.get("bot/users", params=query_params) + for user in res["results"]: + yield user + + query_params["page"] = res["next_page_no"] + async def _sync(self, diff: _Diff) -> None: """Synchronise the database with the user cache of `guild`.""" log.trace("Syncing created users...") - for user in diff.created: - await self.bot.api_client.post('bot/users', json=user._asdict()) + if diff.created: + await self.bot.api_client.post("bot/users", json=diff.created) log.trace("Syncing updated users...") - for user in diff.updated: - await self.bot.api_client.put(f'bot/users/{user.id}', json=user._asdict()) + if diff.updated: + await self.bot.api_client.patch("bot/users/bulk_patch", json=diff.updated) diff --git a/bot/exts/fun/duck_pond.py b/bot/exts/fun/duck_pond.py index 82084ea88..48aa2749c 100644 --- a/bot/exts/fun/duck_pond.py +++ b/bot/exts/fun/duck_pond.py @@ -22,6 +22,7 @@ class DuckPond(Cog): self.bot = bot self.webhook_id = constants.Webhooks.duck_pond self.webhook = None + self.ducked_messages = [] self.bot.loop.create_task(self.fetch_webhook()) self.relay_lock = None @@ -176,7 +177,8 @@ class DuckPond(Cog): duck_count = await self.count_ducks(message) # If we've got more than the required amount of ducks, send the message to the duck_pond. - if duck_count >= constants.DuckPond.threshold: + if duck_count >= constants.DuckPond.threshold and message.id not in self.ducked_messages: + self.ducked_messages.append(message.id) await self.locked_relay(message) @Cog.listener() diff --git a/bot/exts/moderation/dm_relay.py b/bot/exts/moderation/dm_relay.py index 14263e004..4d5142b55 100644 --- a/bot/exts/moderation/dm_relay.py +++ b/bot/exts/moderation/dm_relay.py @@ -90,7 +90,11 @@ class DMRelay(Cog): # Handle any attachments if message.attachments: try: - await send_attachments(message, self.webhook) + await send_attachments( + message, + self.webhook, + username=f"{message.author.display_name} ({message.author.id})" + ) except (discord.errors.Forbidden, discord.errors.NotFound): e = discord.Embed( description=":x: **This message contained an attachment, but it could not be retrieved**", diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index a8b3feb38..7cf7075e6 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -71,6 +71,23 @@ class Infractions(InfractionScheduler, commands.Cog): """Permanently ban a user for the given reason and stop watching them with Big Brother.""" await self.apply_ban(ctx, user, reason) + @command(aliases=('pban',)) + async def purgeban( + self, + ctx: Context, + user: FetchedMember, + purge_days: t.Optional[int] = 1, + *, + reason: t.Optional[str] = None + ) -> None: + """ + Same as ban but removes all their messages for the given number of days, default being 1. + + `purge_days` can only be values between 0 and 7. + Anything outside these bounds are automatically adjusted to their respective limits. + """ + await self.apply_ban(ctx, user, reason, max(min(purge_days, 7), 0)) + # endregion # region: Temporary infractions @@ -246,7 +263,14 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy(member_arg=2) - async def apply_ban(self, ctx: Context, user: UserSnowflake, reason: t.Optional[str], **kwargs) -> None: + async def apply_ban( + self, + ctx: Context, + user: UserSnowflake, + reason: t.Optional[str], + purge_days: t.Optional[int] = 0, + **kwargs + ) -> None: """ Apply a ban infraction with kwargs passed to `post_infraction`. @@ -278,7 +302,7 @@ class Infractions(InfractionScheduler, commands.Cog): if reason: reason = textwrap.shorten(reason, width=512, placeholder="...") - action = ctx.guild.ban(user, reason=reason, delete_message_days=0) + action = ctx.guild.ban(user, reason=reason, delete_message_days=purge_days) await self.apply_infraction(ctx, infraction, user, action) if infraction.get('expires_at') is not None: diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index eec63f5b3..adfe42fcd 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -135,7 +135,8 @@ class Superstarify(InfractionScheduler, Cog): return # Post the infraction to the API - reason = reason or f"old nick: {member.display_name}" + old_nick = member.display_name + reason = reason or f"old nick: {old_nick}" infraction = await _utils.post_infraction(ctx, member, "superstar", reason, duration, active=True) id_ = infraction["id"] @@ -148,7 +149,7 @@ class Superstarify(InfractionScheduler, Cog): await member.edit(nick=forced_nick, reason=reason) self.schedule_expiration(infraction) - old_nick = escape_markdown(member.display_name) + old_nick = escape_markdown(old_nick) forced_nick = escape_markdown(forced_nick) # Send a DM to the user to notify them of their new infraction. diff --git a/bot/exts/moderation/verification.py b/bot/exts/moderation/verification.py index c3ad8687e..d28114298 100644 --- a/bot/exts/moderation/verification.py +++ b/bot/exts/moderation/verification.py @@ -547,6 +547,16 @@ class Verification(Cog): # video. if raw_member.get("is_pending"): await self.member_gating_cache.set(member.id, True) + + # TODO: Temporary, remove soon after asking joe. + await self.mod_log.send_log_message( + icon_url=self.bot.user.avatar_url, + colour=discord.Colour.blurple(), + title="New native gated user", + channel_id=constants.Channels.user_log, + text=f"<@{member.id}> ({member.id})", + ) + return log.trace(f"Sending on join message to new member: {member.id}") diff --git a/bot/exts/utils/ping.py b/bot/exts/utils/ping.py index a9ca3dbeb..572fc934b 100644 --- a/bot/exts/utils/ping.py +++ b/bot/exts/utils/ping.py @@ -33,7 +33,7 @@ class Latency(commands.Cog): """ # datetime.datetime objects do not have the "milliseconds" attribute. # It must be converted to seconds before converting to milliseconds. - bot_ping = (datetime.utcnow() - ctx.message.created_at).total_seconds() / 1000 + bot_ping = (datetime.utcnow() - ctx.message.created_at).total_seconds() * 1000 bot_ping = f"{bot_ping:.{ROUND_LATENCY}f} ms" try: diff --git a/bot/utils/messages.py b/bot/utils/messages.py index d0b2342b3..b6c7cab50 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -56,15 +56,24 @@ async def wait_for_deletion( async def send_attachments( message: discord.Message, destination: Union[discord.TextChannel, discord.Webhook], - link_large: bool = True + link_large: bool = True, + use_cached: bool = False, + **kwargs ) -> List[str]: """ Re-upload the message's attachments to the destination and return a list of their new URLs. Each attachment is sent as a separate message to more easily comply with the request/file size limit. If link_large is True, attachments which are too large are instead grouped into a single - embed which links to them. + embed which links to them. Extra kwargs will be passed to send() when sending the attachment. """ + webhook_send_kwargs = { + 'username': message.author.display_name, + 'avatar_url': message.author.avatar_url, + } + webhook_send_kwargs.update(kwargs) + webhook_send_kwargs['username'] = sub_clyde(webhook_send_kwargs['username']) + large = [] urls = [] for attachment in message.attachments: @@ -78,18 +87,14 @@ async def send_attachments( # but some may get through hence the try-catch. if attachment.size <= destination.guild.filesize_limit - 512: with BytesIO() as file: - await attachment.save(file, use_cached=True) + await attachment.save(file, use_cached=use_cached) attachment_file = discord.File(file, filename=attachment.filename) if isinstance(destination, discord.TextChannel): - msg = await destination.send(file=attachment_file) + msg = await destination.send(file=attachment_file, **kwargs) urls.append(msg.attachments[0].url) else: - await destination.send( - file=attachment_file, - username=sub_clyde(message.author.display_name), - avatar_url=message.author.avatar_url - ) + await destination.send(file=attachment_file, **webhook_send_kwargs) elif link_large: large.append(attachment) else: @@ -106,13 +111,9 @@ async def send_attachments( embed.set_footer(text="Attachments exceed upload size limit.") if isinstance(destination, discord.TextChannel): - await destination.send(embed=embed) + await destination.send(embed=embed, **kwargs) else: - await destination.send( - embed=embed, - username=sub_clyde(message.author.display_name), - avatar_url=message.author.avatar_url - ) + await destination.send(embed=embed, **webhook_send_kwargs) return urls diff --git a/docker-compose.yml b/docker-compose.yml index cff7d33d6..8be5aac0e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,7 @@ services: - postgres environment: DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite + METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity SECRET_KEY: suitable-for-development-only STATIC_ROOT: /var/www/static diff --git a/tests/bot/exts/backend/sync/test_users.py b/tests/bot/exts/backend/sync/test_users.py index c0a1da35c..9f380a15d 100644 --- a/tests/bot/exts/backend/sync/test_users.py +++ b/tests/bot/exts/backend/sync/test_users.py @@ -1,7 +1,6 @@ import unittest -from unittest import mock -from bot.exts.backend.sync._syncers import UserSyncer, _Diff, _User +from bot.exts.backend.sync._syncers import UserSyncer, _Diff from tests import helpers @@ -10,7 +9,7 @@ def fake_user(**kwargs): kwargs.setdefault("id", 43) kwargs.setdefault("name", "bob the test man") kwargs.setdefault("discriminator", 1337) - kwargs.setdefault("roles", (666,)) + kwargs.setdefault("roles", [666]) kwargs.setdefault("in_guild", True) return kwargs @@ -40,22 +39,42 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): return guild + @staticmethod + def get_mock_member(member: dict): + member = member.copy() + del member["in_guild"] + mock_member = helpers.MockMember(**member) + mock_member.roles = [helpers.MockRole(id=role_id) for role_id in member["roles"]] + return mock_member + async def test_empty_diff_for_no_users(self): """When no users are given, an empty diff should be returned.""" + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [] + } guild = self.get_guild() actual_diff = await self.syncer._get_diff(guild) - expected_diff = (set(), set(), None) + expected_diff = ([], [], None) self.assertEqual(actual_diff, expected_diff) async def test_empty_diff_for_identical_users(self): """No differences should be found if the users in the guild and DB are identical.""" - self.bot.api_client.get.return_value = [fake_user()] + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user()] + } guild = self.get_guild(fake_user()) + guild.get_member.return_value = self.get_mock_member(fake_user()) actual_diff = await self.syncer._get_diff(guild) - expected_diff = (set(), set(), None) + expected_diff = ([], [], None) self.assertEqual(actual_diff, expected_diff) @@ -63,59 +82,102 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): """Only updated users should be added to the 'updated' set of the diff.""" updated_user = fake_user(id=99, name="new") - self.bot.api_client.get.return_value = [fake_user(id=99, name="old"), fake_user()] + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user(id=99, name="old"), fake_user()] + } guild = self.get_guild(updated_user, fake_user()) + guild.get_member.side_effect = [ + self.get_mock_member(updated_user), + self.get_mock_member(fake_user()) + ] actual_diff = await self.syncer._get_diff(guild) - expected_diff = (set(), {_User(**updated_user)}, None) + expected_diff = ([], [{"id": 99, "name": "new"}], None) self.assertEqual(actual_diff, expected_diff) async def test_diff_for_new_users(self): - """Only new users should be added to the 'created' set of the diff.""" + """Only new users should be added to the 'created' list of the diff.""" new_user = fake_user(id=99, name="new") - self.bot.api_client.get.return_value = [fake_user()] + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user()] + } guild = self.get_guild(fake_user(), new_user) - + guild.get_member.side_effect = [ + self.get_mock_member(fake_user()), + self.get_mock_member(new_user) + ] actual_diff = await self.syncer._get_diff(guild) - expected_diff = ({_User(**new_user)}, set(), None) + expected_diff = ([new_user], [], None) self.assertEqual(actual_diff, expected_diff) async def test_diff_sets_in_guild_false_for_leaving_users(self): """When a user leaves the guild, the `in_guild` flag is updated to `False`.""" - leaving_user = fake_user(id=63, in_guild=False) - - self.bot.api_client.get.return_value = [fake_user(), fake_user(id=63)] + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user(), fake_user(id=63)] + } guild = self.get_guild(fake_user()) + guild.get_member.side_effect = [ + self.get_mock_member(fake_user()), + None + ] actual_diff = await self.syncer._get_diff(guild) - expected_diff = (set(), {_User(**leaving_user)}, None) + expected_diff = ([], [{"id": 63, "in_guild": False}], None) self.assertEqual(actual_diff, expected_diff) async def test_diff_for_new_updated_and_leaving_users(self): """When users are added, updated, and removed, all of them are returned properly.""" new_user = fake_user(id=99, name="new") + updated_user = fake_user(id=55, name="updated") - leaving_user = fake_user(id=63, in_guild=False) - self.bot.api_client.get.return_value = [fake_user(), fake_user(id=55), fake_user(id=63)] + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user(), fake_user(id=55), fake_user(id=63)] + } guild = self.get_guild(fake_user(), new_user, updated_user) + guild.get_member.side_effect = [ + self.get_mock_member(fake_user()), + self.get_mock_member(updated_user), + None + ] actual_diff = await self.syncer._get_diff(guild) - expected_diff = ({_User(**new_user)}, {_User(**updated_user), _User(**leaving_user)}, None) + expected_diff = ([new_user], [{"id": 55, "name": "updated"}, {"id": 63, "in_guild": False}], None) self.assertEqual(actual_diff, expected_diff) async def test_empty_diff_for_db_users_not_in_guild(self): - """When the DB knows a user the guild doesn't, no difference is found.""" - self.bot.api_client.get.return_value = [fake_user(), fake_user(id=63, in_guild=False)] + """When the DB knows a user, but the guild doesn't, no difference is found.""" + self.bot.api_client.get.return_value = { + "count": 3, + "next_page_no": None, + "previous_page_no": None, + "results": [fake_user(), fake_user(id=63, in_guild=False)] + } guild = self.get_guild(fake_user()) + guild.get_member.side_effect = [ + self.get_mock_member(fake_user()), + None + ] actual_diff = await self.syncer._get_diff(guild) - expected_diff = (set(), set(), None) + expected_diff = ([], [], None) self.assertEqual(actual_diff, expected_diff) @@ -131,13 +193,10 @@ class UserSyncerSyncTests(unittest.IsolatedAsyncioTestCase): """Only POST requests should be made with the correct payload.""" users = [fake_user(id=111), fake_user(id=222)] - user_tuples = {_User(**user) for user in users} - diff = _Diff(user_tuples, set(), None) + diff = _Diff(users, [], None) await self.syncer._sync(diff) - calls = [mock.call("bot/users", json=user) for user in users] - self.bot.api_client.post.assert_has_calls(calls, any_order=True) - self.assertEqual(self.bot.api_client.post.call_count, len(users)) + self.bot.api_client.post.assert_called_once_with("bot/users", json=diff.created) self.bot.api_client.put.assert_not_called() self.bot.api_client.delete.assert_not_called() @@ -146,13 +205,10 @@ class UserSyncerSyncTests(unittest.IsolatedAsyncioTestCase): """Only PUT requests should be made with the correct payload.""" users = [fake_user(id=111), fake_user(id=222)] - user_tuples = {_User(**user) for user in users} - diff = _Diff(set(), user_tuples, None) + diff = _Diff([], users, None) await self.syncer._sync(diff) - calls = [mock.call(f"bot/users/{user['id']}", json=user) for user in users] - self.bot.api_client.put.assert_has_calls(calls, any_order=True) - self.assertEqual(self.bot.api_client.put.call_count, len(users)) + self.bot.api_client.patch.assert_called_once_with("bot/users/bulk_patch", json=diff.updated) self.bot.api_client.post.assert_not_called() self.bot.api_client.delete.assert_not_called() |