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

Implement audit_log_reason argument #2021

Closed
wants to merge 65 commits into from
Closed

Implement audit_log_reason argument #2021

wants to merge 65 commits into from

Conversation

SenseiHiraku
Copy link
Contributor

I've noticed, that adding a reason for the audit_log, as implemented in other Discord API libraries, is not yet fully implemented here.
It may be breaking change for many functions, but it's a necessary one in my eyes, basically an addition to #1354

nickelc and others added 30 commits June 24, 2022 16:30
The audit log types are no longer re-exported in the `model::guild` module.

BREAKING CHANGES:
- The `audit_log::Action*` enum sub types are renamed to `*Action`.
  For instance, `ActionChannel` to `ChannelAction`.

- The audit log types must be imported directly from the
  `model::guild::audit_log` module.
…1893)

The newtype struct `gateway::WsClient` hides the WebSocket
implementation details and is the first step to remove the
`async_tungstenite` & `tungstenite` types from the public API.

The `WebSocketGatewayClientExt`, `ReceiverExt` and `SenderExt` trait
methods are merged into the implementation of the newtype struct.

BREAKING CHANGES:
- `gateway::WsStream` is replaced by `gateway::WsClient`.
- The `gateway::WebSocketGatewayClientExt` trait is removed.
Since `&` is Copy, it’s useless to have a reference on `Option<&'a [u8]>`.
This changes the return type of `http::Typing::stop` to be a simple
`bool` rather than `Option<()>`, which doesn't provide useful
information that couldn't be expressed with just a `bool`.

Co-authored-by: Michael Krasnitski <42564254+mkrasnitski@users.noreply.github.com>
Signed-off-by: Miezhiko <Miezhiko@gmail.com>
This changes instances of `ToString` to `Into<String>` whenever ownership of
the `String` is necessary. In case it is not, it's simply changed to `&str` to
lessen the effects of monomorphization on compile times.
Problems with the current setup:

- The content of messages with these types is not documented, meaning that it can change anytime.
- Not every message type is handled, e.g. boost or thread creation messages do not get their content added.
- Overall more possibly unexpected behaviour and less consistency with the API.
This adds a method `Webhook::execute_in_thread` which takes an additional
thread ID (which is just a `ChannelId`) parameter. Then, the webhook will
execute in that thread, provided it belongs to the webhook's assigned guild
channel.

This also adds a `thread_id` parameter to `Http::execute_webhook`, which is a
breaking change.
This also removes `GuildContainer`, as this was (and historically has been) the only use of it.
This replaces the implementation of `From<Ref>` for `CacheRef` with a
private method in order to not expose the dashmap type as part of the
public API.

Also, this rewords and removes a few clones on types returned from the cache.
The interaction payload for autocompletes is slightly different. It's
missing the `resolved` data map and reports only for the current
autocompleted option value the `focused` field and its value as a string.

The focused option value is modeled as an enum variant of `CommandDataOptionValue`.

```
enum CommandDataOptionValue {
    Autocomplete { name: String, kind: CommandOptionType, value: String },
}
```

The focused option value can be retrieved via `AutocompleteData::focused_option()`
from the `AutocompleteData::options` vector.
…known` events (#1938, #1951)

BREAKING CHANGES: Removes the `guild_unavailable` & `unknown` methods
from the `EventHandler` trait.
BREAKING CHANGE: This removes methods only usable on user accounts.
GnomedDev and others added 17 commits June 24, 2022 16:49
The message cache is currently stored as `DashMap<ChannelId, DashMap<MessageId,
Message>>` however since the outer `DashMap` already provides interior
mutability, it doesn't need another `DashMap` inside. 

This also stops dashmap being exposed to the public API via
`MessageIterator::Item` by removing it (and instead exposes the inner
`HashMap`).
The derive macros are grouped into `std` & third party and
sorted alphabetically.

```
#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)]
struct Foo;
```
BREAKING CHANGE: Some enums were missing the `[non_exhaustive]` attribute.
The file was forgotten to delete when the types were moved into the
root. See 34bc5fe
The derive macros are grouped into `std` & third party and
sorted alphabetically.

```
#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)]
struct Foo;
```
BREAKING CHANGE: `Cache::channel_messages_field` is replaced by `Cache::channel_messages`
returning a reference to the messages map.
This replaces the `[u64; 2]` shard info with `ShardInfo { id: u32, total: u32 }`.

Co-authored-by: nickelc <constantin.nickel@gmail.com>
The `enum_number!` macro now takes the whole enum definition and generates
`From` trait implementations to convert a value to the enum and back.

The implementations can then be picked up by `serde` with
`#[serde(from = "u8", into = "u8")]` to (de)serialize the types.

```
enum_number! {
    #[derive(Clone, Copy, Debug, Deserialize, Serialize)]
    #[serde(from = "u8", into = "u8")]
    pub enum Foo {
        A = 1,
        B = 2,
        _ => Unknown(u8),
    }
}
```

BREAKING CHANGE: The `Unknown` variant now takes the unknown value
and the removed `fn num() -> u64` method can be replaced
with `let v = u8::from(kind)`.
Before `ShardInfo`:
`[serenity::gateway::shard]: [Shard [15, 33]] ...`

Currently:
`[serenity::gateway::shard]: [Shard ShardInfo { id: 15, total: 33 }] ...`

After:
`[serenity::gateway::shard]: [ShardInfo { id: 15, total: 33 }] ...`
The returned `Result` is already marked as `#[must_use]`.
@github-actions github-actions bot added builder Related to the `builder` module. examples Related to Serenity's examples. model Related to the `model` module. labels Jun 28, 2022
@mkrasnitski
Copy link
Collaborator

mkrasnitski commented Jun 28, 2022

Thanks for the PR! Unfortunate timing on your part, I suppose. If you look at #1967 #2024, we're actually in the middle of an overhaul to how the builders are constructed and executed, which I expect will be finished and merged sometime next month. But, with the new system, adding support for audit_log_reason ought to be pretty easy, either as an additional method on the builder, or as a parameter to the execute function being introduced with every builder. I'm in favor of the former, which implies adding an audit_log_reason field to the builder struct and just marking it #[serde(skip)], and feature-flagging it behind "http".

@arqunis arqunis force-pushed the next branch 3 times, most recently from f06e6e7 to b6d6ce0 Compare July 20, 2022 02:43
@mkrasnitski
Copy link
Collaborator

@SenseiHiraku Now that #2024 has been merged, if you'd like, you can take a stab at re-implementing this (if not let me know and I can tackle it). I think the best way to do so would be to add an optional audit_log_reason field to every builder marked #[serde(skip)], as well as a corresponding method on the builder to set its value. Unfortunately I think it's best to close this PR in its current form and open a new one, as the changes it makes are no longer compatible with the current codebase.

@arqunis
Copy link
Member

arqunis commented Aug 27, 2022

Completed by #2105

@arqunis arqunis closed this Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder Related to the `builder` module. examples Related to Serenity's examples. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants