Skip to content

Commit

Permalink
Fixes #547 where editing channel/group name crashed RoomManager's ope…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
rwakida committed Aug 22, 2015
1 parent c0fce84 commit 13fae7d
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 11 deletions.
6 changes: 5 additions & 1 deletion client/lib/RoomHistoryManager.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@
ts = new Date

Meteor.call 'loadHistory', rid, ts, limit, 0, (err, result) ->
if err
console.log err
return

ChatMessage.insert item for item in result
room.isLoading.set false
room.loaded += result.length
if result.length < limit
if result?.length < limit
room.hasMore.set false

hasMore = (rid) ->
Expand Down
44 changes: 37 additions & 7 deletions client/lib/RoomManager.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ Meteor.startup ->
RoomHistoryManager.clear openedRooms[typeName].rid
ChatMessage.remove rid: openedRooms[typeName].rid

remove = (typeName) ->
# this is called when the room name is changed and the cached room is no longer valid
if openedRooms[typeName]
close typeName
delete openedRooms[typeName]

computation = Tracker.autorun ->
for typeName, record of openedRooms when record.active is true
do (typeName, record) ->
Expand All @@ -68,15 +74,15 @@ Meteor.startup ->

room = ChatRoom.findOne query, { reactive: false }

openedRooms[typeName].rid = room._id
if room
openedRooms[typeName].rid = room._id
msgStream.on openedRooms[typeName].rid, (msg) ->
ChatMessage.upsert { _id: msg._id }, msg

msgStream.on openedRooms[typeName].rid, (msg) ->
ChatMessage.upsert { _id: msg._id }, msg
deleteMsgStream.on openedRooms[typeName].rid, (msg) ->
ChatMessage.remove _id: msg._id

deleteMsgStream.on openedRooms[typeName].rid, (msg) ->
ChatMessage.remove _id: msg._id

Dep.changed()
Dep.changed()

setRoomExpireExcept = (except) ->

Expand Down Expand Up @@ -126,6 +132,27 @@ Meteor.startup ->
room = openedRooms[typeName]
return room?.dom?

refreshDomOfRoom = (typeName, rid) ->
mainNode = document.querySelector('.main-content')
if mainNode?
for child in mainNode.children
mainNode.removeChild child if child?
roomDom = getDomOfRoom(typeName, rid)
mainNode.appendChild roomDom
if roomDom.classList.contains('room-container')
roomDom.querySelector('.messages-box > .wrapper').scrollTop = roomDom.oldScrollTop



removeDomOfRoom = () ->
mainNode = document.querySelector('.main-content')
if mainNode?
for child in mainNode.children
if child?
if child.classList.contains('room-container')
child.oldScrollTop = child.querySelector('.messages-box > .wrapper').scrollTop
mainNode.removeChild child

updateUserStatus = (user, status, utcOffset) ->
onlineUsersValue = onlineUsers.curValue

Expand All @@ -143,7 +170,10 @@ Meteor.startup ->
init: init
getDomOfRoom: getDomOfRoom
existsDomOfRoom: existsDomOfRoom
refreshDomOfRoom: refreshDomOfRoom
removeDomOfRoom: removeDomOfRoom
msgStream: msgStream
openedRooms: openedRooms
updateUserStatus: updateUserStatus
onlineUsers: onlineUsers
remove:remove
7 changes: 6 additions & 1 deletion client/routes/roomRoute.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ openRoom = (type, name) ->
FlowRouter.go 'home'
return

RoomManager.refreshDomOfRoom(type + name, room._id)
###
mainNode = document.querySelector('.main-content')
if mainNode?
for child in mainNode.children
Expand All @@ -33,6 +35,7 @@ openRoom = (type, name) ->
if roomDom.classList.contains('room-container')
roomDom.querySelector('.messages-box > .wrapper').scrollTop = roomDom.oldScrollTop
###
Session.set 'openedRoom', room._id

Session.set 'editRoomTitle', false
Expand All @@ -44,15 +47,17 @@ openRoom = (type, name) ->
$('.message-form .input-message').focus()
, 100


roomExit = ->
RoomManager.removeDomOfRoom()
###
mainNode = document.querySelector('.main-content')
if mainNode?
for child in mainNode.children
if child?
if child.classList.contains('room-container')
child.oldScrollTop = child.querySelector('.messages-box > .wrapper').scrollTop
mainNode.removeChild child
###


FlowRouter.route '/channel/:name',
Expand Down
30 changes: 28 additions & 2 deletions client/startup/roomObserve.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,33 @@ Meteor.startup ->
ChatRoom.find().observe
added: (data) ->
Session.set('roomData' + data._id, data)
changed: (data) ->
Session.set('roomData' + data._id, data)
changed: (newData, oldData) ->
handleRoomNameChanged( newData, oldData)
Session.set('roomData' + newData._id, null)
Session.set('roomData' + newData._id, newData)
removed: (data) ->
Session.set('roomData' + data._id, undefined)

handleRoomNameChanged = (newData, oldData) ->
if newData.t in ['p','c'] and newData.name isnt oldData.name
oldTypeName = oldData.t + oldData.name
RoomManager.remove oldTypeName
openedRoom = Session.get('openedRoom')
if openedRoom is newData._id
newTypeName = newData.t + newData.name
Tracker.autorun (c) ->
if RoomManager.open(newTypeName).ready() isnt true
return
c.stop()
RoomManager.removeDomOfRoom()
RoomManager.refreshDomOfRoom( newTypeName, newData._id )
Session.set 'openedRoom', newData._id
Session.set 'editRoomTitle', false
Meteor.call 'readMessages', newData._id if Meteor.userId()?
# KonchatNotification.removeRoomNotification(params._id)

if Meteor.Device.isDesktop()
setTimeout ->
$('.message-form .input-message').focus()
, 100

0 comments on commit 13fae7d

Please sign in to comment.