Merge #1608 - enforce one help channel per user

4 files changed, 23 insertions, 109 deletions

diff --git a/bot/constants.py b/bot/constants.py
index 885b5c822..ab55da482 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -602,7 +602,6 @@ class HelpChannels(metaclass=YAMLGetter):
     section = 'help_channels'
 
     enable: bool
-    claim_minutes: int
     cmd_whitelist: List[int]
     idle_minutes_claimant: int
     idle_minutes_others: int
diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py
index 262b18e16..5c410a0a1 100644
--- a/bot/exts/help_channels/_cog.py
+++ b/bot/exts/help_channels/_cog.py
@@ -12,7 +12,7 @@ from discord.ext import commands
 
 from bot import constants
 from bot.bot import Bot
-from bot.exts.help_channels import _caches, _channel, _cooldown, _message, _name, _stats
+from bot.exts.help_channels import _caches, _channel, _message, _name, _stats
 from bot.utils import channel as channel_utils, lock, scheduling
 
 log = logging.getLogger(__name__)
@@ -94,6 +94,24 @@ class HelpChannels(commands.Cog):
 
         self.scheduler.cancel_all()
 
+    async def _handle_role_change(self, member: discord.Member, coro: t.Callable[..., t.Coroutine]) -> None:
+        """
+        Change `member`'s cooldown role via awaiting `coro` and handle errors.
+
+        `coro` is intended to be `discord.Member.add_roles` or `discord.Member.remove_roles`.
+        """
+        try:
+            await coro(self.bot.get_guild(constants.Guild.id).get_role(constants.Roles.help_cooldown))
+        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}")
+
     @lock.lock_arg(NAMESPACE, "message", attrgetter("channel.id"))
     @lock.lock_arg(NAMESPACE, "message", attrgetter("author.id"))
     @lock.lock_arg(f"{NAMESPACE}.unclaim", "message", attrgetter("author.id"), wait=True)
@@ -106,9 +124,10 @@ class HelpChannels(commands.Cog):
         """
         log.info(f"Channel #{message.channel} was claimed by `{message.author.id}`.")
         await self.move_to_in_use(message.channel)
-        await _cooldown.revoke_send_permissions(message.author, self.scheduler)
+        await self._handle_role_change(message.author, message.author.add_roles)
 
         await _message.pin(message)
+
         try:
             await _message.dm_on_open(message)
         except Exception as e:
@@ -276,7 +295,6 @@ class HelpChannels(commands.Cog):
 
         log.trace("Initialising the cog.")
         await self.init_categories()
-        await _cooldown.check_cooldowns(self.scheduler)
 
         self.channel_queue = self.create_channel_queue()
         self.name_queue = _name.create_name_queue(
@@ -407,16 +425,11 @@ class HelpChannels(commands.Cog):
         """Actual implementation of `unclaim_channel`. See that for full documentation."""
         await _caches.claimants.delete(channel.id)
 
-        # Ignore missing tasks because a channel may still be dormant after the cooldown expires.
-        if claimant_id in self.scheduler:
-            self.scheduler.cancel(claimant_id)
-
         claimant = self.bot.get_guild(constants.Guild.id).get_member(claimant_id)
         if claimant is None:
             log.info(f"{claimant_id} left the guild during their help session; the cooldown role won't be removed")
-        elif not any(claimant.id == user_id for _, user_id in await _caches.claimants.items()):
-            # Remove the cooldown role if the claimant has no other channels left
-            await _cooldown.remove_cooldown_role(claimant)
+        else:
+            await self._handle_role_change(claimant, claimant.remove_roles)
 
         await _message.unpin(channel)
         await _stats.report_complete_session(channel.id, closed_on)
diff --git a/bot/exts/help_channels/_cooldown.py b/bot/exts/help_channels/_cooldown.py
deleted file mode 100644
index c5c39297f..000000000
--- a/bot/exts/help_channels/_cooldown.py
+++ /dev/null
@@ -1,95 +0,0 @@
-import logging
-from typing import Callable, Coroutine
-
-import discord
-
-import bot
-from bot import constants
-from bot.exts.help_channels import _caches, _channel
-from bot.utils.scheduling import Scheduler
-
-log = logging.getLogger(__name__)
-CoroutineFunc = Callable[..., Coroutine]
-
-
-async def add_cooldown_role(member: discord.Member) -> None:
-    """Add the help cooldown role to `member`."""
-    log.trace(f"Adding cooldown role for {member} ({member.id}).")
-    await _change_cooldown_role(member, member.add_roles)
-
-
-async def check_cooldowns(scheduler: Scheduler) -> None:
-    """Remove expired cooldowns and re-schedule active ones."""
-    log.trace("Checking all cooldowns to remove or re-schedule them.")
-    guild = bot.instance.get_guild(constants.Guild.id)
-    cooldown = constants.HelpChannels.claim_minutes * 60
-
-    for channel_id, member_id in await _caches.claimants.items():
-        member = guild.get_member(member_id)
-        if not member:
-            continue  # Member probably left the guild.
-
-        in_use_time = await _channel.get_in_use_time(channel_id)
-
-        if not in_use_time or in_use_time.seconds > cooldown:
-            # Remove the role if no claim time could be retrieved or if the cooldown expired.
-            # Since the channel is in the claimants cache, it is definitely strange for a time
-            # to not exist. However, it isn't a reason to keep the user stuck with a cooldown.
-            await remove_cooldown_role(member)
-        else:
-            # The member is still on a cooldown; re-schedule it for the remaining time.
-            delay = cooldown - in_use_time.seconds
-            scheduler.schedule_later(delay, member.id, remove_cooldown_role(member))
-
-
-async def remove_cooldown_role(member: discord.Member) -> None:
-    """Remove the help cooldown role from `member`."""
-    log.trace(f"Removing cooldown role for {member} ({member.id}).")
-    await _change_cooldown_role(member, member.remove_roles)
-
-
-async def revoke_send_permissions(member: discord.Member, scheduler: Scheduler) -> None:
-    """
-    Disallow `member` to send messages in the Available category for a certain time.
-
-    The time until permissions are reinstated can be configured with
-    `HelpChannels.claim_minutes`.
-    """
-    log.trace(
-        f"Revoking {member}'s ({member.id}) send message permissions in the Available category."
-    )
-
-    await 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).
-    if member.id in scheduler:
-        scheduler.cancel(member.id)
-
-    delay = constants.HelpChannels.claim_minutes * 60
-    scheduler.schedule_later(delay, member.id, remove_cooldown_role(member))
-
-
-async def _change_cooldown_role(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 = bot.instance.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
-
-    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}")
diff --git a/config-default.yml b/config-default.yml
index 8099a0860..55388247c 100644
--- a/config-default.yml
+++ b/config-default.yml
@@ -463,9 +463,6 @@ free:
 help_channels:
     enable: true
 
-    # Minimum interval before allowing a certain user to claim a new help channel
-    claim_minutes: 15
-
     # Roles which are allowed to use the command which makes channels dormant
     cmd_whitelist:
         - *HELPERS_ROLE