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

Overhaul command permission builders #2399

Merged
merged 1 commit into from
May 3, 2023

Conversation

kangalio
Copy link
Collaborator

@kangalio kangalio commented Apr 23, 2023

  • 🔤 Renames CreateCommandPermissionsData builder to EditCommandPermissions
    • Name is more fitting because the builder calls the edit_guild_command_permissions endpoint
  • 🔤 Renames CreateCommandPermissionData builder to CreateCommandPermission
  • 🔤 Renames CommandPermission model type to CommandPermissions
    • Name is more fitting because it actually stores a Vec of all permissions for a single command
  • 🔤 Renames CommandPermissionData to CommandPermission
  • 🔤 Renames [GuildId, Guild, PartialGuild]::create_command_permission to edit_command_permission
    • Less misleading - this method overwrites all permissions of the given command
    • Matches the HTTP endpoint name
  • ✨ Replaces EditCommandPermissions methods new(), add_permission(CreateCommandPermission), set_permissions(Vec<CreateCommandPermission>) with just new(Vec<CreateCommandPermission>)
    • Not providing any CreateCommandPermission instances makes no sense, and new(vec![a, b, c]) is easier for both serenity and the user than new().add_permission(a).add_permission(b).add_permission(c)
  • ✨ Replaces CreateCommandPermissionData methods new(), kind(CommandPermissionType), id(CommandPermissionId), permission(bool) with role(RoleId, bool), user(UserId, bool), channel(ChannelId, bool)
    • Much shorter to invoke
    • Cannot be misused (previously your configured kind could mismatch the type of ID you passed in)
    • Shows everything you can do with a command permission on one glance
  • ✨ Adds CreateCommandPermission::everyone(GuildId, bool) and CreateCommandPermission::all_channels(GuildId, bool)
    • These were previously missing
  • ❌ Removes edit_guild_commands_permissions endpoint
    • This endpoint has been disabled with Permissions V2 (Discord docs)
    • Don't confuse with edit_guild_command_permissions (note the plural) - that endpoint is still active!
  • 🔧 Replaces CreateCommandPermission's inner private fields by making CreateCommandPermission a newtype around CommandPermission
    • They share the same schema in the Discord docs so we can also share the same type struct definition

This PR has not been runtime tested

@github-actions github-actions bot added builder Related to the `builder` module. client Related to the `client` module. http Related to the `http` module. model Related to the `model` module. labels Apr 23, 2023
@kangalio
Copy link
Collaborator Author

kangalio commented May 1, 2023

Noticing the delay of merging: I made these changes because it makes more sense to do things this way imho - however, as always, I'm not sure if this gain for new code outweighs the churn for existing code. If others believe the breaking change of this PR is not worth it, I'm ok with not having this PR merged.

@arqunis arqunis added enhancement An improvement to Serenity. breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users labels May 3, 2023
@arqunis arqunis merged commit 891cd1b into serenity-rs:next May 3, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 7, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 7, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 7, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 7, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 18, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 19, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 21, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 3, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 3, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 26, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 26, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Aug 28, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Aug 28, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 6, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 6, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 23, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 23, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 1, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 1, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 19, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users builder Related to the `builder` module. client Related to the `client` module. enhancement An improvement to Serenity. http Related to the `http` module. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants