-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(User): fix bot and system properties being incorrect in some cases #5923
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When patching an already cached user with partial data (happens in audit logs and wherever we'd get a partial user in the future), the bot
and system
property would be assigned false
.
Can be reproduced by the following steps:
- Enable the
USER
partial - Add/remove a role from/to a bot
- Fetch audit log
- Inspect
User#bot
of that bot and findfalse
Same thing should apply to system
, but I don't have a system user to assign a role to at hand 😄
The ideal behavior of structure._patch({})
for potentially partial structures would be no changes. (Although this is not yet the case for all instances, but we are slowly getting there.)
I tried to reproduce this and got the same behavior but that's because the user is not classified as a partial since it's username is a string (at least it was in my case). |
The previously mentioned issue should be fixed as I made it so that it takes both the received data and the existing data for the user into account. So if one isn't present, the other one will be kept instead of being overwritten with false for non-partial users. Please let me know if there are any other issues with this approach |
Please describe the changes this PR makes and why it should be merged:
Due to the system property only being present in user objects when the user is a system user, it would end up always being null instead of false like it should be. This PR fixes that so that it's true for system users, false for non-system users and null for partial users (regardless of whether they're system or not, since we don't know that)
Another issue in the User class was that the bot property would always be false for partial users due to the check
typeof this.bot !== 'boolean'
which would return true since this.bot was set to null previously. This fixes that so that it's null for partial users, true for bots and false for regular users.Status and versioning classification: