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

Updated member list to show offline members also. #42

Merged
merged 7 commits into from
Aug 13, 2015
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions client/lib/RoomManager.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Meteor.startup ->
subscription = null
msgStream = new Meteor.Stream 'messages'
deleteMsgStream = new Meteor.Stream 'delete-message'
onlineUsers = new ReactiveVar {}
allUsers = new ReactiveVar {}

Dep = new Tracker.Dependency

Expand Down Expand Up @@ -105,16 +105,14 @@ Meteor.startup ->
return room?.dom?

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

Choose a reason for hiding this comment

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

Does .curValue return the same thing as .get() for a reactive var?

Copy link
Author

Choose a reason for hiding this comment

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

I dunno. I just changed their currrent one from online. You're much more familiar with it.

allUsersValue[user.username] =
firstName: user.profile.first_name
lastName: user.profile.last_name
name: user.name
status: user.status

Choose a reason for hiding this comment

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

This isn't a bug here, but rather in how we're populating our users collection from LDAP. We're not populating a 'status' field on startup, so only users that have logged in before have it set.

The list of users on the right pane will use this field to determine how to style the name and the dot next to it (e.g., green == available, red == away). Normally, if a user is offline, it will appear as gray, making it obvious that the user is offline. However, in our case, if a user hasn't logged in before, the field is missing, so the name shows up in black with no dot.

I'll file an actual bug ticket about this. If we want, we can also work around it here by setting 'status' to:

status: if user.status? then user.status else 'offline'

but probably isn't worth it. Thoughts?

Choose a reason for hiding this comment

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

Ha ha! Looks like Reid made this a non-issue with pull request #50!

Copy link
Author

Choose a reason for hiding this comment

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

Yep. Fixed it while we were sleeping!


if status is 'offline'
delete onlineUsersValue[user.username]
else
onlineUsersValue[user.username] =
status: status
utcOffset: utcOffset

onlineUsers.set onlineUsersValue
allUsers.set allUsersValue

open: open
close: close
Expand All @@ -124,4 +122,4 @@ Meteor.startup ->
msgStream: msgStream
openedRooms: openedRooms
updateUserStatus: updateUserStatus
onlineUsers: onlineUsers
allUsers: allUsers
2 changes: 1 addition & 1 deletion client/startup/usersObserve.coffee
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Meteor.startup ->
Meteor.users.find({}, { fields: { name: 1, username: 1, pictures: 1, status: 1, emails: 1, phone: 1, services: 1, utcOffset: 1 } }).observe
Meteor.users.find({}, { fields: { 'profile.first_name': 1, 'profile.last_name': 1, name: 1, username: 1, pictures: 1, status: 1, emails: 1, phone: 1, services: 1, utcOffset: 1 } }).observe
added: (user) ->
Session.set('user_' + user.username + '_status', user.status)
RoomManager.updateUserStatus user, user.status, user.utcOffset
Expand Down
5 changes: 2 additions & 3 deletions client/stylesheets/base.less
Original file line number Diff line number Diff line change
Expand Up @@ -2605,9 +2605,8 @@ a.github-fork {
text-overflow: ellipsis;
position: relative;
padding-left: 10px;
color: @secondary-font-color;
.calc(width,
~"100% - 45px");
//color: @secondary-font-color;

Choose a reason for hiding this comment

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

Do we want the whole name colored according to status, or just rely on the dot on the left of every name entry?

Copy link
Author

Choose a reason for hiding this comment

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

I thought the whole name because the dot is so small. And I like it better.

Choose a reason for hiding this comment

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

Works for me. It definitely makes it super clear/intuitive to tell when a user is offline.

.calc(width, ~"100% - 45px");
}
}
a {
Expand Down
36 changes: 14 additions & 22 deletions client/views/app/room.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Template.room.helpers
return roomData.u?._id is Meteor.userId() and roomData.t in ['c', 'p']

canPrivateMsg: ->
console.og 'room.helpers canPrivateMsg' if window.rocketDebug
console.log 'room.helpers canPrivateMsg' if window.rocketDebug

Choose a reason for hiding this comment

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

I prefer console.og better ;)

Copy link
Author

Choose a reason for hiding this comment

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

I think someone used a url shortener while programming.

return Meteor.userId() isnt this.username

roomNameEdit: ->
Expand Down Expand Up @@ -182,33 +182,25 @@ Template.room.helpers
status = Session.get 'user_' + username + '_status'
if status in ['online', 'away', 'busy']
return {username: username, status: status}
return
return {username: username, status: 'offline'}

roomUsers: ->
room = ChatRoom.findOne(this._id, { reactive: false })
users = []
onlineUsers = RoomManager.onlineUsers.get()
allUsers = RoomManager.allUsers.get()

for username in room?.usernames or []
if onlineUsers[username]?
utcOffset = onlineUsers[username]?.utcOffset
if utcOffset?
if utcOffset > 0
utcOffset = "+#{utcOffset}"
users.push
firstName: allUsers[username]?.firstName
lastName: allUsers[username]?.lastName
name: allUsers[username]?.name
username: username
status: allUsers[username]?.status

utcOffset = "(UTC #{utcOffset})"

users.push
username: username
status: onlineUsers[username]?.status
utcOffset: utcOffset

users = _.sortBy users, 'username'
users = _.sortBy users, 'lastName'

ret =
_id: this._id
total: room.usernames.length
totalOnline: users.length
users: users

Choose a reason for hiding this comment

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

Nice. The UTC time offset was kinda cool, possibly useful-ish, but it definitely cluttered things up over there.

return ret
Expand Down Expand Up @@ -292,7 +284,7 @@ Template.room.helpers

securityBannerText: ->
return this.get 'text'

maxMessageLength: ->
return RocketChat.settings.get('Message_MaxAllowedSize')

Expand Down Expand Up @@ -420,15 +412,15 @@ Template.room.events

'click .user-view nav .pvt-msg': (e) ->
console.log 'room click .user-view nav .pvt-msg' if window.rocketDebug
# determine if we're creating new room or opening existing room. DM uses
# determine if we're creating new room or opening existing room. DM uses
# usernames for room id
me = Meteor.user().username
to = Session.get('showUserInfo')
rid = [me, to].sort().join('')
if ChatSubscription.findOne({rid:rid})
# conversation already exists
FlowRouter.go('room', {_id: rid})
else
else
# close side nav if it's open
if SideNav.flexStatus
SideNav.setFlex null
Expand Down Expand Up @@ -587,7 +579,7 @@ Template.room.onCreated ->
self = this
# this.scrollOnBottom = true
# this.typing = new msgTyping this.data._id
this.showUsersOffline = new ReactiveVar false
this.showUsersOffline = new ReactiveVar true
this.atBottom = true

this.bannerData = new ReactiveDict
Expand Down
10 changes: 3 additions & 7 deletions client/views/app/room.html
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,14 @@ <h2>
<div class="list-view{{#if $.Session.get 'showUserInfo'}} -hidden{{/if}}">
{{#with roomUsers}}
<div class="status">
<h2>{{_ "Members_List"}}</h2> {{!--
<p>
{{{_ "Showing_online_users" total_online=totalOnline total=total}}}
<button class="see-all">{{seeAll}}</button>
</p>--}}
<h2>{{_ "Members_List"}}</h2>
</div>
<ul class='list clearfix lines'>
{{#each users}}
<li class='user-image user-card-room status-{{status}}'>
<a data-username="{{username}}" tabindex="0" title="{{username}}">
<a data-username="{{username}}" tabindex="0" title="{{username}}" class="status-{{status}}">
{{> avatar username=username}}
<p>{{username}} {{utcOffset}}</p>
<p class="status-{{status}}">{{firstName}} {{lastName}}</p>
</a>
</li>
{{/each}}
Expand Down
11 changes: 10 additions & 1 deletion server/publications/userData.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,19 @@ Meteor.publish 'userData', ->

console.log '[publish] userData'.green

Meteor.users.find this.userId,
###
Please note this returns the information for all users back to the client.
Make sure to not add any more fields that are sensitive like access inside
the profile or the entire profile object which would contain the access.
###
Meteor.users.find {},
fields:
name: 1
'profile.first_name': 1
'profile.last_name': 1
username: 1
emails: 1
phone: 1
status: 1
statusDefault: 1
statusConnection: 1
Expand Down