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

Add GuildChannel.deleteOverwrite() #5234

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/structures/GuildChannel.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,26 @@ class GuildChannel extends Channel {
.then(() => this);
}

/**
* Deletes existing permission overwrites for a user or role in this channel.
* @param {RoleResolvable|UserResolvable} userOrRole The user or role to update
ImRodry marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Promise<GuildChannel>}
* @example
* // Delete permission overwrites for a message author
* message.channel.deleteOverwrite(message.author)
* .then(() => console.log("Deleted permission overwrites for " + message.author.tag + " in " + message.channel))
* .catch(console.error);
*/
deleteOverwrite(userOrRole) {
userOrRole = this.guild.roles.resolve(userOrRole) || this.client.users.resolve(userOrRole);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will throw for valid but uncached users. Ideally this function should only make an API call and let the API decide whether the role/user exists.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed it will, the other methods which this is likely based on do this as well, there is an open discussion about this #5230
I think if this is being added, in this instance it should definitely be passed directly to discords API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I simply followed the way it is currently handled in other methods, although I definitely agree it should use an ID instead of the user or role. Even then I’m not sure if I should change that before your changes get accepted and committed since that would create inconsistencies

Copy link
Member

Choose a reason for hiding this comment

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

What I linked is simply a discussion for how we should handle this, it is not a PR yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know, but you proposed changes that weren't accepted yet so it's not worth changing it here since it could create inconsistencies if those changes are never implementes

if (!userOrRole) return Promise.reject(new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role'));

return this.client.api
.channels(this.id)
.permissions[userOrRole.id].delete(userOrRole.id)
.then(() => this);
}

/**
* Locks in the permission overwrites from the parent channel.
* @returns {Promise<GuildChannel>}
Expand Down
1 change: 1 addition & 0 deletions typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,7 @@ declare module 'discord.js' {
options: PermissionOverwriteOption,
reason?: string,
): Promise<this>;
public deleteOverwrite(userOrRole: RoleResolvable | UserResolvable): Promise<this>;
public edit(data: ChannelData, reason?: string): Promise<this>;
public equals(channel: GuildChannel): boolean;
public fetchInvites(): Promise<Collection<string, Invite>>;
Expand Down