(rroles): Expiry system and small bugfixes.

Completed the `duration` reaction role setting implementation.
Improved the `Duration` setting type format.
Moved reaction roles to their own module.
Various small bugfixes.

bot/modules/guild_admin/reaction_roles/__init__.py

@@ -1,3 +1,5 @@
+from .module import module
+
 from . import data
 from . import settings
 from . import tracker

bot/modules/guild_admin/reaction_roles/command.py

@@ -1,6 +1,5 @@
 import asyncio
 import discord
-from collections import defaultdict
 from discord import PartialEmoji
 
 from cmdClient.lib import ResponseTimedOut, UserCancelled
@@ -8,8 +7,7 @@ from wards import guild_admin
 from settings import UserInputError
 from utils.lib import tick, cross
 
-from ..module import module
+from .module import module
-
 from .tracker import ReactionRoleMessage
 from .data import reaction_role_reactions, reaction_role_messages
 from . import settings
@@ -176,20 +174,20 @@ _message_setting_flags = {
 }
 _reaction_setting_flags = {
     'price': settings.price,
-    'timeout': settings.timeout
+    'duration': settings.duration
 }
 
 
 @module.cmd(
     "reactionroles",
-    group="Guild Admin",
+    group="Guild Configuration",
-    desc="Create or configure reaction role messages.",
+    desc="Create and configure reaction role messages.",
     aliases=('rroles',),
     flags=(
         'delete', 'remove==',
         'enable', 'disable',
         'required_role==', 'removable=', 'maximum=', 'refunds=', 'log=', 'default_price=',
-        'price=', 'timeout=='
+        'price=', 'duration=='
     )
 )
 @guild_admin()
@@ -238,12 +236,13 @@ async def cmd_reactionroles(ctx, flags):
         required_role: The role required to use these reactions roles.
     Reaction Settings::
         price: The price of this reaction role.
-        timeout: The amount of time the role lasts. (TBD)
+        tduration: How long this role will last after being selected or bought.
     Configuration Examples``:
         {prefix}rroles {ctx.msg.id} --maximum 5
         {prefix}rroles {ctx.msg.id} --default_price 20
         {prefix}rroles {ctx.msg.id} --required_role None
         {prefix}rroles {ctx.msg.id} 🧮 --price 1024
+        {prefix}rroles {ctx.msg.id} 🧮 --duration 7 days
     """
     if not ctx.args:
         # No target message provided, list the current reaction messages
@@ -909,7 +908,7 @@ async def cmd_reactionroles(ctx, flags):
         "{settings_table}\n"
         "To update a message setting: `{prefix}rroles messageid --setting value`\n"
         "To update an emoji setting: `{prefix}rroles messageid emoji --setting value`\n"
-        "See more usage information and examples with `{prefix}help rroles`."
+        "See examples and more usage information with `{prefix}help rroles`."
     ).format(
         prefix=ctx.best_prefix,
         settings_table=target.settings.tabulated()

bot/modules/guild_admin/reaction_roles/data.py

@@ -1,4 +1,4 @@
-from data import RowTable
+from data import Table, RowTable
 
 
 reaction_role_messages = RowTable(
@@ -17,3 +17,6 @@ reaction_role_reactions = RowTable(
     ('reactionid', 'messageid', 'roleid', 'emoji_name', 'emoji_id', 'emoji_animated', 'price', 'timeout'),
     'reactionid'
 )
+
+
+reaction_role_expiring = Table('reaction_role_expiring')

bot/modules/guild_admin/reaction_roles/expiry.py

@@ -0,0 +1,172 @@
+import logging
+import traceback
+import asyncio
+import discord
+
+from meta import client
+from utils.lib import utc_now
+from settings import GuildSettings
+
+from .module import module
+from .data import reaction_role_expiring
+
+_expiring = {}
+_wakeup_event = asyncio.Event()
+
+
+# TODO: More efficient data structure for min optimisation, e.g. pre-sorted with bisection insert
+
+
+# Public expiry interface
+def schedule_expiry(guildid, userid, roleid, expiry, reactionid=None):
+    """
+    Schedule expiry of the given role for the given member at the given time.
+    This will also cancel any existing expiry for this member, role pair.
+    """
+    reaction_role_expiring.delete_where(
+        guildid=guildid,
+        userid=userid,
+        roleid=roleid,
+    )
+    reaction_role_expiring.insert(
+        guildid=guildid,
+        userid=userid,
+        roleid=roleid,
+        expiry=expiry,
+        reactionid=reactionid
+    )
+    key = (guildid, userid, roleid)
+    _expiring[key] = expiry.timestamp()
+    _wakeup_event.set()
+
+
+def cancel_expiry(key):
+    """
+    Cancel expiry for the given member and role, if it exists.
+    """
+    guildid, userid, roleid = key
+    reaction_role_expiring.delete_where(
+        guildid=guildid,
+        userid=userid,
+        roleid=roleid,
+    )
+    if _expiring.pop(key, None) is not None:
+        # Wakeup the expiry tracker for recalculation
+        _wakeup_event.set()
+
+
+def _next():
+    """
+    Calculate the next member, role pair to expire.
+    """
+    if _expiring:
+        key, _ = min(_expiring.items(), key=lambda pair: pair[1])
+        return key
+    else:
+        return None
+
+
+async def _expire(key):
+    """
+    Execute reaction role expiry for the given member and role.
+    This removes the role and logs the removal if applicable.
+    If the user is no longer in the guild, it removes the role from the persistent roles instead.
+    """
+    guildid, userid, roleid = key
+    guild = client.get_guild(guildid)
+    if guild:
+        role = guild.get_role(roleid)
+        if role:
+            member = guild.get_member(userid)
+            if member:
+                log = GuildSettings(guildid).event_log.log
+                if role in member.roles:
+                    # Remove role from member, and log if applicable
+                    try:
+                        await member.remove_roles(
+                            role,
+                            atomic=True,
+                            reason="Expiring temporary reaction role."
+                        )
+                    except discord.HTTPException:
+                        log(
+                            "Failed to remove expired reaction role {} from {}.".format(
+                                role.mention,
+                                member.mention
+                            ),
+                            colour=discord.Colour.red(),
+                            title="Could not remove expired Reaction Role!"
+                        )
+                    else:
+                        log(
+                            "Removing expired reaction role {} from {}.".format(
+                                role.mention,
+                                member.mention
+                            ),
+                            title="Reaction Role expired!"
+                        )
+            else:
+                # Remove role from stored persistent roles, if existent
+                client.data.past_member_roles.delete_where(
+                    guildid=guildid,
+                    userid=userid,
+                    roleid=roleid
+                )
+    reaction_role_expiring.delete_where(
+        guildid=guildid,
+        userid=userid,
+        roleid=roleid
+    )
+
+
+async def _expiry_tracker(client):
+    """
+    Track and launch role expiry.
+    """
+    while True:
+        try:
+            key = _next()
+            diff = utc_now().timestamp() - _expiring[key] if key else None
+            await asyncio.wait_for(_wakeup_event.wait(), timeout=diff)
+        except asyncio.TimeoutError:
+            # Timeout means next doesn't exist or is ready to expire
+            if key and key in _expiring and _expiring[key] <= utc_now().timestamp() + 1:
+                _expiring.pop(key)
+                asyncio.create_task(_expire(key))
+        except Exception:
+            # This should be impossible, but catch and log anyway
+            client.log(
+                "Exception occurred while tracking reaction role expiry. Exception traceback follows.\n{}".format(
+                    traceback.format_exc()
+                ),
+                context="REACTION_ROLE_EXPIRY",
+                level=logging.ERROR
+            )
+        else:
+            # Wakeup event means that we should recalculate next
+            _wakeup_event.clear()
+
+
+@module.launch_task
+async def launch_expiry_tracker(client):
+    """
+    Launch the role expiry tracker.
+    """
+    asyncio.create_task(_expiry_tracker(client))
+    client.log("Reaction role expiry tracker launched.", context="REACTION_ROLE_EXPIRY")
+
+
+@module.init_task
+def load_expiring_roles(client):
+    """
+    Initialise the expiring reaction role map, and attach it to the client.
+    """
+    rows = reaction_role_expiring.select_where()
+    _expiring.clear()
+    _expiring.update({(row['guildid'], row['userid'], row['roleid']): row['expiry'].timestamp() for row in rows})
+    client.objects['expiring_reaction_roles'] = _expiring
+    if _expiring:
+        client.log(
+            "Loaded {} expiring reaction roles.".format(len(_expiring)),
+            context="REACTION_ROLE_EXPIRY"
+        )

bot/modules/guild_admin/reaction_roles/module.py

@@ -0,0 +1,3 @@
+from LionModule import LionModule
+
+module = LionModule("Reaction_Roles")

bot/modules/guild_admin/reaction_roles/settings.py

@@ -223,11 +223,11 @@ class price(setting_types.Integer, ReactionSetting):
 
 
 @ReactionSettings.attach_setting
-class timeout(setting_types.Duration, ReactionSetting):
+class duration(setting_types.Duration, ReactionSetting):
-    attr_name = 'timeout'
+    attr_name = 'duration'
     _data_column = 'timeout'
 
-    display_name = "timeout"
+    display_name = "duration"
     desc = "How long this reaction role will last."
 
     long_desc = (
@@ -235,20 +235,22 @@ class timeout(setting_types.Duration, ReactionSetting):
         "Note that this does not affect existing members with the role, or existing expiries."
     )
 
-    _default_multiplier = 1
+    _default_multiplier = 3600
+    _show_days = True
+    _min = 600
 
     @classmethod
     def _format_data(cls, id, data, **kwargs):
         if data is None:
-            return "Never"
+            return "Permanent"
         else:
             return super()._format_data(id, data, **kwargs)
 
     @property
     def success_response(self):
         if self.value is not None:
-            return "{{reaction.emoji}} {{reaction.role.mention}} will timeout `{}` after selection.".format(
+            return "{{reaction.emoji}} {{reaction.role.mention}} will expire `{}` after selection.".format(
                 self.formatted
             )
         else:
-            return "{reaction.emoji} {reaction.role.mention} will never timeout after selection."
+            return "{reaction.emoji} {reaction.role.mention} will not expire."

bot/modules/guild_admin/reaction_roles/tracker.py

@@ -1,6 +1,7 @@
 import asyncio
 import logging
 import traceback
+import datetime
 from collections import defaultdict
 from typing import List, Mapping, Optional
 from cachetools import LFUCache
@@ -11,11 +12,13 @@ from discord import PartialEmoji
 from meta import client
 from core import Lion
 from data import Row
+from utils.lib import utc_now
 from settings import GuildSettings
 
 from ..module import module
-from .data import reaction_role_messages, reaction_role_reactions  # , reaction_role_expiry
+from .data import reaction_role_messages, reaction_role_reactions
 from .settings import RoleMessageSettings, ReactionSettings
+from .expiry import schedule_expiry, cancel_expiry
 
 
 class ReactionRoleReaction:
@@ -404,23 +407,30 @@ class ReactionRoleMessage:
                             except discord.HTTPException:
                                 pass
 
+                        # Schedule the expiry, if required
+                        duration = reaction.settings.duration.value
+                        if duration:
+                            expiry = utc_now() + datetime.timedelta(seconds=duration)
+                            schedule_expiry(self.guild.id, member.id, role.id, expiry, reaction.reactionid)
+                        else:
+                            expiry = None
+
                         # Log the role modification if required
                         if self.settings.log.value:
                             event_log.log(
                                 "Added [reaction role]({}) {} "
-                                "to {}{}.".format(
+                                "to {}{}.{}".format(
                                     self.message_link,
                                     role.mention,
                                     member.mention,
-                                    " for `{}` coins.".format(price) if price else ''
+                                    " for `{}` coins.".format(price) if price else '',
+                                    "\nThis role will expire at <t:{:.0f}>.".format(
+                                        expiry.timestamp()
+                                    ) if expiry else ''
                                 ),
                                 title="Reaction Role Added"
                             )
 
-                        # Start the timeout, if required
-                        # TODO: timeout system
-                        ...
-
     async def process_raw_reaction_remove(self, payload):
         """
         Process a general reaction remove payload.
@@ -519,18 +529,17 @@ class ReactionRoleMessage:
                                     "Removed [reaction role]({}) {} "
                                     "from {}.".format(
                                         self.message_link,
-                                        reaction.role.mention,
+                                        role.mention,
                                         member.mention
                                     ),
                                     title="Reaction Role Removed"
                                 )
 
-                            # TODO: Cancel timeout
+                            # Cancel any existing expiry
-                            ...
+                            cancel_expiry(self.guild.id, member.id, role.id)
 
 
 # TODO: Make all the embeds a bit nicer, and maybe make a consistent interface for them
-# We could use the reaction url as the author url! At least for the non-builtin emojis.
 # TODO: Handle RawMessageDelete event
 # TODO: Handle permission errors when fetching message in config
 

bot/modules/meta/help.py

@@ -10,7 +10,7 @@ from .module import module
 
 # Set the command groups to appear in the help
 group_hints = {
-    'Productivity': "*Various productivity tools.*",
+    'Productivity': "*Use these to help you stay focused and productive!*",
     'Statistics': "*StudyLion leaderboards and study statistics.*",
     'Economy': "*Buy, sell, and trade with your hard-earned coins!*",
     'Personal Settings': "*Tell me about yourself!*",
@@ -34,7 +34,7 @@ admin_group_order = (
 )
 
 bot_admin_group_order = (
-    ('Bot Admin', 'Guild Configuration', 'Moderation', 'Meta'),
+    ('Bot Admin', 'Guild Admin', 'Guild Configuration', 'Moderation', 'Meta'),
     ('Productivity', 'Statistics', 'Economy', 'Personal Settings')
 )
 

bot/settings/setting_types.py

@@ -624,6 +624,9 @@ class Duration(SettingType):
     # This is particularly useful since the duration parser will return 0 for most non-duration strings
     allow_zero = False
 
+    # Whether to show days on the output
+    _show_days = False
+
     @classmethod
     def _data_from_value(cls, id: int, value: Optional[bool], **kwargs):
         """
@@ -654,12 +657,22 @@ class Duration(SettingType):
             num = parse_dur(userstr)
 
         if num == 0 and not cls.allow_zero:
-            raise UserInputError("The provided duration cannot be `0`! (Please enter in the format `1d 2h 3m 4s`.)")
+            raise UserInputError(
+                "The provided duration cannot be `0`! (Please enter in the format `1d 2h 3m 4s`, or `None` to unset.)"
+            )
 
         if cls._max is not None and num > cls._max:
-            raise UserInputError("Duration cannot be longer than `{}`!".format(strfdur(cls._max)))
+            raise UserInputError(
+                "Duration cannot be longer than `{}`!".format(
+                    strfdur(cls._max, short=False, show_days=cls._show_days)
+                )
+            )
         if cls._min is not None and num < cls._min:
-            raise UserInputError("Duration connot be shorter than `{}`!".format(strfdur(cls._min)))
+            raise UserInputError(
+                "Duration connot be shorter than `{}`!".format(
+                    strfdur(cls._min, short=False, show_days=cls._show_days)
+                )
+            )
 
         return num
 
@@ -671,7 +684,7 @@ class Duration(SettingType):
         if data is None:
             return None
         else:
-            return "`{}`".format(strfdelta(datetime.timedelta(seconds=data)))
+            return "`{}`".format(strfdur(data, short=False, show_days=cls._show_days))
 
 
 class SettingList(SettingType):

bot/utils/lib.py

@@ -205,23 +205,35 @@ def parse_dur(time_str):
     return seconds
 
 
-def strfdur(duration):
+def strfdur(duration, short=True, show_days=False):
     """
     Convert a duration given in seconds to a number of hours, minutes, and seconds.
     """
+    days = duration // (3600 * 24) if show_days else 0
     hours = duration // 3600
+    if days:
+        hours %= 24
     minutes = duration // 60 % 60
     seconds = duration % 60
 
     parts = []
+    if days:
+        unit = 'd' if short else (' days' if days != 1 else ' day')
+        parts.append('{}{}'.format(days, unit))
     if hours:
-        parts.append('{}h'.format(hours))
+        unit = 'h' if short else (' hours' if hours != 1 else ' hour')
+        parts.append('{}{}'.format(hours, unit))
     if minutes:
-        parts.append('{}m'.format(minutes))
+        unit = 'm' if short else (' minutes' if minutes != 1 else ' minute')
+        parts.append('{}{}'.format(minutes, unit))
     if seconds or duration == 0:
-        parts.append('{}s'.format(seconds))
+        unit = 's' if short else (' seconds' if seconds != 1 else ' second')
+        parts.append('{}{}'.format(seconds, unit))
 
-    return ' '.join(parts)
+    if short:
+        return ' '.join(parts)
+    else:
+        return ', '.join(parts)
 
 
 def substitute_ranges(ranges_str, max_match=20, max_range=1000, separator=','):

data/schema.sql

@@ -501,14 +501,14 @@ CREATE TABLE reaction_role_reactions(
 );
 CREATE INDEX reaction_role_reaction_messages ON reaction_role_reactions (messageid);
 
-CREATE TABLE reaction_role_expiry(
+CREATE TABLE reaction_role_expiring(
   guildid BIGINT NOT NULL,
   userid BIGINT NOT NULL,
   roleid BIGINT NOT NULL,
   expiry TIMESTAMPTZ NOT NULL,
   reactionid INTEGER REFERENCES reaction_role_reactions (reactionid) ON DELETE SET NULL
 );
-CREATE INDEX reaction_role_expiry_members ON reaction_role_expiry (guildid, userid, roleid);
+CREATE UNIQUE INDEX reaction_role_expiry_members ON reaction_role_expiring (guildid, userid, roleid);
 
 -- }}}
 -- vim: set fdm=marker: