-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
refactor(http,model)!: refactor permission overwrites #1521
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR contains two changes. First, `InteractionResponse` (and `Attachment`) from `http` to a new `model` module, `model::http`. This is a first step in separating models that we send and that we receive from Discord over HTTP. Eventually, all models in `model::http` will exclusively be used when sending data to Discord. Second, `InteractionResponse` itself is no longer a complicated enum, and now simply serializes and deserializes just as Discord expects. This saves developer effort in writing and testing custom serde code, which allows for adding more fields much faster. Users will either need to manually create an `InteractionResponseData` themselves, or use the renamed `InteractionResponseDataBuilder` (formerly `CallbackDataBuilder`).
This PR does a few things. First, for our received `PermissionOverwrite` type, remove the custom de/serialization in favor of a model that more closely matches what Discord sends us. Second, add a `model::http` `PermissionOverwrite`, with the difference being `allow` and `deny` are now `Option`s. This type, while it does not directly match what we send Discord, simplifies the `http` API only requiring one request builder for `UpdateChannelPermission`.
github-actions
bot
added
c-cache
Affects the cache crate
c-http
Affects the http crate
c-model
Affects the model crate
c-util
Affects the util crate
m-breaking change
Breaks the public API.
t-refactor
Refactors APIs or code.
labels
Feb 8, 2022
Erk-
previously requested changes
Feb 12, 2022
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.
The changes look fine, just a small nit from me.
Though maybe we should add a example of a permission overwrite where cast
is used, as that may not be clear to everyone.
zeylahellyer
approved these changes
Feb 19, 2022
Erk-
previously requested changes
Feb 19, 2022
Co-authored-by: Erk <[email protected]>
7596ff
commented
Feb 19, 2022
Erk-
approved these changes
Feb 19, 2022
7596ff
added a commit
that referenced
this pull request
Mar 10, 2022
InteractionClient `InteractionClient` functions have been renamed with a consistent style ([#1433] - [@7596ff]): | New | Old | | ---------------------------- | ------------------------------- | | `create_response` | `interaction_callback` | | `delete_response` | `delete_interaction_original` | | `response` | `get_interaction_original` | | `update_response` | `update_interaction_original` | | `create_followup` | `create_followup_message` | | `delete_followup` | `delete_followup_message` | | `followup` | `followup_message` | | `update_followup` | `update_followup_message` | | `create_global_command` | unchanged | | `delete_global_command` | unchanged | | `global_command` | `get_global_command` | | `global_commands` | `get_global_commands` | | `set_global_commands` | unchanged | | `update_global_command` | unchanged | | `create_guild_command` | unchanged | | `delete_guild_command` | unchanged | | `guild_command` | `get_guild_command` | | `guild_commands` | `get_guild_commands` | | `set_guild_commands` | unchanged | | `update_guild_command` | unchanged | | `command_permissions` | `get_command_permissions` | | `guild_command_permissions` | `get_guild_command_permissions` | | `set_command_permissions` | unchanged | | `update_command_permissions` | unchanged | Respective request builders have been renamed to match. Sending Messages Sending messages has been refactored and made consistent across all methods ([#1354] - [@7596ff]). This is a full log of all message-sending related changes, with [#1354] being the majority of the changes, and other PRs being noted. `AttachmentFile` has been renamed to `model::http::Attachment`, and now holds owned values ([#1508] - [@7596ff]). `Client::default_allowed_mentions` now returns a reference instead of cloning. Documentation for request builder methods have been updated and made consistent with each other. `CreateMessage` - `allowed_mentions` is actually used in `try_into_request` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` - rename `stickers` to `sticker_ids` - `attachments` now validates filenames ([#1530] - [@7596ff]) `UpdateMessage` - `allowed_mentions` is actually used in `try_into_request` - `allowed_mentions` now takes a reference, and is nullable - `attachments` now takes a list of `http::attachment::Attachment`s - `attachments` now validates filenames ([#1530] - [@7596ff]) - `embeds` now accepts an `Option` instead of a slice - add `payload_json` `ExecuteWebhook`: - `allowed_mentions` is actually used in `try_into_request` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` - `attachments` now validates filenames ([#1530] - [@7596ff]) - refactored `wait` to use a field on the request itself instead of calling a `request` method `UpdateWebhookMessage` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` and `keep_attachment_ids` - `attachments` now validates filenames ([#1530] - [@7596ff]) - no longer auditable `CreateResponse` - now takes `model::http::InteractionResponse` ([#1508] - [@7596ff]) - send attachments using a form rather than JSON ([#1509] - [@7596ff]) `UpdateResponse` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` and `keep_attachment_ids` - `attachments` now validates filenames ([#1530] - [@7596ff]) `CreateFollowup` - `allowed_mentions` is actually used in `try_into_request` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` - `attachments` now validates filenames ([#1530] - [@7596ff]) `UpdateFollowup` - `allowed_mentions` now takes a reference, and is nullable - `attach` and `files` have been replaced with `attachments` and `keep_attachment_ids` - `attachments` now validates filenames ([#1530] - [@7596ff]) Changes Many integer sizes used for functions such as `CreateInvite::max_age` have been reduced to `u32`s or `u16`s based on their documented maximum value ([#1505] - [@laralove143]). `Client::update_channel_permissions` now takes a new type, `model::http::PermissionOverwrite`, instead of involving a more complicated request builder ([#1521] - [@7596ff]). This removes `UpdateChannelPermissionConfigured`. `AuditLogReason::reason` is now validated with `twilight_validate`, and returns a `ValidationError` ([#1527] - [@7596ff]). `AuditLogReasonError` has been removed. Update to Discord API version 10 ([#1540] - [@zeylahellyer]). This involves two changes: - remove `Route::CreateBan`'s `reason` struct field - update `CreateBan` to specify the reason in the headers, not URL [#1354]: #1354 [#1433]: #1433 [#1508]: #1508 [#1509]: #1508 [#1521]: #1521 [#1540]: #1540 [@7596ff]: https://github.com/7596ff [@zeylahellyer]: https://github.com/zeylahellyer
7596ff
added a commit
that referenced
this pull request
Mar 10, 2022
Channels The `Channel` type has been unified into a struct ([#1449] - [@zeylahellyer], [@itohatweb]). All possible fields of every channel variant and thread variant are now present on this type. This change was prompted by Discord's own storage of channels, and that variants do not necessarily have guaranteed fields. See the PR description for more details. New `http` module Add a new module, `http` ([#1508], [#1521] - [@7596ff]). This module contains types that are only sent to Discord. `AttachmentFile` has been moved from `twilight-http` and renamed to `model::http::attachment::Attachment`. `InteractionResponse` has been moved to `model::http::interaction::InteractionResponse`. `CallbackData` has been renamed to `InteractionResponseData`. `PermissionOverwrite` now has a separate type in `model::http`; it differs from a received `PermissionOverwrite` in that its `allow` and `deny` fields are optional. Additions Add support for modals ([#1300] - [@itohatweb], [@7596ff]): - Sending - add a new component, `TextInput` - move `Component` de/serialization to the enum itself, and remove all de/serialization from its variants - Receiving - add `Interaction::Modal`, `InteractionType::ModalSubmit` - add `ModalSubmitInteraction`, `ModalInteractionData`, `ModalInteractionDataActionRow`, `ModalInteractionDataComponent` Add `GuildStickersUpdate` to the `Event` enum ([#1520] - [@HTG-YT]). Changes `Event` variants have been boxed or unboxed based on a new threshold, making the size of the enum more consistent ([#1436] - [@vilgotf]). Rename `Intents::GUILD_EMOJIS` to `GUILD_EMOJIS_AND_STICKERS` ([#1520] - [@HTG-YT]). `PermissionOverwrite` has been refactored to more closely represent Discord's model ([#1521] - [@7596ff]). Its ID is stored with a generic marker, and can be casted to a member or role ID as needed. Update to Discord API version 10 ([#1540] - [@zeylahellyer]). This involves two changes: - remove `CurrentApplicationInfo`'s `summary` field - add `Intents::MESSAGE_CONTENT` [#1300]: #1300 [#1436]: #1436 [#1449]: #1449 [#1508]: #1508 [#1520]: #1520 [#1521]: #1521 [#1540]: #1540 [@7596ff]: https://github.com/7596ff [@HTG-YT]: https://github.com/HTG-YT [@itohatweb]: https://github.com/itohatweb [@vilgotf]: https://github.com/vilgotf [@zeylahellyer]: https://github.com/zeylahellyer
7596ff
added a commit
that referenced
this pull request
Mar 10, 2022
Changes Rename `CallbackDataBuilder` to `InteractionResponseDataBuilder`, and add the methods `attachments`, `choices`, `custom_id`, and `title` ([#1300], [#1508] - [@itohatweb], [@7596ff]). Update the permission calculator to use the new `PermissionOverwrite` type ([#1521] - [@7596ff]). [#1300]: #1300 [#1508]: #1508 [#1521]: #1521 [@7596ff]: https://github.com/7596ff [@itohatweb]: https://github.com/itohatweb
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
c-cache
Affects the cache crate
c-http
Affects the http crate
c-model
Affects the model crate
c-util
Affects the util crate
m-breaking change
Breaks the public API.
t-refactor
Refactors APIs or code.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR does a few things.
First, for our received
PermissionOverwrite
type, remove the custom de/serialization in favor of a model that more closely matches what Discord sends us.Second, add a
model::http
PermissionOverwrite
, with the difference beingallow
anddeny
are nowOption
s. This type, while it does not directly match what we send Discord, simplifies thehttp
API only requiring one request builder forUpdateChannelPermission
.