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

Connect users on room create #7

Merged
merged 4 commits into from
Jul 16, 2022
Merged

Conversation

gibsonbailey
Copy link
Contributor

@ mention of reviewers

@ckcollab

A brief description of the purpose of the changes contained in this PR.

Before:

  • user_1 and user_2 connect to chat websocket
  • user_1 creates new chatroom with members user_1 and user_2
  • user_1 sends message to new chatroom
  • user_1 never receives the repeated message from the websocket to confirm that the message was successfully sent
  • user_2 never receives message, because he connects to the rooms when he connects the websocket

Now:

  • user_1 and user_2 connect to chat websocket
  • user_1 creates new chatroom with members user_1 and user_2
  • The list of chatrooms that user_1anduser_2` are listening to is automatically refreshed
  • user_1 sends message to new chatroom
  • user_1 receives reflected message back from server, confirming that his message was successfully consumed by server
  • user_2 receives message from the room broadcast

Misc. comments

Running into this case in Udayos. This should fix it.

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • Ready to merge

Copy link
Contributor

@ckcollab ckcollab left a comment

Choose a reason for hiding this comment

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

Nice test yo

@gibsonbailey gibsonbailey merged commit d7681bd into master Jul 16, 2022
@gibsonbailey gibsonbailey deleted the connect-users-on-room-create branch July 16, 2022 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants