Bookmarking messages #479

meatsnails · 2024-08-28T22:28:28Z

Description

Adds Bookmarking to tux whenever you react with 🔖 on a message in a server it gets linked and its contents forwarded to your dms

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Other: (write here)

Guidelines

My code follows the style guidelines of this project (formatted with Ruff)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if needed
My changes generate no new warnings
I have tested this change
Any dependent changes have been merged and published in downstream modules
I have followed all of these guidelines.

How Has This Been Tested? (if applicable)

Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)

Screenshots (if applicable)

Please add screenshots to help explain your changes.

Additional Information

Please add any other information that is important to this PR.

Summary by Sourcery

Add a new bookmarking feature that enables users to bookmark messages by reacting with a bookmark emoji, sending the message details to their DMs. Update the README to reorganize the contributors section.

New Features:

Introduce a bookmarking feature that allows users to bookmark messages by reacting with a bookmark emoji, which sends the message link and content to the user's direct messages.

Enhancements:

Reorganize the contributors section in the README file to update the order and presentation of contributors.

sourcery-ai · 2024-08-28T22:28:36Z

Reviewer's Guide by Sourcery

This pull request implements a new bookmarking feature for the Tux Discord bot. The feature allows users to bookmark messages by reacting with a 🔖 emoji, which then sends the message content and link to the user's DMs. The implementation includes a new Bookmarks cog and updates to the README.md file.

File-Level Changes

Change	Details	Files
Implement bookmarking functionality	Create a new Bookmarks cog Add an event listener for reaction additions Check for the 🔖 emoji reaction Generate a message link for the bookmarked message Send bookmarked message content, author, and link to user's DMs Include attachment URLs if present in the bookmarked message Handle potential errors when sending DMs Set up the Bookmarks cog to be added to the bot	`tux/cogs/services/bookmarks.py`
Update README.md contributors section	Rearrange contributor entries Remove some contributor entries Add new contributor entries	`README.md`

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @GabberBalls - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider implementing a rate limit or cooldown mechanism to prevent potential abuse of the bookmarking feature.
It might be beneficial to add an option for users to disable receiving bookmark messages in their DMs if they prefer not to use this feature.

Here's what I looked at during the review

🟡 General issues: 3 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-08-28T22:29:32Z

tux/cogs/services/bookmarks.py

+                    await user.send(
+                        f"Bookmarked: {message_link}\nAuthor: {author_username}\nContent: {message_contents}",
+                    )
+            except (discord.Forbidden, discord.HTTPException):


suggestion: Provide feedback to the user on bookmark failure

When an error occurs during bookmarking, the user is not notified. Consider adding a way to inform the user about the failure, perhaps by reacting to the message with an error emoji.

Suggested change

except (discord.Forbidden, discord.HTTPException):

except (discord.Forbidden, discord.HTTPException):

logger.error(f"An error occurred while bookmarking {message_link} for {user}")

await message.add_reaction('❌')

return

sourcery-ai · 2024-08-28T22:29:32Z

tux/cogs/services/bookmarks.py

+            message_contents = reaction.message.content
+            message_attachments = reaction.message.attachments
+            author_username = reaction.message.author.name
+            try:


suggestion: Extract message construction logic into a separate method

The logic for constructing the bookmark message could be extracted into a separate method. This would improve readability and make the code more maintainable.

try: bookmark_message = self.construct_bookmark_message(reaction.message)

sourcery-ai · 2024-08-28T22:29:32Z

tux/cogs/services/bookmarks.py

+    @commands.Cog.listener()
+    async def on_reaction_add(self, reaction: discord.Reaction, user: discord.User) -> None:
+        if str(reaction.emoji) == "🔖":
+            message_link = f"https://discord.com/channels/{reaction.message.guild.id}/{reaction.message.channel.id}/{reaction.message.id}"


suggestion: Use Discord's jump_url property instead of constructing the link manually

Instead of manually constructing the message link, consider using Discord's built-in jump_url property. This would be more idiomatic and potentially more robust: message_link = reaction.message.jump_url

Suggested change

message_link = f"https://discord.com/channels/{reaction.message.guild.id}/{reaction.message.channel.id}/{reaction.message.id}"

message_link = reaction.message.jump_url

meatsnails · 2024-08-28T22:29:50Z

forgot to mention this in the origin post but it has some issues with discord's message caching

meatsnails · 2024-08-28T23:23:46Z

updated with embeds at @kzndotsh's request should be all good for commit now

teilorr

on_raw_reaction_add will fix those 'caching' issues

tux/cogs/services/bookmarks.py

teilorr · 2024-08-28T23:58:14Z

tux/cogs/services/bookmarks.py

+            message_link = f"https://discord.com/channels/{reaction.message.guild.id}/{reaction.message.channel.id}/{reaction.message.id}"
+            message_contents = reaction.message.content
+            message_attachments = reaction.message.attachments
+            author_username = reaction.message.author.name


unnecessary variable

tux/cogs/services/bookmarks.py

meatsnails · 2024-08-29T00:23:06Z

shout out to @teilorr for pointing out those fixes and issues

electron271 · 2024-08-29T02:41:42Z

tux/cogs/services/bookmarks.py

@@ -0,0 +1,56 @@
+import discord


move this file to the utility cogs dir

electron271 · 2024-08-29T02:43:40Z

tux/cogs/services/bookmarks.py

+                message_link = message.jump_url
+                message_contents = message.content
+                message_attachments = message.attachments
+                author_username = message.author.name


redundant variables

ruff is also complaining about typechecking

electron271 · 2024-08-29T02:44:16Z

tux/cogs/services/bookmarks.py

+                    logger.error("Channel not found")
+                    return
+
+                message = await channel.fetch_message(payload.message_id)


typechecking warnings

electron271 · 2024-08-29T02:44:24Z

tux/cogs/services/bookmarks.py

+                embed.add_field(name="Jump to Message", value=f"[Click Here]({message_link})", inline=False)
+
+                if message_attachments:
+                    attachments_info = "\n".join([attachment.url for attachment in message_attachments])


typechecking errors

electron271 · 2024-08-29T02:47:05Z

tux/cogs/services/bookmarks.py

+                    else:
+                        logger.error(f"User not found for ID: {payload.user_id}")
+                except (discord.Forbidden, discord.HTTPException) as e:
+                    logger.error(f"An error occurred while bookmarking {message_link} for {payload.user_id}: {e}")


add some special handling for if the user has dms off, maybe like react also with like a error dms off emoji or something or send a message in chat for a bit then delete it

tux/cogs/services/bookmarks.py

sourcery-ai bot reviewed Aug 28, 2024

View reviewed changes

teilorr suggested changes Aug 29, 2024

View reviewed changes

electron271 requested review from electron271 and kzndotsh August 29, 2024 01:56

electron271 assigned electron271, kzndotsh and meatsnails and unassigned kzndotsh Aug 29, 2024

electron271 added category: commands priority: low type: enhancement labels Aug 29, 2024

electron271 requested changes Aug 29, 2024

View reviewed changes

meatsnails closed this Aug 29, 2024

meatsnails mentioned this pull request Aug 29, 2024

Added Bookmarks #485

Merged

	message_link = f"https://discord.com/channels/{reaction.message.guild.id}/{reaction.message.channel.id}/{reaction.message.id}"
	message_link = reaction.message.jump_url

Uh oh!

Bookmarking messages #479

Bookmarking messages #479

Uh oh!

Conversation

meatsnails commented Aug 28, 2024 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Guidelines

How Has This Been Tested? (if applicable)

Screenshots (if applicable)

Additional Information

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

File-Level Changes

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

meatsnails commented Aug 28, 2024

Uh oh!

meatsnails commented Aug 28, 2024

Uh oh!

teilorr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teilorr Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meatsnails commented Aug 29, 2024

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

electron271 Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

meatsnails commented Aug 28, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Aug 28, 2024 •

edited

Loading