Add the current nickname of a user in infraction search

[kraktus]

If the user has not changed its nickname, display None, otherwise display its current nickname.

closes #337

[MarkKoz]

The nickname is supposed to be for the user, not the actor. To be clear, the user is the one which received the infraction. The actor is the one who gave out the infraction. You can see that the actor is already mentioned so there's no need to show their nickname anyway.

Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.

[kraktus]

The nickname is supposed to be for the user, not the actor. To be clear, the user is the one which received the infraction. The actor is the one who gave out the infraction. You can see that the actor is already mentioned so there's no need to show their nickname anyway.

Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.

Ok, because I muted myself during tests I didn't see the difference

[kraktus]

I think using the <@id> is the best method because it always works and resolves and the issues raised.

Also I am pretty sure that actor.mention relies on it.

[jchristgit]

Disagree about <@id> being the best method. It doesn't always work, as Discord's clients will often only load online users into the cache, which means that these will display as something like the following:

This is especially true on mobile.

[kraktus]

Disagree about <@id> being the best method. It doesn't always work, as Discord's clients will often only load online users into the cache, which means that these will display as something like the following:

This is especially true on mobile.

Yes I thought that even if the user was not in the cache, clicking on the mention would load it, unfortunately it's not true. I will switch to fetch_user instead, even if I would have preferred not to relie on the API.

[kraktus]

After some more tests, to use fetch_user I have to turn infraction_to_string into a async function which tons of errors. Do you have any idea to resolve them?

[kraktus]

I don't really understand why actor is searched in a different way than user, so I leaved it the way it was, but instead of doing this:

        actor_id = infraction["actor"]
        guild = self.bot.get_guild(constants.Guild.id)
        actor = guild.get_member(actor_id)

I think this would be better:

       actor_id = infraction["actor"]
       actor = self.bot.get_user(actor_id)

I would be also more consistent.

@jchristgit no more use of <@id>

[jchristgit]

@jchristgit no more use of <@id>

Using .mention is the same as building <@id> yourself though: https://github.com/Rapptz/discord.py/blob/c7a1f5e6e9f18fff3128d67b6d098a4ab842fcb7/discord/user.py#L204-L207

The only exception is that you're also fetching the user now which can make this command very slow due to the rate limiting that endpoint has set. get_user loads from cache.

I think the most simple approach to this is to:

• Leave the current display of user#discrim as-is
• Move the get_user call before the string
• If the user's nickname != the user's username, display something like "{user} (nickname '{user}')" instead of the current "{user}" setup

[kraktus]

Using .mention is the same as building <@id> yourself though: https://github.com/Rapptz/discord.py/blob/c7a1f5e6e9f18fff3128d67b6d098a4ab842fcb7/discord/user.py#L204-L207

Damn, how could I forget? I said it myself...

Also I am pretty sure that actor.mention relies on it.

The only exception is that you're also fetching the user now which can make this command very slow due to the rate limiting that endpoint has set. get_user loads from cache.

Fetching only if the user is not in the cache

If the user's nickname != the user's username, display something like "{user} (nickname '{user}')" instead of the current "{user}" setup

Well that's not what MarkKoz says:

Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.

I agree with him because adding the nickname to the username will longer too much the line

Move the get_user call before the string

Already the case I think?

[kraktus]

Simplest way to do this I think is not to worry about mentions at all, and display the infos as raw text. I will modify it tonight.

[SebastiaanZ]

Hey, @kraktus, I was wondering, are you also a member of our Discord community? What's your handle on there?

[kraktus]

@SebastiaanZ I changed it to take the same username as here: kraktus :)

[MarkKoz]

Well that's not what MarkKoz says:

Also, I think it'd be better to replace the user's name with a nickname if it's available rather than adding it on a separate line.

I agree with him because adding the nickname to the username will longer too much the line

Well, depends on what the staff deems to be useful information. I was thinking both the nickname and discriminator are not needed because the ID will always be shown anyway.

[MarkKoz]

Sorry for taking so long to get back to you.

It is not adequate to simply log the exception and proceed because the object will still be None. Furthermore, the way nicknames are currently being fetched may raise AttributeError because the user or actor could be a discord.User instead of discord.Member at those points.

That being said, I've actually remembered that the infractions API has "extended" endpoints which are useful here because they will return full user information along with infractions (as opposed to just user IDs). The user information in the database is kept in sync. I believe the only case of it being out of sync would be the username (not the nickname) of a user who is not in the guild. This is because the bot's syncer cog cannot update the username until the user re-joins the guild. However, I do not think that is relevant since in all such cases the nickname would be available anyway and we prefer nicknames.

Using the get and fetch functions was getting too complicated; the extended endpoint seems like the better, cleaner approach. It should be a utility function in moderation/utils.py because it will get used twice here (for the user and actor) and will in the future be used elsewhere in the moderation cogs.

[kraktus]

@MarkKoz I honestly think that's out of reach for me, because I don't understand all the Api-related code of the repo and I've no idea where to begin to implement the requested function in utils.py.

[MarkKoz]

@kraktus Are you still interested in trying? We can help you through it on Discord as needed. For starters, you could take a look at other code in the bot to see how requests to the API are made.

[kraktus]

Unfortunately I won't have time, and after 7 commits didn't progress at all on the objective, I'm closing this PR.