Merge branch 'hotfix/0.5.8.H'

Author: JayDwee

koala/cogs/verification/cog.py

@@ -306,29 +306,22 @@ async def check_verifications(self, ctx):
     @commands.check(koalabot.is_admin)
     @commands.command(name="reVerify")
     @commands.check(verify_is_enabled)
-    async def re_verify(self, ctx, role):
+    async def re_verify(self, ctx, role: discord.Role):
         """
         Removes a role from all users who have it and marks them as needing to re-verify before giving it back
         :param ctx: the context of the discord message
         :param role: the role to be removed and re-verified (e.g. @students)
         :return:
         """
         with session_manager() as session:
-            try:
-                role_id = int(role[3:-1])
-            except ValueError:
-                raise self.InvalidArgumentError("Please give a role by @mentioning it")
-            except TypeError:
-                raise self.InvalidArgumentError("Please give a role by @mentioning it")
-
-            exists = session.execute(select(Roles).filter_by(s_id=ctx.guild.id, r_id=role_id)).all()
+            exists = session.execute(select(Roles).filter_by(s_id=ctx.guild.id, r_id=role.id)).all()
 
             if not exists:
                 raise self.VerifyError("Verification is not enabled for that role")
-            role = discord.utils.get(ctx.guild.roles, id=role_id)
-            for member in ctx.guild.members:
-                if role in member.roles:
-                    await member.remove_roles(role)
+            existing_reverify = session.execute(select(ToReVerify.u_id).filter_by(r_id=role.id)).scalars().all()
+            for member in role.members:
+                await member.remove_roles(role)
+                if member.id not in existing_reverify:
                     session.add(ToReVerify(u_id=member.id, r_id=role.id))
 
             session.commit()
@@ -373,7 +366,7 @@ async def assign_roles_for_user(self, user_id, email):
                     # bot not in guild
                     logger.error(e)
                 except discord.errors.NotFound:
-                    logger.error(f"user with id {user_id} not found")
+                    logger.warn(f"user with id {user_id} not found")
 
     async def remove_roles_for_user(self, user_id, email):
         with session_manager() as session:

koalabot.py

@@ -143,6 +143,12 @@ def check_guild_has_ext(ctx, extension_id):
 
 @bot.event
 async def on_command_error(ctx, error: Exception):
+    if ctx.guild is None:
+        guild_id = "UNKNOWN"
+        logger.warn("Unknown guild ID threw exception", exc_info=error)
+    else:
+        guild_id = ctx.guild.id
+
     if error.__class__ in [commands.MissingRequiredArgument,
                            commands.CommandNotFound]:
         await ctx.send(embed=error_embed(description=error))
@@ -156,10 +162,10 @@ async def on_command_error(ctx, error: Exception):
     elif isinstance(error, commands.errors.ChannelNotFound):
         await ctx.send(embed=error_embed(description=f"The channel ID provided is either invalid, or not in this server."))
     elif isinstance(error, commands.CommandInvokeError):
-        # logger.warning("CommandInvokeError(%s), guild_id: %s, message: %s", error.original, ctx.guild.id, ctx.message)
+        logger.error("CommandInvokeError(%s), guild_id: %s, message: %s", error.original, guild_id, ctx.message, exc_info=error)
         await ctx.send(embed=error_embed(description=error.original))
     else:
-        logger.error(f"Unexpected Error in guild %s : %s", ctx.guild.name, error, exc_info=error)
+        logger.error(f"Unexpected Error in guild %s : %s", guild_id, error, exc_info=error)
         await ctx.send(embed=error_embed(
             description=f"An unexpected error occurred, please contact an administrator Timestamp: {time.time()}")) # FIXME: better timestamp
         raise error

tests/cogs/verification/test_cog.py

@@ -10,8 +10,10 @@
 import asyncio
 
 # Libs
+import discord
 import discord.ext.test as dpytest
 import pytest
+import sqlalchemy.orm
 from discord.ext import commands
 from sqlalchemy import select, delete
 
@@ -226,3 +228,31 @@ async def test_re_verify():
         session.delete(test_role)
         session.execute(delete(ToReVerify).filter_by(u_id=member.id))
         session.commit()
+
+@pytest.mark.asyncio
+async def test_re_verify_duplicate():
+    with session_manager() as session:
+        test_config = dpytest.get_config()
+        guild = test_config.guilds[0]
+        role = dpytest.back.make_role("testRole", guild, id_num=555)
+        member = test_config.members[0]
+        await dpytest.add_role(member, role)
+        test_verified_email = VerifiedEmails(u_id=member.id, email='test@egg.com')
+        test_role = Roles(s_id=guild.id, r_id=role.id, email_suffix='egg.com')
+        test_re_verify = ToReVerify(u_id=member.id, r_id=role.id)
+        session.add(test_verified_email)
+        session.add(test_role)
+        session.add(test_re_verify)
+        session.commit()
+
+        await dpytest.message(koalabot.COMMAND_PREFIX + "reVerify <@&555>")
+        assert role not in member.roles
+        blacklisted = session.execute(select(ToReVerify).filter_by(u_id=member.id)).all()
+        assert blacklisted
+        assert dpytest.verify().message().content(
+            "That role has now been removed from all users and they will need to re-verify the associated email.")
+        session.delete(test_verified_email)
+        session.delete(test_role)
+        session.execute(delete(ToReVerify).filter_by(u_id=member.id))
+        session.commit()
+