From e29e8e35643b1444388871f63192fc57276e92cf Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Mon, 20 Mar 2023 18:50:00 +1000 Subject: [PATCH 01/28] handle starboard message deletions gracefully --- .env.example | 3 ++- uqcsbot/starboard.py | 60 ++++++++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/.env.example b/.env.example index e1aba8a..a7bbde4 100644 --- a/.env.example +++ b/.env.example @@ -8,4 +8,5 @@ POSTGRES_URI_BOT=sqlite:/// # Variables for various command cogs. AOC_SESSION_ID= SB_BASE_THRESHOLD=8 -SB_BIG_THRESHOLD=24 \ No newline at end of file +SB_BIG_THRESHOLD=24 +SB_RATELIMIT=30 \ No newline at end of file diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 7a3402c..436fcb1 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -40,15 +40,23 @@ def _rm_big_ratelimit(self, id: int): """ Callback to remove a message from the big-ratelimited list """ self.big_blocked_messages.remove(id) - def _query_sb_message(self, recv: int) -> Optional[int]: - """ Get the starboard message ID corresponding to the recieved message """ + async def _query_sb_message(self, recv: int) -> discord.Message: + """ Get the starboard message corresponding to the recieved message. Handles messages no longer existing and cleans up the DB accordingly. """ db_session = self.bot.create_db_session() - id = db_session.query(models.Starboard).filter(models.Starboard.recv == recv).one_or_none() - db_session.close() - - if id is not None: - return id.sent - return id + entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv) + message_id = entry.one_or_none() + + message = None + if message_id is not None: + try: + message = await self.starboard_channel.fetch_message(message_id.sent) + except discord.errors.NotFound: + entry.delete(synchronize_session=False) + db_session.commit() + finally: + db_session.close() + + return message def _update_sb_message(self, recv: int, sent: int): """ Sets the ID of the starboard message corresponding to the recieved message """ @@ -112,10 +120,10 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): if reaction is not None: new_reaction_count = reaction.count - sb_message_id = self._query_sb_message(recv_message.id) + sb_message = await self._query_sb_message(recv_message.id) if new_reaction_count >= self.base_threshold and \ - sb_message_id is None and recv_message.id not in self.base_blocked_messages: + sb_message is None and recv_message.id not in self.base_blocked_messages: new_sb_message = await self.starboard_channel.send( content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}", embeds=self._create_sb_embed(recv_message) @@ -135,15 +143,14 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): self.base_blocked_messages += [recv_message.id] Timer(self.ratelimit, self._rm_base_ratelimit, [recv_message.id]).start() elif new_reaction_count > self.base_threshold and \ - sb_message_id is not None and recv_message.id not in self.big_blocked_messages: - old_sb_message = await self.starboard_channel.fetch_message(sb_message_id) + sb_message is not None and recv_message.id not in self.big_blocked_messages: - await old_sb_message.edit( + await sb_message.edit( content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}" ) - if new_reaction_count >= self.big_threshold and not old_sb_message.pinned: - await old_sb_message.pin(reason=f"Reached {self.big_threshold} starboard reactions") + if new_reaction_count >= self.big_threshold and not sb_message.pinned: + await sb_message.pin(reason=f"Reached {self.big_threshold} starboard reactions") self.big_blocked_messages += [recv_message.id] Timer(self.ratelimit, self._rm_big_ratelimit, [recv_message.id]).start() @@ -170,12 +177,10 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): if reaction is not None: new_reaction_count = reaction.count - sb_message_id = self._query_sb_message(recv_message.id) - if sb_message_id is None: + sb_message = await self._query_sb_message(recv_message.id) + if sb_message is None: return - sb_message = await self.starboard_channel.fetch_message(sb_message_id) - if new_reaction_count < self.base_threshold: await sb_message.delete() # delete will also unpin self._remove_sb_message(payload.message_id) @@ -183,9 +188,8 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): if new_reaction_count < self.big_threshold and sb_message.pinned: await sb_message.unpin() - - old_sb_message = await self.starboard_channel.fetch_message(sb_message_id) - await old_sb_message.edit( + + await sb_message.edit( content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}" ) @@ -203,11 +207,9 @@ async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): recv_message = await channel.fetch_message(payload.message_id) - sb_message_id = self._query_sb_message(recv_message.id) - if sb_message_id is None: + sb_message= await self._query_sb_message(recv_message.id) + if sb_message is None: return - - sb_message = await self.starboard_channel.fetch_message(sb_message_id) # delete will also unpin await sb_message.delete() @@ -230,11 +232,9 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo recv_message = await channel.fetch_message(payload.message_id) - sb_message_id = self._query_sb_message(recv_message.id) - if sb_message_id is None: + sb_message = await self._query_sb_message(recv_message.id) + if sb_message is None: return - - sb_message = await self.starboard_channel.fetch_message(sb_message_id) # delete will also unpin await sb_message.delete() From 4c3183805dc0ddc576600a6b5df806e3ff855cdd Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Mon, 20 Mar 2023 19:32:45 +1000 Subject: [PATCH 02/28] timezones are fixed. admin-blacklist for starboard implemented. --- uqcsbot/models.py | 2 +- uqcsbot/starboard.py | 97 +++++++++++++++++++++++++++++++++++++++----- uqcsbot/text.py | 10 ++--- 3 files changed, 92 insertions(+), 17 deletions(-) diff --git a/uqcsbot/models.py b/uqcsbot/models.py index e56a2b7..11307aa 100644 --- a/uqcsbot/models.py +++ b/uqcsbot/models.py @@ -31,4 +31,4 @@ class Starboard(Base): __tablename__ = 'starboard' recv = Column("recv", BigInteger, primary_key=True, nullable=False) - sent = Column("sent", BigInteger, nullable=False, unique=True) \ No newline at end of file + sent = Column("sent", BigInteger, nullable=True, unique=True) \ No newline at end of file diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 436fcb1..a85ff90 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -1,16 +1,22 @@ import os -from typing import Optional from threading import Timer +from datetime import timezone import discord +from discord import app_commands from discord.ext import commands +from pytz import timezone from uqcsbot import models # needs to be models and not just starboard because of namespacing with this class +class BlacklistedMessageError(Exception): + pass + class Starboard(commands.Cog): CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" + BRISBANE_TZ = timezone('Australia/Brisbane') def __init__(self, bot: commands.Bot): self.bot = bot @@ -20,6 +26,18 @@ def __init__(self, bot: commands.Bot): self.base_blocked_messages = [] # messages that are temp blocked from being resent to the starboard self.big_blocked_messages = [] # messages that are temp blocked from being re-pinned in the starboard + + self.whitelist_menu = app_commands.ContextMenu( + name="Starboard Whitelist", + callback=self.context_whitelist_sb_message, + ) + self.bot.tree.add_command(self.whitelist_menu) + + self.blacklist_menu = app_commands.ContextMenu( + name="Starboard Blacklist", + callback=self.context_blacklist_sb_message, + ) + self.bot.tree.add_command(self.blacklist_menu) @commands.Cog.listener() async def on_ready(self): @@ -40,16 +58,55 @@ def _rm_big_ratelimit(self, id: int): """ Callback to remove a message from the big-ratelimited list """ self.big_blocked_messages.remove(id) + @app_commands.default_permissions(manage_messages=True) + async def context_blacklist_sb_message(self, interaction: discord.Interaction, message: discord.Message): + """ Blacklists a message from being starboarded. If the message is already starboarded, also deletes it. """ + db_session = self.bot.create_db_session() + entry = db_session.query(models.Starboard).filter(models.Starboard.recv == message.id) + query_val = entry.one_or_none() + + if query_val != None: + if query_val.sent == None: + return + + await (await self.starboard_channel.fetch_message(query_val.sent)).delete() + query_val.sent = None + else: + db_session.add(models.Starboard(recv=message.id, sent=None)) + + db_session.commit() + db_session.close() + + await interaction.response.send_message(f"Blacklisted message {message.id}.") + + @app_commands.default_permissions(manage_messages=True) + async def context_whitelist_sb_message(self, interaction: discord.Interaction, message: discord.Message): + """ Removes a message from the starboard blacklist. Doesn't perform an 'update' of the message. """ + db_session = self.bot.create_db_session() + entry = db_session.query(models.Starboard).filter( + models.Starboard.recv == message.id, + models.Starboard.sent == None + ) + if entry.one_or_none() != None: + entry.delete(synchronize_session=False) + db_session.commit() + db_session.close() + await interaction.response.send_message(f"Whitelisted message {message.id}.") + async def _query_sb_message(self, recv: int) -> discord.Message: - """ Get the starboard message corresponding to the recieved message. Handles messages no longer existing and cleans up the DB accordingly. """ + """ Get the starboard message corresponding to the recieved message. + Handles messages no longer existing and cleans up the DB accordingly. """ db_session = self.bot.create_db_session() entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv) - message_id = entry.one_or_none() + query_val = entry.one_or_none() message = None - if message_id is not None: + if query_val is not None: try: - message = await self.starboard_channel.fetch_message(message_id.sent) + if query_val.sent == None: + raise BlacklistedMessageError() + + message = await self.starboard_channel.fetch_message(query_val.sent) except discord.errors.NotFound: entry.delete(synchronize_session=False) db_session.commit() @@ -76,7 +133,7 @@ def _remove_sb_message(self, recv: int): def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: embed = discord.Embed(color=recv.author.top_role.color, description=recv.content) embed.set_author(name=recv.author.display_name, icon_url=recv.author.display_avatar.url) - embed.set_footer(text=recv.created_at.strftime('%b %d, %H:%M:%S')) + embed.set_footer(text=recv.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) if len(recv.attachments) > 0: embed.set_image(url = recv.attachments[0].url) @@ -93,7 +150,9 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: icon_url=recv.reference.resolved.author.display_avatar.url ) - replied.set_footer(text=recv.reference.resolved.created_at.strftime('%b %d, %H:%M:%S')) + replied.set_footer( + text=recv.reference.resolved.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S') + ) return [replied, embed] @@ -120,7 +179,10 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): if reaction is not None: new_reaction_count = reaction.count - sb_message = await self._query_sb_message(recv_message.id) + try: + sb_message = await self._query_sb_message(recv_message.id) + except BlacklistedMessageError: + return if new_reaction_count >= self.base_threshold and \ sb_message is None and recv_message.id not in self.base_blocked_messages: @@ -177,7 +239,11 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): if reaction is not None: new_reaction_count = reaction.count - sb_message = await self._query_sb_message(recv_message.id) + try: + sb_message = await self._query_sb_message(recv_message.id) + except BlacklistedMessageError: + return + if sb_message is None: return @@ -207,7 +273,11 @@ async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): recv_message = await channel.fetch_message(payload.message_id) - sb_message= await self._query_sb_message(recv_message.id) + try: + sb_message = await self._query_sb_message(recv_message.id) + except BlacklistedMessageError: + return + if sb_message is None: return @@ -232,7 +302,11 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo recv_message = await channel.fetch_message(payload.message_id) - sb_message = await self._query_sb_message(recv_message.id) + try: + sb_message = await self._query_sb_message(recv_message.id) + except BlacklistedMessageError: + return + if sb_message is None: return @@ -240,5 +314,6 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo await sb_message.delete() self._remove_sb_message(payload.message_id) + async def setup(bot: commands.Bot): await bot.add_cog(Starboard(bot)) \ No newline at end of file diff --git a/uqcsbot/text.py b/uqcsbot/text.py index 7fc99b6..77fbfa6 100644 --- a/uqcsbot/text.py +++ b/uqcsbot/text.py @@ -21,11 +21,11 @@ def __init__(self, bot: commands.Bot): ) self.bot.tree.add_command(self.mock_menu) - self.scare_menu = app_commands.ContextMenu( - name="Scare", - callback=self.scare_context, - ) - self.bot.tree.add_command(self.scare_menu) + #self.scare_menu = app_commands.ContextMenu( + # name="Scare", + # callback=self.scare_context, + #) + #self.bot.tree.add_command(self.scare_menu) @app_commands.command() @app_commands.describe(message="Input string") From 3c542eaec5f5e30966781d26bdf5b4ea854a7094 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Tue, 21 Mar 2023 09:09:55 +1000 Subject: [PATCH 03/28] track reactions on OG and SB messages --- uqcsbot/models.py | 1 + uqcsbot/starboard.py | 188 ++++++++++++++++++++++++++++++------------- 2 files changed, 133 insertions(+), 56 deletions(-) diff --git a/uqcsbot/models.py b/uqcsbot/models.py index 11307aa..ebf29e2 100644 --- a/uqcsbot/models.py +++ b/uqcsbot/models.py @@ -31,4 +31,5 @@ class Starboard(Base): __tablename__ = 'starboard' recv = Column("recv", BigInteger, primary_key=True, nullable=False) + recv_location = Column("recv_location", BigInteger, nullable=True, unique=False) sent = Column("sent", BigInteger, nullable=True, unique=True) \ No newline at end of file diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index a85ff90..c3f1fef 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -1,5 +1,6 @@ import os from threading import Timer +from typing import List from datetime import timezone import discord @@ -65,14 +66,14 @@ async def context_blacklist_sb_message(self, interaction: discord.Interaction, m entry = db_session.query(models.Starboard).filter(models.Starboard.recv == message.id) query_val = entry.one_or_none() - if query_val != None: - if query_val.sent == None: + if query_val is not None: + if query_val.sent is None: return await (await self.starboard_channel.fetch_message(query_val.sent)).delete() query_val.sent = None else: - db_session.add(models.Starboard(recv=message.id, sent=None)) + db_session.add(models.Starboard(recv=message.id, recv_location=message.channel.id, sent=None)) db_session.commit() db_session.close() @@ -87,7 +88,7 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m models.Starboard.recv == message.id, models.Starboard.sent == None ) - if entry.one_or_none() != None: + if entry.one_or_none() is not None: entry.delete(synchronize_session=False) db_session.commit() db_session.close() @@ -103,7 +104,7 @@ async def _query_sb_message(self, recv: int) -> discord.Message: message = None if query_val is not None: try: - if query_val.sent == None: + if query_val.sent is None: raise BlacklistedMessageError() message = await self.starboard_channel.fetch_message(query_val.sent) @@ -115,10 +116,51 @@ async def _query_sb_message(self, recv: int) -> discord.Message: return message - def _update_sb_message(self, recv: int, sent: int): + async def _query_og_message(self, sent: int) -> discord.Message: + """ Get the og message corresponding to this starboard message. + Handles messages no longer existing. """ + db_session = self.bot.create_db_session() + entry = db_session.query(models.Starboard).filter( + models.Starboard.sent == sent + ).one_or_none() + + message = None + if entry is not None: + if entry.recv is not None and entry.recv_location is not None: + try: + channel = self.bot.get_channel(entry.recv_location) + if channel is None: + raise discord.errors.NotFound() + + message = await channel.fetch_message(entry.recv) + except discord.errors.NotFound: + entry.recv_location = None + entry.recv = None + db_session.commit() + + db_session.close() + return message + + async def _find_num_reactions(self, messages: List[discord.Message]) -> int: + """ Find the total number of starboard_emoji reacts across this list of messages. """ + users = [] + authors = [] + + for message in messages: + if message is None: + continue + + authors += [message.author.id] + reaction = discord.utils.get(message.reactions, emoji=self.starboard_emoji) + if reaction is not None: + users += [user.id async for user in reaction.users()] + + return len(set([user for user in users if user not in authors])) + + def _update_sb_message(self, recv: int, recv_location: int, sent: int): """ Sets the ID of the starboard message corresponding to the recieved message """ db_session = self.bot.create_db_session() - db_session.add(models.Starboard(recv=recv, sent=sent)) + db_session.add(models.Starboard(recv=recv, recv_location=recv_location, sent=sent)) db_session.commit() db_session.close() @@ -157,6 +199,24 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: return [replied, embed] return [embed] + + @app_commands.command() + @app_commands.default_permissions(manage_messages=True) + async def cleanup_starboard(self, interaction: discord.Interaction): + """ Cleans up the last 100 messages from the starboard. + Removes any uqcsbot message that doesn't have a corresponding message id. """ + sb_messages = await self.starboard_channel.history(limit=100) + db_session = self.bot.create_db_session() + + await interaction.defer(thinking=True) + + for message in sb_messages: + query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() + if query is None and message.author.id == self.user.id: + message.delete() + + db_session.close() + await interaction.followup.send("Finished cleaning up.") @commands.Cog.listener() async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): @@ -168,26 +228,24 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): return channel = self.bot.get_channel(payload.channel_id) - if channel == self.starboard_channel or channel.category.name.startswith("admin"): - # TODO: "reaction count" for starboard could take into account (original + starboard) - return - - recv_message = await channel.fetch_message(payload.message_id) - reaction = discord.utils.get(recv_message.reactions, emoji=self.starboard_emoji) - - new_reaction_count = 0 - if reaction is not None: - new_reaction_count = reaction.count try: - sb_message = await self._query_sb_message(recv_message.id) + messages = [await channel.fetch_message(payload.message_id)] + if channel == self.starboard_channel: + messages += [await self._query_og_message(payload.message_id)] + sb_message, recv_message = messages + else: + messages += [await self._query_sb_message(payload.message_id)] + recv_message, sb_message = messages except BlacklistedMessageError: return + + new_reaction_count = await self._find_num_reactions(messages) if new_reaction_count >= self.base_threshold and \ sb_message is None and recv_message.id not in self.base_blocked_messages: new_sb_message = await self.starboard_channel.send( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}", + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}", embeds=self._create_sb_embed(recv_message) # note that the embed is never edited, which means the content of the starboard post is fixed as # soon as the 5th reaction is processed @@ -200,7 +258,7 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): )) ) - self._update_sb_message(recv_message.id, new_sb_message.id) + self._update_sb_message(recv_message.id, recv_message.channel.id, new_sb_message.id) self.base_blocked_messages += [recv_message.id] Timer(self.ratelimit, self._rm_base_ratelimit, [recv_message.id]).start() @@ -208,7 +266,7 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): sb_message is not None and recv_message.id not in self.big_blocked_messages: await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}" + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" ) if new_reaction_count >= self.big_threshold and not sb_message.pinned: @@ -228,24 +286,22 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): return channel = self.bot.get_channel(payload.channel_id) - if channel == self.starboard_channel or channel.category.name.startswith("admin"): - # TODO: "reaction count" for starboard could take into account (original + starboard) - return - - recv_message = await channel.fetch_message(payload.message_id) - reaction = discord.utils.get(recv_message.reactions, emoji=self.starboard_emoji) - - new_reaction_count = 0 - if reaction is not None: - new_reaction_count = reaction.count try: - sb_message = await self._query_sb_message(recv_message.id) + messages = [await channel.fetch_message(payload.message_id)] + if channel == self.starboard_channel: + messages += [await self._query_og_message(payload.message_id)] + sb_message, recv_message = messages + else: + messages += [await self._query_sb_message(payload.message_id)] + recv_message, sb_message = messages except BlacklistedMessageError: return - if sb_message is None: + if sb_message == None: return + + new_reaction_count = await self._find_num_reactions(messages) if new_reaction_count < self.base_threshold: await sb_message.delete() # delete will also unpin @@ -256,7 +312,7 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): await sb_message.unpin() await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | {recv_message.channel.mention}" + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" ) @@ -267,23 +323,33 @@ async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): return channel = self.bot.get_channel(payload.channel_id) - if channel == self.starboard_channel or channel.category.name.startswith("admin"): - # TODO: "reaction count" for starboard could take into account (original + starboard) - return - - recv_message = await channel.fetch_message(payload.message_id) - try: - sb_message = await self._query_sb_message(recv_message.id) + messages = [await channel.fetch_message(payload.message_id)] + if channel == self.starboard_channel: + messages += [await self._query_og_message(payload.message_id)] + sb_message, recv_message = messages + else: + messages += [await self._query_sb_message(payload.message_id)] + recv_message, sb_message = messages except BlacklistedMessageError: return - if sb_message is None: + if sb_message == None: + return + + new_reaction_count = await self._find_num_reactions(messages) + + if new_reaction_count < self.base_threshold: + await sb_message.delete() # delete will also unpin + self._remove_sb_message(payload.message_id) return + + if new_reaction_count < self.big_threshold and sb_message.pinned: + await sb_message.unpin() - # delete will also unpin - await sb_message.delete() - self._remove_sb_message(payload.message_id) + await sb_message.edit( + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" + ) @commands.Cog.listener() @@ -296,23 +362,33 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo return channel = self.bot.get_channel(payload.channel_id) - if channel == self.starboard_channel or channel.category.name.startswith("admin"): - # TODO: "reaction count" for starboard could take into account (original + starboard) - return - - recv_message = await channel.fetch_message(payload.message_id) - try: - sb_message = await self._query_sb_message(recv_message.id) + messages = [await channel.fetch_message(payload.message_id)] + if channel == self.starboard_channel: + messages += [await self._query_og_message(payload.message_id)] + sb_message, recv_message = messages + else: + messages += [await self._query_sb_message(payload.message_id)] + recv_message, sb_message = messages except BlacklistedMessageError: return - if sb_message is None: + if sb_message == None: return + + new_reaction_count = await self._find_num_reactions(messages) - # delete will also unpin - await sb_message.delete() - self._remove_sb_message(payload.message_id) + if new_reaction_count < self.base_threshold: + await sb_message.delete() # delete will also unpin + self._remove_sb_message(payload.message_id) + return + + if new_reaction_count < self.big_threshold and sb_message.pinned: + await sb_message.unpin() + + await sb_message.edit( + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" + ) async def setup(bot: commands.Bot): From 2f228b2979ded61f8cba9f986e4fb1abb79c9104 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Wed, 22 Mar 2023 12:47:22 +1000 Subject: [PATCH 04/28] more bugfixes --- uqcsbot/starboard.py | 42 +++++++++++++++++++++++++++--------------- uqcsbot/text.py | 11 ++++++----- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index c3f1fef..cc81366 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -13,6 +13,8 @@ class BlacklistedMessageError(Exception): pass +class ReferenceDeletedError(Exception): + pass class Starboard(commands.Cog): CHANNEL_NAME = "starboard" @@ -173,6 +175,9 @@ def _remove_sb_message(self, recv: int): db_session.close() def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: + if recv.reference is not None and isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): + raise ReferenceDeletedError() + embed = discord.Embed(color=recv.author.top_role.color, description=recv.content) embed.set_author(name=recv.author.display_name, icon_url=recv.author.display_avatar.url) embed.set_footer(text=recv.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) @@ -181,7 +186,7 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: embed.set_image(url = recv.attachments[0].url) # only takes the first attachment to avoid sending large numbers of images to starboard. - if recv.reference is not None and not isinstance(recv.reference, discord.DeletedReferencedMessage): + if recv.reference is not None and not isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): replied = discord.Embed( color=recv.reference.resolved.author.top_role.color, description=recv.reference.resolved.content @@ -197,15 +202,14 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: ) return [replied, embed] - return [embed] @app_commands.command() @app_commands.default_permissions(manage_messages=True) async def cleanup_starboard(self, interaction: discord.Interaction): - """ Cleans up the last 100 messages from the starboard. + """ Cleans up the last 30 messages from the starboard. Removes any uqcsbot message that doesn't have a corresponding message id. """ - sb_messages = await self.starboard_channel.history(limit=100) + sb_messages = await self.starboard_channel.history(limit=30) db_session = self.bot.create_db_session() await interaction.defer(thinking=True) @@ -237,19 +241,27 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): else: messages += [await self._query_sb_message(payload.message_id)] recv_message, sb_message = messages - except BlacklistedMessageError: + except (discord.errors.NotFound, BlacklistedMessageError): return new_reaction_count = await self._find_num_reactions(messages) if new_reaction_count >= self.base_threshold and \ - sb_message is None and recv_message.id not in self.base_blocked_messages: - new_sb_message = await self.starboard_channel.send( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}", - embeds=self._create_sb_embed(recv_message) - # note that the embed is never edited, which means the content of the starboard post is fixed as - # soon as the 5th reaction is processed - ) + sb_message is None and recv_message.id not in self.base_blocked_messages and recv_message.content != "": + try: + new_sb_message = await self.starboard_channel.send( + content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}", + embeds=self._create_sb_embed(recv_message) + # note that the embed is never edited, which means the content of the starboard post is fixed as + # soon as the 5th reaction is processed + ) + except ReferenceDeletedError: + db_session = self.bot.create_db_session() + db_session.add(models.Starboard(recv=recv_message.id, recv_location=recv_message.channel.id, sent=None)) + db_session.commit() + db_session.close() + return + await new_sb_message.edit( view=discord.ui.View.from_message(new_sb_message).add_item(discord.ui.Button( label="Original Message", @@ -295,7 +307,7 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): else: messages += [await self._query_sb_message(payload.message_id)] recv_message, sb_message = messages - except BlacklistedMessageError: + except (discord.errors.NotFound, BlacklistedMessageError): return if sb_message == None: @@ -331,7 +343,7 @@ async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): else: messages += [await self._query_sb_message(payload.message_id)] recv_message, sb_message = messages - except BlacklistedMessageError: + except (discord.errors.NotFound, BlacklistedMessageError): return if sb_message == None: @@ -370,7 +382,7 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo else: messages += [await self._query_sb_message(payload.message_id)] recv_message, sb_message = messages - except BlacklistedMessageError: + except (discord.errors.NotFound, BlacklistedMessageError): return if sb_message == None: diff --git a/uqcsbot/text.py b/uqcsbot/text.py index 77fbfa6..d073b7e 100644 --- a/uqcsbot/text.py +++ b/uqcsbot/text.py @@ -21,11 +21,12 @@ def __init__(self, bot: commands.Bot): ) self.bot.tree.add_command(self.mock_menu) - #self.scare_menu = app_commands.ContextMenu( - # name="Scare", - # callback=self.scare_context, - #) - #self.bot.tree.add_command(self.scare_menu) + # casualty of the starboard's blacklist/whitelist commands, kept for posterity + # self.scare_menu = app_commands.ContextMenu( + # name="Scare", + # callback=self.scare_context, + # ) + # self.bot.tree.add_command(self.scare_menu) @app_commands.command() @app_commands.describe(message="Input string") From 44f0722e66ea83e3865ee52bd332552f35ce3492 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 12:13:17 +1000 Subject: [PATCH 05/28] switch to zoneinfo per PR #76 --- uqcsbot/starboard.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index cc81366..4fb8992 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -1,12 +1,11 @@ import os from threading import Timer from typing import List -from datetime import timezone +from zoneinfo import ZoneInfo import discord from discord import app_commands from discord.ext import commands -from pytz import timezone from uqcsbot import models # needs to be models and not just starboard because of namespacing with this class @@ -19,7 +18,7 @@ class ReferenceDeletedError(Exception): class Starboard(commands.Cog): CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" - BRISBANE_TZ = timezone('Australia/Brisbane') + BRISBANE_TZ = ZoneInfo("Australia/Brisbane") def __init__(self, bot: commands.Bot): self.bot = bot From f6df619052e4117ac959748a9748fccd8715afbc Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 13:56:56 +1000 Subject: [PATCH 06/28] added a lot of comments. fixed a lot of things. --- uqcsbot/starboard.py | 124 ++++++++++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 43 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 4fb8992..b9ff132 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -60,31 +60,47 @@ def _rm_big_ratelimit(self, id: int): """ Callback to remove a message from the big-ratelimited list """ self.big_blocked_messages.remove(id) + async def _blacklist_log(self, message: discord.Message, user: discord.Member, blacklist: bool): + channels = self.bot.get_all_channels() + modlog = discord.utils.get(channels, name="admin-alerts") + state = "blacklisted" if blacklist else "whitelisted" + + await modlog.send(f"{str(user)} {state} message {message.id} (link: {message.jump_url})") + @app_commands.default_permissions(manage_messages=True) async def context_blacklist_sb_message(self, interaction: discord.Interaction, message: discord.Message): """ Blacklists a message from being starboarded. If the message is already starboarded, also deletes it. """ db_session = self.bot.create_db_session() + # can't use the db query functions for this, they error out if a message hits the blacklist entry = db_session.query(models.Starboard).filter(models.Starboard.recv == message.id) query_val = entry.one_or_none() if query_val is not None: if query_val.sent is None: + # if the table has (recv, none) then it's already blacklisted. return + # otherwise the table has (recv, something), we should delete the something and then make it (recv, none) await (await self.starboard_channel.fetch_message(query_val.sent)).delete() query_val.sent = None else: + # other-otherwise the table doesn't have recv, so we add (recv, none) db_session.add(models.Starboard(recv=message.id, recv_location=message.channel.id, sent=None)) db_session.commit() db_session.close() + await self._blacklist_log(message, interaction.user, blacklist=True) await interaction.response.send_message(f"Blacklisted message {message.id}.") @app_commands.default_permissions(manage_messages=True) async def context_whitelist_sb_message(self, interaction: discord.Interaction, message: discord.Message): - """ Removes a message from the starboard blacklist. Doesn't perform an 'update' of the message. """ + """ Removes a message from the starboard blacklist. + N.B. Doesn't perform an 'update' of the message. This may result in messages meeting the threshold + but not being starboarded if they don't get any more reacts. """ db_session = self.bot.create_db_session() + + # if we find a (recv, none) for this message, delete it. otherwise the message is already not blacklisted. entry = db_session.query(models.Starboard).filter( models.Starboard.recv == message.id, models.Starboard.sent == None @@ -93,11 +109,13 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m entry.delete(synchronize_session=False) db_session.commit() db_session.close() + + await self._blacklist_log(message.id, blacklist=False) await interaction.response.send_message(f"Whitelisted message {message.id}.") async def _query_sb_message(self, recv: int) -> discord.Message: - """ Get the starboard message corresponding to the recieved message. - Handles messages no longer existing and cleans up the DB accordingly. """ + """ Get the starboard message corresponding to the recieved message. Returns None if no sb message exists. + Handles messages potentially being deleted and cleans up the DB accordingly. """ db_session = self.bot.create_db_session() entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv) query_val = entry.one_or_none() @@ -106,10 +124,13 @@ async def _query_sb_message(self, recv: int) -> discord.Message: if query_val is not None: try: if query_val.sent is None: + # (recv, none) is a blacklist entry raise BlacklistedMessageError() message = await self.starboard_channel.fetch_message(query_val.sent) except discord.errors.NotFound: + # if we can't find the sb message anymore, then it's been deleted and we return None + # but we also delete the SB entry to save future lookups. entry.delete(synchronize_session=False) db_session.commit() finally: @@ -118,8 +139,8 @@ async def _query_sb_message(self, recv: int) -> discord.Message: return message async def _query_og_message(self, sent: int) -> discord.Message: - """ Get the og message corresponding to this starboard message. - Handles messages no longer existing. """ + """ Get the og message corresponding to this starboard message. Returns None if the message has been deleted. + Handles messages no longer existing and cleans up the DB accordingly. """ db_session = self.bot.create_db_session() entry = db_session.query(models.Starboard).filter( models.Starboard.sent == sent @@ -131,10 +152,16 @@ async def _query_og_message(self, sent: int) -> discord.Message: try: channel = self.bot.get_channel(entry.recv_location) if channel is None: + # for some reason get_channel doesn't handle errors the same as fetch_message + # but we want to treat them the same anyway raise discord.errors.NotFound() message = await channel.fetch_message(entry.recv) except discord.errors.NotFound: + # if we can't find the recv message anymore, then it's been deleted and we return None + # however, SB messages can still generate :starhaj:'s, so we don't necessarily delete the + # message - the caller will handle that. + entry.recv_location = None entry.recv = None db_session.commit() @@ -142,18 +169,21 @@ async def _query_og_message(self, sent: int) -> discord.Message: db_session.close() return message - async def _find_num_reactions(self, messages: List[discord.Message]) -> int: + async def _find_num_reactions(self, recv: discord.Message, sent: discord.Message) -> int: """ Find the total number of starboard_emoji reacts across this list of messages. """ users = [] authors = [] - for message in messages: + for message in (recv, sent): if message is None: continue + # store the message authors so we can discard their reacts later authors += [message.author.id] reaction = discord.utils.get(message.reactions, emoji=self.starboard_emoji) if reaction is not None: + # we use the user.id as a marker of the reaction so we can set() it and + # eliminate duplicates (only count one reaction per person) users += [user.id async for user in reaction.users()] return len(set([user for user in users if user not in authors])) @@ -175,6 +205,8 @@ def _remove_sb_message(self, recv: int): def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: if recv.reference is not None and isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): + # we raise this exception and auto-blacklist replies to deleted messages on the logic that those + # messages were probably deleted for a reason raise ReferenceDeletedError() embed = discord.Embed(color=recv.author.top_role.color, description=recv.content) @@ -186,6 +218,7 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: # only takes the first attachment to avoid sending large numbers of images to starboard. if recv.reference is not None and not isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): + # if the reference exists, add it. isinstance here prevents race conditions replied = discord.Embed( color=recv.reference.resolved.author.top_role.color, description=recv.reference.resolved.content @@ -203,20 +236,35 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: return [replied, embed] return [embed] + async def _get_message_pair(self, channel, message_id: int) -> List[discord.Message]: + message = await channel.fetch_message(message_id) + + if channel == self.starboard_channel: + sent = message + recv = await self._query_og_message(message_id) + else: + recv = message + sent = await self._query_sb_message(message_id) + + return [recv, sent] + @app_commands.command() @app_commands.default_permissions(manage_messages=True) async def cleanup_starboard(self, interaction: discord.Interaction): - """ Cleans up the last 30 messages from the starboard. - Removes any uqcsbot message that doesn't have a corresponding message id. """ - sb_messages = await self.starboard_channel.history(limit=30) + """ Cleans up the last 100 messages from the starboard. + Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. """ + sb_messages = await self.starboard_channel.history(limit=100) db_session = self.bot.create_db_session() + # in case it takes a while, we need to defer the interaction so it doesn't die await interaction.defer(thinking=True) for message in sb_messages: query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() if query is None and message.author.id == self.user.id: + # only delete messages that uqcsbot itself sent message.delete() + db_session.close() await interaction.followup.send("Finished cleaning up.") @@ -233,18 +281,20 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): channel = self.bot.get_channel(payload.channel_id) try: - messages = [await channel.fetch_message(payload.message_id)] - if channel == self.starboard_channel: - messages += [await self._query_og_message(payload.message_id)] - sb_message, recv_message = messages - else: - messages += [await self._query_sb_message(payload.message_id)] - recv_message, sb_message = messages + recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) except (discord.errors.NotFound, BlacklistedMessageError): + # if the message has already been deleted, or is blacklisted, we're done here + return + + # delete starhaj self-reacts instantly + if recv_message.author.id == payload.user_id: + # payload.member is guaranteed to be available because we're adding and we're in a server + await recv_message.remove_reaction(payload.emoji, payload.member) return - new_reaction_count = await self._find_num_reactions(messages) + new_reaction_count = await self._find_num_reactions(recv_message, sb_message) + # if above threshold, not yet sent, not ratelimited, and message has some text to starboard if new_reaction_count >= self.base_threshold and \ sb_message is None and recv_message.id not in self.base_blocked_messages and recv_message.content != "": try: @@ -252,7 +302,7 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}", embeds=self._create_sb_embed(recv_message) # note that the embed is never edited, which means the content of the starboard post is fixed as - # soon as the 5th reaction is processed + # soon as the Nth reaction is processed ) except ReferenceDeletedError: db_session = self.bot.create_db_session() @@ -273,6 +323,9 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): self.base_blocked_messages += [recv_message.id] Timer(self.ratelimit, self._rm_base_ratelimit, [recv_message.id]).start() + # elif above threshold and sent and not ratelimited. note that this means a big-blocked message won't see + # message text updates for the duration of its ratelimit. in practice this is rare enough that i think we're + # all good. elif new_reaction_count > self.base_threshold and \ sb_message is not None and recv_message.id not in self.big_blocked_messages: @@ -299,20 +352,15 @@ async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): channel = self.bot.get_channel(payload.channel_id) try: - messages = [await channel.fetch_message(payload.message_id)] - if channel == self.starboard_channel: - messages += [await self._query_og_message(payload.message_id)] - sb_message, recv_message = messages - else: - messages += [await self._query_sb_message(payload.message_id)] - recv_message, sb_message = messages + recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) except (discord.errors.NotFound, BlacklistedMessageError): + # if the message has already been deleted, or is blacklisted, we're done here return if sb_message == None: return - new_reaction_count = await self._find_num_reactions(messages) + new_reaction_count = await self._find_num_reactions(recv_message, sb_message) if new_reaction_count < self.base_threshold: await sb_message.delete() # delete will also unpin @@ -335,20 +383,15 @@ async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): channel = self.bot.get_channel(payload.channel_id) try: - messages = [await channel.fetch_message(payload.message_id)] - if channel == self.starboard_channel: - messages += [await self._query_og_message(payload.message_id)] - sb_message, recv_message = messages - else: - messages += [await self._query_sb_message(payload.message_id)] - recv_message, sb_message = messages + recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) except (discord.errors.NotFound, BlacklistedMessageError): + # if the message has already been deleted, or is blacklisted, we're done here return if sb_message == None: return - new_reaction_count = await self._find_num_reactions(messages) + new_reaction_count = await self._find_num_reactions(recv_message, sb_message) if new_reaction_count < self.base_threshold: await sb_message.delete() # delete will also unpin @@ -374,20 +417,15 @@ async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmo channel = self.bot.get_channel(payload.channel_id) try: - messages = [await channel.fetch_message(payload.message_id)] - if channel == self.starboard_channel: - messages += [await self._query_og_message(payload.message_id)] - sb_message, recv_message = messages - else: - messages += [await self._query_sb_message(payload.message_id)] - recv_message, sb_message = messages + recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) except (discord.errors.NotFound, BlacklistedMessageError): + # if the message has already been deleted, or is blacklisted, we're done here return if sb_message == None: return - new_reaction_count = await self._find_num_reactions(messages) + new_reaction_count = await self._find_num_reactions(recv_message, sb_message) if new_reaction_count < self.base_threshold: await sb_message.delete() # delete will also unpin From 8da96f39c42db46b7b8d92a5a931ff605965eb5b Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 17:41:48 +1000 Subject: [PATCH 07/28] switch blacklist interactions to ephemeral replies --- uqcsbot/starboard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index b9ff132..7ad9199 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -91,7 +91,7 @@ async def context_blacklist_sb_message(self, interaction: discord.Interaction, m db_session.close() await self._blacklist_log(message, interaction.user, blacklist=True) - await interaction.response.send_message(f"Blacklisted message {message.id}.") + await interaction.response.send_message(f"Blacklisted message {message.id}.", ephemeral=True) @app_commands.default_permissions(manage_messages=True) async def context_whitelist_sb_message(self, interaction: discord.Interaction, message: discord.Message): @@ -111,7 +111,7 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m db_session.close() await self._blacklist_log(message.id, blacklist=False) - await interaction.response.send_message(f"Whitelisted message {message.id}.") + await interaction.response.send_message(f"Whitelisted message {message.id}.", ephemeral=True) async def _query_sb_message(self, recv: int) -> discord.Message: """ Get the starboard message corresponding to the recieved message. Returns None if no sb message exists. From e7685d212885dbfc229f881507762e345aa6823f Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 17:53:55 +1000 Subject: [PATCH 08/28] beefed up the log entries for blacklisting/whitelisting --- uqcsbot/starboard.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 7ad9199..a7e4b5d 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -61,11 +61,30 @@ def _rm_big_ratelimit(self, id: int): self.big_blocked_messages.remove(id) async def _blacklist_log(self, message: discord.Message, user: discord.Member, blacklist: bool): + """ Logs a blacklist/whitelist command to the modlog. """ channels = self.bot.get_all_channels() modlog = discord.utils.get(channels, name="admin-alerts") state = "blacklisted" if blacklist else "whitelisted" - await modlog.send(f"{str(user)} {state} message {message.id} (link: {message.jump_url})") + embed = discord.Embed(color=message.author.top_role.color, description=message.content) + embed.set_author(name=message.author.display_name, icon_url=message.author.display_avatar.url) + embed.set_footer(text=message.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) + + if len(message.attachments) > 0: + embed.set_image(url = message.attachments[0].url) + # only takes the first attachment to avoid sending large numbers of images to starboard. + + log_item = await modlog.send( + f"{str(user)} {state} message {message.id}", + embeds=embed) + + await log_item.edit( + view=discord.ui.View.from_message(log_item).add_item(discord.ui.Button( + label="Original Message", + style=discord.ButtonStyle.link, + url=message.jump_url + )) + ) @app_commands.default_permissions(manage_messages=True) async def context_blacklist_sb_message(self, interaction: discord.Interaction, message: discord.Message): @@ -204,6 +223,7 @@ def _remove_sb_message(self, recv: int): db_session.close() def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: + """ Creates the starboard embed for a message, including author, colours, replies, etc. """ if recv.reference is not None and isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): # we raise this exception and auto-blacklist replies to deleted messages on the logic that those # messages were probably deleted for a reason @@ -237,6 +257,8 @@ def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: return [embed] async def _get_message_pair(self, channel, message_id: int) -> List[discord.Message]: + """ Given some message ID, return the two Message objects relevant to it: + the starboard message (or None) and the original message (or None) """ message = await channel.fetch_message(message_id) if channel == self.starboard_channel: From f5980a9414f6fed8f5ce376210f550e03f6a2fae Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 17:58:42 +1000 Subject: [PATCH 09/28] real --- uqcsbot/starboard.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index a7e4b5d..30dfc2c 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -129,7 +129,7 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m db_session.commit() db_session.close() - await self._blacklist_log(message.id, blacklist=False) + await self._blacklist_log(message, interaction.user, blacklist=False) await interaction.response.send_message(f"Whitelisted message {message.id}.", ephemeral=True) async def _query_sb_message(self, recv: int) -> discord.Message: @@ -275,13 +275,13 @@ async def _get_message_pair(self, channel, message_id: int) -> List[discord.Mess async def cleanup_starboard(self, interaction: discord.Interaction): """ Cleans up the last 100 messages from the starboard. Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. """ - sb_messages = await self.starboard_channel.history(limit=100) + sb_messages = self.starboard_channel.history(limit=100) db_session = self.bot.create_db_session() # in case it takes a while, we need to defer the interaction so it doesn't die await interaction.defer(thinking=True) - for message in sb_messages: + async for message in sb_messages: query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() if query is None and message.author.id == self.user.id: # only delete messages that uqcsbot itself sent From 32703900dee898dee02c52da05a543ac9133a75b Mon Sep 17 00:00:00 2001 From: James Dearlove Date: Fri, 7 Apr 2023 18:15:15 +1000 Subject: [PATCH 10/28] And now I'm fixing stuff --- uqcsbot/starboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 30dfc2c..cc4d3f4 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -76,7 +76,7 @@ async def _blacklist_log(self, message: discord.Message, user: discord.Member, b log_item = await modlog.send( f"{str(user)} {state} message {message.id}", - embeds=embed) + embeds=[embed]) await log_item.edit( view=discord.ui.View.from_message(log_item).add_item(discord.ui.Button( From 51218e6f40852a48da7fc5169bd55f5e8c5a711e Mon Sep 17 00:00:00 2001 From: James Dearlove Date: Fri, 7 Apr 2023 18:21:53 +1000 Subject: [PATCH 11/28] More quick fixes --- uqcsbot/starboard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index cc4d3f4..f42ca6a 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -279,11 +279,11 @@ async def cleanup_starboard(self, interaction: discord.Interaction): db_session = self.bot.create_db_session() # in case it takes a while, we need to defer the interaction so it doesn't die - await interaction.defer(thinking=True) + await interaction.response.defer(thinking=True) async for message in sb_messages: query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() - if query is None and message.author.id == self.user.id: + if query is None and message.author.id == self.bot.user.id: # only delete messages that uqcsbot itself sent message.delete() From 0001af515d0403d10054b9b5bc49c1acdc4e08fb Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 18:31:23 +1000 Subject: [PATCH 12/28] recv is NOT NULL --- uqcsbot/starboard.py | 1 - 1 file changed, 1 deletion(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index f42ca6a..4834f1e 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -182,7 +182,6 @@ async def _query_og_message(self, sent: int) -> discord.Message: # message - the caller will handle that. entry.recv_location = None - entry.recv = None db_session.commit() db_session.close() From 1993d014a7505e0d6be60b9e4ae1c8552f4d82f8 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 18:39:40 +1000 Subject: [PATCH 13/28] fix cleanup command --- uqcsbot/starboard.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 4834f1e..3ba19ff 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -274,6 +274,11 @@ async def _get_message_pair(self, channel, message_id: int) -> List[discord.Mess async def cleanup_starboard(self, interaction: discord.Interaction): """ Cleans up the last 100 messages from the starboard. Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. """ + + if interaction.channel == self.starboard_channel: + await interaction.response.send_message("Can't cleanup from inside the starboard!", ephemeral=True) + return + sb_messages = self.starboard_channel.history(limit=100) db_session = self.bot.create_db_session() @@ -284,7 +289,8 @@ async def cleanup_starboard(self, interaction: discord.Interaction): query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() if query is None and message.author.id == self.bot.user.id: # only delete messages that uqcsbot itself sent - message.delete() + print("\n\ndeleting\n") + await message.delete() db_session.close() From 36b5c74c275506b1720b7ccad936dbf37d297454 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Fri, 7 Apr 2023 18:58:56 +1000 Subject: [PATCH 14/28] fix message deletion... again --- uqcsbot/starboard.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 3ba19ff..e5ae02a 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -197,6 +197,7 @@ async def _find_num_reactions(self, recv: discord.Message, sent: discord.Message continue # store the message authors so we can discard their reacts later + # grandfathering old messages where their reacts were not auto-deleted, also failsafes, etc authors += [message.author.id] reaction = discord.utils.get(message.reactions, emoji=self.starboard_emoji) if reaction is not None: @@ -314,7 +315,7 @@ async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): return # delete starhaj self-reacts instantly - if recv_message.author.id == payload.user_id: + if recv_message is not None and recv_message.author.id == payload.user_id: # payload.member is guaranteed to be available because we're adding and we're in a server await recv_message.remove_reaction(payload.emoji, payload.member) return From b7fbb1fa4519f65b713df650edccc75d14008ba0 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sat, 8 Apr 2023 16:46:23 +1000 Subject: [PATCH 15/28] rewrite the majority of the querying code --- uqcsbot/models.py | 8 +- uqcsbot/starboard.py | 433 ++++++++++++++++--------------------------- 2 files changed, 162 insertions(+), 279 deletions(-) diff --git a/uqcsbot/models.py b/uqcsbot/models.py index ea0e845..0c8438b 100644 --- a/uqcsbot/models.py +++ b/uqcsbot/models.py @@ -43,6 +43,10 @@ class Reminders(Base): class Starboard(Base): __tablename__ = 'starboard' - recv = Column("recv", BigInteger, primary_key=True, nullable=False) + # composite key on recv, sent. + # recv == null implies deleted recv message. + # recv_location == null implies deleted recv channel. recv should also be null. + # sent == null implies blacklisted recv message. + recv = Column("recv", BigInteger, primary_key=True, nullable=True) recv_location = Column("recv_location", BigInteger, nullable=True, unique=False) - sent = Column("sent", BigInteger, nullable=True, unique=True) + sent = Column("sent", BigInteger, primary_key=True, nullable=True, unique=True) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index e5ae02a..50062d9 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -1,6 +1,6 @@ import os from threading import Timer -from typing import List +from typing import Tuple, List from zoneinfo import ZoneInfo import discord @@ -8,11 +8,12 @@ from discord.ext import commands from uqcsbot import models -# needs to be models and not just starboard because of namespacing with this class class BlacklistedMessageError(Exception): + # always caught. used to differentiate between "starboard message doesn't exist" and "starboard message doesn't exist (for a reason)" pass -class ReferenceDeletedError(Exception): +class SomethingsFucked(Exception): + # never caught. used because i don't trust myself and i want it to be clear that something's wrong. pass class Starboard(commands.Cog): @@ -52,14 +53,6 @@ async def on_ready(self): self.starboard_emoji = discord.utils.get(self.bot.emojis, name=self.EMOJI_NAME) self.starboard_channel = discord.utils.get(self.bot.get_all_channels(), name=self.CHANNEL_NAME) - def _rm_base_ratelimit(self, id: int): - """ Callback to remove a message from the base-ratelimited list """ - self.base_blocked_messages.remove(id) - - def _rm_big_ratelimit(self, id: int): - """ Callback to remove a message from the big-ratelimited list """ - self.big_blocked_messages.remove(id) - async def _blacklist_log(self, message: discord.Message, user: discord.Member, blacklist: bool): """ Logs a blacklist/whitelist command to the modlog. """ channels = self.bot.get_all_channels() @@ -70,10 +63,6 @@ async def _blacklist_log(self, message: discord.Message, user: discord.Member, b embed.set_author(name=message.author.display_name, icon_url=message.author.display_avatar.url) embed.set_footer(text=message.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) - if len(message.attachments) > 0: - embed.set_image(url = message.attachments[0].url) - # only takes the first attachment to avoid sending large numbers of images to starboard. - log_item = await modlog.send( f"{str(user)} {state} message {message.id}", embeds=[embed]) @@ -131,68 +120,88 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m await self._blacklist_log(message, interaction.user, blacklist=False) await interaction.response.send_message(f"Whitelisted message {message.id}.", ephemeral=True) + + def _rm_base_ratelimit(self, id: int) -> None: + """ Callback to remove a message from the base-ratelimited list """ + self.base_blocked_messages.remove(id) + + def _rm_big_ratelimit(self, id: int) -> None: + """ Callback to remove a message from the big-ratelimited list """ + self.big_blocked_messages.remove(id) + + def _update_sb_message(self, recv: int, recv_location: int, sent: int) -> None: + """ Sets the ID of the starboard message corresponding to the recieved message """ + db_session = self.bot.create_db_session() + db_session.add(models.Starboard(recv=recv, recv_location=recv_location, sent=sent)) + db_session.commit() + db_session.close() - async def _query_sb_message(self, recv: int) -> discord.Message: - """ Get the starboard message corresponding to the recieved message. Returns None if no sb message exists. - Handles messages potentially being deleted and cleans up the DB accordingly. """ + def _remove_sb_message(self, recv: int, sent: int) -> None: + """ Deletes the DB entry for the starboard message for this recieved message """ db_session = self.bot.create_db_session() - entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv) - query_val = entry.one_or_none() - - message = None - if query_val is not None: + entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv and models.Starboard.sent == sent) + entry.delete(synchronize_session=False) + db_session.commit() + db_session.close() + + async def _fetch_message_or_none(self, channel: (discord.TextChannel | None), id: (int | None)) -> (discord.Message | None): + """ Translates a lot of None into much less None. Also translates NotFound exceptions. """ + if channel is None or id is None: + return None + else: try: - if query_val.sent is None: - # (recv, none) is a blacklist entry - raise BlacklistedMessageError() - - message = await self.starboard_channel.fetch_message(query_val.sent) - except discord.errors.NotFound: - # if we can't find the sb message anymore, then it's been deleted and we return None - # but we also delete the SB entry to save future lookups. - entry.delete(synchronize_session=False) - db_session.commit() - finally: - db_session.close() - - return message + return await channel.fetch_message(id) + except discord.NotFound(): + return None + + async def _lookup_from_id(self, channel_id: int, message_id: int) -> Tuple[discord.Message | None, discord.Message | None]: + """ Takes a channel ID and a message ID. These may be a starboard message or a recieved message. - async def _query_og_message(self, sent: int) -> discord.Message: - """ Get the og message corresponding to this starboard message. Returns None if the message has been deleted. - Handles messages no longer existing and cleans up the DB accordingly. """ + Returns (Recieved Message, Starboard Message), or Nones, as applicable. + """ db_session = self.bot.create_db_session() - entry = db_session.query(models.Starboard).filter( - models.Starboard.sent == sent - ).one_or_none() - - message = None - if entry is not None: - if entry.recv is not None and entry.recv_location is not None: - try: - channel = self.bot.get_channel(entry.recv_location) - if channel is None: - # for some reason get_channel doesn't handle errors the same as fetch_message - # but we want to treat them the same anyway - raise discord.errors.NotFound() - - message = await channel.fetch_message(entry.recv) - except discord.errors.NotFound: - # if we can't find the recv message anymore, then it's been deleted and we return None - # however, SB messages can still generate :starhaj:'s, so we don't necessarily delete the - # message - the caller will handle that. - - entry.recv_location = None - db_session.commit() - db_session.close() - return message - - async def _find_num_reactions(self, recv: discord.Message, sent: discord.Message) -> int: - """ Find the total number of starboard_emoji reacts across this list of messages. """ + entry = None + if self.starboard_channel.id == channel_id: + # we're primarily looking up a recieved message and a location. + # first, get the entry, then the location, then _fetch_message_or_none the remaining IDs. + entry = db_session.query(models.Starboard).filter(models.Starboard.sent == message_id).one_or_none() + + if entry.recv_location is not None: + channel = self.bot.get_channel(entry.recv_location) + + return (self._fetch_message_or_none(channel, entry.recv), + self._fetch_message_or_none(self.starboard_channel, entry.sent)) + else: + # if the recieved location is None, then the message won't exist either. + # The only thing we can possibly return is a starboard message. + return (None, + self._fetch_message_or_none(self.starboard_channel, entry.sent)) + + else: + # we're primarily looking up a starboard message. + entry = db_session.query(models.Starboard).filter(models.Starboard.recv == id).one_or_none() + + if entry.recv_location != channel_id: + # This also implies entry.recv_location is not None + raise SomethingsFucked("Recieved message from different channel to DB lookup!") + if entry.sent is None: + raise BlacklistedMessageError() + + channel = self.bot.get_channel(channel_id) + + return (self._fetch_message_or_none(channel, entry.recv), + self._fetch_message_or_none(self.starboard_channel, entry.sent)) + + async def _count_num_reacts(self, data: Tuple[discord.Message | None, discord.Message | None]) -> int: + """ Takes _lookup_from_id data (a channel and two messages, maybe None). + + Returns the number of unique reactions across both messages, not including the authors of those messages. + """ users = [] authors = [] - for message in (recv, sent): + for message in data: if message is None: continue @@ -201,271 +210,141 @@ async def _find_num_reactions(self, recv: discord.Message, sent: discord.Message authors += [message.author.id] reaction = discord.utils.get(message.reactions, emoji=self.starboard_emoji) if reaction is not None: - # we use the user.id as a marker of the reaction so we can set() it and + # we use the user.id to describe the reaction so we can set() it and # eliminate duplicates (only count one reaction per person) users += [user.id async for user in reaction.users()] return len(set([user for user in users if user not in authors])) - def _update_sb_message(self, recv: int, recv_location: int, sent: int): - """ Sets the ID of the starboard message corresponding to the recieved message """ - db_session = self.bot.create_db_session() - db_session.add(models.Starboard(recv=recv, recv_location=recv_location, sent=sent)) - db_session.commit() - db_session.close() - - def _remove_sb_message(self, recv: int): - """ Deletes the DB entry for the starboard message for this recieved message """ - db_session = self.bot.create_db_session() - entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv) - entry.delete(synchronize_session=False) - db_session.commit() - db_session.close() - - def _create_sb_embed(self, recv: discord.Message) -> discord.Embed: - """ Creates the starboard embed for a message, including author, colours, replies, etc. """ - if recv.reference is not None and isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): - # we raise this exception and auto-blacklist replies to deleted messages on the logic that those - # messages were probably deleted for a reason - raise ReferenceDeletedError() + def _generate_message_text(self, reaction_count: int, recieved_msg: (discord.Message | None)) -> str: + return f"{str(self.starboard_emoji)} {reaction_count} | {('#' + recieved_msg.channel.name) if recieved_msg is not None else 'OoOoO... ghost message!'}" - embed = discord.Embed(color=recv.author.top_role.color, description=recv.content) - embed.set_author(name=recv.author.display_name, icon_url=recv.author.display_avatar.url) - embed.set_footer(text=recv.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) + def _generate_message_embeds(self, recieved_msg: discord.Message) -> List[discord.Embed]: + """ Creates the starboard embed for a message, including author, colours, replies, etc. """ + embed = discord.Embed(color=recieved_msg.author.top_role.color, description=recieved_msg.content) + embed.set_author(name=recieved_msg.author.display_name, icon_url=recieved_msg.author.display_avatar.url) + embed.set_footer(text=recieved_msg.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) - if len(recv.attachments) > 0: - embed.set_image(url = recv.attachments[0].url) + if len(recieved_msg.attachments) > 0: + embed.set_image(url = recieved_msg.attachments[0].url) # only takes the first attachment to avoid sending large numbers of images to starboard. - if recv.reference is not None and not isinstance(recv.reference.resolved, discord.DeletedReferencedMessage): - # if the reference exists, add it. isinstance here prevents race conditions + if recieved_msg.reference is not None and not isinstance(recieved_msg.reference.resolved, discord.DeletedReferencedMessage): + # if the reference exists, add it. isinstance here tightens race conditions replied = discord.Embed( - color=recv.reference.resolved.author.top_role.color, - description=recv.reference.resolved.content + color=recieved_msg.reference.resolved.author.top_role.color, + description=recieved_msg.reference.resolved.content ) replied.set_author( - name=f"Replying to {recv.reference.resolved.author.display_name}", - icon_url=recv.reference.resolved.author.display_avatar.url + name=f"Replying to {recieved_msg.reference.resolved.author.display_name}", + icon_url=recieved_msg.reference.resolved.author.display_avatar.url ) replied.set_footer( - text=recv.reference.resolved.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S') + text=recieved_msg.reference.resolved.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S') ) return [replied, embed] return [embed] - async def _get_message_pair(self, channel, message_id: int) -> List[discord.Message]: - """ Given some message ID, return the two Message objects relevant to it: - the starboard message (or None) and the original message (or None) """ - message = await channel.fetch_message(message_id) - - if channel == self.starboard_channel: - sent = message - recv = await self._query_og_message(message_id) - else: - recv = message - sent = await self._query_sb_message(message_id) - - return [recv, sent] - - @app_commands.command() - @app_commands.default_permissions(manage_messages=True) - async def cleanup_starboard(self, interaction: discord.Interaction): - """ Cleans up the last 100 messages from the starboard. - Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. """ - - if interaction.channel == self.starboard_channel: - await interaction.response.send_message("Can't cleanup from inside the starboard!", ephemeral=True) - return - - sb_messages = self.starboard_channel.history(limit=100) - db_session = self.bot.create_db_session() - - # in case it takes a while, we need to defer the interaction so it doesn't die - await interaction.response.defer(thinking=True) - - async for message in sb_messages: - query = db_session.query(models.Starboard).filter(models.Starboard.sent == message.id).one_or_none() - if query is None and message.author.id == self.bot.user.id: - # only delete messages that uqcsbot itself sent - print("\n\ndeleting\n") - await message.delete() - - - db_session.close() - await interaction.followup.send("Finished cleaning up.") - - @commands.Cog.listener() - async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent): - if payload.emoji.id != self.starboard_emoji.id: - return - - guild = self.bot.get_guild(payload.guild_id) - if guild is None: - return - - channel = self.bot.get_channel(payload.channel_id) - - try: - recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) - except (discord.errors.NotFound, BlacklistedMessageError): - # if the message has already been deleted, or is blacklisted, we're done here - return - - # delete starhaj self-reacts instantly - if recv_message is not None and recv_message.author.id == payload.user_id: - # payload.member is guaranteed to be available because we're adding and we're in a server - await recv_message.remove_reaction(payload.emoji, payload.member) - return - - new_reaction_count = await self._find_num_reactions(recv_message, sb_message) + async def _process_sb_updates(self, reaction_count: int, recieved_msg: (discord.Message | None), starboard_msg: (discord.Message | None)) -> None: + if reaction_count >= self.base_threshold and recieved_msg is not None and recieved_msg.id not in self.base_blocked_messages and starboard_msg is None and (recieved_msg.reference is None or not isinstance(recieved_msg.reference.resolved, discord.DeletedReferencedMessage)): + # Above threshold, not blocked, not replying to deleted msg, no current starboard message? post it. + new_sb_message = await self.starboard_channel.send( + content=self._generate_message_text(), + embeds=self._generate_message_embeds() + ) - # if above threshold, not yet sent, not ratelimited, and message has some text to starboard - if new_reaction_count >= self.base_threshold and \ - sb_message is None and recv_message.id not in self.base_blocked_messages and recv_message.content != "": - try: - new_sb_message = await self.starboard_channel.send( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}", - embeds=self._create_sb_embed(recv_message) - # note that the embed is never edited, which means the content of the starboard post is fixed as - # soon as the Nth reaction is processed - ) - except ReferenceDeletedError: - db_session = self.bot.create_db_session() - db_session.add(models.Starboard(recv=recv_message.id, recv_location=recv_message.channel.id, sent=None)) - db_session.commit() - db_session.close() - return - await new_sb_message.edit( view=discord.ui.View.from_message(new_sb_message).add_item(discord.ui.Button( label="Original Message", style=discord.ButtonStyle.link, - url=recv_message.jump_url + url=recieved_msg.jump_url )) ) - self._update_sb_message(recv_message.id, recv_message.channel.id, new_sb_message.id) + # start the base ratelimit + self.base_blocked_messages += [recieved_msg.id] + Timer(self.ratelimit, self._rm_base_ratelimit, [recieved_msg.id]).start() + elif reaction_count >= self.base_threshold and starboard_msg is not None: + # Above threshold, existing message? update it. + if reaction_count >= self.big_threshold: + await starboard_msg.pin(reason=f"Reached {self.big_threshold} starboard reactions.") + + # start the pin ratelimit + self.big_blocked_messages += [starboard_msg.id] + Timer(self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id]).start() + else: + await starboard_msg.unpin(reason=f"Fell below starboard threshold ({self.big_threshold}).") - self.base_blocked_messages += [recv_message.id] - Timer(self.ratelimit, self._rm_base_ratelimit, [recv_message.id]).start() - # elif above threshold and sent and not ratelimited. note that this means a big-blocked message won't see - # message text updates for the duration of its ratelimit. in practice this is rare enough that i think we're - # all good. - elif new_reaction_count > self.base_threshold and \ - sb_message is not None and recv_message.id not in self.big_blocked_messages: - - await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" - ) - - if new_reaction_count >= self.big_threshold and not sb_message.pinned: - await sb_message.pin(reason=f"Reached {self.big_threshold} starboard reactions") - - self.big_blocked_messages += [recv_message.id] - Timer(self.ratelimit, self._rm_big_ratelimit, [recv_message.id]).start() + await starboard_msg.edit(content=self._generate_message_text(reaction_count, recieved_msg)) + else: + # Below threshold, or blocked from sending. Might need to delete. + if starboard_msg is not None: + await starboard_msg.delete() + """ + The four handlers below here are more or less identical. + The differences are just that `add` deletes :starhaj:'s that come from the author of the message, + and `clear` doesn't need to check that the payload references :starhaj:. + """ @commands.Cog.listener() - async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent): - if payload.emoji.id != self.starboard_emoji.id: + async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent) -> None: + if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: return - guild = self.bot.get_guild(payload.guild_id) - if guild is None: - return - - channel = self.bot.get_channel(payload.channel_id) - try: - recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) - except (discord.errors.NotFound, BlacklistedMessageError): - # if the message has already been deleted, or is blacklisted, we're done here - return - - if sb_message == None: - return - - new_reaction_count = await self._find_num_reactions(recv_message, sb_message) - - if new_reaction_count < self.base_threshold: - await sb_message.delete() # delete will also unpin - self._remove_sb_message(payload.message_id) + recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + except BlacklistedMessageError: return - if new_reaction_count < self.big_threshold and sb_message.pinned: - await sb_message.unpin() - - await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" - ) - + if recieved_msg is not None and recieved_msg.author.id == payload.user_id: + # payload.member is guaranteed to be available because we're adding and we're in a server + return await recieved_msg.remove_reaction(payload.emoji, payload.member) + new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) + @commands.Cog.listener() - async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent): - guild = self.bot.get_guild(payload.guild_id) - if guild is None: + async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent) -> None: + if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: return - channel = self.bot.get_channel(payload.channel_id) try: - recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) - except (discord.errors.NotFound, BlacklistedMessageError): - # if the message has already been deleted, or is blacklisted, we're done here - return - - if sb_message == None: + recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + except BlacklistedMessageError: return - - new_reaction_count = await self._find_num_reactions(recv_message, sb_message) - - if new_reaction_count < self.base_threshold: - await sb_message.delete() # delete will also unpin - self._remove_sb_message(payload.message_id) - return - - if new_reaction_count < self.big_threshold and sb_message.pinned: - await sb_message.unpin() - - await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" - ) + new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) @commands.Cog.listener() - async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmojiEvent): - if payload.emoji.id != self.starboard_emoji.id: - return - - guild = self.bot.get_guild(payload.guild_id) - if guild is None: + async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent) -> None: + if self.bot.get_guild(payload.guild_id) is None: return - channel = self.bot.get_channel(payload.channel_id) try: - recv_message, sb_message = await self._get_message_pair(channel, payload.message_id) - except (discord.errors.NotFound, BlacklistedMessageError): - # if the message has already been deleted, or is blacklisted, we're done here + recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + except BlacklistedMessageError: return - if sb_message == None: + new_reaction_count = await self._find_num_reactions(recieved_msg, starboard_msg) + await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) + + @commands.Cog.listener() + async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmojiEvent) -> None: + if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: return - new_reaction_count = await self._find_num_reactions(recv_message, sb_message) - - if new_reaction_count < self.base_threshold: - await sb_message.delete() # delete will also unpin - self._remove_sb_message(payload.message_id) + try: + recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + except BlacklistedMessageError: return - - if new_reaction_count < self.big_threshold and sb_message.pinned: - await sb_message.unpin() - await sb_message.edit( - content=f"{str(self.starboard_emoji)} {new_reaction_count} | #{recv_message.channel.name}" - ) + new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) async def setup(bot: commands.Bot): From c0769d575457f18664b575aaa4df70024ebac619 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sat, 8 Apr 2023 17:29:29 +1000 Subject: [PATCH 16/28] re-worked db support; black for style --- uqcsbot/models.py | 1 + uqcsbot/starboard.py | 385 ++++++++++++++++++++++++++++++------------- 2 files changed, 268 insertions(+), 118 deletions(-) diff --git a/uqcsbot/models.py b/uqcsbot/models.py index 0c8438b..00be811 100644 --- a/uqcsbot/models.py +++ b/uqcsbot/models.py @@ -44,6 +44,7 @@ class Starboard(Base): __tablename__ = 'starboard' # composite key on recv, sent. + # recv == null implies deleted recv message. # recv_location == null implies deleted recv channel. recv should also be null. # sent == null implies blacklisted recv message. diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 50062d9..00c0720 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -9,13 +9,17 @@ from uqcsbot import models + class BlacklistedMessageError(Exception): - # always caught. used to differentiate between "starboard message doesn't exist" and "starboard message doesn't exist (for a reason)" + # always caught. used to differentiate "starboard message doesn't exist" and "starboard message is blacklisted" pass + + class SomethingsFucked(Exception): # never caught. used because i don't trust myself and i want it to be clear that something's wrong. pass + class Starboard(commands.Cog): CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" @@ -27,60 +31,82 @@ def __init__(self, bot: commands.Bot): self.big_threshold = int(os.environ.get("SB_BIG_THRESHOLD")) self.ratelimit = int(os.environ.get("SB_RATELIMIT")) - self.base_blocked_messages = [] # messages that are temp blocked from being resent to the starboard - self.big_blocked_messages = [] # messages that are temp blocked from being re-pinned in the starboard + self.base_blocked_messages = ( + [] + ) # messages that are temp blocked from being resent to the starboard + self.big_blocked_messages = ( + [] + ) # messages that are temp blocked from being re-pinned in the starboard self.whitelist_menu = app_commands.ContextMenu( name="Starboard Whitelist", callback=self.context_whitelist_sb_message, ) self.bot.tree.add_command(self.whitelist_menu) - + self.blacklist_menu = app_commands.ContextMenu( name="Starboard Blacklist", callback=self.context_blacklist_sb_message, ) self.bot.tree.add_command(self.blacklist_menu) - + @commands.Cog.listener() async def on_ready(self): - """ + """ Really this should be in __init__ but this stuff doesn't exist until the bot is ready. N.B. this does assume the bot only has access to one channel called "starboard" and one emoji called "starhaj". If this assumption stops holding, we may need to move back to IDs (cringe) or ensure we only get them from the correct guild (cringer). """ self.starboard_emoji = discord.utils.get(self.bot.emojis, name=self.EMOJI_NAME) - self.starboard_channel = discord.utils.get(self.bot.get_all_channels(), name=self.CHANNEL_NAME) - - async def _blacklist_log(self, message: discord.Message, user: discord.Member, blacklist: bool): - """ Logs a blacklist/whitelist command to the modlog. """ + self.starboard_channel = discord.utils.get( + self.bot.get_all_channels(), name=self.CHANNEL_NAME + ) + + async def _blacklist_log( + self, message: discord.Message, user: discord.Member, blacklist: bool + ): + """Logs a blacklist/whitelist command to the modlog.""" channels = self.bot.get_all_channels() modlog = discord.utils.get(channels, name="admin-alerts") state = "blacklisted" if blacklist else "whitelisted" - embed = discord.Embed(color=message.author.top_role.color, description=message.content) - embed.set_author(name=message.author.display_name, icon_url=message.author.display_avatar.url) - embed.set_footer(text=message.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) + embed = discord.Embed( + color=message.author.top_role.color, description=message.content + ) + embed.set_author( + name=message.author.display_name, icon_url=message.author.display_avatar.url + ) + embed.set_footer( + text=message.created_at.astimezone(tz=self.BRISBANE_TZ).strftime( + "%b %d, %H:%M:%S" + ) + ) log_item = await modlog.send( - f"{str(user)} {state} message {message.id}", - embeds=[embed]) - + content=f"{str(user)} {state} message {message.id}", embeds=[embed] + ) + await log_item.edit( - view=discord.ui.View.from_message(log_item).add_item(discord.ui.Button( + view=discord.ui.View.from_message(log_item).add_item( + discord.ui.Button( label="Original Message", style=discord.ButtonStyle.link, - url=message.jump_url - )) + url=message.jump_url, + ) ) - + ) + @app_commands.default_permissions(manage_messages=True) - async def context_blacklist_sb_message(self, interaction: discord.Interaction, message: discord.Message): - """ Blacklists a message from being starboarded. If the message is already starboarded, also deletes it. """ + async def context_blacklist_sb_message( + self, interaction: discord.Interaction, message: discord.Message + ): + """Blacklists a message from being starboarded. If the message is already starboarded, also deletes it.""" db_session = self.bot.create_db_session() # can't use the db query functions for this, they error out if a message hits the blacklist - entry = db_session.query(models.Starboard).filter(models.Starboard.recv == message.id) + entry = db_session.query(models.Starboard).filter( + models.Starboard.recv == message.id + ) query_val = entry.one_or_none() if query_val is not None: @@ -89,29 +115,43 @@ async def context_blacklist_sb_message(self, interaction: discord.Interaction, m return # otherwise the table has (recv, something), we should delete the something and then make it (recv, none) - await (await self.starboard_channel.fetch_message(query_val.sent)).delete() + try: + await ( + await self.starboard_channel.fetch_message(query_val.sent) + ).delete() + except discord.NotFound(): + # if the message has already been deleted without a DB update, fetch may error out + pass + query_val.sent = None else: # other-otherwise the table doesn't have recv, so we add (recv, none) - db_session.add(models.Starboard(recv=message.id, recv_location=message.channel.id, sent=None)) + db_session.add( + models.Starboard( + recv=message.id, recv_location=message.channel.id, sent=None + ) + ) db_session.commit() db_session.close() await self._blacklist_log(message, interaction.user, blacklist=True) - await interaction.response.send_message(f"Blacklisted message {message.id}.", ephemeral=True) + await interaction.response.send_message( + f"Blacklisted message {message.id}.", ephemeral=True + ) @app_commands.default_permissions(manage_messages=True) - async def context_whitelist_sb_message(self, interaction: discord.Interaction, message: discord.Message): - """ Removes a message from the starboard blacklist. - N.B. Doesn't perform an 'update' of the message. This may result in messages meeting the threshold - but not being starboarded if they don't get any more reacts. """ + async def context_whitelist_sb_message( + self, interaction: discord.Interaction, message: discord.Message + ): + """Removes a message from the starboard blacklist. + N.B. Doesn't perform an 'update' of the message. This may result in messages meeting the threshold + but not being starboarded if they don't get any more reacts.""" db_session = self.bot.create_db_session() # if we find a (recv, none) for this message, delete it. otherwise the message is already not blacklisted. entry = db_session.query(models.Starboard).filter( - models.Starboard.recv == message.id, - models.Starboard.sent == None + models.Starboard.recv == message.id, models.Starboard.sent == None ) if entry.one_or_none() is not None: entry.delete(synchronize_session=False) @@ -119,33 +159,48 @@ async def context_whitelist_sb_message(self, interaction: discord.Interaction, m db_session.close() await self._blacklist_log(message, interaction.user, blacklist=False) - await interaction.response.send_message(f"Whitelisted message {message.id}.", ephemeral=True) + await interaction.response.send_message( + f"Whitelisted message {message.id}.", ephemeral=True + ) def _rm_base_ratelimit(self, id: int) -> None: - """ Callback to remove a message from the base-ratelimited list """ + """Callback to remove a message from the base-ratelimited list""" self.base_blocked_messages.remove(id) - + def _rm_big_ratelimit(self, id: int) -> None: - """ Callback to remove a message from the big-ratelimited list """ + """Callback to remove a message from the big-ratelimited list""" self.big_blocked_messages.remove(id) - - def _update_sb_message(self, recv: int, recv_location: int, sent: int) -> None: - """ Sets the ID of the starboard message corresponding to the recieved message """ + + def _starboard_db_add(self, recv: int, recv_location: int, sent: int) -> None: + """Creates a starboard DB entry. Only called from _process_updates when the messages are not None, so doesn't + need to handle any None-checks - we can just pass ints straight in.""" db_session = self.bot.create_db_session() - db_session.add(models.Starboard(recv=recv, recv_location=recv_location, sent=sent)) + db_session.add( + models.Starboard(recv=recv, recv_location=recv_location, sent=sent) + ) db_session.commit() db_session.close() - - def _remove_sb_message(self, recv: int, sent: int) -> None: - """ Deletes the DB entry for the starboard message for this recieved message """ + + def _starboard_db_remove( + self, recv: (discord.Message | None), sent: (discord.Message | None) + ) -> None: + """Removes a starboard DB entry. Only called from process_updates, but no caller guarantees about recv being + None, so we take Messages and only get ids if they're not-None.""" + recv_id = recv.id if recv is not None else None + sent_id = sent.id if sent is not None else None + db_session = self.bot.create_db_session() - entry = db_session.query(models.Starboard).filter(models.Starboard.recv == recv and models.Starboard.sent == sent) + entry = db_session.query(models.Starboard).filter( + models.Starboard.recv == recv_id and models.Starboard.sent == sent_id + ) entry.delete(synchronize_session=False) db_session.commit() db_session.close() - - async def _fetch_message_or_none(self, channel: (discord.TextChannel | None), id: (int | None)) -> (discord.Message | None): - """ Translates a lot of None into much less None. Also translates NotFound exceptions. """ + + async def _fetch_message_or_none( + self, channel: (discord.TextChannel | None), id: (int | None) + ) -> (discord.Message | None): + """Translates a lot of None into much less None. Also translates NotFound exceptions.""" if channel is None or id is None: return None else: @@ -153,50 +208,70 @@ async def _fetch_message_or_none(self, channel: (discord.TextChannel | None), id return await channel.fetch_message(id) except discord.NotFound(): return None - - async def _lookup_from_id(self, channel_id: int, message_id: int) -> Tuple[discord.Message | None, discord.Message | None]: - """ Takes a channel ID and a message ID. These may be a starboard message or a recieved message. - Returns (Recieved Message, Starboard Message), or Nones, as applicable. + async def _lookup_from_id( + self, channel_id: int, message_id: int + ) -> Tuple[discord.Message | None, discord.Message | None]: + """Takes a channel ID and a message ID. These may be for a starboard message or a recieved message. + + Returns (Recieved Message, Starboard Message), or Nones, as applicable. """ db_session = self.bot.create_db_session() - + entry = None if self.starboard_channel.id == channel_id: # we're primarily looking up a recieved message and a location. # first, get the entry, then the location, then _fetch_message_or_none the remaining IDs. - entry = db_session.query(models.Starboard).filter(models.Starboard.sent == message_id).one_or_none() + entry = ( + db_session.query(models.Starboard) + .filter(models.Starboard.sent == message_id) + .one_or_none() + ) if entry.recv_location is not None: channel = self.bot.get_channel(entry.recv_location) - return (self._fetch_message_or_none(channel, entry.recv), - self._fetch_message_or_none(self.starboard_channel, entry.sent)) + return ( + self._fetch_message_or_none(channel, entry.recv), + self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) else: # if the recieved location is None, then the message won't exist either. # The only thing we can possibly return is a starboard message. - return (None, - self._fetch_message_or_none(self.starboard_channel, entry.sent)) + return ( + None, + self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) else: # we're primarily looking up a starboard message. - entry = db_session.query(models.Starboard).filter(models.Starboard.recv == id).one_or_none() + entry = ( + db_session.query(models.Starboard) + .filter(models.Starboard.recv == message_id) + .one_or_none() + ) if entry.recv_location != channel_id: # This also implies entry.recv_location is not None - raise SomethingsFucked("Recieved message from different channel to DB lookup!") - if entry.sent is None: + raise SomethingsFucked( + "Recieved message from different channel to DB lookup!" + ) + elif entry.sent is None: raise BlacklistedMessageError() channel = self.bot.get_channel(channel_id) - return (self._fetch_message_or_none(channel, entry.recv), - self._fetch_message_or_none(self.starboard_channel, entry.sent)) - - async def _count_num_reacts(self, data: Tuple[discord.Message | None, discord.Message | None]) -> int: - """ Takes _lookup_from_id data (a channel and two messages, maybe None). - - Returns the number of unique reactions across both messages, not including the authors of those messages. + return ( + self._fetch_message_or_none(channel, entry.recv), + self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) + + async def _count_num_reacts( + self, data: Tuple[discord.Message | None, discord.Message | None] + ) -> int: + """Takes _lookup_from_id data (two messages, maybe None). + + Returns the number of unique reactions across both messages, not including the authors of those messages. """ users = [] authors = [] @@ -204,7 +279,7 @@ async def _count_num_reacts(self, data: Tuple[discord.Message | None, discord.Me for message in data: if message is None: continue - + # store the message authors so we can discard their reacts later # grandfathering old messages where their reacts were not auto-deleted, also failsafes, etc authors += [message.author.id] @@ -216,72 +291,121 @@ async def _count_num_reacts(self, data: Tuple[discord.Message | None, discord.Me return len(set([user for user in users if user not in authors])) - def _generate_message_text(self, reaction_count: int, recieved_msg: (discord.Message | None)) -> str: - return f"{str(self.starboard_emoji)} {reaction_count} | {('#' + recieved_msg.channel.name) if recieved_msg is not None else 'OoOoO... ghost message!'}" + def _generate_message_text( + self, reaction_count: int, recieved_msg: (discord.Message | None) + ) -> str: + return ( + f"{str(self.starboard_emoji)} {reaction_count} | " + "{('#' + recieved_msg.channel.name) if recieved_msg is not None else 'OoOoO... ghost message!'}" + ) - def _generate_message_embeds(self, recieved_msg: discord.Message) -> List[discord.Embed]: - """ Creates the starboard embed for a message, including author, colours, replies, etc. """ - embed = discord.Embed(color=recieved_msg.author.top_role.color, description=recieved_msg.content) - embed.set_author(name=recieved_msg.author.display_name, icon_url=recieved_msg.author.display_avatar.url) - embed.set_footer(text=recieved_msg.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S')) + def _generate_message_embeds( + self, recieved_msg: discord.Message + ) -> List[discord.Embed]: + """Creates the starboard embed for a message, including author, colours, replies, etc.""" + embed = discord.Embed( + color=recieved_msg.author.top_role.color, description=recieved_msg.content + ) + embed.set_author( + name=recieved_msg.author.display_name, + icon_url=recieved_msg.author.display_avatar.url, + ) + embed.set_footer( + text=recieved_msg.created_at.astimezone(tz=self.BRISBANE_TZ).strftime( + "%b %d, %H:%M:%S" + ) + ) if len(recieved_msg.attachments) > 0: - embed.set_image(url = recieved_msg.attachments[0].url) + embed.set_image(url=recieved_msg.attachments[0].url) # only takes the first attachment to avoid sending large numbers of images to starboard. - - if recieved_msg.reference is not None and not isinstance(recieved_msg.reference.resolved, discord.DeletedReferencedMessage): + + if recieved_msg.reference is not None and not isinstance( + recieved_msg.reference.resolved, discord.DeletedReferencedMessage + ): # if the reference exists, add it. isinstance here tightens race conditions replied = discord.Embed( color=recieved_msg.reference.resolved.author.top_role.color, - description=recieved_msg.reference.resolved.content + description=recieved_msg.reference.resolved.content, ) replied.set_author( name=f"Replying to {recieved_msg.reference.resolved.author.display_name}", - icon_url=recieved_msg.reference.resolved.author.display_avatar.url + icon_url=recieved_msg.reference.resolved.author.display_avatar.url, ) replied.set_footer( - text=recieved_msg.reference.resolved.created_at.astimezone(tz=self.BRISBANE_TZ).strftime('%b %d, %H:%M:%S') + text=recieved_msg.reference.resolved.created_at.astimezone( + tz=self.BRISBANE_TZ + ).strftime("%b %d, %H:%M:%S") ) return [replied, embed] return [embed] - async def _process_sb_updates(self, reaction_count: int, recieved_msg: (discord.Message | None), starboard_msg: (discord.Message | None)) -> None: - if reaction_count >= self.base_threshold and recieved_msg is not None and recieved_msg.id not in self.base_blocked_messages and starboard_msg is None and (recieved_msg.reference is None or not isinstance(recieved_msg.reference.resolved, discord.DeletedReferencedMessage)): + async def _process_sb_updates( + self, + reaction_count: int, + recieved_msg: (discord.Message | None), + starboard_msg: (discord.Message | None), + ) -> None: + if ( + reaction_count >= self.base_threshold + and recieved_msg is not None + and recieved_msg.id not in self.base_blocked_messages + and starboard_msg is None + and ( + recieved_msg.reference is None + or not isinstance( + recieved_msg.reference.resolved, discord.DeletedReferencedMessage + ) + ) + ): # Above threshold, not blocked, not replying to deleted msg, no current starboard message? post it. new_sb_message = await self.starboard_channel.send( content=self._generate_message_text(), - embeds=self._generate_message_embeds() + embeds=self._generate_message_embeds(), ) await new_sb_message.edit( - view=discord.ui.View.from_message(new_sb_message).add_item(discord.ui.Button( - label="Original Message", - style=discord.ButtonStyle.link, - url=recieved_msg.jump_url - )) + view=discord.ui.View.from_message(new_sb_message).add_item( + discord.ui.Button( + label="Original Message", + style=discord.ButtonStyle.link, + url=recieved_msg.jump_url, + ) + ) ) + self._starboard_db_add(recieved_msg, recieved_msg.channel, new_sb_message) + # start the base ratelimit self.base_blocked_messages += [recieved_msg.id] Timer(self.ratelimit, self._rm_base_ratelimit, [recieved_msg.id]).start() elif reaction_count >= self.base_threshold and starboard_msg is not None: # Above threshold, existing message? update it. if reaction_count >= self.big_threshold: - await starboard_msg.pin(reason=f"Reached {self.big_threshold} starboard reactions.") + await starboard_msg.pin( + reason=f"Reached {self.big_threshold} starboard reactions." + ) # start the pin ratelimit self.big_blocked_messages += [starboard_msg.id] - Timer(self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id]).start() + Timer( + self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id] + ).start() else: - await starboard_msg.unpin(reason=f"Fell below starboard threshold ({self.big_threshold}).") - - await starboard_msg.edit(content=self._generate_message_text(reaction_count, recieved_msg)) + await starboard_msg.unpin( + reason=f"Fell below starboard threshold ({self.big_threshold})." + ) + + await starboard_msg.edit( + content=self._generate_message_text(reaction_count, recieved_msg) + ) else: # Below threshold, or blocked from sending. Might need to delete. if starboard_msg is not None: + self._starboard_db_remove(recieved_msg, starboard_msg) await starboard_msg.delete() """ @@ -291,61 +415,86 @@ async def _process_sb_updates(self, reaction_count: int, recieved_msg: (discord. """ @commands.Cog.listener() - async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent) -> None: - if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: + async def on_raw_reaction_add( + self, payload: discord.RawReactionActionEvent + ) -> None: + if ( + payload.emoji != self.starboard_emoji + or self.bot.get_guild(payload.guild_id) is None + ): return - + try: - recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + recieved_msg, starboard_msg = await self._lookup_from_id( + payload.channel_id, payload.message_id + ) except BlacklistedMessageError: return - + if recieved_msg is not None and recieved_msg.author.id == payload.user_id: # payload.member is guaranteed to be available because we're adding and we're in a server return await recieved_msg.remove_reaction(payload.emoji, payload.member) - new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + new_reaction_count = await self._count_num_reacts((recieved_msg, starboard_msg)) await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) - + @commands.Cog.listener() - async def on_raw_reaction_remove(self, payload: discord.RawReactionActionEvent) -> None: - if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: + async def on_raw_reaction_remove( + self, payload: discord.RawReactionActionEvent + ) -> None: + if ( + payload.emoji != self.starboard_emoji + or self.bot.get_guild(payload.guild_id) is None + ): return - + try: - recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + recieved_msg, starboard_msg = await self._lookup_from_id( + payload.channel_id, payload.message_id + ) except BlacklistedMessageError: return - new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + new_reaction_count = await self._count_num_reacts((recieved_msg, starboard_msg)) await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) @commands.Cog.listener() - async def on_raw_reaction_clear(self, payload: discord.RawReactionClearEvent) -> None: + async def on_raw_reaction_clear( + self, payload: discord.RawReactionClearEvent + ) -> None: if self.bot.get_guild(payload.guild_id) is None: return - + try: - recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + recieved_msg, starboard_msg = await self._lookup_from_id( + payload.channel_id, payload.message_id + ) except BlacklistedMessageError: return - - new_reaction_count = await self._find_num_reactions(recieved_msg, starboard_msg) + + new_reaction_count = await self._count_num_reacts((recieved_msg, starboard_msg)) await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) - + @commands.Cog.listener() - async def on_raw_reaction_clear_emoji(self, payload: discord.RawReactionClearEmojiEvent) -> None: - if payload.emoji != self.starboard_emoji or self.bot.get_guild(payload.guild_id) is None: + async def on_raw_reaction_clear_emoji( + self, payload: discord.RawReactionClearEmojiEvent + ) -> None: + if ( + payload.emoji != self.starboard_emoji + or self.bot.get_guild(payload.guild_id) is None + ): return - + try: - recieved_msg, starboard_msg = await self._lookup_from_id(payload.channel_id, payload.message_id) + recieved_msg, starboard_msg = await self._lookup_from_id( + payload.channel_id, payload.message_id + ) except BlacklistedMessageError: return - new_reaction_count = self._count_num_reacts((recieved_msg, starboard_msg)) + new_reaction_count = await self._count_num_reacts((recieved_msg, starboard_msg)) await self._process_sb_updates(new_reaction_count, recieved_msg, starboard_msg) async def setup(bot: commands.Bot): - await bot.add_cog(Starboard(bot)) \ No newline at end of file + await bot.add_cog(Starboard(bot)) From e436697639d1079a5d3eaccf5430ca0a44a9516d Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sat, 8 Apr 2023 17:39:44 +1000 Subject: [PATCH 17/28] touchups pt1 --- uqcsbot/starboard.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 00c0720..34875cb 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -296,7 +296,7 @@ def _generate_message_text( ) -> str: return ( f"{str(self.starboard_emoji)} {reaction_count} | " - "{('#' + recieved_msg.channel.name) if recieved_msg is not None else 'OoOoO... ghost message!'}" + f"{('#' + recieved_msg.channel.name) if recieved_msg is not None else 'OoOoO... ghost message!'}" ) def _generate_message_embeds( @@ -323,7 +323,8 @@ def _generate_message_embeds( if recieved_msg.reference is not None and not isinstance( recieved_msg.reference.resolved, discord.DeletedReferencedMessage ): - # if the reference exists, add it. isinstance here tightens race conditions + # if the reference exists, add it. isinstance here just tightens race conditions; we check + # that messages aren't deleted before calling this anyway. replied = discord.Embed( color=recieved_msg.reference.resolved.author.top_role.color, description=recieved_msg.reference.resolved.content, @@ -377,6 +378,7 @@ async def _process_sb_updates( ) ) + # recieved_msg isn't None and we just sent the sb message, so it also shouldn't be None self._starboard_db_add(recieved_msg, recieved_msg.channel, new_sb_message) # start the base ratelimit @@ -394,7 +396,7 @@ async def _process_sb_updates( Timer( self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id] ).start() - else: + elif starboard_msg.pinned: await starboard_msg.unpin( reason=f"Fell below starboard threshold ({self.big_threshold})." ) From 5ae3b7e5e7e9084715f3831c479a8749c8c321f7 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sat, 8 Apr 2023 17:46:27 +1000 Subject: [PATCH 18/28] recreated cleanup. cleanup now also updates messages. --- uqcsbot/starboard.py | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 34875cb..559c650 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -63,6 +63,51 @@ async def on_ready(self): self.bot.get_all_channels(), name=self.CHANNEL_NAME ) + @app_commands.command() + @app_commands.default_permissions(manage_messages=True) + async def cleanup_starboard(self, interaction: discord.Interaction): + """Cleans up the last 100 messages from the starboard. + Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. + """ + + if interaction.channel == self.starboard_channel: + return await interaction.response.send_message( + "Can't cleanup from inside the starboard!", ephemeral=True + ) + + sb_messages = self.starboard_channel.history(limit=100) + db_session = self.bot.create_db_session() + + # in case it takes a while, we need to defer the interaction so it doesn't die + await interaction.response.defer(thinking=True) + + async for message in sb_messages: + query = ( + db_session.query(models.Starboard) + .filter(models.Starboard.sent == message.id) + .one_or_none() + ) + if query is None and message.author.id == self.bot.user.id: + # only delete messages that uqcsbot itself sent + await message.delete() + else: + try: + recieved_msg, starboard_msg = await self._lookup_from_id( + self.starboard_channel.id, message.id + ) + except BlacklistedMessageError: + return + + new_reaction_count = await self._count_num_reacts( + (recieved_msg, starboard_msg) + ) + await self._process_sb_updates( + new_reaction_count, recieved_msg, starboard_msg + ) + + db_session.close() + await interaction.followup.send("Finished cleaning up.") + async def _blacklist_log( self, message: discord.Message, user: discord.Member, blacklist: bool ): From 95826f94091963c3967cd373d091dc1abf8e8981 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sat, 8 Apr 2023 18:24:18 +1000 Subject: [PATCH 19/28] touchups pt2. now modlogs Very Bad Things --- uqcsbot/starboard.py | 85 ++++++++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 559c650..16bf315 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -17,12 +17,16 @@ class BlacklistedMessageError(Exception): class SomethingsFucked(Exception): # never caught. used because i don't trust myself and i want it to be clear that something's wrong. - pass + def __init__(self, modlog: discord.TextChannel, message: str, *args, **kwargs): + super().__init__(*args, **kwargs) + modlog.send(f"Bad Starboard state: {message}") + class Starboard(commands.Cog): - CHANNEL_NAME = "starboard" + SB_CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" + MODLOG_CHANNEL_NAME = "admin-alerts" BRISBANE_TZ = ZoneInfo("Australia/Brisbane") def __init__(self, bot: commands.Bot): @@ -60,7 +64,10 @@ async def on_ready(self): """ self.starboard_emoji = discord.utils.get(self.bot.emojis, name=self.EMOJI_NAME) self.starboard_channel = discord.utils.get( - self.bot.get_all_channels(), name=self.CHANNEL_NAME + self.bot.get_all_channels(), name=self.SB_CHANNEL_NAME + ) + self.modlog = discord.utils.get( + self.bot.get_all_channels(), name=self.MODLOG_CHANNEL_NAME ) @app_commands.command() @@ -90,7 +97,7 @@ async def cleanup_starboard(self, interaction: discord.Interaction): if query is None and message.author.id == self.bot.user.id: # only delete messages that uqcsbot itself sent await message.delete() - else: + elif message.author.id == self.bot.user.id: try: recieved_msg, starboard_msg = await self._lookup_from_id( self.starboard_channel.id, message.id @@ -112,8 +119,6 @@ async def _blacklist_log( self, message: discord.Message, user: discord.Member, blacklist: bool ): """Logs a blacklist/whitelist command to the modlog.""" - channels = self.bot.get_all_channels() - modlog = discord.utils.get(channels, name="admin-alerts") state = "blacklisted" if blacklist else "whitelisted" embed = discord.Embed( @@ -128,7 +133,7 @@ async def _blacklist_log( ) ) - log_item = await modlog.send( + log_item = await self.modlog.send( content=f"{str(user)} {state} message {message.id}", embeds=[embed] ) @@ -273,20 +278,23 @@ async def _lookup_from_id( .one_or_none() ) - if entry.recv_location is not None: - channel = self.bot.get_channel(entry.recv_location) + if entry is not None: + if entry.recv_location is not None: + channel = self.bot.get_channel(entry.recv_location) - return ( - self._fetch_message_or_none(channel, entry.recv), - self._fetch_message_or_none(self.starboard_channel, entry.sent), - ) + return ( + await self._fetch_message_or_none(channel, entry.recv), + await self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) + else: + # if the recieved location is None, then the message won't exist either. + # The only thing we can possibly return is a starboard message. + return ( + None, + await self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) else: - # if the recieved location is None, then the message won't exist either. - # The only thing we can possibly return is a starboard message. - return ( - None, - self._fetch_message_or_none(self.starboard_channel, entry.sent), - ) + raise await SomethingsFucked(modlog=self.modlog, message="Couldn't find an DB entry for this starboard message!") else: # we're primarily looking up a starboard message. @@ -296,20 +304,29 @@ async def _lookup_from_id( .one_or_none() ) - if entry.recv_location != channel_id: - # This also implies entry.recv_location is not None - raise SomethingsFucked( - "Recieved message from different channel to DB lookup!" - ) - elif entry.sent is None: - raise BlacklistedMessageError() + if entry is not None: + if entry.recv_location != channel_id: + # This also implies entry.recv_location is not None + raise await SomethingsFucked( + modlog=self.modlog, + message="Recieved message from different channel to DB lookup!" + ) + elif entry.sent is None: + raise BlacklistedMessageError() - channel = self.bot.get_channel(channel_id) + channel = self.bot.get_channel(channel_id) - return ( - self._fetch_message_or_none(channel, entry.recv), - self._fetch_message_or_none(self.starboard_channel, entry.sent), - ) + return ( + await self._fetch_message_or_none(channel, message_id), + await self._fetch_message_or_none(self.starboard_channel, entry.sent), + ) + else: + channel = self.bot.get_channel(channel_id) + + return ( + await self._fetch_message_or_none(channel, message_id), + None, + ) async def _count_num_reacts( self, data: Tuple[discord.Message | None, discord.Message | None] @@ -409,8 +426,8 @@ async def _process_sb_updates( ): # Above threshold, not blocked, not replying to deleted msg, no current starboard message? post it. new_sb_message = await self.starboard_channel.send( - content=self._generate_message_text(), - embeds=self._generate_message_embeds(), + content=self._generate_message_text(reaction_count, recieved_msg), + embeds=self._generate_message_embeds(recieved_msg), ) await new_sb_message.edit( @@ -424,7 +441,7 @@ async def _process_sb_updates( ) # recieved_msg isn't None and we just sent the sb message, so it also shouldn't be None - self._starboard_db_add(recieved_msg, recieved_msg.channel, new_sb_message) + self._starboard_db_add(recieved_msg.id, recieved_msg.channel.id, new_sb_message.id) # start the base ratelimit self.base_blocked_messages += [recieved_msg.id] From 799845211b4949a724bf6bb7de3cc3559b4ad975 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Mon, 10 Apr 2023 13:24:32 +1000 Subject: [PATCH 20/28] black again --- uqcsbot/starboard.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 16bf315..84457ed 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -22,7 +22,6 @@ def __init__(self, modlog: discord.TextChannel, message: str, *args, **kwargs): modlog.send(f"Bad Starboard state: {message}") - class Starboard(commands.Cog): SB_CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" @@ -284,17 +283,24 @@ async def _lookup_from_id( return ( await self._fetch_message_or_none(channel, entry.recv), - await self._fetch_message_or_none(self.starboard_channel, entry.sent), + await self._fetch_message_or_none( + self.starboard_channel, entry.sent + ), ) else: # if the recieved location is None, then the message won't exist either. # The only thing we can possibly return is a starboard message. return ( None, - await self._fetch_message_or_none(self.starboard_channel, entry.sent), + await self._fetch_message_or_none( + self.starboard_channel, entry.sent + ), ) else: - raise await SomethingsFucked(modlog=self.modlog, message="Couldn't find an DB entry for this starboard message!") + raise SomethingsFucked( + modlog=self.modlog, + message="Couldn't find an DB entry for this starboard message!", + ) else: # we're primarily looking up a starboard message. @@ -307,9 +313,9 @@ async def _lookup_from_id( if entry is not None: if entry.recv_location != channel_id: # This also implies entry.recv_location is not None - raise await SomethingsFucked( + raise SomethingsFucked( modlog=self.modlog, - message="Recieved message from different channel to DB lookup!" + message="Recieved message from different channel to DB lookup!", ) elif entry.sent is None: raise BlacklistedMessageError() @@ -318,11 +324,13 @@ async def _lookup_from_id( return ( await self._fetch_message_or_none(channel, message_id), - await self._fetch_message_or_none(self.starboard_channel, entry.sent), + await self._fetch_message_or_none( + self.starboard_channel, entry.sent + ), ) else: channel = self.bot.get_channel(channel_id) - + return ( await self._fetch_message_or_none(channel, message_id), None, @@ -441,7 +449,9 @@ async def _process_sb_updates( ) # recieved_msg isn't None and we just sent the sb message, so it also shouldn't be None - self._starboard_db_add(recieved_msg.id, recieved_msg.channel.id, new_sb_message.id) + self._starboard_db_add( + recieved_msg.id, recieved_msg.channel.id, new_sb_message.id + ) # start the base ratelimit self.base_blocked_messages += [recieved_msg.id] From b24ca23259652124e168fa76478c1fa2522fa3cf Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Thu, 13 Apr 2023 20:55:41 +1000 Subject: [PATCH 21/28] fix error handling --- uqcsbot/starboard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 84457ed..2df65a0 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -17,9 +17,9 @@ class BlacklistedMessageError(Exception): class SomethingsFucked(Exception): # never caught. used because i don't trust myself and i want it to be clear that something's wrong. - def __init__(self, modlog: discord.TextChannel, message: str, *args, **kwargs): + def __init__(self, client: discord.Client, modlog: discord.TextChannel, message: str, *args, **kwargs): super().__init__(*args, **kwargs) - modlog.send(f"Bad Starboard state: {message}") + client.loop.create_task(modlog.send(f"Bad Starboard state: {message}")) class Starboard(commands.Cog): From 2def182e73784edcbee8d082082f7911ebd35b9e Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Thu, 13 Apr 2023 22:36:19 +1000 Subject: [PATCH 22/28] more commenting & error message stuff --- uqcsbot/starboard.py | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 2df65a0..ee1a1fe 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -18,7 +18,7 @@ class BlacklistedMessageError(Exception): class SomethingsFucked(Exception): # never caught. used because i don't trust myself and i want it to be clear that something's wrong. def __init__(self, client: discord.Client, modlog: discord.TextChannel, message: str, *args, **kwargs): - super().__init__(*args, **kwargs) + super().__init__(message, *args, **kwargs) client.loop.create_task(modlog.send(f"Bad Starboard state: {message}")) @@ -57,9 +57,8 @@ def __init__(self, bot: commands.Bot): async def on_ready(self): """ Really this should be in __init__ but this stuff doesn't exist until the bot is ready. - N.B. this does assume the bot only has access to one channel called "starboard" and one emoji called - "starhaj". If this assumption stops holding, we may need to move back to IDs (cringe) or ensure we only get - them from the correct guild (cringer). + N.B. this does assume the server only has one channel called "starboard" and one emoji called + "starhaj". If this assumption stops holding, we may need to move back to IDs (cringe) """ self.starboard_emoji = discord.utils.get(self.bot.emojis, name=self.EMOJI_NAME) self.starboard_channel = discord.utils.get( @@ -70,13 +69,18 @@ async def on_ready(self): ) @app_commands.command() - @app_commands.default_permissions(manage_messages=True) + @app_commands.default_permissions(manage_server=True) async def cleanup_starboard(self, interaction: discord.Interaction): """Cleans up the last 100 messages from the starboard. Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. + Otherwise, causes a starboard update on the messages. + + manage_server perms: for committee and infra use. """ if interaction.channel == self.starboard_channel: + # because if you do it from in the starboard, it deletes its own interaction response + # and i cba making it not do that, so i'll just forbid doing it in starboard. return await interaction.response.send_message( "Can't cleanup from inside the starboard!", ephemeral=True ) @@ -150,7 +154,10 @@ async def _blacklist_log( async def context_blacklist_sb_message( self, interaction: discord.Interaction, message: discord.Message ): - """Blacklists a message from being starboarded. If the message is already starboarded, also deletes it.""" + """Blacklists a message from being starboarded. If the message is already starboarded, also deletes it. + + manage_messages perms: committee-only. + """ db_session = self.bot.create_db_session() # can't use the db query functions for this, they error out if a message hits the blacklist entry = db_session.query(models.Starboard).filter( @@ -195,7 +202,9 @@ async def context_whitelist_sb_message( ): """Removes a message from the starboard blacklist. N.B. Doesn't perform an 'update' of the message. This may result in messages meeting the threshold - but not being starboarded if they don't get any more reacts.""" + but not being starboarded if they don't get any more reacts. + + manage_messages perms: committee-only""" db_session = self.bot.create_db_session() # if we find a (recv, none) for this message, delete it. otherwise the message is already not blacklisted. @@ -299,7 +308,10 @@ async def _lookup_from_id( else: raise SomethingsFucked( modlog=self.modlog, - message="Couldn't find an DB entry for this starboard message!", + message=f"Couldn't find an DB entry for this starboard message ({message_id})!", + # note that this will also trigger on Isaac's initial-starboard message (1076779482637144105). + # quite honestly, this is the most comprehensible way for us to handle it; the bot will gracefully + # crap out and the only notice will be the error in #admin-alerts. ) else: @@ -312,10 +324,9 @@ async def _lookup_from_id( if entry is not None: if entry.recv_location != channel_id: - # This also implies entry.recv_location is not None raise SomethingsFucked( modlog=self.modlog, - message="Recieved message from different channel to DB lookup!", + message=f"Recieved message ({message_id}) from different channel to what the DB expects!", ) elif entry.sent is None: raise BlacklistedMessageError() @@ -351,7 +362,7 @@ async def _count_num_reacts( continue # store the message authors so we can discard their reacts later - # grandfathering old messages where their reacts were not auto-deleted, also failsafes, etc + # grandfathering old messages where their reacts were not auto-deleted, also failsafes are nice, etc authors += [message.author.id] reaction = discord.utils.get(message.reactions, emoji=self.starboard_emoji) if reaction is not None: From d5419a141c0b20021d572bef9bb28128dc2b8e2d Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Mon, 24 Apr 2023 10:47:15 +1000 Subject: [PATCH 23/28] update perms & misc --- uqcsbot/starboard.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index ee1a1fe..1f75c03 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -16,7 +16,7 @@ class BlacklistedMessageError(Exception): class SomethingsFucked(Exception): - # never caught. used because i don't trust myself and i want it to be clear that something's wrong. + # never caught. used for bad db states, which should never occur, but just in case, y'know? def __init__(self, client: discord.Client, modlog: discord.TextChannel, message: str, *args, **kwargs): super().__init__(message, *args, **kwargs) client.loop.create_task(modlog.send(f"Bad Starboard state: {message}")) @@ -69,13 +69,13 @@ async def on_ready(self): ) @app_commands.command() - @app_commands.default_permissions(manage_server=True) + @app_commands.default_permissions(manage_guild=True) async def cleanup_starboard(self, interaction: discord.Interaction): """Cleans up the last 100 messages from the starboard. Removes any uqcsbot message that doesn't have a corresponding message id in the db, regardless of recv. Otherwise, causes a starboard update on the messages. - manage_server perms: for committee and infra use. + manage_guild perms: for committee and infra use. """ if interaction.channel == self.starboard_channel: @@ -176,7 +176,7 @@ async def context_blacklist_sb_message( await self.starboard_channel.fetch_message(query_val.sent) ).delete() except discord.NotFound(): - # if the message has already been deleted without a DB update, fetch may error out + # if the message has already been deleted without a DB update, fetch may error out, but we don't care pass query_val.sent = None @@ -469,7 +469,7 @@ async def _process_sb_updates( Timer(self.ratelimit, self._rm_base_ratelimit, [recieved_msg.id]).start() elif reaction_count >= self.base_threshold and starboard_msg is not None: # Above threshold, existing message? update it. - if reaction_count >= self.big_threshold: + if reaction_count >= self.big_threshold and starboard_msg.id not in self.big_blocked_messages: await starboard_msg.pin( reason=f"Reached {self.big_threshold} starboard reactions." ) @@ -479,7 +479,7 @@ async def _process_sb_updates( Timer( self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id] ).start() - elif starboard_msg.pinned: + elif starboard_msg.pinned and starboard_msg.id not in self.big_blocked_messages: await starboard_msg.unpin( reason=f"Fell below starboard threshold ({self.big_threshold})." ) From 9ebcf4ad109d719e47c28aed64904035b8f38fe0 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Mon, 24 Apr 2023 14:22:26 +1000 Subject: [PATCH 24/28] black again --- uqcsbot/starboard.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 1f75c03..66c161a 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -17,7 +17,14 @@ class BlacklistedMessageError(Exception): class SomethingsFucked(Exception): # never caught. used for bad db states, which should never occur, but just in case, y'know? - def __init__(self, client: discord.Client, modlog: discord.TextChannel, message: str, *args, **kwargs): + def __init__( + self, + client: discord.Client, + modlog: discord.TextChannel, + message: str, + *args, + **kwargs, + ): super().__init__(message, *args, **kwargs) client.loop.create_task(modlog.send(f"Bad Starboard state: {message}")) @@ -155,7 +162,7 @@ async def context_blacklist_sb_message( self, interaction: discord.Interaction, message: discord.Message ): """Blacklists a message from being starboarded. If the message is already starboarded, also deletes it. - + manage_messages perms: committee-only. """ db_session = self.bot.create_db_session() @@ -203,7 +210,7 @@ async def context_whitelist_sb_message( """Removes a message from the starboard blacklist. N.B. Doesn't perform an 'update' of the message. This may result in messages meeting the threshold but not being starboarded if they don't get any more reacts. - + manage_messages perms: committee-only""" db_session = self.bot.create_db_session() @@ -469,7 +476,10 @@ async def _process_sb_updates( Timer(self.ratelimit, self._rm_base_ratelimit, [recieved_msg.id]).start() elif reaction_count >= self.base_threshold and starboard_msg is not None: # Above threshold, existing message? update it. - if reaction_count >= self.big_threshold and starboard_msg.id not in self.big_blocked_messages: + if ( + reaction_count >= self.big_threshold + and starboard_msg.id not in self.big_blocked_messages + ): await starboard_msg.pin( reason=f"Reached {self.big_threshold} starboard reactions." ) @@ -479,7 +489,10 @@ async def _process_sb_updates( Timer( self.ratelimit, self._rm_big_ratelimit, [starboard_msg.id] ).start() - elif starboard_msg.pinned and starboard_msg.id not in self.big_blocked_messages: + elif ( + starboard_msg.pinned + and starboard_msg.id not in self.big_blocked_messages + ): await starboard_msg.unpin( reason=f"Fell below starboard threshold ({self.big_threshold})." ) From e99f722b57cb48620e35b5e16a14b44a194c2984 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Tue, 25 Apr 2023 19:24:00 +1000 Subject: [PATCH 25/28] resolving review comments --- uqcsbot/starboard.py | 51 ++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 66c161a..049b6e1 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -41,18 +41,16 @@ def __init__(self, bot: commands.Bot): self.big_threshold = int(os.environ.get("SB_BIG_THRESHOLD")) self.ratelimit = int(os.environ.get("SB_RATELIMIT")) - self.base_blocked_messages = ( - [] - ) # messages that are temp blocked from being resent to the starboard - self.big_blocked_messages = ( - [] - ) # messages that are temp blocked from being re-pinned in the starboard - - self.whitelist_menu = app_commands.ContextMenu( - name="Starboard Whitelist", - callback=self.context_whitelist_sb_message, + # messages that are temp blocked from being resent to the starboard + self.base_blocked_messages = [] + # messages that are temp blocked from being repinned in the starboard + self.big_blocked_messages = [] + + self.unblacklist_menu = app_commands.ContextMenu( + name="Starboard unblacklist", + callback=self.context_unblacklist_sb_message, ) - self.bot.tree.add_command(self.whitelist_menu) + self.bot.tree.add_command(self.unblacklist_menu) self.blacklist_menu = app_commands.ContextMenu( name="Starboard Blacklist", @@ -113,7 +111,8 @@ async def cleanup_starboard(self, interaction: discord.Interaction): self.starboard_channel.id, message.id ) except BlacklistedMessageError: - return + if starboard_msg is not None: + await starboard_msg.delete() new_reaction_count = await self._count_num_reacts( (recieved_msg, starboard_msg) @@ -128,8 +127,8 @@ async def cleanup_starboard(self, interaction: discord.Interaction): async def _blacklist_log( self, message: discord.Message, user: discord.Member, blacklist: bool ): - """Logs a blacklist/whitelist command to the modlog.""" - state = "blacklisted" if blacklist else "whitelisted" + """Logs the use of a blacklist/unblacklist command to the modlog.""" + state = "blacklisted" if blacklist else "unblacklisted" embed = discord.Embed( color=message.author.top_role.color, description=message.content @@ -175,7 +174,9 @@ async def context_blacklist_sb_message( if query_val is not None: if query_val.sent is None: # if the table has (recv, none) then it's already blacklisted. - return + return await interaction.response.send_message( + "Message already blacklisted!", ephemeral=True + ) # otherwise the table has (recv, something), we should delete the something and then make it (recv, none) try: @@ -204,7 +205,7 @@ async def context_blacklist_sb_message( ) @app_commands.default_permissions(manage_messages=True) - async def context_whitelist_sb_message( + async def context_unblacklist_sb_message( self, interaction: discord.Interaction, message: discord.Message ): """Removes a message from the starboard blacklist. @@ -221,11 +222,16 @@ async def context_whitelist_sb_message( if entry.one_or_none() is not None: entry.delete(synchronize_session=False) db_session.commit() - db_session.close() + db_session.close() + else: + db_session.close() + return await interaction.response.send_message( + "Message already unblacklisted!", ephemeral=True + ) await self._blacklist_log(message, interaction.user, blacklist=False) await interaction.response.send_message( - f"Whitelisted message {message.id}.", ephemeral=True + f"unblacklisted message {message.id}.", ephemeral=True ) def _rm_base_ratelimit(self, id: int) -> None: @@ -313,12 +319,15 @@ async def _lookup_from_id( ), ) else: + if message_id == 1076779482637144105: + # This is Isaac's initial-starboard message. I know, IDs are bad. BUT + # consider that this doesn't cause any of the usual ID-related issues + # like breaking lookups in other servers. + return + raise SomethingsFucked( modlog=self.modlog, message=f"Couldn't find an DB entry for this starboard message ({message_id})!", - # note that this will also trigger on Isaac's initial-starboard message (1076779482637144105). - # quite honestly, this is the most comprehensible way for us to handle it; the bot will gracefully - # crap out and the only notice will be the error in #admin-alerts. ) else: From 6e05bd1e4b04d0a870e629e6fc5daa71de33a9be Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Tue, 25 Apr 2023 20:29:22 +1000 Subject: [PATCH 26/28] switch error handling to new globally-accessible exception --- uqcsbot/starboard.py | 27 +++++++-------------------- uqcsbot/utils/err_log_utils.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 20 deletions(-) create mode 100644 uqcsbot/utils/err_log_utils.py diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 049b6e1..50f264d 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -8,6 +8,7 @@ from discord.ext import commands from uqcsbot import models +from uqcsbot.utils.err_log_utils import FatalErrorWithLog class BlacklistedMessageError(Exception): @@ -15,20 +16,6 @@ class BlacklistedMessageError(Exception): pass -class SomethingsFucked(Exception): - # never caught. used for bad db states, which should never occur, but just in case, y'know? - def __init__( - self, - client: discord.Client, - modlog: discord.TextChannel, - message: str, - *args, - **kwargs, - ): - super().__init__(message, *args, **kwargs) - client.loop.create_task(modlog.send(f"Bad Starboard state: {message}")) - - class Starboard(commands.Cog): SB_CHANNEL_NAME = "starboard" EMOJI_NAME = "starhaj" @@ -325,9 +312,9 @@ async def _lookup_from_id( # like breaking lookups in other servers. return - raise SomethingsFucked( - modlog=self.modlog, - message=f"Couldn't find an DB entry for this starboard message ({message_id})!", + raise FatalErrorWithLog( + client=self.bot, + message=f"Starboard state error: Couldn't find an DB entry for this starboard message ({message_id})!", ) else: @@ -340,9 +327,9 @@ async def _lookup_from_id( if entry is not None: if entry.recv_location != channel_id: - raise SomethingsFucked( - modlog=self.modlog, - message=f"Recieved message ({message_id}) from different channel to what the DB expects!", + raise FatalErrorWithLog( + client=self.bot, + message=f"Starboard state error: Recieved message ({message_id}) from different channel to what the DB expects!", ) elif entry.sent is None: raise BlacklistedMessageError() diff --git a/uqcsbot/utils/err_log_utils.py b/uqcsbot/utils/err_log_utils.py new file mode 100644 index 0000000..5b02c54 --- /dev/null +++ b/uqcsbot/utils/err_log_utils.py @@ -0,0 +1,20 @@ +import discord +from discord.ext import commands + +MODLOG_CHANNEL_NAME = "admin-alerts" + +class FatalErrorWithLog(Exception): + def __init__( + self, + client: commands.Bot, + message: str, + *args, + **kwargs, + ): + modlog = discord.utils.get(client.get_all_channels(), name=MODLOG_CHANNEL_NAME) + if modlog is not None: + client.loop.create_task(modlog.send(message)) + else: + message += f" ...And also, I couldn't find #{MODLOG_CHANNEL_NAME} to log this properly." + + super().__init__(message, *args, **kwargs) From 6209ef08192fa2e25774eb640a5c414d68639bf6 Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sun, 7 May 2023 13:34:24 +1000 Subject: [PATCH 27/28] pokemon: gotta style them all --- uqcsbot/utils/err_log_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uqcsbot/utils/err_log_utils.py b/uqcsbot/utils/err_log_utils.py index 5b02c54..f0c1483 100644 --- a/uqcsbot/utils/err_log_utils.py +++ b/uqcsbot/utils/err_log_utils.py @@ -3,6 +3,7 @@ MODLOG_CHANNEL_NAME = "admin-alerts" + class FatalErrorWithLog(Exception): def __init__( self, @@ -16,5 +17,5 @@ def __init__( client.loop.create_task(modlog.send(message)) else: message += f" ...And also, I couldn't find #{MODLOG_CHANNEL_NAME} to log this properly." - + super().__init__(message, *args, **kwargs) From c1e89d84c853d982456606b59f496917919155cf Mon Sep 17 00:00:00 2001 From: andrewj-brown <92134285+andrewj-brown@users.noreply.github.com> Date: Sun, 7 May 2023 23:42:42 +1000 Subject: [PATCH 28/28] fortify the logging ever so slightly --- uqcsbot/starboard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uqcsbot/starboard.py b/uqcsbot/starboard.py index 50f264d..f932fbb 100644 --- a/uqcsbot/starboard.py +++ b/uqcsbot/starboard.py @@ -34,7 +34,7 @@ def __init__(self, bot: commands.Bot): self.big_blocked_messages = [] self.unblacklist_menu = app_commands.ContextMenu( - name="Starboard unblacklist", + name="Starboard Unblacklist", callback=self.context_unblacklist_sb_message, ) self.bot.tree.add_command(self.unblacklist_menu) @@ -329,7 +329,7 @@ async def _lookup_from_id( if entry.recv_location != channel_id: raise FatalErrorWithLog( client=self.bot, - message=f"Starboard state error: Recieved message ({message_id}) from different channel to what the DB expects!", + message=f"Starboard state error: Recieved message ({message_id}) from different channel ({channel_id}) to what the DB expects ({entry.recv_location})!", ) elif entry.sent is None: raise BlacklistedMessageError()