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

fix(GuildManager): add missing types and converts #6683

Merged
merged 4 commits into from Oct 2, 2021
Merged

fix(GuildManager): add missing types and converts #6683

merged 4 commits into from Oct 2, 2021

Conversation

NyanSpaghetti
Copy link
Contributor

@NyanSpaghetti NyanSpaghetti commented Sep 23, 2021

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

Updated PartialChannelData at /typings/index.d.ts#L4509 to match /src/managers/GuildManager.js#L83.
Added missing converts (userLimit, rateLimitPerUser, overwrite.type) to GuildManager#create.
Made PartialOverwriteData.type to take OverwriteType instead of string.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@Jiralite
Copy link
Member

It seems it's a little more than the typings that aren't wholly correct with this.

  1. A channel's rateLimitPerUser is not respected. You can easily see this in the code:

for (const channel of channels) {
if (channel.type) channel.type = ChannelTypes[channel.type.toUpperCase()];
channel.parent_id = channel.parentId;
delete channel.parentId;
if (!channel.permissionOverwrites) continue;
for (const overwrite of channel.permissionOverwrites) {
if (overwrite.allow) overwrite.allow = Permissions.resolve(overwrite.allow).toString();
if (overwrite.deny) overwrite.deny = Permissions.resolve(overwrite.deny).toString();
}
channel.permission_overwrites = channel.permissionOverwrites;
delete channel.permissionOverwrites;
}

There is no mention of converting a provided rateLimitPerUser to rate_limit_per_user here, so this doesn't actually work. This should work just like parentId and parent_id.

  1. The overwrite type of a channel's permissionOverwrites documents to take a string, but discord.js does not resolve this string to an integer which is accepted by the API. This fails.

These were the only problems I found with GuildManager#create - I'd be grateful if you could include these also!

@NyanSpaghetti NyanSpaghetti changed the title fix(types): update PartialChannelData typings fix(GuildManager): add missing types and converts Sep 24, 2021
@blainegrissom
Copy link

I feel like the changes are good, but I don't know the code enough to feel comfortable saying "Yes this is good", so I will leave the judgement of approval to someone more knowledgeable in the code.

@Jiralite Jiralite mentioned this pull request Sep 25, 2021
typings/index.d.ts Outdated Show resolved Hide resolved
src/managers/GuildManager.js Show resolved Hide resolved
@iCrawl iCrawl requested a review from kyranet September 26, 2021 19:37
@iCrawl iCrawl added this to the Version 13.2 milestone Sep 26, 2021
Co-authored-by: Jiralite <33201955+Jiralite@users.noreply.github.com>
@Jiralite
Copy link
Member

Mind modifying this to ChannelType|number?

* @property {ChannelType} [type] The type of the channel

@iCrawl iCrawl merged commit cdf65f7 into discordjs:main Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants