disnake icon indicating copy to clipboard operation
disnake copied to clipboard

feat(interactions): deserialize `channel` from data

Open shiftinv opened this issue 2 years ago • 2 comments

Summary

Implements https://github.com/discord/discord-api-docs/commit/88a761816b26d6891f8a276683bf57a4133aa233, deserializing the channel field from the interaction payload and no longer using the channel_id field.

This has no effect for channels which are already cached, but it now provides a proper channel object in cases where it isn't (no bot in guild, private threads, ...).

A few comments on safety

The continued assumption is that all non-PING interactions contain this channel field, which is what we're already assuming for channel_id. However, this is not documented. Making the channel field optional in the future is definitely worth consideration, imo, but for now it might be too much of a breaking change in terms of typing.

Similarly, we expect some fields to exist in this channel field, particularly id + type (and name + parent_id + thread_metadata in the case of threads) While all these are documented to exist in interaction.resolved.channels, this might not be the case for interaction.channel, which really only guarantees id and type. In the end it's probably a fair assumption, but still something we should improve in the future.

related thread

Checklist

  • [x] If code changes were made, then they have been tested
    • [x] I have updated the documentation to reflect the changes
    • [x] I have formatted the code properly by running pdm lint
    • [x] I have type-checked the code by running pdm pyright
  • [x] This PR fixes an issue
  • [x] This PR adds something new (e.g. new method or parameters)
  • [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • [ ] This PR is not a code change (e.g. documentation, README, ...)

shiftinv avatar Apr 24 '23 17:04 shiftinv

The continued assumption is that all non-PING interactions contain this channel field, which is what we're already assuming for channel_id. However, this is not documented.

I really don't like this assumption, and would like some fallback to current behavior in the event that the channel data is not present (it was already rolled back once for breaking d.js).

onerandomusername avatar Jun 09 '23 18:06 onerandomusername

Looks like the assumption that parent_id is included in threads doesn't always hold for interaction.channel, unlike interaction.data.resolved.channels: https://github.com/discord/discord-api-docs/discussions/6270

I can't reproduce it, but I was afraid this might happen. It makes this field a lot more annoying to work with; not sure how to handle this properly right now, I don't want to just slap a try/except around it.

edit: looked into it further, the linked issue may be slightly inaccurate and we actually do get this data here

shiftinv avatar Jul 05 '23 14:07 shiftinv