fix(ranks): More thorough role scan.

src/modules/ranks/cog.py

@@ -286,27 +286,78 @@ class RankCog(LionCog):
                     await task
 
     async def _role_check(self, session_rank: SeasonRank):
-        guild = self.bot.get_guild(session_rank.guildid)
+        """
-        member = guild.get_member(session_rank.userid)
+        Update the member's rank roles, if required.
-        crank = session_rank.current_rank
+        """
-        roleid = crank.roleid if crank else None
+        guildid = session_rank.guildid
-        last_roleid = session_rank.rankrow.last_roleid
+        guild = self.bot.get_guild(guildid)
-        if guild is not None and member is not None and roleid != last_roleid:
+
-            new_role = guild.get_role(roleid) if roleid else None
+        userid = session_rank.userid
-            last_role = guild.get_role(last_roleid) if last_roleid else None
+        member = guild.get_member(userid)
+
+        if guild is not None and member is not None and guild.me.guild_permissions.manage_roles:
+            ranks = await self.get_guild_ranks(guildid)
+
+            crank = session_rank.current_rank
+            current_roleid = crank.roleid if crank else None
+
+            # First gather rank roleids, note that the last_roleid is an 'honourary' roleid
+            last_roleid = session_rank.rankrow.last_roleid
+            rank_roleids = {rank.roleid for rank in ranks}
+            rank_roleids.add(last_roleid)
+
+            # Gather member roleids
+            mem_roleids = {role.id: role for role in member.roles}
+
+            # Calculate diffs
+            to_add = guild.get_role(current_roleid) if (current_roleid not in mem_roleids) else None
+            to_rm = [
+                role for roleid, role in mem_roleids.items()
+                if roleid in rank_roleids and roleid != current_roleid
+            ]
+
+            # Now update roles
             new_last_roleid = last_roleid
-            if guild.me.guild_permissions.manage_roles:
+
+            # TODO: Event log here, including errors
+            to_rm = [role for role in to_rm if role.is_assignable()]
+            if to_rm:
                 try:
-                    if last_role and last_role.is_assignable():
+                    await member.remove_roles(
-                        await member.remove_roles(last_role)
+                        *to_rm,
-                        new_last_roleid = None
+                        reason="Removing Old Rank Roles",
-                    if new_role and new_role.is_assignable():
+                        atomic=True
-                        await member.add_roles(new_role)
+                    )
-                        new_last_roleid = roleid
+                    roleids = ', '.join(str(role.id) for role in to_rm)
-                except discord.HTTPClient:
+                    logger.info(
-                    pass
+                        f"Removed old rank roles from <uid:{userid}> in <gid:{guildid}>: {roleids}"
-                if new_last_roleid != last_roleid:
+                    )
-                    await session_rank.rankrow.update(last_roleid=new_last_roleid)
+                    new_last_roleid = None
+                except discord.HTTPException:
+                    logger.warning(
+                        f"Unexpected error removing old rank roles from <uid:{member.id}> in <gid:{guild.id}>: {to_rm}",
+                        exc_info=True
+                    )
+
+            if to_add and to_add.is_assignable():
+                try:
+                    await member.add_roles(
+                        to_add,
+                        reason="Rewarding Activity Rank",
+                        atomic=True
+                    )
+                    logger.info(
+                        f"Rewarded rank role <rid:{to_add.id}> to <uid:{userid}> in <gid:{guildid}>."
+                    )
+                    new_last_roleid = to_add.id
+                except discord.HTTPException:
+                    logger.warning(
+                        f"Unexpected error giving <uid:{userid}> in <gid:{guildid}> their rank role <rid:{to_add.id}>",
+                        exc_info=True
+                    )
+
+            if new_last_roleid != last_roleid:
+                await session_rank.rankrow.update(last_roleid=new_last_roleid)
 
     @log_wrap(action="Update Rank")
     async def update_rank(self, session_rank):
@@ -336,23 +387,61 @@ class RankCog(LionCog):
         if member is None:
             return
 
-        new_role = guild.get_role(new_rank.roleid)
+        last_roleid = session_rank.rankrow.last_roleid
-        if last_roleid := session_rank.rankrow.last_roleid:
-            last_role = guild.get_role(last_roleid)
-        else:
-            last_role = None
 
+        # Update ranks
         if guild.me.guild_permissions.manage_roles:
-            try:
+            # First gather rank roleids, note that the last_roleid is an 'honourary' roleid
-                if last_role and last_role.is_assignable():
+            rank_roleids = {rank.roleid for rank in ranks}
-                    await member.remove_roles(last_role)
+            rank_roleids.add(last_roleid)
+
+            # Gather member roleids
+            mem_roleids = {role.id: role for role in member.roles}
+
+            # Calculate diffs
+            to_add = guild.get_role(new_rank.roleid) if (new_rank.roleid not in mem_roleids) else None
+            to_rm = [
+                role for roleid, role in mem_roleids.items()
+                if roleid in rank_roleids and roleid != new_rank.roleid
+            ]
+
+            # Now update roles
+            # TODO: Event log here, including errors
+            to_rm = [role for role in to_rm if role.is_assignable()]
+            if to_rm:
+                try:
+                    await member.remove_roles(
+                        *to_rm,
+                        reason="Removing Old Rank Roles",
+                        atomic=True
+                    )
+                    roleids = ', '.join(str(role.id) for role in to_rm)
+                    logger.info(
+                        f"Removed old rank roles from <uid:{userid}> in <gid:{guildid}>: {roleids}"
+                    )
                     last_roleid = None
-                if new_role and new_role.is_assignable():
+                except discord.HTTPException:
-                    await member.add_roles(new_role)
+                    logger.warning(
-                    last_roleid = new_role.id
+                        f"Unexpected error removing old rank roles from <uid:{member.id}> in <gid:{guild.id}>: {to_rm}",
-            except discord.HTTPException:
+                        exc_info=True
-                # TODO: Event log either way
+                    )
-                pass
+
+            if to_add and to_add.is_assignable():
+                try:
+                    await member.add_roles(
+                        to_add,
+                        reason="Rewarding Activity Rank",
+                        atomic=True
+                    )
+                    logger.info(
+                        f"Rewarded rank role <rid:{to_add.id}> to <uid:{userid}> in <gid:{guildid}>."
+                    )
+                    last_roleid=to_add.id
+                except discord.HTTPException:
+                    logger.warning(
+                        f"Unexpected error giving <uid:{userid}> in <gid:{guildid}> their rank role <rid:{to_add.id}>",
+                        exc_info=True
+                    )
 
         # Update MemberRank row
         column = {