feat(model, http): Add support for guild onboarding #2130
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
github-actions
bot
added
c-http
zeylahellyer
added
d-api
vilgotf
requested changes
Feb 10, 2023
|
Looks good, I'll re-review this once it lands (as there will likely be changes) |
zeylahellyer
added
w-do-not-merge
…2132) Remove all of the deprecated APIs: - `twilight_gateway::EventTypeFlags::GUILD_BANS` - `exec` on all `twilight_http` request builders - `twilight_http::response::StatusCode::raw` - `twilight_model::gateway::Intents::GUILD_BANS` - `twilight_model::guild::GuildFeature::Commerce` - `twilight_model::guild::GuildFeature::MonetizationEnabled` - `twilight_model::oauth::current_application_info` - `twilight_model::user::UserFlags::CERTIFIED_MODERATOR`
The `http` depednency was exposed in `twilight-http`'s `Method`, used when sending requests. `hyper` (the underlying HTTP crate), however, accepts string representations of HTTP methods as well, so this switches to using that instead. Closes twilight-rs#1456.
Save a bit of space on gateway REST API models by reducing shard ID and session concurrency and total integer sizes.
Remove the `twilight_model::user::UserProfile` type because it isn't used anywhere and there doesn't appear to be a history of it being used. This appears to have been added, likely mistakenly, in the initial model commit years ago.
A common complaint of users is that HTTP request builder validation is too verbose. Every field of a request builder that does validation returns a `Result`. This is verbose at best, a point of contention in the middle, and a frustration at worst. It means every HTTP call has to be wrapped in order to use `?`, or errors have to be handled locally. Additionally, this is a footgun for Twilight development: if a field has validation added, then it's a breaking change. This encourages us to make every field return a Result, which is even worse for end users. The solution here is to report errors when the request builder has finalized.
We can achieve this by refactoring HTTP request builders to internally wrap the set of fields in a `Result`. Request builder field methods can simply mutate the fields if they aren't wrapped in an error, or make use of `Result::and_then` when performing validation. The builder can then simply return itself instead of wrapping itself in a `Result`.
These errors are realized by end users when calling `IntoFuture::into_future` or `TryIntoRequest::try_into_request` on request builders when they're done building the request. When a validation error occurred during request building, the resulting `ResponseFuture` will resolve to an `Error` of a new error type, `Validation`.
Use Case
People want to be able to create typed HTTP requests with ease. At the same time, these requests should be valid so users aren't sending invalid requests, unnecessarily eating into their global request ratelimit and providing slow responses for users of their application that something went wrong. People can create custom request bodies and they can do their best to make sure they've got it right, but we provide builders for them, and these builders automatically do validation on inputs. At the same time, each field input returns a Result, since something about the field can be wrong.
This gets unruly at scale: do you handle every Result in place? Do you wrap HTTP builder construction so you can `?` your problems away? Or do you take the easy way out and `.unwrap()` it all? What people go with is probably a mixture of all of these, but it's not a great experience in any case. Users already handle a `Result` as the `Output` of the `ResponseFuture`, so they can now also handle another case for validation! This wraps up validation errors with other types of errors nicely.
Technical Implementation
Request builders often store a set of fields, which can contain request body parameters or query parameters
```rust
pub struct CreateMessage<'a> {
attachment_manager: AttachmentManager<'a>,
channel_id: Id<ChannelMarker>,
fields: CreateMessageFields<'a>,
http: &'a Client,
}
```
The only change here is we wrap the fields in a `Result`:
```diff
pub struct CreateMessage<'a> {
attachment_manager: AttachmentManager<'a>,
channel_id: Id<ChannelMarker>,
- fields: CreateMessageFields<'a>,
+ fields: Result<CreateMessageFields<'a>, MessageValidationError>,
http: &'a Client,
}
```
Methods that don't validate fields can't just mutate the fields directly, so now they only do so if the fields haven't errored:
```diff
pub fn allowed_mentions(mut self, allowed_mentions: Option<&'a AllowedMentions>) -> Self {
- fields.allowed_mentions = Some(Nullable(allowed_mentions));
+ if let Ok(fields) = self.fields.as_mut() {
+ fields.allowed_mentions = Some(Nullable(allowed_mentions));
+ }
self
}
```
Meanwhile, methods that *do* validate their input use `Result::and_then` so that validation can be cleanly done:
```diff
-pub fn embeds(mut self, embeds: &'a [Embed]) -> Result<Self, MessageValidationError> {
+pub fn embeds(mut self, embeds: &'a [Embed]) -> Self {
- validate_embeds(embeds)?;
-
- self.fields.embeds = Some(embeds);
+ self.fields = self.fields.and_then(|mut fields| {
+ validate_embeds(embeds)?;
+ fields.embeds = Some(embeds);
+
+ Ok(fields)
+ });
- Ok(self)
+ self
}
```
Audit log reasons have been similarly changed:
```diff
pub trait AuditLogReason<'a>: private::Sealed {
- fn reason(self, reason: &'a str) -> Result<Self, ValidationError>
+ fn reason(self, reason: &'a str) -> Self
where
Self: Sized;
}
```
```diff
impl<'a> AuditLogReason<'a> for DeleteMessage<'a> {
- fn reason(mut self, reason: &'a str) -> Self {
- validate_audit_reason(reason)?;
-
- self.reason = Some(reason);
+ fn reason(mut self, reason: &'a str) -> Self {
+ self.reason = validate_audit_reason(reason).and(Ok(Some(reason)));
self
}
}
```
This is the heart of the changes made. Nothing Earth-shattering, it just had to be done.. for every. Request. We have. Along with removing now-extraneous `?`s on method calls in examples. So the diff is large.
Example
Before:
```rust
let response = client.create_message(channel_id)
.content("test")?
.embeds(&embeds)?
.tts(true)
.await?;
```
After:
```rust
let response = client.create_message(channel_id)
.content("test")
.embeds(&embeds)
.tts(true)
.await?;
```
Closes issue twilight-rs#1187.
|
Feature is live, could this be updated? |
Yeah, I'll try to finish it out this weekend. |
github-actions
bot
added
c-cache
laralove143
reviewed
Apr 25, 2023
github-actions
bot
added
c-gateway
github-actions
bot
removed
c-gateway-queue
suneettipirneni
removed
c-all
laralove143
previously requested changes
Apr 26, 2023
…eat/guild-onboarding
HTGAzureX1212
approved these changes
Jun 17, 2023
vilgotf
requested changes
Jun 22, 2023
…eat/guild-onboarding
vilgotf
approved these changes
Jul 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implements guild onboarding ref: discord/discord-api-docs#5915
This adds the following structures:
OnboardingOnboardingPromptOnboardingPromptOptionOnboardingPromptTypeAs well as supporting the http GET for retrieving guild onboarding information.
Closes #2134