draft: Add DM communications #31

jamievlin · 2023-12-19T01:14:44Z

Closes #8

No767

This is a first brief overview and quick look at the code. I'd recommend at #20 for how I set up views, etc. What I'll more than likely do is to take some of the code in this PR, and merge it with #20 as that PR has a complete full impl of the core tickets system

Also make sure to run the formatters, and check if Pyright and Ruff catches any errors or not

No767 · 2023-12-19T02:08:56Z

sql/create_user_threads_map.sql

Please use the built in migrations system in order to handle this. You can use python migrations.py migrate to create a new revision to work on

No767 · 2023-12-19T02:18:44Z

bot/cogs/dm_comms.py

+
+    async def on_timeout(self) -> None:
+        if not self._replied:
+            await self._base_msg.reply("You have not responded within 30 seconds")


I highly don't recommend using discord.Message.reply, as it quickly becomes annoying. The user already knows that the bot already responded, so it's better to use discord.Message.send instead

Suggested change

await self._base_msg.reply("You have not responded within 30 seconds")

async def on_timeout(self) -> None:

if self.message:

await self.message.edit(content="You have not responded within 30 seconds")

also realistically, the 30 second timeout is kinda too little. ~300-500 seconds is a good amount as the user may or may be deciding on whether they should be creating the ticket or not

No767 · 2023-12-19T02:19:39Z

bot/cogs/dm_comms.py

+from rodhaj import Rodhaj
+
+
+class StubView(discord.ui.View):


libs.utils.views.RoboView exists, and is designed to aid with default views

bot/cogs/dm_comms.py

No767 · 2023-12-19T02:24:25Z

bot/cogs/dm_comms.py

+    @commands.Cog.listener()
+    async def on_message(self, message: Message):
+        if isinstance(message.channel, DMChannel):
+            if message.author not in self._bot.users:


Ah yes the classic mistake of thinking discord.Member has an __eq__ method. If you wish to compare them, compare the ids instead. And I'm not so sure to say about comparing the bot's internal user cache as it might not be accurate (maybe ask soheab about this?)

jamievlin · 2023-12-22T21:09:21Z

@No767 I believe I fixed most of the issues (aside from formatting or pyright, but I will later today)

No767 · 2023-12-23T00:16:38Z

I'll be submitting a review fairly soon

No767

I can see the improvements. I've left some comments

No767 · 2023-12-25T04:45:02Z

bot/cogs/dm_comms.py

+            if not self._bot.user_is_this_bot(message.author):
+                await self.handle_dm(message)


As this code lies within the on_message, this event takes an argument, which is discord.Message. With this knowledge, the canonical method for preventing bot recursion is shown below:

if message.author.bot: return ... # do the rest of the code

The way Solstice phrased it was a bit confusing

No767 · 2023-12-25T04:50:13Z

bot/cogs/dm_comms.py

+
+    @commands.Cog.listener()
+    async def on_message(self, message: Message):
+        if isinstance(message.channel, DMChannel):


A cleaner check can be implemented as such:

Suggested change

if isinstance(message.channel, DMChannel):

# If the guild associated with the message is None, we know that it has to be a guaranteed to come from a DM instead

if message.guild is None: ...

bot/cogs/dm_comms.py

No767 · 2023-12-27T22:06:06Z

bot/cogs/dm_comms.py

+
+    async def handle_user_no_thread(self, message: Message):
+        channel = message.channel
+        context = await self._bot.get_context(message)


It would be more efficient just to construct the context once as you would need it in both cases anyways

No767 · 2023-12-27T22:07:20Z

bot/cogs/dm_comms.py

+
+        # Code to create thread here
+        if result:
+            await channel.send(


message.channel sends it to the message's channel. I'd not recommend doing this as it can get confusing and does introduce a bug into the code. In reality, use the context here instead

No767 · 2023-12-27T22:08:46Z

bot/rodhaj.py

@@ -83,6 +83,10 @@ def stop():
            self.logger.info("Dev mode is enabled. Loading FSWatcher")
            self.loop.create_task(self.fs_watcher())

+    def user_is_this_bot(self, user: Union[discord.User, discord.Member]):


just check the attr of discord.User's bot

for example:

# Assume we have the user variable, which is the type of discord.User return user.bot

in reality you probably don't even need this helper method to begin with

Deleted this method

for more information, see https://pre-commit.ci

This reverts commit e825d0d.

sonarcloud · 2024-01-05T08:11:38Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

No767

Everything looks good to me. What I'll probably do is to merge this into main, and then merge it back into my branch (noelle/tickets), and then edit the code as needed on my branch

jamievlin requested a review from No767 December 19, 2023 01:14

jamievlin self-assigned this Dec 19, 2023

jamievlin force-pushed the jamie/dm-comms branch 2 times, most recently from 9444ecc to e825d0d Compare December 19, 2023 01:21

No767 requested changes Dec 19, 2023

View reviewed changes

jamievlin force-pushed the jamie/dm-comms branch 2 times, most recently from ee0492a to 4eed780 Compare December 22, 2023 21:08

jamievlin force-pushed the jamie/dm-comms branch 2 times, most recently from ae89937 to 480f560 Compare December 22, 2023 21:14

jamievlin requested a review from No767 December 22, 2023 21:15

No767 reviewed Dec 27, 2023

View reviewed changes

jamievlin force-pushed the jamie/dm-comms branch from 480f560 to c431687 Compare December 28, 2023 03:09

jamievlin and others added 5 commits January 4, 2024 23:43

dm comms: Add Cog to handle DMS sent to bot.

d32e57a

SQL: Add stub sql script to create user to thread mapping.

9d6a327

[pre-commit.ci] auto fixes from pre-commit.com hooks

fa25bcd

for more information, see https://pre-commit.ci

Revert "SQL: Add stub sql script to create user to thread mapping."

a92a6bf

This reverts commit e825d0d.

SQL: Fix query in handling DM.

8c2d472

jamievlin force-pushed the jamie/dm-comms branch from c431687 to 045f710 Compare January 5, 2024 06:43

jamievlin added 5 commits January 4, 2024 23:50

DM: Fix checking that dm's author is not the bot.

c83979e

DMCOMMS: Make handle_user_no_thread instance method.

6f1512a

VIEWS: Fix ConfirmationView's invocation of super's ctor.

5541483

VIEWS: Use Confirmation view for stub view for now.

2c1180f

RODHAJ: Fix style as Noelle's comments.

ddb8426

jamievlin force-pushed the jamie/dm-comms branch from 045f710 to ddb8426 Compare January 5, 2024 08:11

jamievlin requested a review from No767 January 5, 2024 08:11

jamievlin marked this pull request as ready for review January 5, 2024 08:12

No767 approved these changes Jan 5, 2024

View reviewed changes

No767 merged commit 8082e64 into main Jan 5, 2024
9 checks passed

No767 mentioned this pull request Jan 5, 2024

Bi-directional communication #10

Closed

No767 deleted the jamie/dm-comms branch January 6, 2024 00:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

draft: Add DM communications #31

draft: Add DM communications #31

jamievlin commented Dec 19, 2023

No767 left a comment •

edited

Loading

No767 Dec 19, 2023

No767 Dec 19, 2023

No767 Dec 19, 2023

No767 Dec 19, 2023

jamievlin commented Dec 22, 2023 •

edited

Loading

No767 commented Dec 23, 2023

No767 left a comment

No767 Dec 25, 2023

No767 Dec 25, 2023

No767 Dec 27, 2023

No767 Dec 27, 2023

No767 Dec 27, 2023

jamievlin Jan 5, 2024

sonarcloud bot commented Jan 5, 2024

No767 left a comment

		if not self._bot.user_is_this_bot(message.author):
		await self.handle_dm(message)

-        if isinstance(message.channel, DMChannel):
+# If the guild associated with the message is None, we know that it has to be a guaranteed to come from a DM instead
+if message.guild is None: ...

draft: Add DM communications #31

draft: Add DM communications #31

Conversation

jamievlin commented Dec 19, 2023

No767 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamievlin commented Dec 22, 2023 • edited Loading

No767 commented Dec 23, 2023

No767 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 5, 2024

Quality Gate passed

No767 left a comment

Choose a reason for hiding this comment

No767 left a comment •

edited

Loading

jamievlin commented Dec 22, 2023 •

edited

Loading