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

Fixes #547 where editing channel/group name crashed RoomManager #548

Closed
wants to merge 1 commit into from

Conversation

rwakida
Copy link
Contributor

@rwakida rwakida commented Aug 22, 2015

Adds code that observes room name changes, closes the old room, and
opens the new room via the RoomManager. There is still a small bug
where the room name changed message is not displayed to the user who changed
the name probably due to the message stream subscribing after the message was sent.

…nRoom

tracker

Bug is a side effect of User friendly URLs #18.  The commit changed Room
subscription based on room id to room name.. It created a new room
subscription, but didn't remove the old name from the RoomManager's
openedRooms cache.  Whenever the tracker ran, an 'undefined' room
was returned because the old room name was invalid.

Adds code that observes room name changes, closes the old room, and
opens the new room via the RoomManager.  There is still a small bug
where the room name changed message is not displayed to the user who changed
the name probably due to the message stream subscribing after the message was sent.
@rodrigok
Copy link
Member

I will review ASAP

@rodrigok
Copy link
Member

Hi @rwakida, I tested this PR and seems that the url doesn't change when the name of the current room changes. This can create a lot of problems, mainly with refresh.

I think that the better solution is close the current room and redirect user the the new one. I did some tests, you can review here #570

@rodrigok rodrigok closed this Aug 24, 2015
@rwakida
Copy link
Contributor Author

rwakida commented Aug 24, 2015

@rodrigok looks good. thanks.

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.

None yet

2 participants