From d9ed643c41c8cf96ec208d6fc096882fc64c5d15 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 21:06:39 -0700 Subject: ModLog: fix AttributeError in on_member_update `iterable_item_removed` and `iterable_item_added` lack `new_value` and `old_value`. Instead, they just contain the actual value added or removed. The code was incorrectly trying to access old and new values for the iterable changes. The iterable changes are only useful for the role diff, but they aren't even needed for that. The role diff calculation has been refactored to always get the diff rather than doing it only if it sees there has been a change to the `_roles` attribute. To be clear, `_roles` only has IDs, which is why its diff isn't that useful anyway. To use it, the code would have to get the Role objects, which is basically what the `roles` property already does. `_cs_roles` seems to be some Role object cache, but its reliability is unclear. --- bot/cogs/moderation/modlog.py | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 41472c64c..02396e1c5 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -452,6 +452,21 @@ class ModLog(Cog, name="ModLog"): channel_id=Channels.mod_log ) + @staticmethod + def get_role_diff(before: t.List[discord.Role], after: t.List[discord.Role]) -> t.List[str]: + """Return a list of strings describing the roles added and removed.""" + changes = [] + before_roles = set(before) + after_roles = set(after) + + for role in (before_roles - after_roles): + changes.append(f"**Role removed:** {role.name} (`{role.id}`)") + + for role in (after_roles - before_roles): + changes.append(f"**Role added:** {role.name} (`{role.id}`)") + + return changes + @Cog.listener() async def on_member_update(self, before: discord.Member, after: discord.Member) -> None: """Log member update event to user log.""" @@ -463,22 +478,18 @@ class ModLog(Cog, name="ModLog"): return diff = DeepDiff(before, after) - changes = [] + changes = self.get_role_diff(before.roles, after.roles) done = [] diff_values = {} diff_values.update(diff.get("values_changed", {})) diff_values.update(diff.get("type_changes", {})) - diff_values.update(diff.get("iterable_item_removed", {})) - diff_values.update(diff.get("iterable_item_added", {})) diff_user = DeepDiff(before._user, after._user) diff_values.update(diff_user.get("values_changed", {})) diff_values.update(diff_user.get("type_changes", {})) - diff_values.update(diff_user.get("iterable_item_removed", {})) - diff_values.update(diff_user.get("iterable_item_added", {})) for key, value in diff_values.items(): if not key: # Not sure why, but it happens @@ -495,24 +506,11 @@ class ModLog(Cog, name="ModLog"): if key in done or key in MEMBER_CHANGES_SUPPRESSED: continue - if key == "_roles": - new_roles = after.roles - old_roles = before.roles + new = value.get("new_value") + old = value.get("old_value") - for role in old_roles: - if role not in new_roles: - changes.append(f"**Role removed:** {role.name} (`{role.id}`)") - - for role in new_roles: - if role not in old_roles: - changes.append(f"**Role added:** {role.name} (`{role.id}`)") - - else: - new = value.get("new_value") - old = value.get("old_value") - - if new and old: - changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") + if new and old: + changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") done.append(key) -- cgit v1.2.3 From 9133c4a7b79020d507b9cecbb9ce6d957b52fd9d Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 21:57:53 -0700 Subject: ModLog: remove user diff in on_member_update The correct event for user changes is on_user_update, so this code does nothing in the on_member_update event. --- bot/cogs/moderation/modlog.py | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 02396e1c5..703da4ee7 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -24,7 +24,7 @@ GUILD_CHANNEL = t.Union[discord.CategoryChannel, discord.TextChannel, discord.Vo CHANNEL_CHANGES_UNSUPPORTED = ("permissions",) CHANNEL_CHANGES_SUPPRESSED = ("_overwrites", "position") -MEMBER_CHANGES_SUPPRESSED = ("status", "activities", "_client_status", "nick") +MEMBER_CHANGES_SUPPRESSED = ("status", "activities", "_client_status") ROLE_CHANGES_UNSUPPORTED = ("colour", "permissions") VOICE_STATE_ATTRIBUTES = { @@ -486,11 +486,6 @@ class ModLog(Cog, name="ModLog"): diff_values.update(diff.get("values_changed", {})) diff_values.update(diff.get("type_changes", {})) - diff_user = DeepDiff(before._user, after._user) - - diff_values.update(diff_user.get("values_changed", {})) - diff_values.update(diff_user.get("type_changes", {})) - for key, value in diff_values.items(): if not key: # Not sure why, but it happens continue @@ -514,21 +509,6 @@ class ModLog(Cog, name="ModLog"): done.append(key) - if before.name != after.name: - changes.append( - f"**Username:** `{before.name}` **→** `{after.name}`" - ) - - if before.discriminator != after.discriminator: - changes.append( - f"**Discriminator:** `{before.discriminator}` **→** `{after.discriminator}`" - ) - - if before.display_name != after.display_name: - changes.append( - f"**Display name:** `{before.display_name}` **→** `{after.display_name}`" - ) - if not changes: return -- cgit v1.2.3 From 17858e4d65d5592d1da6178cb80415de615f21ab Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 22:05:18 -0700 Subject: ModLog: fix excluded None values in on_member_update This was preventing diffs for added nicknames from showing, among other things. --- bot/cogs/moderation/modlog.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 703da4ee7..163721e1c 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -504,8 +504,7 @@ class ModLog(Cog, name="ModLog"): new = value.get("new_value") old = value.get("old_value") - if new and old: - changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") + changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") done.append(key) -- cgit v1.2.3 From 35fc846e671192199bde7e98e43b2ac21513f629 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 21:16:02 -0700 Subject: ModLog: refactor on_member_update * Exclude all sequences/mapping types rather than excluding by name * Replace MEMBER_CHANGES_SUPPRESSED with excludes as DeepDiff args * Don't keep track of "done" attributes - there shouldn't be dupes --- bot/cogs/moderation/modlog.py | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 163721e1c..bd805f590 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -24,7 +24,6 @@ GUILD_CHANNEL = t.Union[discord.CategoryChannel, discord.TextChannel, discord.Vo CHANNEL_CHANGES_UNSUPPORTED = ("permissions",) CHANNEL_CHANGES_SUPPRESSED = ("_overwrites", "position") -MEMBER_CHANGES_SUPPRESSED = ("status", "activities", "_client_status") ROLE_CHANGES_UNSUPPORTED = ("colour", "permissions") VOICE_STATE_ATTRIBUTES = { @@ -477,36 +476,27 @@ class ModLog(Cog, name="ModLog"): self._ignored[Event.member_update].remove(before.id) return - diff = DeepDiff(before, after) changes = self.get_role_diff(before.roles, after.roles) - done = [] - diff_values = {} + # The regex is a simple way to exclude all sequence and mapping types. + diff = DeepDiff(before, after, exclude_regex_paths=r".*\[.*") - diff_values.update(diff.get("values_changed", {})) - diff_values.update(diff.get("type_changes", {})) + # A type change seems to always take precedent over a value change. Furthermore, it will + # include the value change along with the type change anyway. Therefore, it's OK to + # "overwrite" values_changed; in practice there will never even be anything to overwrite. + diff_values = {**diff.get("values_changed", {}), **diff.get("type_changes", {})} - for key, value in diff_values.items(): - if not key: # Not sure why, but it happens + for attr, value in diff_values.items(): + if not attr: # Not sure why, but it happens. continue - key = key[5:] # Remove "root." prefix - - if "[" in key: - key = key.split("[", 1)[0] - - if "." in key: - key = key.split(".", 1)[0] - - if key in done or key in MEMBER_CHANGES_SUPPRESSED: - continue + attr = attr[len("root."):] # Remove "root." prefix. + attr = attr.replace("_", " ").replace(".", " ").capitalize() new = value.get("new_value") old = value.get("old_value") - changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") - - done.append(key) + changes.append(f"**{attr}:** `{old}` **→** `{new}`") if not changes: return @@ -520,8 +510,10 @@ class ModLog(Cog, name="ModLog"): message = f"**{member_str}** (`{after.id}`)\n{message}" await self.send_log_message( - Icons.user_update, Colour.blurple(), - "Member updated", message, + icon_url=Icons.user_update, + colour=Colour.blurple(), + title="Member updated", + text=message, thumbnail=after.avatar_url_as(static_format="png"), channel_id=Channels.user_log ) -- cgit v1.2.3 From b2972e0f816c60395517412011e312a3040491a0 Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 16 Jun 2020 13:12:58 -0700 Subject: Use int literal instead of len for slice Co-authored-by: Kieran Siek --- bot/cogs/moderation/modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index bd805f590..ffbb87bbe 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -490,7 +490,7 @@ class ModLog(Cog, name="ModLog"): if not attr: # Not sure why, but it happens. continue - attr = attr[len("root."):] # Remove "root." prefix. + attr = attr[5:] # Remove "root." prefix. attr = attr.replace("_", " ").replace(".", " ").capitalize() new = value.get("new_value") -- cgit v1.2.3