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

feat(GuildMember): Added communicationDisabledUntil property #6949

Closed
wants to merge 14 commits into from
Closed

feat(GuildMember): Added communicationDisabledUntil property #6949

wants to merge 14 commits into from

Conversation

ZeldaFan0225
Copy link

@ZeldaFan0225 ZeldaFan0225 commented Nov 6, 2021

Please describe the changes this PR makes and why it should be merged:

This PR includes the already present communication_disabled_until property returned by the API.
This should be merged so as soon Discord releases that feature people can use it.

Relevant PR in discord-api-docs:

Status and versioning classification:

  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)

Copy link
Contributor

@ImRodry ImRodry left a comment

Choose a reason for hiding this comment

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

This PR should be marked as a draft until the upstream PR is created/merged

Copy link
Contributor

@ImRodry ImRodry left a comment

Choose a reason for hiding this comment

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

Still missing the new parameter in GuildMemberEditData

Copy link
Member

@ckohen ckohen left a comment

Choose a reason for hiding this comment

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

Based on the some of things seen by testing this (yes you can actually test it already, client doesn't do anything with it though), the API seems highly likely to change, just as an FYI

if (_data.communicationDisabledUntil) {
if (_data.communicationDisabledUntil instanceof Date) {
_data.communication_disabled_until = _data.communicationDisabledUntil;
if(_data.communication_disabled_until.getTime() < Date.now()) throw new Error('COMMUNICATION_DISABLED_PAST_DATE');
Copy link
Member

Choose a reason for hiding this comment

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

Discord accepts dates in the past currently (and doesn't reset on sending null), so the lib shouldn't throw

if(_data.communication_disabled_until.getTime() < Date.now()) throw new Error('COMMUNICATION_DISABLED_PAST_DATE');
} else {
_data.communication_disabled_until = new Date(Date.now() + _data.communicationDisabledUntil * 1000);
if(_data.communication_disabled_until.getTime() < Date.now()) throw new Error('COMMUNICATION_DISABLED_PAST_DATE');
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -150,6 +150,9 @@ const Messages = {
NOT_IMPLEMENTED: (what, name) => `Method ${what} not implemented on ${name}.`,

SWEEP_FILTER_RETURN: 'The return value of the sweepFilter function was not false or a Function',

COMMUNICATION_DISABLED_PAST_DATE: 'The given date can\'t be in the past',
COMMUNICATION_DISABLED_NEGATIVE_DURATION: 'The given duration can\' be negative',
Copy link
Member

Choose a reason for hiding this comment

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

This error is unused

@ZeldaFan0225 ZeldaFan0225 marked this pull request as draft November 7, 2021 09:46
@GodderE2D
Copy link
Contributor

I feel like the terms "muted" and "unmuted" shouldn't be used here, but we'll wait for UI descriptions to see what we can replace them with, if at all.

@ImRodry
Copy link
Contributor

ImRodry commented Nov 22, 2021

Please add the PR to the description: discord/discord-api-docs#4075

@milanmdev
Copy link

milanmdev commented Nov 22, 2021

Discord released a "MODERATE_MEMBERS" permission - this should be added to this PR

discord/discord-api-docs#4075 (comment)

* Disables or enables communication for this member.
* @param {?number | Date} seconds The duration in seconds or a date in the future until the member should have
* their communication disabled. Passing `null` will enable communication for them.
* @param {string} [reason] Reason for setting the nickname

Choose a reason for hiding this comment

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

Typo as a nickname is not set here, but instead the reason for the mute.


/**
* Disables or enables communication for this member.
* @param {?number | Date} seconds The duration in seconds or a date in the future until the member should have
Copy link
Member

Choose a reason for hiding this comment

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

The documentation doesn't render the ? properly otherwise

Suggested change
* @param {?number | Date} seconds The duration in seconds or a date in the future until the member should have
* @param {?(number|Date)} seconds The duration in seconds or a date in the future until the member should have

* @param {string} [reason] Reason for setting the nickname
* @returns {Promise<GuildMember>}
*/
disableCommunication(seconds, reason) {
Copy link
Member

Choose a reason for hiding this comment

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

The name of the first argument doesn't match the one in the typings
Not to mention that seconds sounds misleading since it accepts dates

@ImRodry
Copy link
Contributor

ImRodry commented Dec 12, 2021

@ZeldaFan0225 are you no longer going to maintain this PR?

@ZeldaFan0225
Copy link
Author

@ImRodry no thats why i closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants