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

Document guild time outs #4075

Merged
merged 23 commits into from
Dec 20, 2021
Merged

Document guild time outs #4075

merged 23 commits into from
Dec 20, 2021

Conversation

NurMarvin
Copy link
Contributor

This PR adds documentation for guild time outs, which currently mainly only consist of the communication_disabled_until field on guild member objects. Since this field has been showing up on guild member objects for quite some time now and the field getting functionality on the API-side, I feel like it's at least worth creating a draft PR mentioning this field, since this can affect bots on servers where this gets tested. For example, someone could be time outing the bot and the bot would be getting errors regarding "Missing Permissions" with the bot owner possibly being puzzled as to why this happens despite checking for permissions etc.
I know this information is technically just data-mined but due to the aforementioned reasons I felt like still creating at least draft PR.

@Lulalaby
Copy link
Contributor

Lulalaby commented Nov 9, 2021

Yeah, a few users already reported that they played with it and can't remove it anymore 😅

docs/resources/Guild.md Outdated Show resolved Hide resolved
Co-authored-by: Jake Ward <[email protected]>
docs/resources/Guild.md Outdated Show resolved Hide resolved
Co-authored-by: Vitor <[email protected]>
@msciotti msciotti added the not released This issue or PR is referencing a change that is not yet widely released and/or subject to change. label Nov 9, 2021
Copy link
Contributor

@advaith1 advaith1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify Guild Member needs to be updated too

(also, is it definitely optional?)

docs/resources/Guild.md Outdated Show resolved Hide resolved
Marvin Witt and others added 3 commits November 11, 2021 16:48
docs/resources/Guild.md Outdated Show resolved Hide resolved
@milanmdev
Copy link

milanmdev commented Nov 22, 2021

Discord added a permission for managing guild time outs - I've never documented permissions, but I assume it should be documented as 1 << 40 (don't quote me on this)

image

@suneettipirneni
Copy link

suneettipirneni commented Nov 22, 2021

Discord added a permission for managing guild time outs - I've never documented permissions, but I assume it should be documented as 1 << 40 (don't quote me on this)

image

The new permission seems to be named MODERATE_MEMBERS = 1n << 40n

@venda8035
Copy link

Free

Co-authored-by: Advaith <[email protected]>
docs/resources/Guild.md Outdated Show resolved Hide resolved
| mute | boolean | whether the user is muted in voice channels |
| pending? | boolean | whether the user has not yet passed the guild's [Membership Screening](#DOCS_RESOURCES_GUILD/membership-screening-object) requirements |
| permissions? | string | total permissions of the member in the channel, including overwrites, returned when in the interaction object |
| communication_disabled_until? | ?ISO8601 timestamp | when the user's [timeout](https://support.discord.com/hc/en-us/articles/4413305239191-Time-Out-FAQ) will expire and the user will be able to communicate in the guild again, null or a time in the past if the user is not timed out |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When can this property be missing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be on every guild member object.
Had no case yet, where it was missing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh this is a good point - @hemu is it optional?

Copy link
Contributor

@hemu hemu Dec 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah to be honest the optionality of fields from the perspective of the public api is always a tricky question to answer definitively for me. b/c the objects we define in the docs are defined more for convenience, so they can be referred to from other places in the docs rather than something that always maps 1:1 to how we use models internally. I think the best answer I can give is when we serialize the guild member model, we currently always seem to include the communication_disabled_until field. But this still doesn't 100% guarantee it is always included unless we thoroughly check every single endpoint where this model is included in some way currently. And this could change in the future at any time. But I think that means we can remove the optional?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since from a pure data model perspective communication_disabled_until isn't a core field for the member model like joined_at is, how about we leave it as optional just in case, since I'd imagine the opposite -- assuming it is required now and at some point in the future an endpoint whose document references this Guild Member Object in docs fails to include this field -- is a worse developer experience. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should leave it as optional :)

Copy link
Contributor

@advaith1 advaith1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the link and wording tweak in the other tables; after this it should be good to go 🚀

docs/resources/Guild.md Outdated Show resolved Hide resolved
docs/topics/Gateway.md Outdated Show resolved Hide resolved
@advaith1
Copy link
Contributor

also, is this field supported in the Add Guild Member endpoint?

docs/resources/Guild.md Outdated Show resolved Hide resolved
@MinnDevelopment
Copy link
Contributor

Could the permissions section be updated to explain how being timed out affects the permissions? Such as, which permissions it disables.

@hemu
Copy link
Contributor

hemu commented Dec 20, 2021

Honest question: if the UI shows it as "Timeout Members", would it be wrong to not keep a similar name for consistency reasons with the API as TIMEOUT_MEMBERS? All of the other permissions are kept generally 1:1 in naming with the API and UI because of (reasonably assuming) consistency and avoiding confusion. MODERATE_MEMBERS as the permission for timeouts in the API does add a slight amount of confusion imo as @MinnDevelopment mentioned. I think the actual naming of the permission is misleading because of the other moderation-specific permissions BAN_MEMBERS, KICK_MEMBERS and etc.

The hesitation here is that in the future if another action is introduced and gated by the MODERATE_MEMBERS perm, then we'd have to introduce api / docs churn for all devs to keep it consistent. But changing the client facing description to accommodate the additional action is less impactful

@hemu
Copy link
Contributor

hemu commented Dec 20, 2021

Could the permissions section be updated to explain how being timed out affects the permissions? Such as, which permissions it disables.

great idea, I can add this as a followup

@hemu
Copy link
Contributor

hemu commented Dec 20, 2021

also, is this field supported in the Add Guild Member endpoint?

not currently. would the use case be to add a member that is already timed out initially?

@suneettipirneni
Copy link

also, is this field supported in the Add Guild Member endpoint?

not currently. would the use case be to add a member that is already timed out initially?

Wouldn't this overlap with the features of membership screening?

@advaith1
Copy link
Contributor

I was just asking to make sure it wasn't being missed in the docs, I don't have a use case

@hemu
Copy link
Contributor

hemu commented Dec 20, 2021

alright going to merge this in and deploy at this point so we can make these core parts available -- can save improvements for follow up PR's at this point. Big thank you to everyone who contributed!! 🙂

@hemu hemu merged commit b51c34f into discord:master Dec 20, 2021
7596ff pushed a commit to twilight-rs/twilight that referenced this pull request Dec 27, 2021
Discord API PR (initial documentation): discord/discord-api-docs#4075
Discord API PR (further documentation): discord/discord-api-docs#4266
@night night removed the not released This issue or PR is referencing a change that is not yet widely released and/or subject to change. label Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.