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 granting / denying permissions #769

Merged

Conversation

schnapster
Copy link
Contributor

Resolves #767, please see that issue for details.

@schnapster
Copy link
Contributor Author

PermissionOverride po = tc.createPermissionOverride(role).complete();
//wait
print(po.getAllowedRaw() + " " + po.getDeniedRaw());
po.getManager().grant(Permission.MESSAGE_ATTACH_FILES.getRawValue()).complete();
//wait
print(po.getAllowedRaw() + " " + po.getDeniedRaw());
po.getManager().deny(Permission.MESSAGE_ATTACH_FILES.getRawValue()).complete();
//wait
print(po.getAllowedRaw() + " " + po.getDeniedRaw());
po.getManager().grant(Permission.MESSAGE_ATTACH_FILES.getRawValue()).complete();
//wait
print(po.getAllowedRaw() + " " + po.getDeniedRaw());
po.getManager().clear(Permission.MESSAGE_ATTACH_FILES.getRawValue()).complete();
//wait
print(po.getAllowedRaw() + " " + po.getDeniedRaw());

results in

0 0
32768 0
0 32768
32768 0
0 0

which looks correct to me.

{
this.allowed |= permissions;
this.set |= ALLOWED;
}
Copy link
Member

Choose a reason for hiding this comment

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

So what this code now does: if none of the granted permissions are currently granted we set them.
This is really not how this is supposed to go, we want to update if any of the permissions are newly granted.

My suggestion was to only change the this.set |= ALLOWED.

Copy link
Member

@MinnDevelopment MinnDevelopment left a comment

Choose a reason for hiding this comment

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

lgtm

@MinnDevelopment MinnDevelopment added bug status: completed has been completed but is not yet released labels Sep 9, 2018
@MinnDevelopment MinnDevelopment merged commit 7b3e9dc into discord-jda:development Sep 9, 2018
@schnapster schnapster deleted the fix-permission-override branch September 9, 2018 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: completed has been completed but is not yet released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants