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

Consider messages deleted if their channel is deleted #3519

Merged
merged 1 commit into from Oct 4, 2019

Conversation

@almostSouji
Copy link
Member

commented Oct 3, 2019

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

Message#deleted as well as Message#deletable currently return false/true if the messages channel is deleted. Since our users use these properties to check if a message can be deleted before doing so this can lead to unexpected behavior.

If the channel is deleted the API will respond with an error since the respective endpoint is unavailable.

This PR attempts to change this behavior by setting messages deleted on channel_delete.
Since discord.js receives channel close events if DM channels are closed they should be excluded from this change. The typeof check should be more robust than checking the .type property and is possibly more likely to be detected should the class responsible for direct message channels be changed in the future.

The deletable getter will automatically carry the changes, since it checks .deleted implicitly.

I see no need to update typings, might have missed something though.
Unsure if this functionality should be considered breaking, as it does change the behavior of a property.

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.
@bdistin
bdistin approved these changes Oct 3, 2019
@Fyko
Fyko approved these changes Oct 3, 2019
@kyranet
kyranet approved these changes Oct 3, 2019
@SpaceEEC SpaceEEC merged commit 48856c0 into discordjs:master Oct 4, 2019
3 checks passed
3 checks passed
ESLint
Details
TSLint
Details
Documentation
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.