refactor: Eliminate presence_info params.

It's easier to reason about the activity.js code if we just
make it clear that presence_info is a global singleton.  Going
forward, functions that use that data will explicitly refer to
exports.presence_info.

This change makes some tests slightly more awkward to set up, but
with the actual code you now don't have to question whether there
are multiple copies of presence_info to maintain.

The diff for compare_function is a bit confusing to read, but it's
basically just de-dented since we don't need to parameterize the
compare function any more.
This commit is contained in:
Steve Howell
2017-03-06 07:14:52 -08:00
committed by Tim Abbott
parent 89d362656a
commit a246d2dcb3
2 changed files with 42 additions and 41 deletions

View File

@@ -90,12 +90,13 @@ global.compile_template('user_presence_rows');
(function test_sort_users() {
var user_ids = [alice.user_id, fred.user_id, jill.user_id];
var user_info = {};
user_info[alice.user_id] = { status: 'inactive' };
user_info[fred.user_id] = { status: 'active' };
user_info[jill.user_id] = { status: 'active' };
var presence_info = {};
presence_info[alice.user_id] = { status: 'inactive' };
presence_info[fred.user_id] = { status: 'active' };
presence_info[jill.user_id] = { status: 'active' };
activity._sort_users(user_ids, user_info);
activity.presence_info = presence_info;
activity._sort_users(user_ids);
assert.deepEqual(user_ids, [
fred.user_id,
@@ -193,14 +194,15 @@ global.compile_template('user_presence_rows');
var huddle = 'alice@zulip.com,fred@zulip.com,jill@zulip.com,mark@zulip.com';
huddle = people.emails_strings_to_user_ids_string(huddle);
var presence_list = {};
presence_list[alice.user_id] = { status: 'active' };
presence_list[fred.user_id] = { status: 'idle' }; // counts as present
var presence_info = {};
presence_info[alice.user_id] = { status: 'active' };
presence_info[fred.user_id] = { status: 'idle' }; // counts as present
// jill not in list
presence_list[mark.user_id] = { status: 'offline' }; // does not count
presence_info[mark.user_id] = { status: 'offline' }; // does not count
activity.presence_info = presence_info;
assert.equal(
activity.huddle_fraction_present(huddle, presence_list),
activity.huddle_fraction_present(huddle),
'0.50');
}());

View File

@@ -160,7 +160,8 @@ exports.short_huddle_name = function (huddle) {
return names.join(', ');
};
exports.huddle_fraction_present = function (huddle, presence_info) {
exports.huddle_fraction_present = function (huddle) {
var presence_info = exports.presence_info;
var user_ids = huddle.split(',');
var num_present = 0;
@@ -178,8 +179,9 @@ exports.huddle_fraction_present = function (huddle, presence_info) {
return ratio.toFixed(2);
};
function get_compare_function(presence_info) {
return function (a, b) {
function compare_function(a, b) {
var presence_info = exports.presence_info;
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') {
@@ -202,12 +204,11 @@ function get_compare_function(presence_info) {
full_name_b = people.get_person_from_user_id(b).full_name;
}
return util.strcmp(full_name_a, full_name_b);
};
}
function sort_users(user_ids, presence_info) {
function sort_users(user_ids) {
// TODO sort by unread count first, once we support that
user_ids.sort(get_compare_function(presence_info));
user_ids.sort(compare_function);
return user_ids;
}
@@ -258,7 +259,7 @@ function matches_filter(user_id) {
function filter_and_sort(users) {
var user_ids = Object.keys(users);
user_ids = filter_user_ids(user_ids);
user_ids = sort_users(user_ids, exports.presence_info);
user_ids = sort_users(user_ids);
return user_ids;
}
@@ -300,15 +301,13 @@ exports.insert_user_into_list = function (user_id) {
var items = $('#user_presences li').toArray();
var compare = get_compare_function(exports.presence_info);
function insert() {
var i = 0;
for (i = 0; i < items.length; i += 1) {
var li = $(items[i]);
var list_user_id = li.attr('data-user-id');
if (compare(user_id, list_user_id) < 0) {
if (compare_function(user_id, list_user_id) < 0) {
li.before(html);
return;
}
@@ -375,7 +374,7 @@ exports.update_huddles = function () {
user_ids_string: huddle,
name: exports.full_huddle_name(huddle),
href: narrow.huddle_with_uri(huddle),
fraction_present: exports.huddle_fraction_present(huddle, exports.presence_info),
fraction_present: exports.huddle_fraction_present(huddle),
short_name: exports.short_huddle_name(huddle),
};
});