Merge pull request #885 from python-discord/feat/frontend/839/help-channel-role

Use a role overwrite to keep track of help channel cooldowns

3 files changed, 60 insertions, 69 deletions

diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py
index a61f30deb..ef58ca9a1 100644
--- a/bot/cogs/help_channels.py
+++ b/bot/cogs/help_channels.py
@@ -39,8 +39,9 @@ channels in the Help: Available category.
 AVAILABLE_MSG = f"""
 This help channel is now **available**, which means that you can claim it by simply typing your \
 question into it. Once claimed, the channel will move into the **Python Help: Occupied** category, \
-and will be yours until it has been inactive for {constants.HelpChannels.idle_minutes} minutes. When \
-that happens, it will be set to **dormant** and moved into the **Help: Dormant** category.
+and will be yours until it has been inactive for {constants.HelpChannels.idle_minutes} minutes or \
+is closed manually with `!close`. When that happens, it will be set to **dormant** and moved into \
+the **Help: Dormant** category.
 
 You may claim a new channel once every {constants.HelpChannels.claim_minutes} minutes. If you \
 currently cannot send a message in this channel, it means you are on cooldown and need to wait.
@@ -66,6 +67,8 @@ IN_USE_ANSWERED_EMOJI = "⌛"
 IN_USE_UNANSWERED_EMOJI = "⏳"
 NAME_SEPARATOR = "｜"
 
+CoroutineFunc = t.Callable[..., t.Coroutine]
+
 
 class TaskData(t.NamedTuple):
     """Data for a scheduled task."""
@@ -89,12 +92,15 @@ class HelpChannels(Scheduler, commands.Cog):
         * If there are no more dormant channels, the bot will automatically create a new one
         * If there are no dormant channels to move, helpers will be notified (see `notify()`)
     * When a channel becomes available, the dormant embed will be edited to show `AVAILABLE_MSG`
+    * User can only claim a channel at an interval `constants.HelpChannels.claim_minutes`
+        * To keep track of cooldowns, user which claimed a channel will have a temporary role
 
     In Use Category
 
     * Contains all channels which are occupied by someone needing help
     * Channel moves to dormant category after `constants.HelpChannels.idle_minutes` of being idle
     * Command can prematurely mark a channel as dormant
+        * Channel claimant is allowed to use the command
         * Allowed roles for the command are configurable with `constants.HelpChannels.cmd_whitelist`
     * When a channel becomes dormant, an embed with `DORMANT_MSG` will be sent
 
@@ -217,8 +223,8 @@ class HelpChannels(Scheduler, commands.Cog):
 
         return role_check
 
-    @commands.command(name="dormant", aliases=["close"], enabled=False)
-    async def dormant_command(self, ctx: commands.Context) -> None:
+    @commands.command(name="close", aliases=["dormant"], enabled=False)
+    async def close_command(self, ctx: commands.Context) -> None:
         """
         Make the current in-use help channel dormant.
 
@@ -226,14 +232,15 @@ class HelpChannels(Scheduler, commands.Cog):
         delete the message that invoked this,
         and reset the send permissions cooldown for the user who started the session.
         """
-        log.trace("dormant command invoked; checking if the channel is in-use.")
+        log.trace("close command invoked; checking if the channel is in-use.")
         if ctx.channel.category == self.in_use_category:
             if await self.dormant_check(ctx):
                 with suppress(KeyError):
                     del self.help_channel_claimants[ctx.channel]
 
-                with suppress(discord.errors.HTTPException, discord.errors.NotFound):
-                    await self.reset_claimant_send_permission(ctx.channel)
+                await self.remove_cooldown_role(ctx.author)
+                # Ignore missing task when cooldown has passed but the channel still isn't dormant.
+                self.cancel_task(ctx.author.id, ignore_missing=True)
 
                 await self.move_to_dormant(ctx.channel, "command")
                 self.cancel_task(ctx.channel.id)
@@ -271,7 +278,7 @@ class HelpChannels(Scheduler, commands.Cog):
             log.trace(f"The clean name for `{channel}` is `{name}`")
         except ValueError:
             # If, for some reason, the channel name does not contain "help-" fall back gracefully
-            log.info(f"Can't get clean name as `{channel}` does not follow the `{prefix}` naming convention.")
+            log.info(f"Can't get clean name because `{channel}` isn't prefixed by `{prefix}`.")
             name = channel.name
 
         return name
@@ -400,7 +407,7 @@ class HelpChannels(Scheduler, commands.Cog):
         # The ready event wasn't used because channels could change categories between the time
         # the command is invoked and the cog is ready (e.g. if move_idle_channel wasn't called yet).
         # This may confuse users. So would potentially long delays for the cog to become ready.
-        self.dormant_command.enabled = True
+        self.close_command.enabled = True
 
         await self.init_available()
 
@@ -419,6 +426,11 @@ class HelpChannels(Scheduler, commands.Cog):
         self.bot.stats.gauge("help.total.available", total_available)
         self.bot.stats.gauge("help.total.dormant", total_dormant)
 
+    @staticmethod
+    def is_claimant(member: discord.Member) -> bool:
+        """Return True if `member` has the 'Help Cooldown' role."""
+        return any(constants.Roles.help_cooldown == role.id for role in member.roles)
+
     def is_dormant_message(self, message: t.Optional[discord.Message]) -> bool:
         """Return True if the contents of the `message` match `DORMANT_MSG`."""
         if not message or not message.embeds:
@@ -484,11 +496,6 @@ class HelpChannels(Scheduler, commands.Cog):
             topic=AVAILABLE_TOPIC,
         )
 
-        log.trace(
-            f"Ensuring that all channels in `{self.available_category}` have "
-            f"synchronized permissions after moving `{channel}` into it."
-        )
-        await self.ensure_permissions_synchronization(self.available_category)
         self.report_stats()
 
     async def move_to_dormant(self, channel: discord.TextChannel, caller: str) -> None:
@@ -658,67 +665,49 @@ class HelpChannels(Scheduler, commands.Cog):
         # be put in the queue.
         await self.move_to_available()
 
-    @staticmethod
-    async def ensure_permissions_synchronization(category: discord.CategoryChannel) -> None:
-        """
-        Ensure that all channels in the `category` have their permissions synchronized.
-
-        This method mitigates an issue we have yet to find the cause for: Every so often, a channel in the
-        `Help: Available` category gets in a state in which it will no longer synchronizes its permissions
-        with the category. To prevent that, we iterate over the channels in the category and edit the channels
-        that are observed to be in such a state. If no "out of sync" channels are observed, this method will
-        not make API calls and should be fairly inexpensive to run.
-        """
-        for channel in category.channels:
-            if not channel.permissions_synced:
-                log.info(f"The permissions of channel `{channel}` were out of sync with category `{category}`.")
-                await channel.edit(sync_permissions=True)
-
-    async def update_category_permissions(
-        self, category: discord.CategoryChannel, member: discord.Member, **permissions
-    ) -> None:
-        """
-        Update the permissions of the given `member` for the given `category` with `permissions` passed.
-
-        After updating the permissions for the member in the category, this helper function will call the
-        `ensure_permissions_synchronization` method to ensure that all channels are still synchronizing their
-        permissions with the category. It's currently unknown why some channels get "out of sync", but this
-        hopefully mitigates the issue.
-        """
-        log.trace(f"Updating permissions for `{member}` in `{category}` with {permissions}.")
-        await category.set_permissions(member, **permissions)
-
-        log.trace(f"Ensuring that all channels in `{category}` are synchronized after permissions update.")
-        await self.ensure_permissions_synchronization(category)
-
     async def reset_send_permissions(self) -> None:
-        """Reset send permissions for members with it set to False in the Available category."""
+        """Reset send permissions in the Available category for claimants."""
         log.trace("Resetting send permissions in the Available category.")
+        guild = self.bot.get_guild(constants.Guild.id)
 
-        for member, overwrite in self.available_category.overwrites.items():
-            if isinstance(member, discord.Member) and overwrite.send_messages is False:
-                log.trace(f"Resetting send permissions for {member} ({member.id}).")
+        # TODO: replace with a persistent cache cause checking every member is quite slow
+        for member in guild.members:
+            if self.is_claimant(member):
+                await self.remove_cooldown_role(member)
 
-                # We don't use the permissions helper function here as we may have to reset multiple overwrites
-                # and we don't want to enforce the permissions synchronization in each iteration.
-                await self.available_category.set_permissions(member, overwrite=None)
+    async def add_cooldown_role(self, member: discord.Member) -> None:
+        """Add the help cooldown role to `member`."""
+        log.trace(f"Adding cooldown role for {member} ({member.id}).")
+        await self._change_cooldown_role(member, member.add_roles)
 
-        log.trace(f"Ensuring channels in `Help: Available` are synchronized after permissions reset.")
-        await self.ensure_permissions_synchronization(self.available_category)
+    async def remove_cooldown_role(self, member: discord.Member) -> None:
+        """Remove the help cooldown role from `member`."""
+        log.trace(f"Removing cooldown role for {member} ({member.id}).")
+        await self._change_cooldown_role(member, member.remove_roles)
 
-    async def reset_claimant_send_permission(self, channel: discord.TextChannel) -> None:
-        """Reset send permissions in the Available category for the help `channel` claimant."""
-        log.trace(f"Attempting to find claimant for #{channel.name} ({channel.id}).")
-        try:
-            member = self.help_channel_claimants[channel]
-        except KeyError:
-            log.trace(f"Channel #{channel.name} ({channel.id}) not in claimant cache, permissions unchanged.")
+    async def _change_cooldown_role(self, member: discord.Member, coro_func: CoroutineFunc) -> None:
+        """
+        Change `member`'s cooldown role via awaiting `coro_func` and handle errors.
+
+        `coro_func` is intended to be `discord.Member.add_roles` or `discord.Member.remove_roles`.
+        """
+        guild = self.bot.get_guild(constants.Guild.id)
+        role = guild.get_role(constants.Roles.help_cooldown)
+        if role is None:
+            log.warning(f"Help cooldown role ({constants.Roles.help_cooldown}) could not be found!")
             return
 
-        log.trace(f"Resetting send permissions for {member} ({member.id}).")
-        await self.update_category_permissions(self.available_category, member, overwrite=None)
-        # Ignore missing task when claim cooldown has passed but the channel still isn't dormant.
-        self.cancel_task(member.id, ignore_missing=True)
+        try:
+            await coro_func(role)
+        except discord.NotFound:
+            log.debug(f"Failed to change role for {member} ({member.id}): member not found")
+        except discord.Forbidden:
+            log.debug(
+                f"Forbidden to change role for {member} ({member.id}); "
+                f"possibly due to role hierarchy"
+            )
+        except discord.HTTPException as e:
+            log.error(f"Failed to change role for {member} ({member.id}): {e.status} {e.code}")
 
     async def revoke_send_permissions(self, member: discord.Member) -> None:
         """
@@ -731,14 +720,14 @@ class HelpChannels(Scheduler, commands.Cog):
             f"Revoking {member}'s ({member.id}) send message permissions in the Available category."
         )
 
-        await self.update_category_permissions(self.available_category, member, send_messages=False)
+        await self.add_cooldown_role(member)
 
         # Cancel the existing task, if any.
         # Would mean the user somehow bypassed the lack of permissions (e.g. user is guild owner).
         self.cancel_task(member.id, ignore_missing=True)
 
         timeout = constants.HelpChannels.claim_minutes * 60
-        callback = self.update_category_permissions(self.available_category, member, overwrite=None)
+        callback = self.remove_cooldown_role(member)
 
         log.trace(f"Scheduling {member}'s ({member.id}) send message permissions to be reinstated.")
         self.schedule_task(member.id, TaskData(timeout, callback))
diff --git a/bot/constants.py b/bot/constants.py
index 2add028e7..49098c9f2 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -421,6 +421,7 @@ class Roles(metaclass=YAMLGetter):
     announcements: int
     contributors: int
     core_developers: int
+    help_cooldown: int
     helpers: int
     jammers: int
     moderators: int
diff --git a/config-default.yml b/config-default.yml
index f2b0bfa9f..b0165adf6 100644
--- a/config-default.yml
+++ b/config-default.yml
@@ -201,6 +201,7 @@ guild:
     roles:
         announcements:                          463658397560995840
         contributors:                           295488872404484098
+        help_cooldown:                          699189276025421825
         muted:              &MUTED_ROLE         277914926603829249
         partners:                               323426753857191936
         python_community:   &PY_COMMUNITY_ROLE  458226413825294336