feat(cache,http,model): guild timeouts #1342

HTGAzureX1212 · 2021-12-21T09:45:26Z

Discord API PR (initial documentation): discord/discord-api-docs#4075
Discord API PR (further documentation): discord/discord-api-docs#4266

baptiste0928

The permission calculator in the util crate should also be updated to reflect changes in permissions introduced by timeout:

Timed out members will temporarily lose all permissions except VIEW_CHANNEL and READ_MESSAGE_HISTORY. Owners and admin users with ADMINISTRATOR permissions are exempt.

cache/in-memory/src/event/member.rs

HTGAzureX1212 · 2021-12-21T11:04:43Z

The permission calculator in the util crate should also be updated to reflect changes in permissions introduced by timeout:

Timed out members will temporarily lose all permissions except VIEW_CHANNEL and READ_MESSAGE_HISTORY. Owners and admin users with ADMINISTRATOR permissions are exempt.

That could be a bit problematic - that requires checking whether the member in question is timed out or not, which needs to take the communication_disabled_until field of *Member into consideration.

baptiste0928

Looks good! Permission calculator need to be updated as well, but I think it can be done in a separate PR.

7596ff

As far as I can tell, this isn't breaking. We should target this at main.

http/src/request/guild/member/update_guild_member.rs

model/src/application/interaction/application_command/data/resolved.rs

model/src/guild/member.rs

model/src/guild/permissions.rs

vilgotf · 2021-12-21T17:45:00Z

Field is always present according to this discussion discord/discord-api-docs#4075 (comment)

The base branch was changed.

codecov-commenter · 2021-12-22T09:34:08Z

Codecov Report

Merging #1342 (581d394) into main (581d394) will not change coverage.
The diff coverage is n/a.

❗ Current head 581d394 differs from pull request most recent head caa8d86. Consider uploading reports for the commit caa8d86 to get more accurate results

@@           Coverage Diff           @@
##             main    #1342   +/-   ##
=======================================
  Coverage   32.92%   32.92%           
=======================================
  Files         366      366           
  Lines       14541    14541           
=======================================
  Hits         4788     4788           
  Misses       9753     9753

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 581d394...caa8d86. Read the comment docs.

baptiste0928

I noticed that no events are sent when the timeout is over (discord/discord-api-docs#4269). Therefore, relying on whether communication_disabled_until has a value does not allow knowing if a given member is actually timed out (especially when using cached data).

As checking if a member is timed out is a common use case, it should be highlighted that the presence of the field is not enough to know if a user is currently timed out. I suggest wrapping the Option<Timestamp> in another type with specific methods for this use case.

Example implementation:

pub struct TimeoutState(Option<Timestamp>);

impl TimeoutState {
   pub fn is_timed_out(&self) -> bool {
     // Check if a member is currently timed out
   }
   
   pub fn until(&self) -> Option<&Timestamp> {
     // Return the timestamp only if the user is currently timed out
   }
   
   pub fn inner(&self) -> Option<&Timestamp> {
      // Return the raw (unchecked) timestamp value
   }
}

A drawback is that the current time will be retrieved each time the is_timed_out or until method is called, but I think this could prevent some bugs. Anyway, the inner method will allow getting the unchecked timestamp without performance drawback.

http/src/request/guild/member/update_guild_member.rs

…for various cached member types

7596ff

Small things, looks good otherwise.

model/src/guild/member.rs

http/src/request/validate.rs

baptiste0928

I agree with @7596ff, but I think that it should be highlighted in the CachedMember documentation that the presence of the value is not enough to know if a member is timed out.

cache/in-memory/src/model/member.rs

HTGAzureX1212 · 2021-12-24T13:05:56Z

I agree with @7596ff, but I think that it should be highlighted in the CachedMember documentation that the presence of the value is not enough to know if a member is timed out.

Does it make sense to highlight as such in *Member types in the model crate as well?

http/src/request/validate.rs

baptiste0928 · 2021-12-25T13:03:39Z

Does it make sense to highlight as such in *Member types in the model crate as well?

As other *Member types are directly obtained from HTTP requests or events, their communication_disabled_until should reflect the current state of the user. I don't think it's necessary.

HTGAzureX1212 · 2021-12-25T13:23:52Z

Does it make sense to highlight as such in *Member types in the model crate as well?

As other *Member types are directly obtained from HTTP requests or events, their communication_disabled_until should reflect the current state of the user. I don't think it's necessary.

Alright!

Additions Support guild member timeouts via `Member::communication_disabled_until` and `Permissions::MODERATE_MEMBERS` ([#1342] - [@HTG-YT]). [@HTG-YT]: https://github.com/HTG-YT [#1342]: #1342 Signed-off-by: Zeyla Hellyer <[email protected]>

@zeylahellyer

Additions Support guild member timeouts via `UpdateGuildMember::communication_disabled_until` ([#1342] - [@HTG-YT]). Support pre-flight cancelation to cancel requests after passing ratelimit queues but before sending ([#1353] - [@zeylahellyer]). Fixes Fix display implementation for Get Active Threads route ([#1386] - [@zeylahellyer]). Fix display implementation for Get Current User Application Info route ([#1389] - [@Erk-]). [@Erk-]: https://github.com/Erk- [@HTG-YT]: https://github.com/HTG-YT [@zeylahellyer]: https://github.com/zeylahellyer [#1389]: #1389 [#1386]: #1386 [#1353]: #1353 [#1342]: #1342 Signed-off-by: Zeyla Hellyer <[email protected]>

@zeylahellyer

Additions Support guild member timeouts ([#1342] - [@HTG-YT]). Support iterating over a channel's list of cached messages via `InMemoryCache::channel_messages` ([#1362] - [@zeylahellyer]). [@HTG-YT]: https://github.com/HTG-YT [@zeylahellyer]: https://github.com/zeylahellyer [#1362]: #1362 [#1342]: #1342 Signed-off-by: Zeyla Hellyer <[email protected]>

Nextcord: nextcord/nextcord#272 https://github.com/nextcord/nextcord/releases/tag/2.0.0a6 eris: abalabahaha/eris#1317 twilight: twilight-rs/twilight#1342 https://github.com/twilight-rs/twilight/releases/tag/model-0.8.4

baptiste0928 requested changes Dec 21, 2021

View reviewed changes

cache/in-memory/src/event/member.rs Outdated Show resolved Hide resolved

cache/in-memory/src/event/member.rs Outdated Show resolved Hide resolved

cache/in-memory/src/event/member.rs Outdated Show resolved Hide resolved

baptiste0928 added c-cache Affects the cache crate c-http Affects the http crate c-model Affects the model crate d-api Change related to Discord's API. labels Dec 21, 2021

baptiste0928 previously approved these changes Dec 21, 2021

View reviewed changes

7596ff suggested changes Dec 21, 2021

View reviewed changes

HTGAzureX1212 changed the base branch from next to main December 22, 2021 06:17

HTGAzureX1212 changed the base branch from main to next December 22, 2021 06:39

HTGAzureX1212 changed the base branch from next to main December 22, 2021 06:39

HTGAzureX1212 changed the base branch from main to next December 22, 2021 07:53

baptiste0928 reviewed Dec 22, 2021

View reviewed changes

http/src/request/guild/member/update_guild_member.rs Outdated Show resolved Hide resolved

7596ff marked this pull request as draft December 22, 2021 21:58

HTGAzureX1212 marked this pull request as ready for review December 23, 2021 06:02

HTGAzureX1212 requested review from baptiste0928 and 7596ff December 23, 2021 06:03

HTGAzureX1212 changed the base branch from next to main December 23, 2021 09:07

HTGAzureX1212 added 9 commits December 23, 2021 17:27

feature: add support for guild timeouts

74ad224

fix: fix build errors

d36638f

fix: fix build errors again

b0d08e9

fix: fix build errors again

a88d4a6

fix: fix build errors again

81afde5

fix: fix code mistake of using none for communication_disabled_until …

bdcfdf1

…for various cached member types

enhancement: resolve some comments

ba040d6

rustfmt

4f57ea4

fix: fix model tests

db462a8

HTGAzureX1212 changed the title ~~feature: add support for guild timeouts~~ feat(cache, http, model): guild timeouts Dec 24, 2021

HTGAzureX1212 changed the title ~~feat(cache, http, model): guild timeouts~~ feat(cache,http,model): guild timeouts Dec 24, 2021

7596ff suggested changes Dec 24, 2021

View reviewed changes

model/src/guild/member.rs Outdated Show resolved Hide resolved

http/src/request/validate.rs Show resolved Hide resolved

remove membertimeoutstate

3a3b72a

HTGAzureX1212 dismissed baptiste0928’s stale review via 3a3b72a December 24, 2021 09:12

HTGAzureX1212 requested review from 7596ff and baptiste0928 December 24, 2021 09:13

rustfmt

2f78ff9

baptiste0928 requested changes Dec 24, 2021

View reviewed changes

cache/in-memory/src/model/member.rs Show resolved Hide resolved

remark the insufficiency of communication_disabled_until

552bc82

HTGAzureX1212 requested a review from baptiste0928 December 24, 2021 12:03

baptiste0928 previously approved these changes Dec 24, 2021

View reviewed changes

itohatweb reviewed Dec 24, 2021

View reviewed changes

http/src/request/validate.rs Outdated Show resolved Hide resolved

use match instead of early return

3be9efc

HTGAzureX1212 dismissed baptiste0928’s stale review via 3be9efc December 25, 2021 00:33

fix stupid mistake

caa8d86

HTGAzureX1212 requested a review from baptiste0928 December 25, 2021 11:30

baptiste0928 approved these changes Dec 25, 2021

View reviewed changes

itohatweb approved these changes Dec 25, 2021

View reviewed changes

7596ff approved these changes Dec 26, 2021

View reviewed changes

7596ff merged commit 22cbf63 into twilight-rs:main Dec 27, 2021

HTGAzureX1212 deleted the feature/timeout branch December 27, 2021 08:52

SQKo mentioned this pull request Jan 20, 2022

Update merged PR (nextcord, eris, twilight) advaith1/libs#63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cache,http,model): guild timeouts #1342

feat(cache,http,model): guild timeouts #1342

HTGAzureX1212 commented Dec 21, 2021

baptiste0928 left a comment

HTGAzureX1212 commented Dec 21, 2021 •

edited

Loading

baptiste0928 left a comment

7596ff left a comment

vilgotf commented Dec 21, 2021

codecov-commenter commented Dec 22, 2021 •

edited

Loading

baptiste0928 left a comment

7596ff left a comment

baptiste0928 left a comment

HTGAzureX1212 commented Dec 24, 2021

baptiste0928 commented Dec 25, 2021

HTGAzureX1212 commented Dec 25, 2021

feat(cache,http,model): guild timeouts #1342

feat(cache,http,model): guild timeouts #1342

Conversation

HTGAzureX1212 commented Dec 21, 2021

baptiste0928 left a comment

Choose a reason for hiding this comment

HTGAzureX1212 commented Dec 21, 2021 • edited Loading

baptiste0928 left a comment

Choose a reason for hiding this comment

7596ff left a comment

Choose a reason for hiding this comment

vilgotf commented Dec 21, 2021

codecov-commenter commented Dec 22, 2021 • edited Loading

Codecov Report

baptiste0928 left a comment

Choose a reason for hiding this comment

7596ff left a comment

Choose a reason for hiding this comment

baptiste0928 left a comment

Choose a reason for hiding this comment

HTGAzureX1212 commented Dec 24, 2021

baptiste0928 commented Dec 25, 2021

HTGAzureX1212 commented Dec 25, 2021

HTGAzureX1212 commented Dec 21, 2021 •

edited

Loading

codecov-commenter commented Dec 22, 2021 •

edited

Loading