Add GuildChannel.deleteOverwrite()

[ImRodry]

Please describe the changes this PR makes and why it should be merged: There were methods for creating and updating permission overwrites in a channel, but not for deleting them, so I added one. It doesn't support a reason due to apparent Discord API limitations so the only parameter it supports is the User or Role resolvable

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.

[almostSouji]

I see no benefit in this over https://discord.js.org/#/docs/main/stable/class/PermissionOverwrites?scrollTo=delete. Deletions in Discord.js always happen on the structure itself, not the corresponding manager.

[anandre]

I see no benefit in this over https://discord.js.org/#/docs/main/stable/class/PermissionOverwrites?scrollTo=delete. Deletions in Discord.js always happen on the structure itself, not the corresponding manager.

Then why did we add https://discord.js.org/#/docs/main/stable/class/MessageManager?scrollTo=delete recently?

[almostSouji]

MessageManager#delete can operate on uncached messages.
PermissionOverwrites intrinsically belong to the channel, they don't need to be fetched.

[ImRodry]

I see no benefit in this over https://discord.js.org/#/docs/main/stable/class/PermissionOverwrites?scrollTo=delete. Deletions in Discord.js always happen on the structure itself, not the corresponding manager.

It doesn’t make any sense to have methods inside GuildChannel to create and update permission overwrites (which do exactly the same) and not have one to delete. The thing you linked only works on specific situations and, for example, didn’t work for what I was looking for, and this did, so I think it would be useful and would make sense

[wasdennnoch]

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.

[ckohen]

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

[ImRodry]

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

[ckohen]

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

[ImRodry]

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

[wasdennnoch]

The thing you linked only works on specific situations and, for example, didn’t work for what I was looking for

Can you give an example for when it doesn't work?

[almostSouji]

It doesn’t make any sense to have methods inside GuildChannel to create and update permission overwrites (which do exactly the same) and not have one to delete. The thing you linked only works on specific situations and, for example, didn’t work for what I was looking for, and this did, so I think it would be useful and would make sense

This is a misconception on your end.

It doesn’t make any sense to have methods inside GuildChannel to create and update permission overwrites [...] and not have one to delete.

GuildChannel#permissionOverwrites is not a manager. There is no data that may need to be fetched and can be uncached (managing that data is the sole purpose of managers)

Of course, it makes sense to have a method to create and update overwrites, and since they can't be on the Manager (since it just doesn't exist in the first place) they will have to be on GuildChannel.

Delete however makes no sense to have on the GuildChannel class, as there is - again - no need to ever fetch overwrites, which you could save an API call for by introducing this method; you get them with the channel, always.

To delete an overwrite - as is the case with other structures that are not handled by managers - you can retrieve the overwrite from GuildChannel#permissionOverwrites and call delete on the resulting structure

const ov = message.channel.permissionOverwrites.get("<User or Role ID>"):
ov.delete()

didn’t work for what I was looking for, and this did, so I think it would be useful and would make sense

There is no case where this would not work. Even with intents GUILD_CHANNEL_UPDATE, GUILD_CHANNEL_CREATE, GUILD_CREATE are all categorized under the GUILD intent.

(which do exactly the same)

They do not.

GuildChannel#updateOverwrite updates the existing overwrites with provided data, instead of completely overwriting the overwrite for this specific user or role. Only if no overwrite for this specific user exists it will instead create one and be the same as GuildChannel#createOverwrite

[ImRodry]

The thing you linked only works on specific situations and, for example, didn’t work for what I was looking for

Can you give an example for when it doesn't work?

Tried to delete the permission overwrite for role with ID 768435276191891456 on channel 613015467984158742 and it didn't work

guild.channels.cache.get("613015467984158742").permissionOverwrites.delete(guild.roles.cache.find(r => r.id === "768435276191891456"))```

[ImRodry]

To delete an overwrite - as is the case with other structures that are not handled by managers - you can retrieve the overwrite from GuildChannel#permissionOverwrites and call delete on the resulting structure

I don't want to delete the permission overwrites for that channel, I wanna delete the permission overwrites for a specific role/user on that channel.

There is no case where this would not work. Even with intents GUILD_CHANNEL_UPDATE, GUILD_CHANNEL_CREATE, GUILD_CREATE are all categorized under the GUILD intent.

Refer to my comment above

[almostSouji]

I don't want to delete the permission overwrites for that channel, I wanna delete the permission overwrites for a specific role/user on that channel.

Yes, and I instructed how to do that above. Which you choose to ignore and instead demonstrate your misunderstanding once again.

Refer to my comment above

I can't refer to something that's a misunderstanding of how things work on your end. You are demonstrating the wrong usage of the method instead of listening to what I was saying.

again. the way to do this is to retrieve the PermissionOverwrite instance from the <GuildChannel>.permissionOverwrites collection, where the key is the ID of the role or user the overwrite is meant for. You then call the delete method on that structure. Not the collection holding it.

Collection#delete - which you call in your example above is inherited from the parent class (Map) and deletes the entry from the map in question. this does not cause any API call to go to Discord to actually delete things. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/delete

[ImRodry]

Yes, and I instructed how to do that above. Which you choose to ignore and instead demonstrate your misunderstanding once again.

Alright, my bad, I missed the user or role id part, I was on mobile, no need to be so aggressive about it jeez

I can't refer to something that's a misunderstanding of how things work on your end. You are demonstrating the wrong usage of the method instead of listening to what I was saying.
again. the way to do this is to retrieve the PermissionOverwrite instance from the .permissionOverwrites collection, where the key is the ID of the role or user the overwrite is meant for. You then call the delete method on that structure. Not the collection holding it.
Collection#delete - which you call in your example above is inherited from the parent class (Map) and deletes the entry from the map in question. this does not cause any API call to go to Discord to actually delete things. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/delete

Alright, I understand all of that. Now can you explain what would be the issue with merging this?

[almostSouji]

Alright, I understand all of that. Now can you explain what would be the issue with merging this?

Calling delete on the structure itself is a coherent approach across all of the library. This adds no functionality (the exception brought up by Mark above is useful due to not needing to fetch uncached structures and does not apply here) but instead code duplication.

After internal discussion to make sure I haven't missed anything and this approach is not preferable to the one already in place I'm going to close this.

---

Edit: If anything we might decide further down the line that the approach for overwrites might be better implemented as a pseudo manager, resulting in , <GuildChannel>.permissionOverwrites.create(...) and <GuildChannel>.permissionOverwrites.delete("")