Repository navigation
Staff role access (admin except bot invite) - #16
Conversation
- Treat Staff role as admin-equivalent for server management, but keep bot invites admin-only - Add actual admin check for invites and staff-aware admin checks elsewhere - Add tests covering Staff role access, admin-only invites, and no-access cases
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new "Staff" role that grants users access to most admin functions except bot invitations. The change adds a distinction between general admin access (including Staff role holders) and actual Discord admin permissions (ADMINISTRATOR or MANAGE_SERVER only).
Changes:
- Modified
isUserAdminInGuildto check for Staff role when Discord admin permissions are absent - Added
isUserActualAdminInGuildmethod to verify actual Discord admin permissions without Staff role consideration - Updated bot invite endpoint to use the stricter admin check, keeping bot invitations restricted to actual admins
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/com/discordbot/web/service/AdminService.java | Added Staff role checking logic with new hasStaffRole helper method and isUserActualAdminInGuild method for restricted operations |
| src/main/java/com/discordbot/web/controller/ServerController.java | Updated bot invite endpoint to use stricter isUserActualAdminInGuild check instead of general admin check |
| src/test/java/com/discordbot/ServerControllerTest.java | Updated bot invite tests to use the new isUserActualAdminInGuild method |
| src/test/java/com/discordbot/AdminServiceBranchesTest.java | Added comprehensive tests for Staff role behavior, actual admin checks, and negative cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wraithfive
left a comment
wraithfive
left a comment
There was a problem hiding this comment.
Addressed feedback: added Staff-role mocks to the negative actual-admin test, added a parse-guard test for non-numeric permissions, and hardened permission parsing with primitive long + null/format handling.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Check if user has actual ADMINISTRATOR or MANAGE_SERVER permissions (no Staff role) | ||
| * Use this for restricted actions like bot invites | ||
| */ | ||
| public boolean isUserActualAdminInGuild(Authentication authentication, String guildId) { |
There was a problem hiding this comment.
The naming of isUserActualAdminInGuild could be clearer. The term "actual" is somewhat ambiguous - it might be more explicit to name this method something like hasDiscordAdminPermissions or isUserDiscordAdminInGuild to clearly indicate that it only checks Discord permissions and excludes role-based access.
| } | ||
|
|
||
| return member.getRoles().stream() | ||
| .anyMatch(role -> role.getName().equalsIgnoreCase("Staff")); |
There was a problem hiding this comment.
The documentation mentions the "Staff" role specifically (with capital S), but the implementation uses case-insensitive matching via equalsIgnoreCase("Staff"). Consider documenting this case-insensitive behavior in the README.md and SECURITY.md, or alternatively, use exact case matching if the role name should be case-sensitive.
| .anyMatch(role -> role.getName().equalsIgnoreCase("Staff")); | |
| .anyMatch(role -> role.getName().equals("Staff")); |
Summary
Testing
Notes