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

Better semantics around message REST methods #587

Open
1 task done
FasterSpeeding opened this issue May 7, 2021 · 1 comment
Open
1 task done

Better semantics around message REST methods #587

FasterSpeeding opened this issue May 7, 2021 · 1 comment
Labels
breaking This change hurts backwards compatibility enhancement New feature or request large Requires a large number of changes and testing, may take several PRs to complete

Comments

@FasterSpeeding
Copy link
Collaborator

FasterSpeeding commented May 7, 2021

Summary

Since webhooks are going to eventually be more commonly used as interactions are introduced it'd be helpful to improve the semantics around "responding" to (message.respond), "editing" (message.edit) and "deleting" (message.delete) messages as to take advantage of the webhook flow.

Problem

This change will likely be breaking and should be brought in to avoid encouraging behaviour which could lead to errors when the bot can't execute the wanted action on a webhook message and inconsistent behaviour like switching between webhook execution and message on response.

Ideal implementation

This may be implemented by differentiating between messages received through a webhook flow and messages received else with different classes being used for each. The main difference between the two would be the signature and implementation of message.respond, message.edit and message.delete, and the fact that the webhook message would enforce that webhook_id is always present while also tracking the webhook token.

There may also be abstract respond, edit and delete methods with more generic signatures (meaning they only have fields that are available for both webhooks and bot execution) on the message class they both inherit from but this might be messy to implement with how we currently handle partial messages.

Checklist

  • I have searched the issue tracker and have made sure it's not a duplicate.
    If it is a follow up of another issue, I have specified it.
@FasterSpeeding FasterSpeeding added the enhancement New feature or request label May 7, 2021
@FasterSpeeding FasterSpeeding changed the title Better semantics around message respond Better semantics around message REST methods May 7, 2021
@FasterSpeeding FasterSpeeding added the breaking This change hurts backwards compatibility label May 7, 2021
@FasterSpeeding FasterSpeeding self-assigned this May 14, 2021
@FasterSpeeding FasterSpeeding added the large Requires a large number of changes and testing, may take several PRs to complete label May 14, 2021
@FasterSpeeding
Copy link
Collaborator Author

This will likely rely on the ExecutableWebhook mixin present in #577 so marked as blocked for now

@FasterSpeeding FasterSpeeding added the blocked This issue cannot yet be completed, or this PR is not yet ready to merge because of a dependency label Jun 16, 2021
@FasterSpeeding FasterSpeeding removed the blocked This issue cannot yet be completed, or this PR is not yet ready to merge because of a dependency label Jun 30, 2021
@FasterSpeeding FasterSpeeding removed their assignment Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change hurts backwards compatibility enhancement New feature or request large Requires a large number of changes and testing, may take several PRs to complete
Projects
None yet
Development

No branches or pull requests

1 participant