-
Notifications
You must be signed in to change notification settings - Fork 635
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
RTM message format has changed, verify eventWrapper assumptions #589
Comments
I also think - yes, we do. The cache is necessary to provide full information about bot users.
This may be also true. That said, for the forthcoming patch releases, I don't want to introduce any breaking changes. If we all are on the same page regarding this, we can safely merge my PR #588 and release a new patch version to address #586 . |
👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. |
Description
We've noticed some changes in the RTM message format but have not actively updated the processing of each event in
eventWrapper
accordingly. It's possible that some of the assumptions made are no longer valid. That was the case in #586 - sincebot_message
events started having auser
property the logic behind when to update thebotUserIdMap
was no longer correct.A few more things to check:
botUserIdMap
? I think yes, because the user ID of the bot is not enough information to put in themessage.user
object, otherwise there will be breaking changes. However, we also see abot_profile
on some (maybe all) events now - is that enough?subtype: 'bot_message'
? It seems to be missing on at least some, but maybe all, events where it should be.This is not an exhaustive list, and we should look for more changes we may need to make to
eventWrapper
.Related to #588
Requirements (place an
x
in each of the[ ]
)The text was updated successfully, but these errors were encountered: