[Core] Multiple mod admin roles (#2783)

* Adds Schema versioning
  - Adds Migration tool
  - Adds tool to migrate to allow multiple admin and mod roles
  - Supports Multiple mod and admin roles

* Ensures migration is run prior to cog load and connection to discord

* Updates to not rely on singular mod/admin role id

* Update requires logic for multiple mod/admin roles

* Add new commands for managing mod/admin roles

* Feedback

Update strings
Update docstrings
Add aliases

* Use snowflakelist

* paginate

* Change variable name

* Fix mistake

* handle settings view fix

* Fix name error

* I'm bad at Ux

* style fix

redbot/__main__.py

@@ -107,8 +107,11 @@ def main():
     red = Red(
         cli_flags=cli_flags, description=description, dm_help=None, fetch_offline_members=True
     )
+    loop = asyncio.get_event_loop()
+    loop.run_until_complete(red.maybe_update_config())
     init_global_checks(red)
     init_events(red, cli_flags)
+
     red.add_cog(Core(red))
     red.add_cog(CogManagerUI())
     if cli_flags.dev:
@@ -117,7 +120,7 @@ def main():
     modlog._init()
     # noinspection PyProtectedMember
     bank._init()
-    loop = asyncio.get_event_loop()
+
     if os.name == "posix":
         loop.add_signal_handler(SIGTERM, lambda: asyncio.ensure_future(sigterm_handler(red, log)))
     tmp_data = {}

redbot/cogs/audio/audio.py

@@ -3051,34 +3051,29 @@ class Audio(commands.Cog):
             return await self._skip_action(ctx, skip_to_track)
 
     async def _can_instaskip(self, ctx, member):
-        mod_role = await ctx.bot.db.guild(ctx.guild).mod_role()
+
-        admin_role = await ctx.bot.db.guild(ctx.guild).admin_role()
         dj_enabled = await self.config.guild(ctx.guild).dj_enabled()
 
-        if dj_enabled:
+        if member.bot:
-            is_active_dj = await self._has_dj_role(ctx, member)
+            return True
-        else:
-            is_active_dj = False
-        is_owner = member.id == self.bot.owner_id
-        is_server_owner = member.id == ctx.guild.owner_id
-        is_coowner = any(x == member.id for x in self.bot._co_owners)
-        is_admin = (
-            discord.utils.get(ctx.guild.get_member(member.id).roles, id=admin_role) is not None
-        )
-        is_mod = discord.utils.get(ctx.guild.get_member(member.id).roles, id=mod_role) is not None
-        is_bot = member.bot is True
-        is_other_channel = await self._channel_check(ctx)
 
-        return (
+        if member.id == ctx.guild.owner_id:
-            is_active_dj
+            return True
-            or is_owner
+
-            or is_server_owner
+        if dj_enabled:
-            or is_coowner
+            if await self._has_dj_role(ctx, member):
-            or is_admin
+                return True
-            or is_mod
+
-            or is_bot
+        if await ctx.bot.is_owner(member):
-            or is_other_channel
+            return True
-        )
+
+        if await ctx.bot.is_mod(member):
+            return True
+
+        if await self._channel_check(ctx):
+            return True
+
+        return False
 
     async def _is_alone(self, ctx, member):
         try:

redbot/cogs/bank/bank.py

@@ -43,10 +43,14 @@ def check_global_setting_admin():
                 return False
             if await ctx.bot.is_owner(author):
                 return True
-            permissions = ctx.channel.permissions_for(author)
+            if author == ctx.guild.owner:
-            is_guild_owner = author == ctx.guild.owner
+                return True
-            admin_role = await ctx.bot.db.guild(ctx.guild).admin_role()
+            if ctx.channel.permissions_for(author).manage_guild:
-            return admin_role in author.roles or is_guild_owner or permissions.manage_guild
+                return True
+            admin_roles = set(await ctx.bot.db.guild(ctx.guild).admin_role())
+            for role in author.roles:
+                if role.id in admin_roles:
+                    return True
         else:
             return await ctx.bot.is_owner(author)
 

redbot/cogs/reports/reports.py

@@ -84,18 +84,14 @@ class Reports(commands.Cog):
             await ctx.send(_("Reporting is now disabled."))
 
     async def internal_filter(self, m: discord.Member, mod=False, perms=None):
-        ret = False
+        if perms and m.guild_permissions >= perms:
-        if mod:
+            return True
-            guild = m.guild
+        if mod and await self.bot.is_mod(m):
-            admin_role = guild.get_role(await self.bot.db.guild(guild).admin_role())
+            return True
-            mod_role = guild.get_role(await self.bot.db.guild(guild).mod_role())
-            ret |= any(r in m.roles for r in (mod_role, admin_role))
-        if perms:
-            ret |= m.guild_permissions >= perms
         # The following line is for consistency with how perms are handled
-        # in Red, though I'm not sure it makse sense to use here.
+        # in Red, though I'm not sure it makes sense to use here.
-        ret |= await self.bot.is_owner(m)
+        if await self.bot.is_owner(m):
-        return ret
+            return True
 
     async def discover_guild(
         self,

redbot/core/bot.py

@@ -67,14 +67,15 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):  # pylint: d
             api_tokens={},
             extra_owner_destinations=[],
             owner_opt_out_list=[],
+            schema_version=0,
         )
 
         self.db.register_guild(
             prefix=[],
             whitelist=[],
             blacklist=[],
-            admin_role=None,
+            admin_role=[],
-            mod_role=None,
+            mod_role=[],
             embeds=None,
             use_bot_color=False,
             fuzzy=False,
@@ -134,6 +135,38 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):  # pylint: d
 
         self._permissions_hooks: List[commands.CheckPredicate] = []
 
+    async def maybe_update_config(self):
+        """
+        This should be run prior to loading cogs or connecting to discord.
+        """
+        schema_version = await self.db.schema_version()
+
+        if schema_version == 0:
+            await self._schema_0_to_1()
+            schema_version += 1
+            await self.db.schema_version.set(schema_version)
+
+    async def _schema_0_to_1(self):
+        """
+        This contains the migration to allow multiple mod and multiple admin roles.
+        """
+
+        log.info("Begin updating guild configs to support multiple mod/admin roles")
+        all_guild_data = await self.db.all_guilds()
+        for guild_id, guild_data in all_guild_data.items():
+            guild_obj = discord.Object(id=guild_id)
+            mod_roles, admin_roles = [], []
+            maybe_mod_role_id = guild_data["mod_role"]
+            maybe_admin_role_id = guild_data["admin_role"]
+
+            if maybe_mod_role_id:
+                mod_roles.append(maybe_mod_role_id)
+                await self.db.guild(guild_obj).mod_role.set(mod_roles)
+            if maybe_admin_role_id:
+                admin_roles.append(maybe_admin_role_id)
+                await self.db.guild(guild_obj).admin_role.set(admin_roles)
+        log.info("Done updating guild configs to support multiple mod/admin roles")
+
     async def send_help_for(
         self, ctx: commands.Context, help_for: Union[commands.Command, commands.GroupMixin, str]
     ):
@@ -191,21 +224,25 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):  # pylint: d
 
     async def is_admin(self, member: discord.Member):
         """Checks if a member is an admin of their guild."""
-        admin_role = await self.db.guild(member.guild).admin_role()
         try:
-            if any(role.id == admin_role for role in member.roles):
+            member_snowflakes = member._roles  # DEP-WARN
-                return True
+            for snowflake in await self.db.guild(member.guild).admin_role():
+                if member_snowflakes.has(snowflake):  # Dep-WARN
+                    return True
         except AttributeError:  # someone passed a webhook to this
             pass
         return False
 
     async def is_mod(self, member: discord.Member):
         """Checks if a member is a mod or admin of their guild."""
-        mod_role = await self.db.guild(member.guild).mod_role()
-        admin_role = await self.db.guild(member.guild).admin_role()
         try:
-            if any(role.id in (mod_role, admin_role) for role in member.roles):
+            member_snowflakes = member._roles  # DEP-WARN
-                return True
+            for snowflake in await self.db.guild(member.guild).admin_role():
+                if member_snowflakes.has(snowflake):  # DEP-WARN
+                    return True
+            for snowflake in await self.db.guild(member.guild).mod_role():
+                if member_snowflakes.has(snowflake):  # DEP-WARN
+                    return True
         except AttributeError:  # someone passed a webhook to this
             pass
         return False

redbot/core/commands/requires.py

@@ -126,16 +126,14 @@ class PrivilegeLevel(enum.IntEnum):
         # The following is simply an optimised way to check if the user has the
         # admin or mod role.
         guild_settings = ctx.bot.db.guild(ctx.guild)
-        admin_role_id = await guild_settings.admin_role()
+
-        mod_role_id = await guild_settings.mod_role()
+        member_snowflakes = ctx.author._roles  # DEP-WARN
-        is_mod = False
+        for snowflake in await guild_settings.admin_role():
-        for role in ctx.author.roles:
+            if member_snowflakes.has(snowflake):  # DEP-WARN
-            if role.id == admin_role_id:
                 return cls.ADMIN
-            elif role.id == mod_role_id:
+        for snowflake in await guild_settings.mod_role():
-                is_mod = True
+            if member_snowflakes.has(snowflake):  # DEP-WARN
-        if is_mod:
+                return cls.MOD
-            return cls.MOD
 
         return cls.NONE
 

redbot/core/core_commands.py

@@ -33,7 +33,7 @@ from redbot.core import (
     i18n,
 )
 from .utils.predicates import MessagePredicate
-from .utils.chat_formatting import humanize_timedelta, pagify, box, inline
+from .utils.chat_formatting import humanize_timedelta, pagify, box, inline, humanize_list
 
 from .commands.requires import PrivilegeLevel
 
@@ -705,15 +705,17 @@ class Core(commands.Cog, CoreLogic):
         if ctx.invoked_subcommand is None:
             if ctx.guild:
                 guild = ctx.guild
-                admin_role = (
+                admin_role_ids = await ctx.bot.db.guild(ctx.guild).admin_role()
-                    guild.get_role(await ctx.bot.db.guild(ctx.guild).admin_role()) or "Not set"
+                admin_role_names = [r.name for r in guild.roles if r.id in admin_role_ids]
-                )
+                admin_roles_str = (
-                mod_role = (
+                    humanize_list(admin_role_names) if admin_role_names else "Not Set."
-                    guild.get_role(await ctx.bot.db.guild(ctx.guild).mod_role()) or "Not set"
                 )
+                mod_role_ids = await ctx.bot.db.guild(ctx.guild).mod_role()
+                mod_role_names = [r.name for r in guild.roles if r.id in mod_role_ids]
+                mod_roles_str = humanize_list(mod_role_names) if mod_role_names else "Not Set."
                 prefixes = await ctx.bot.db.guild(ctx.guild).prefix()
-                guild_settings = _("Admin role: {admin}\nMod role: {mod}\n").format(
+                guild_settings = _("Admin roles: {admin}\nMod roles: {mod}\n").format(
-                    admin=admin_role, mod=mod_role
+                    admin=admin_roles_str, mod=mod_roles_str
                 )
             else:
                 guild_settings = ""
@@ -734,23 +736,60 @@ class Core(commands.Cog, CoreLogic):
                 guild_settings=guild_settings,
                 locale=locale,
             )
-            await ctx.send(box(settings))
+            for page in pagify(settings):
+                await ctx.send(box(page))
 
     @_set.command()
     @checks.guildowner()
     @commands.guild_only()
-    async def adminrole(self, ctx: commands.Context, *, role: discord.Role):
+    async def addadminrole(self, ctx: commands.Context, *, role: discord.Role):
-        """Sets the admin role for this server"""
+        """
-        await ctx.bot.db.guild(ctx.guild).admin_role.set(role.id)
+        Adds an admin role for this guild.
-        await ctx.send(_("The admin role for this guild has been set."))
+        """
+        async with ctx.bot.db.guild(ctx.guild).admin_role() as roles:
+            if role.id in roles:
+                return await ctx.send(_("This role is already an admin role."))
+            roles.append(role.id)
+        await ctx.send(_("That role is now considered an admin role."))
 
     @_set.command()
     @checks.guildowner()
     @commands.guild_only()
-    async def modrole(self, ctx: commands.Context, *, role: discord.Role):
+    async def addmodrole(self, ctx: commands.Context, *, role: discord.Role):
-        """Sets the mod role for this server"""
+        """
-        await ctx.bot.db.guild(ctx.guild).mod_role.set(role.id)
+        Adds a mod role for this guild.
-        await ctx.send(_("The mod role for this guild has been set."))
+        """
+        async with ctx.bot.db.guild(ctx.guild).mod_role() as roles:
+            if role.id in roles:
+                return await ctx.send(_("This role is already a mod role."))
+            roles.append(role.id)
+        await ctx.send(_("That role is now considered a mod role."))
+
+    @_set.command(aliases=["remadmindrole", "deladminrole", "deleteadminrole"])
+    @checks.guildowner()
+    @commands.guild_only()
+    async def removeadminrole(self, ctx: commands.Context, *, role: discord.Role):
+        """
+        Removes an admin role for this guild.
+        """
+        async with ctx.bot.db.guild(ctx.guild).admin_role() as roles:
+            if role.id not in roles:
+                return await ctx.send(_("That role was not an admin role to begin with."))
+            roles.remove(role.id)
+        await ctx.send(_("That role is no longer considered an admin role."))
+
+    @_set.command(aliases=["remmodrole", "delmodrole", "deletemodrole"])
+    @checks.guildowner()
+    @commands.guild_only()
+    async def removemodrole(self, ctx: commands.Context, *, role: discord.Role):
+        """
+        Removes a mod role for this guild.
+        """
+        async with ctx.bot.db.guild(ctx.guild).mod_role() as roles:
+            if role.id not in roles:
+                return await ctx.send(_("That role was not a mod role to begin with."))
+            roles.remove(role.id)
+        await ctx.send(_("That role is no longer considered a mod role."))
 
     @_set.command(aliases=["usebotcolor"])
     @checks.guildowner()

redbot/core/utils/mod.py

@@ -123,29 +123,25 @@ async def is_mod_or_superior(
         If the wrong type of ``obj`` was passed.
 
     """
-    user = None
     if isinstance(obj, discord.Message):
         user = obj.author
     elif isinstance(obj, discord.Member):
         user = obj
     elif isinstance(obj, discord.Role):
-        pass
+        if obj.id in await bot.db.guild(obj.guild).mod_role():
+            return True
+        if obj.id in await bot.db.guild(obj.guild).admin_role():
+            return True
+        return False
     else:
         raise TypeError("Only messages, members or roles may be passed")
 
-    server = obj.guild
-    admin_role_id = await bot.db.guild(server).admin_role()
-    mod_role_id = await bot.db.guild(server).mod_role()
-
-    if isinstance(obj, discord.Role):
-        return obj.id in [admin_role_id, mod_role_id]
-
     if await bot.is_owner(user):
         return True
-    elif discord.utils.find(lambda r: r.id in (admin_role_id, mod_role_id), user.roles):
+    if await bot.is_mod(user):
         return True
-    else:
+
-        return False
+    return False
 
 
 def strfdelta(delta: timedelta):
@@ -208,27 +204,21 @@ async def is_admin_or_superior(
         If the wrong type of ``obj`` was passed.
 
     """
-    user = None
     if isinstance(obj, discord.Message):
         user = obj.author
     elif isinstance(obj, discord.Member):
         user = obj
     elif isinstance(obj, discord.Role):
-        pass
+        return obj.id in await bot.db.guild(obj.guild).admin_role()
     else:
         raise TypeError("Only messages, members or roles may be passed")
 
-    admin_role_id = await bot.db.guild(obj.guild).admin_role()
+    if await bot.is_owner(user):
-
-    if isinstance(obj, discord.Role):
-        return obj.id == admin_role_id
-
-    if user and await bot.is_owner(user):
         return True
-    elif discord.utils.get(user.roles, id=admin_role_id):
+    if await bot.is_admin(user):
         return True
-    else:
+
-        return False
+    return False
 
 
 async def check_permissions(ctx: "Context", perms: Dict[str, bool]) -> bool: