Skip to content

Conversation

@almostSouji
Copy link
Member

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

I decided to make this (original) PR separate from #4598 since it allows a quick fix of the issue at hand in case the required Discord API documentation takes more time than anticipated. The client handles this situation differently, however. After documentation on the API documentation this will be applied as a feature in #4598.

Should #4598 me merged before this issue will auto-close as it is obsolete.

as #4144 has brought to light the lockPermissions option in GuildChannel#setParent (and as direct cause GuildChannel#edit) does currently not lock permissions.

I tracked this down due to the API no longer accepting a lock_permissions field when patching a channel as per https://discord.com/developers/docs/resources/channel#modify-channel, which is how the feature is currently implemented as of #1727

const newData = await this.client.api.channels(this.id).patch({
data: {
name: (data.name || this.name).trim(),
topic: data.topic,
nsfw: data.nsfw,
bitrate: data.bitrate || this.bitrate,
user_limit: typeof data.userLimit !== 'undefined' ? data.userLimit : this.userLimit,
parent_id: data.parentID,
lock_permissions: data.lockPermissions,
rate_limit_per_user: data.rateLimitPerUser,
permission_overwrites,
},
reason,
});

Since locking permissions really just means replacing the channels overwrites with its parents (or future parents) overwrites this proposed solution resolves these overwrites based on the current state and provides them (if applicable along with the new parent) in the patch data.

Considerations/Assumptions made:

  • If both overwrites and the lock option are provided: the overwrites are overwritten
  • If a new parent and lock option are provided: use the overwrites of the to-be parent
  • If lock is supplied without a parent: use the overwrites of this.parent
  • If the new parent is not a category channel or can not be resolved: ignore the lock option

Status

  • 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

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

@iCrawl iCrawl requested a review from SpaceEEC July 17, 2020 08:19
@almostSouji
Copy link
Member Author

Should our suggestion at discord/discord-api-docs#1796 come to pass this might need to be reverted at a later date, reference for visibility

@iCrawl iCrawl merged commit 599cde3 into discordjs:master Aug 12, 2020
@almostSouji almostSouji deleted the fix-lockoption branch September 27, 2020 10:39
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.

setParent() function does not lock permissions

3 participants