Skip to content
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(Permissions): toArray shouldn't check admin #7144

Merged
merged 2 commits into from Dec 26, 2021

Conversation

DTrombett
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

When a member is an administrator, he does have all permissions but it's still possible to add/remove other permissions. Although it doesn't have any effect, the bitfield is still different and the toArray() method should return the permissions of the bitfield.
This can be done by passing false to the toArray() call.
This change also resolves #6267, which seems a regression added in #5911.

I've also considered defaulting the checkAdmin parameter to false in Permissions#toArray() but it would be inconsistent with other methods and useless as I don't see any use case where an array with all the flags is needed for an administrator and could be confused with a person with really every permission.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@iCrawl iCrawl added this to the Version 13.5 milestone Dec 24, 2021
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seriously need to make unit tests for this, it's a really troublesome class.

@kyranet kyranet requested a review from iCrawl December 25, 2021 16:14
@iCrawl iCrawl merged commit fc4292e into discordjs:main Dec 26, 2021
@DTrombett DTrombett deleted the fix/adminpermissions branch December 29, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Permissions#missing returns all permissions in v13
5 participants