presence: Convert presence_info to use user IDs.

When we filtered buddy lists, a recent change introduced some
bugs related to case-insensitive emails.  We now circumvent the
bug by indexing presence_info with user_ids.

frontend_tests/node_tests/activity.js

@@ -19,51 +19,57 @@ set_global('document', {
     }
 });
 
-global.people.add({
+var alice = {
     email: 'alice@zulip.com',
     user_id: 1,
     full_name: 'Alice Smith'
-});
+};
-global.people.add({
+var fred = {
     email: 'fred@zulip.com',
     user_id: 2,
     full_name: "Fred Flintstone"
-});
+};
-global.people.add({
+var jill = {
     email: 'jill@zulip.com',
     user_id: 3,
     full_name: 'Jill Hill'
-});
+};
-global.people.add({
+var mark = {
     email: 'mark@zulip.com',
     user_id: 4,
     full_name: 'Marky Mark'
-});
+};
-global.people.add({
+var norbert = {
     email: 'norbert@zulip.com',
     user_id: 5,
     full_name: 'Norbert Oswald'
-});
+};
+
+global.people.add(alice);
+global.people.add(fred);
+global.people.add(jill);
+global.people.add(mark);
+global.people.add(norbert);
+
 
 var activity = require('js/activity.js');
 
 activity.update_huddles = function () {};
 
 (function test_sort_users() {
-    var users = ['alice@zulip.com', 'fred@zulip.com', 'jill@zulip.com'];
+    var user_ids = [alice.user_id, fred.user_id, jill.user_id];
 
-    var user_info = {
+    var user_info = {};
-        'alice@zulip.com': {status: 'inactive'},
+    user_info[alice.user_id] = {status: 'inactive'};
-        'fred@zulip.com': {status: 'active'},
+    user_info[fred.user_id] = {status: 'active'};
-        'jill@zulip.com': {status: 'active'}
+    user_info[jill.user_id] = {status: 'active'};
-    };
 
-    activity._sort_users(users, user_info);
+    activity._sort_users(user_ids, user_info);
 
-    assert.deepEqual(users, [
+    assert.deepEqual(user_ids, [
-        'fred@zulip.com',
+        fred.user_id,
-        'jill@zulip.com',
+        jill.user_id,
-        'alice@zulip.com'
+        alice.user_id
     ]);
 }());
 
@@ -150,12 +156,11 @@ activity.update_huddles = function () {};
 (function test_huddle_fraction_present() {
     var huddle = 'alice@zulip.com,fred@zulip.com,jill@zulip.com,mark@zulip.com';
 
-    var presence_list = {
+    var presence_list = {};
-        'alice@zulip.com': {status: 'active'},
+    presence_list[alice.user_id] = {status: 'active'};
-        'fred@zulip.com': {status: 'idle'}, // counts as present
+    presence_list[fred.user_id] = {status: 'idle'}; // counts as present
-        // jill not in list
+    // jill not in list
-        'mark@zulip.com': {status: 'offline'} // does not count
+    presence_list[mark.user_id] = {status: 'offline'}; // does not count
-    };
 
     assert.equal(
         activity.huddle_fraction_present(huddle, presence_list),

frontend_tests/node_tests/presence_list_performance.js

@@ -37,41 +37,48 @@ global.compile_template('user_presence_rows');
 
 var people = require("js/people.js");
 var activity = require('js/activity.js');
-activity.presence_info = {
-    'alice@zulip.com': {status: activity.IDLE},
-    'fred@zulip.com': {status: activity.ACTIVE},
-    'jill@zulip.com': {status: activity.ACTIVE},
-    'mark@zulip.com': {status: activity.IDLE},
-    'norbert@zulip.com': {status: activity.ACTIVE}
-};
-
 
 // TODO: de-dup with activity.js
-global.people.add({
+var alice = {
     email: 'alice@zulip.com',
     user_id: 1,
     full_name: 'Alice Smith'
-});
+};
-global.people.add({
+var fred = {
     email: 'fred@zulip.com',
     user_id: 2,
     full_name: "Fred Flintstone"
-});
+};
-global.people.add({
+var jill = {
     email: 'jill@zulip.com',
     user_id: 3,
     full_name: 'Jill Hill'
-});
+};
-global.people.add({
+var mark = {
     email: 'mark@zulip.com',
     user_id: 4,
     full_name: 'Marky Mark'
-});
+};
-global.people.add({
+var norbert = {
     email: 'norbert@zulip.com',
     user_id: 5,
     full_name: 'Norbert Oswald'
-});
+};
+
+global.people.add(alice);
+global.people.add(fred);
+global.people.add(jill);
+global.people.add(mark);
+global.people.add(norbert);
+
+activity.presence_info = {};
+activity.presence_info[alice.user_id] = {status: activity.IDLE};
+activity.presence_info[fred.user_id] = {status: activity.ACTIVE};
+activity.presence_info[jill.user_id] = {status: activity.ACTIVE};
+activity.presence_info[mark.user_id] = {status: activity.IDLE};
+activity.presence_info[norbert.user_id] = {status: activity.ACTIVE};
+
+// console.info(activity.presence_info);
 
 (function test_presence_list_full_update() {
     var users = activity.update_users();
@@ -110,11 +117,12 @@ global.people.add({
 }());
 
 (function test_presence_list_partial_update() {
-    var users = {
+    activity.presence_info[alice.user_id] = {status: 'active'};
-        'alice@zulip.com': {status: 'active'}
+    activity.presence_info[mark.user_id] = {status: 'active'};
-    };
-    activity.presence_info['alice@zulip.com'] = users['alice@zulip.com'];
 
+    var users = {};
+
+    users[alice.user_id] = {status: 'active'};
     users = activity.update_users(users);
     assert.deepEqual(users, [
         { name: 'Alice Smith',
@@ -127,13 +135,11 @@ global.people.add({
 
     // Test if user index in presence_info is the expected one
     var all_users = activity._filter_and_sort(activity.presence_info);
-    assert.equal(all_users.indexOf('alice@zulip.com'), 0);
+    assert.equal(all_users.indexOf(alice.user_id.toString()), 0);
 
     // Test another user
-    users = {
+    users = {};
-        'mark@zulip.com': {status: 'active'}
+    users[mark.user_id] = {status: 'active'};
-    };
-    activity.presence_info['mark@zulip.com'] = users['mark@zulip.com'];
     users = activity.update_users(users);
     assert.deepEqual(users, [
         { name: 'Marky Mark',
@@ -145,6 +151,6 @@ global.people.add({
     ]);
 
     all_users = activity._filter_and_sort(activity.presence_info);
-    assert.equal(all_users.indexOf('mark@zulip.com'), 3);
+    assert.equal(all_users.indexOf(mark.user_id.toString()), 3);
 
 }());

static/js/activity.js

@@ -164,8 +164,9 @@ exports.huddle_fraction_present = function (huddle, presence_info) {
 
     var num_present = 0;
     _.each(emails, function (email) {
-        if (presence_info[email]) {
+        var user_id = people.get_user_id(email);
-            var status = presence_info[email].status;
+        if (presence_info[user_id]) {
+            var status = presence_info[user_id].status;
             if (status && (status !== 'offline')) {
                 ++num_present;
             }
@@ -177,9 +178,9 @@ exports.huddle_fraction_present = function (huddle, presence_info) {
     return ratio.toFixed(2);
 };
 
-function sort_users(users, presence_info) {
+function sort_users(user_ids, presence_info) {
     // TODO sort by unread count first, once we support that
-    users.sort(function (a, b) {
+    user_ids.sort(function (a, b) {
         if (presence_info[a].status === 'active' && presence_info[b].status !== 'active') {
             return -1;
         } else if (presence_info[b].status === 'active' && presence_info[a].status !== 'active') {
@@ -195,16 +196,16 @@ function sort_users(users, presence_info) {
         // Sort equivalent PM names alphabetically
         var full_name_a = a;
         var full_name_b = b;
-        if (people.get_by_email(a)) {
+        if (people.get_person_from_user_id(a)) {
-            full_name_a = people.get_by_email(a).full_name;
+            full_name_a = people.get_person_from_user_id(a).full_name;
         }
-        if (people.get_by_email(b)) {
+        if (people.get_person_from_user_id(b)) {
-            full_name_b = people.get_by_email(b).full_name;
+            full_name_b = people.get_person_from_user_id(b).full_name;
         }
         return util.strcmp(full_name_a, full_name_b);
     });
 
-    return users;
+    return user_ids;
 }
 
 // for testing:
@@ -218,19 +219,19 @@ function focus_lost() {
     exports.has_focus = false;
 }
 
-function filter_emails(emails) {
+function filter_user_ids(user_ids) {
     var user_list = $(".user-list-filter");
     if (user_list.length === 0) {
         // We may have received an activity ping response after
         // initiating a reload, in which case the user list may no
         // longer be available.
         // Return user list: useful for testing user list performance fix
-        return emails;
+        return user_ids;
     }
 
     var search_term = user_list.expectOne().val().trim();
     if (search_term === '') {
-        return emails;
+        return user_ids;
     }
 
     var search_terms = search_term.toLowerCase().split(",");
@@ -238,23 +239,22 @@ function filter_emails(emails) {
         return s.trim();
     });
 
-    var persons = _.map(emails, function (email) {
+    var persons = _.map(user_ids, function (user_id) {
-        return people.get_by_email(email);
+        return people.get_person_from_user_id(user_id);
     });
 
     var email_dict = people.filter_people_by_search_terms(persons, search_terms);
-    emails = _.keys(email_dict);
+    user_ids = _.map(_.keys(email_dict), function (email) {
-    return emails;
+        return people.get_user_id(email);
+    });
+    return user_ids;
 }
 
 function filter_and_sort(users) {
-    var emails = Object.keys(users);
+    var user_ids = Object.keys(users);
-    emails = _.filter(emails, function (email) {
+    user_ids = filter_user_ids(user_ids);
-        return people.get_by_email(email);
+    user_ids = sort_users(user_ids, exports.presence_info);
-    });
+    return user_ids;
-    emails = filter_emails(emails);
-    emails = sort_users(emails, exports.presence_info);
-    return emails;
 }
 
 exports._filter_and_sort = filter_and_sort;
@@ -283,15 +283,17 @@ exports.update_users = function (user_list) {
     // If you want to figure out how to get details for "me", then revert
     // the commit that added this comment.
 
-    function info_for(email) {
+    function info_for(user_id) {
-        var presence = exports.presence_info[email].status;
+        var presence = exports.presence_info[user_id].status;
+        var person = people.get_person_from_user_id(user_id);
+        var email = person.email;
         return {
-            name: people.get_by_email(email).full_name,
+            name: person.full_name,
             email: email,
             num_unread: get_num_unread(email),
             type: presence,
             type_desc: presence_descriptions[presence],
-            mobile: exports.presence_info[email].mobile
+            mobile: exports.presence_info[user_id].mobile
         };
     }
 
@@ -422,7 +424,12 @@ function focus_ping() {
             // Ping returns the active peer list
             _.each(data.presences, function (presence, this_email) {
                 if (!util.is_current_user(this_email)) {
-                    exports.presence_info[this_email] = status_from_timestamp(data.server_timestamp, presence);
+                    var user_id = people.get_user_id(this_email);
+                    if (user_id) {
+                        var status = status_from_timestamp(data.server_timestamp,
+                                                           presence);
+                        exports.presence_info[user_id] = status;
+                    }
                 }
             });
             exports.update_users();
@@ -470,8 +477,13 @@ exports.set_user_statuses = function (users, server_time) {
             return;
         }
         status = status_from_timestamp(server_time, presence);
-        exports.presence_info[email] = status;
+        var user_id = people.get_user_id(email);
-        updated_users[email] = status;
+        if (user_id) {
+            exports.presence_info[user_id] = status;
+            updated_users[user_id] = status;
+        } else {
+            blueslip.warn('unknown email: ' + email);
+        }
     });
 
     exports.update_users(updated_users);