-
-
Notifications
You must be signed in to change notification settings - Fork 42
feat:(bookmarks) added removing bookmarks #916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
700bb40
b9c797a
89691c3
38c22a5
56bebaf
217c364
b8a4072
f16d0f9
34798e9
e9119ab
72d3054
b547dd4
0b4d6d9
ba2c381
1c5f20e
bdb6103
66f4df9
b5dcbb7
2ce131a
5d36f15
6a7918c
aacfee8
b7cf0f4
67110f5
5c959c7
f1e4c72
627c5bb
000bc72
dc15ffe
38f9809
fc56203
8477862
36a5db6
db0d571
8ceef67
502cc5d
7fa2ee0
e0148cd
d7542e0
c5e51ac
18e4e34
bc36aba
23ace84
e8c16a0
0a672c6
fbe4d0f
47f5a9b
9a3f537
0a5e043
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import discord | ||
| from discord.ext import commands | ||
| from loguru import logger | ||
|
|
||
| from tux.utils.embeds import EmbedCreator | ||
|
|
||
|
|
||
| class Bookmarks(commands.Cog): | ||
| def __init__(self, bot: commands.Bot) -> None: | ||
| self.bot = bot | ||
|
|
||
| @commands.Cog.listener() | ||
| async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent) -> None: | ||
| if str(payload.emoji) == "🔖": | ||
| try: | ||
| channel = self.bot.get_channel(payload.channel_id) | ||
| if channel is None: | ||
| logger.error("Channel not found") | ||
| return | ||
|
|
||
| message = await channel.fetch_message(payload.message_id) | ||
| if message is None: | ||
| logger.error("Message not found") | ||
| return | ||
|
|
||
| embed = EmbedCreator.create_info_embed( | ||
| title="Message Bookmarked", | ||
| description=f"> {message.content}", | ||
| ) | ||
| embed.add_field(name="Author", value=message.author.name, inline=False) | ||
| embed.add_field(name="Jump to Message", value=f"[Click Here]({message.jump_url})", inline=False) | ||
|
|
||
| if message.attachments: | ||
| attachments_info = "\n".join([attachment.url for attachment in message.attachments]) | ||
| embed.add_field(name="Attachments", value=attachments_info, inline=False) | ||
|
|
||
| try: | ||
| user = self.bot.get_user(payload.user_id) | ||
| if user is not None: | ||
| await user.send(embed=embed) | ||
| await message.remove_reaction(payload.emoji, user) | ||
| else: | ||
| logger.error(f"User not found for ID: {payload.user_id}") | ||
| except (discord.Forbidden, discord.HTTPException): | ||
| logger.error(f"Cannot send a DM to {user.name}. They may have DMs turned off.") | ||
| await message.remove_reaction(payload.emoji, user) | ||
| temp_message = await channel.send( | ||
| f"{user.mention}, I couldn't send you a DM make sure your DMs are open for bookmarks to work", | ||
| ) | ||
| await temp_message.delete(delay=30) | ||
| except (discord.NotFound, discord.Forbidden, discord.HTTPException) as e: | ||
| logger.error(f"Failed to process the reaction: {e}") | ||
|
|
||
|
|
||
| async def setup(bot: commands.Bot) -> None: | ||
| await bot.add_cog(Bookmarks(bot)) | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,16 @@ | |||||||||
| def __init__(self, bot: Tux) -> None: | ||||||||||
| self.bot = bot | ||||||||||
|
|
||||||||||
| self.valid_add_emojis = CONST.ADD_BOOKMARK | ||||||||||
| self.valid_remove_emojis = CONST.REMOVE_BOOKMARK | ||||||||||
| self.valid_emojis = CONST.ADD_BOOKMARK + CONST.REMOVE_BOOKMARK | ||||||||||
|
|
||||||||||
| # The linter wants to change this but it breaks when it does that | ||||||||||
| def _is_valid_emoji(self, emoji: discord.PartialEmoji, valid_list: str) -> bool: | ||||||||||
| if emoji.name in valid_list: # noqa: SIM103 | ||||||||||
| return True | ||||||||||
| return False | ||||||||||
|
Comment on lines
+22
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (code-quality): We've found these issues:
Suggested change
|
||||||||||
|
|
||||||||||
| @commands.Cog.listener() | ||||||||||
| async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent) -> None: | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (complexity): Consider refactoring on_raw_reaction_add to flatten control flow, centralize fetch logic, and make embed creation synchronous. You can flatten `on_raw_reaction_add` and remove unreachable branches by:
1. Dropping the initial `valid_emojis` check and final `else` (it’s never reached once you branch by add/remove).
2. Pulling user/channel get‐then‐fetch logic into small helpers.
3. Making `_create_bookmark_embed` a sync method again (nothing inside is async).
Example refactor:
```python
class Bookmarks(commands.Cog):
def __init__(…):
…
self.add_emojis = set(CONST.ADD_BOOKMARK)
self.remove_emojis = set(CONST.REMOVE_BOOKMARK)
async def on_raw_reaction_add(self, payload):
emoji = payload.emoji.name
if emoji in self.add_emojis:
user = await self._get_user(payload.user_id) or return
channel = await self._get_channel(payload.channel_id) or return
message = await self._safe_fetch(channel.fetch_message, payload.message_id) or return
embed = self._create_bookmark_embed(message)
await self._send_bookmark(user, message, embed)
elif emoji in self.remove_emojis:
user = await self._get_user(payload.user_id) or return
channel = await self._get_channel(payload.channel_id) or return
message = await self._safe_fetch(channel.fetch_message, payload.message_id) or return
await self._delete_bookmark(message)
# no final else needed
async def _get_user(self, user_id):
user = self.bot.get_user(user_id)
if user: return user
return await self._safe_fetch(self.bot.fetch_user, user_id)
async def _get_channel(self, channel_id):
channel = self.bot.get_channel(channel_id)
if channel: return channel
return await self._safe_fetch(self.bot.fetch_channel, channel_id)
async def _safe_fetch(self, fn, *args):
try:
return await fn(*args)
except discord.NotFound:
logger.error(f"Not found: {fn.__name__}({args[0]})")
except (discord.Forbidden, discord.HTTPException) as e:
logger.error(f"Failed {fn.__name__}: {e}")
def _create_bookmark_embed(self, message: discord.Message) -> discord.Embed:
content = message.content
if len(content) > CONST.EMBED_MAX_DESC_LENGTH:
content = content[:CONST.EMBED_MAX_DESC_LENGTH - 3] + "..."
embed = EmbedCreator.create_embed(
bot=self.bot,
embed_type=EmbedCreator.INFO,
title="Message Bookmarked",
description=f"> {content}",
)
embed.add_field("Author", message.author.name, inline=False)
embed.add_field("Jump to Message", f"[Click Here]({message.jump_url})", inline=False)
if message.attachments:
urls = "\n".join(a.url for a in message.attachments)
embed.add_field("Attachments", urls, inline=False)
return embedThis removes deep nesting, centralizes fetch-and-log, drops the “how’d you get here?” branch, and keeps embed creation synchronous. |
||||||||||
| """ | ||||||||||
|
|
@@ -27,15 +37,30 @@ | |||||||||
| ------- | ||||||||||
| None | ||||||||||
| """ | ||||||||||
|
|
||||||||||
| if str(payload.emoji) != "🔖": | ||||||||||
| if not self._is_valid_emoji(payload.emoji, self.valid_emojis): | ||||||||||
| return | ||||||||||
|
|
||||||||||
| # Get the user who reacted to the message | ||||||||||
| user = self.bot.get_user(payload.user_id) | ||||||||||
| if user is None: | ||||||||||
| try: | ||||||||||
| user = await self.bot.fetch_user(payload.user_id) | ||||||||||
| except discord.NotFound: | ||||||||||
| logger.error(f"User not found for ID: {payload.user_id}") | ||||||||||
| except (discord.Forbidden, discord.HTTPException) as fetch_error: | ||||||||||
| logger.error(f"Failed to fetch user: {fetch_error}") | ||||||||||
| return | ||||||||||
| # Fetch the channel where the reaction was added | ||||||||||
| channel = self.bot.get_channel(payload.channel_id) | ||||||||||
| if channel is None: | ||||||||||
| logger.error(f"Channel not found for ID: {payload.channel_id}") | ||||||||||
| try: | ||||||||||
| channel = await self.bot.fetch_channel(payload.channel_id) | ||||||||||
| except discord.NotFound: | ||||||||||
| logger.error(f"Channel not found for ID: {payload.channel_id}") | ||||||||||
| except (discord.Forbidden, discord.HTTPException) as fetch_error: | ||||||||||
| logger.error(f"Failed to fetch channel: {fetch_error}") | ||||||||||
| return | ||||||||||
|
|
||||||||||
| channel = cast(discord.TextChannel | discord.Thread, channel) | ||||||||||
|
|
||||||||||
| # Fetch the message that was reacted to | ||||||||||
|
|
@@ -48,19 +73,20 @@ | |||||||||
| logger.error(f"Failed to fetch message: {fetch_error}") | ||||||||||
| return | ||||||||||
|
|
||||||||||
| # Create an embed for the bookmarked message | ||||||||||
| embed = self._create_bookmark_embed(message) | ||||||||||
| # check for what to do | ||||||||||
| if self._is_valid_emoji(payload.emoji, self.valid_add_emojis): | ||||||||||
| # Create an embed for the bookmarked message | ||||||||||
| embed = await self._create_bookmark_embed(message) | ||||||||||
|
|
||||||||||
| # Get the user who reacted to the message | ||||||||||
| user = self.bot.get_user(payload.user_id) | ||||||||||
| if user is None: | ||||||||||
| logger.error(f"User not found for ID: {payload.user_id}") | ||||||||||
| return | ||||||||||
| # Send the bookmarked message to the user | ||||||||||
| await self._send_bookmark(user, message, embed, payload.emoji) | ||||||||||
|
|
||||||||||
| # Send the bookmarked message to the user | ||||||||||
| await self._send_bookmark(user, message, embed, payload.emoji) | ||||||||||
| elif self._is_valid_emoji(payload.emoji, self.valid_remove_emojis) and user is not self.bot.user: | ||||||||||
| await self._delete_bookmark(message, user) | ||||||||||
| else: | ||||||||||
| return | ||||||||||
|
|
||||||||||
| def _create_bookmark_embed( | ||||||||||
| async def _create_bookmark_embed( | ||||||||||
| self, | ||||||||||
| message: discord.Message, | ||||||||||
| ) -> discord.Embed: | ||||||||||
|
|
@@ -77,13 +103,16 @@ | |||||||||
| embed.add_field(name="Author", value=message.author.name, inline=False) | ||||||||||
|
|
||||||||||
| embed.add_field(name="Jump to Message", value=f"[Click Here]({message.jump_url})", inline=False) | ||||||||||
|
|
||||||||||
| if message.attachments: | ||||||||||
| attachments_info = "\n".join([attachment.url for attachment in message.attachments]) | ||||||||||
| embed.add_field(name="Attachments", value=attachments_info, inline=False) | ||||||||||
|
|
||||||||||
| return embed | ||||||||||
|
|
||||||||||
| async def _delete_bookmark(self, message: discord.Message, user: discord.User) -> None: | ||||||||||
| if message.author is not self.bot.user: | ||||||||||
| return | ||||||||||
| await message.delete() | ||||||||||
|
|
||||||||||
| @staticmethod | ||||||||||
| async def _send_bookmark( | ||||||||||
| user: discord.User, | ||||||||||
|
|
@@ -107,7 +136,8 @@ | |||||||||
| """ | ||||||||||
|
|
||||||||||
| try: | ||||||||||
| await user.send(embed=embed) | ||||||||||
| dm_message = await user.send(embed=embed) | ||||||||||
| await dm_message.add_reaction(CONST.REMOVE_BOOKMARK) | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (bug_risk): Adding a reaction to the DM message may fail if the bot lacks permissions. Handle potential exceptions from add_reaction with a try/except block to prevent unhandled errors if the bot lacks permissions or DMs are restricted. |
||||||||||
|
|
||||||||||
| except (discord.Forbidden, discord.HTTPException) as dm_error: | ||||||||||
| logger.error(f"Cannot send a DM to {user.name}: {dm_error}") | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (code-quality): Add guard clause (
last-if-guard)