v3.3.2-dev5 - fix edit/delete

Author: Taaku18

.github/workflows/stale.yml

@@ -1,4 +1,5 @@
-name: "Close stale issues"
+name: "Close Stale Issues"
+
 on:
   schedule:
   - cron: "0 0 * * *"

CHANGELOG.md

@@ -7,7 +7,9 @@ This project mostly adheres to [Semantic Versioning](https://semver.org/spec/v2.
 however, insignificant breaking changes does not guarantee a major version bump, see the reasoning [here](https://github.com/kyb3r/modmail/issues/319).
 
 
-# v3.3.2-dev4
+# v3.3.2-dev5
+
+(Development update, very likely to be unstable!)
 
 ### Added
 
@@ -21,20 +23,27 @@ however, insignificant breaking changes does not guarantee a major version bump,
 - Added "perhaps you meant" section to `?config help`.
 - Multi-command alias is now more stable. With support for a single quote escape `\"`.
 - New command `?freply`, which behaves exactly like `?reply` with the addition that you can substitute `{channel}`, `{recipient}`, and `{author}` to be their respective values. 
+- New command `?repair`, repair any broken Modmail thread (with help from @officialpiyush).
+- Recipients gets a feedback when they edit message.
 
 ### Changed
 
 - The look of alias and snippet when previewing.
+- Message ID of the thread embed is saved in DB, instead of the original message.
 
 ### Fixed
 
 - Setting config vars using human time wasn't working.
 - Fixed some bugs with aliases.
+- Fixed a lot of issues with `?edit` and `?delete` and recipient message edit.
+- Masked the error: "AttributeError: 'int' object has no attribute 'name'"
+  - Channel delete event will not be checked until discord.py fixes this issue.
 
 ### Internal
 
 - Commit to black format line width max = 99, consistent with pylint.
 - Alias parser is rewritten without shlex.
+- New checks with thread create / find.
 
 # v3.3.1
 

bot.py

@@ -1,4 +1,4 @@
-__version__ = "3.3.2-dev4"
+__version__ = "3.3.2-dev5"
 
 
 import asyncio
@@ -150,7 +150,7 @@ def session(self) -> ClientSession:
         return self._session
 
     @property
-    def api(self):
+    def api(self) -> ApiClient:
         if self._api is None:
             self._api = ApiClient(self)
         return self._api
@@ -435,8 +435,13 @@ async def on_ready(self):
 
         for recipient_id, items in tuple(closures.items()):
             after = (datetime.fromisoformat(items["time"]) - datetime.utcnow()).total_seconds()
-            if after < 0:
+            if after <= 0:
+                logger.debug("Closing thread for recipient %s.", recipient_id)
                 after = 0
+            else:
+                logger.debug(
+                    "Thread for recipient %s will be closed after %s seconds.", recipient_id, after
+                )
 
             thread = await self.threads.find(recipient_id=int(recipient_id))
 
@@ -447,8 +452,6 @@ async def on_ready(self):
                 await self.config.update()
                 continue
 
-            logger.debug("Closing thread for recipient %s.", recipient_id)
-
             await thread.close(
                 closer=self.get_user(items["closer_id"]),
                 after=after,
@@ -977,15 +980,20 @@ async def on_guild_channel_delete(self, channel):
         if channel.guild != self.modmail_guild:
             return
 
-        audit_logs = self.modmail_guild.audit_logs()
-        entry = await audit_logs.find(lambda e: e.target.id == channel.id)
-        mod = entry.user
+        try:
+            audit_logs = self.modmail_guild.audit_logs()
+            entry = await audit_logs.find(lambda a: a.target == channel)
+            mod = entry.user
+        except AttributeError as e:
+            # discord.py broken implementation with discord API
+            logger.warning("Failed to retrieve audit log.", str(e))
+            return
 
         if mod == self.user:
             return
 
         if isinstance(channel, discord.CategoryChannel):
-            if self.main_category.id == channel.id:
+            if self.main_category == channel:
                 logger.debug("Main category was deleted.")
                 self.config.remove("main_category_id")
                 await self.config.update()
@@ -994,14 +1002,14 @@ async def on_guild_channel_delete(self, channel):
         if not isinstance(channel, discord.TextChannel):
             return
 
-        if self.log_channel is None or self.log_channel.id == channel.id:
+        if self.log_channel is None or self.log_channel == channel:
             logger.info("Log channel deleted.")
             self.config.remove("log_channel_id")
             await self.config.update()
             return
 
         thread = await self.threads.find(channel=channel)
-        if thread:
+        if thread and thread.channel == channel:
             logger.debug("Manually closed channel %s.", channel.name)
             await thread.close(closer=mod, silent=True, delete_channel=False)
 
@@ -1044,19 +1052,24 @@ async def on_bulk_message_delete(self, messages):
         await discord.utils.async_all(self.on_message_delete(msg) for msg in messages)
 
     async def on_message_edit(self, before, after):
-        if before.author.bot:
+        if after.author.bot:
             return
-        if isinstance(before.channel, discord.DMChannel):
+        if isinstance(after.channel, discord.DMChannel):
             thread = await self.threads.find(recipient=before.author)
-            async for msg in thread.channel.history():
-                if msg.embeds:
-                    embed = msg.embeds[0]
-                    matches = str(embed.author.url).split("/")
-                    if matches and matches[-1] == str(before.id):
-                        embed.description = after.content
-                        await msg.edit(embed=embed)
-                        await self.api.edit_message(str(after.id), after.content)
-                        break
+            try:
+                await thread.edit_dm_message(after, after.content)
+            except ValueError:
+                _, blocked_emoji = await self.retrieve_emoji()
+                try:
+                    await after.add_reaction(blocked_emoji)
+                except (discord.HTTPException, discord.InvalidArgument):
+                    pass
+            else:
+                embed = discord.Embed(
+                    description="Successfully Edited Message", color=self.main_color
+                )
+                embed.set_footer(text=f"Message ID: {after.id}")
+                await after.channel.send(embed=embed)
 
     async def on_error(self, event_method, *args, **kwargs):
         logger.error("Ignoring exception in %s.", event_method)

cogs/modmail.py

@@ -1,4 +1,5 @@
 import asyncio
+import re
 from datetime import datetime
 from itertools import zip_longest
 from typing import Optional, Union
@@ -14,6 +15,7 @@
 from core import checks
 from core.models import PermissionLevel, getLogger
 from core.paginator import EmbedPaginatorSession
+from core.thread import Thread
 from core.time import UserFriendlyTime, human_timedelta
 from core.utils import (
     format_preview,
@@ -22,6 +24,8 @@
     format_description,
     trigger_typing,
     escape_code_block,
+    match_user_id,
+    format_channel_name,
 )
 
 logger = getLogger(__name__)
@@ -841,23 +845,6 @@ async def note(self, ctx, *, msg: str = ""):
             msg = await ctx.thread.note(ctx.message)
             await msg.pin()
 
-    async def find_linked_message(self, ctx, message_id):
-        linked_message_id = None
-
-        async for msg in ctx.channel.history():
-            if message_id is None and msg.embeds:
-                embed = msg.embeds[0]
-                if embed.color.value != self.bot.mod_color or not embed.author.url:
-                    continue
-                # TODO: use regex to find the linked message id
-                linked_message_id = str(embed.author.url).split("/")[-1]
-
-            elif message_id and msg.id == message_id:
-                url = msg.embeds[0].author.url
-                linked_message_id = str(url).split("/")[-1]
-
-        return linked_message_id
-
     @commands.command()
     @checks.has_permissions(PermissionLevel.SUPPORTER)
     @checks.thread_only()
@@ -867,12 +854,14 @@ async def edit(self, ctx, message_id: Optional[int] = None, *, message: str):
 
         If no `message_id` is provided,
         the last message sent by a staff will be edited.
+
+        Note: attachments **cannot** be edited.
         """
         thread = ctx.thread
 
-        linked_message_id = await self.find_linked_message(ctx, message_id)
-
-        if linked_message_id is None:
+        try:
+            await thread.edit_message(message_id, message)
+        except ValueError:
             return await ctx.send(
                 embed=discord.Embed(
                     title="Failed",
@@ -881,16 +870,8 @@ async def edit(self, ctx, message_id: Optional[int] = None, *, message: str):
                 )
             )
 
-        await asyncio.gather(
-            thread.edit_message(linked_message_id, message),
-            self.bot.api.edit_message(linked_message_id, message),
-        )
-
         sent_emoji, _ = await self.bot.retrieve_emoji()
-        try:
-            await ctx.message.add_reaction(sent_emoji)
-        except (discord.HTTPException, discord.InvalidArgument):
-            pass
+        return await ctx.message.add_reaction(sent_emoji)
 
     @commands.command()
     @checks.has_permissions(PermissionLevel.SUPPORTER)
@@ -1168,7 +1149,7 @@ async def unblock(self, ctx, *, user: User = None):
     @commands.command()
     @checks.has_permissions(PermissionLevel.SUPPORTER)
     @checks.thread_only()
-    async def delete(self, ctx, message_id: Optional[int] = None):
+    async def delete(self, ctx, message_id: int = None):
         """
         Delete a message that was sent using the reply command or a note.
 
@@ -1179,15 +1160,9 @@ async def delete(self, ctx, message_id: Optional[int] = None):
         """
         thread = ctx.thread
 
-        if message_id is not None:
-            try:
-                message_id = int(message_id)
-            except ValueError:
-                raise commands.BadArgument("A message ID needs to be specified.")
-
-        linked_message_id = await self.find_linked_message(ctx, message_id)
-
-        if linked_message_id is None:
+        try:
+            await thread.delete_message(message_id)
+        except ValueError:
             return await ctx.send(
                 embed=discord.Embed(
                     title="Failed",
@@ -1196,12 +1171,119 @@ async def delete(self, ctx, message_id: Optional[int] = None):
                 )
             )
 
-        await thread.delete_message(linked_message_id)
         sent_emoji, _ = await self.bot.retrieve_emoji()
-        try:
-            await ctx.message.add_reaction(sent_emoji)
-        except (discord.HTTPException, discord.InvalidArgument):
-            pass
+        return await ctx.message.add_reaction(sent_emoji)
+
+    @commands.command()
+    @checks.has_permissions(PermissionLevel.SUPPORTER)
+    async def repair(self, ctx):
+        """
+        Repair a thread broken by Discord.
+        """
+        sent_emoji, blocked_emoji = await self.bot.retrieve_emoji()
+
+        if ctx.thread:
+            user_id = match_user_id(ctx.channel.topic)
+            if user_id == -1:
+                logger.info("Setting current channel's topic to User ID.")
+                await ctx.channel.edit(topic=f"User ID: {ctx.thread.id}")
+            return await ctx.message.add_reaction(sent_emoji)
+
+        logger.info("Attempting to fix a broken thread %s.", ctx.channel.name)
+
+        # Search cache for channel
+        user_id, thread = next(
+            ((k, v) for k, v in self.bot.threads.cache.items() if v.channel == ctx.channel),
+            (-1, None),
+        )
+        if thread is not None:
+            logger.debug("Found thread with tempered ID.")
+            await ctx.channel.edit(reason="Fix broken Modmail thread", topic=f"User ID: {user_id}")
+            return await ctx.message.add_reaction(sent_emoji)
+
+        # find genesis message to retrieve User ID
+        async for message in ctx.channel.history(limit=10, oldest_first=True):
+            if (
+                message.author == self.bot.user
+                and message.embeds
+                and message.embeds[0].color
+                and message.embeds[0].color.value == self.bot.main_color
+                and message.embeds[0].footer.text
+            ):
+                user_id = match_user_id(message.embeds[0].footer.text)
+                if user_id != -1:
+                    recipient = self.bot.get_user(user_id)
+                    if recipient is None:
+                        self.bot.threads.cache[user_id] = thread = Thread(
+                            self.bot.threads, user_id, ctx.channel
+                        )
+                    else:
+                        self.bot.threads.cache[user_id] = thread = Thread(
+                            self.bot.threads, recipient, ctx.channel
+                        )
+                    thread.ready = True
+                    logger.info(
+                        "Setting current channel's topic to User ID and created new thread."
+                    )
+                    await ctx.channel.edit(
+                        reason="Fix broken Modmail thread", topic=f"User ID: {user_id}"
+                    )
+                    return await ctx.message.add_reaction(sent_emoji)
+
+        else:
+            logger.warning("No genesis message found.")
+
+        # match username from channel name
+        # username-1234, username-1234_1, username-1234_2
+        m = re.match(r"^(.+)-(\d{4})(?:_\d+)?$", ctx.channel.name)
+        if m is not None:
+            users = set(
+                filter(
+                    lambda member: member.name == m.group(1)
+                    and member.discriminator == m.group(2),
+                    ctx.guild.members,
+                )
+            )
+            if len(users) == 1:
+                user = users[0]
+                name = format_channel_name(
+                    user, self.bot.modmail_guild, exclude_channel=ctx.channel
+                )
+                recipient = self.bot.get_user(user.id)
+                if user.id in self.bot.threads.cache:
+                    thread = self.bot.threads.cache[user.id]
+                    if thread.channel:
+                        embed = discord.Embed(
+                            title="Delete Channel",
+                            description="This thread channel is no longer in use. "
+                            f"All messages will be directed to {ctx.channel.mention} instead.",
+                            color=self.bot.error_color,
+                        )
+                        embed.set_footer(
+                            text='Please manually delete this channel, do not use "{prefix}close".'
+                        )
+                        try:
+                            await thread.channel.send(embed=embed)
+                        except discord.HTTPException:
+                            pass
+                if recipient is None:
+                    self.bot.threads.cache[user.id] = thread = Thread(
+                        self.bot.threads, user_id, ctx.channel
+                    )
+                else:
+                    self.bot.threads.cache[user.id] = thread = Thread(
+                        self.bot.threads, recipient, ctx.channel
+                    )
+                thread.ready = True
+                logger.info("Setting current channel's topic to User ID and created new thread.")
+                await ctx.channel.edit(
+                    reason="Fix broken Modmail thread", name=name, topic=f"User ID: {user.id}"
+                )
+                return await ctx.message.add_reaction(sent_emoji)
+
+            elif len(users) >= 2:
+                logger.info("Multiple users with the same name and discriminator.")
+        return await ctx.message.add_reaction(blocked_emoji)
 
     @commands.command()
     @checks.has_permissions(PermissionLevel.ADMINISTRATOR)