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

FIX: Add MessageBust.last_id to chat channel subscriptions #19255

Merged

Conversation

martin-brennan
Copy link
Contributor

@martin-brennan martin-brennan commented Nov 30, 2022

This commit adds variousMessageBus.last_ids to serializer payloads
for chat channels and the chat view (for chat live pane) so
we can use those IDs when subscribing to MessageBus channels
from chat.

This allows us to ensure that any messages created between the
server being hit and the UI loaded and subscribing end up being
delivered to the client, rather than just silently dropped.

This commit also fixes an issue where we were subscribing to
the new-messages and new-mentions MessageBus channels multiple
times when following/unfollowing a channel multiple times.

This commit adds the MessageBus.last_id to serializer payloads
for chat channels and the chat view (for chat live pane) so
we can use those IDs when subscribing to MessageBus channels
from chat.

This allows us to ensure that any messages created between the
server being hit and the UI loaded and subscribing end up being
delivered to the client, rather than just silently dropped.

This commit also fixes an issue where we were subscribing to
the new-messages and new-mentions MessageBus channels multiple
times when following/unfollowing a channel multiple times.
@martin-brennan martin-brennan changed the title FIX: Add message_bus_last_id to chat channel subscriptions FIX: Add MessageBust.last_id to chat channel subscriptions Nov 30, 2022
@github-actions github-actions bot added the chat PRs which include a change to Chat plugin label Nov 30, 2022
@martin-brennan martin-brennan force-pushed the issue/add-message-bus-last-id-to-chat-subscriptions branch from ffa5ddc to 8c588cc Compare November 30, 2022 07:22
@martin-brennan martin-brennan marked this pull request as ready for review December 1, 2022 04:00
Copy link
Contributor

@jjaffeux jjaffeux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I will refactor a lot of it in next weeks, there are way too many subscripions and similar ones IMO

@martin-brennan martin-brennan merged commit 8437081 into main Dec 2, 2022
@martin-brennan martin-brennan deleted the issue/add-message-bus-last-id-to-chat-subscriptions branch December 2, 2022 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat PRs which include a change to Chat plugin
Development

Successfully merging this pull request may close these issues.

2 participants