Refactor: Split up add_messages api.

We now have two functions:

    add_new_messages
    add_old_messages

This is a lot easier on the eyes, and it will also
prevent us from exceeding line length in future commits.

We also remove an unneeded stub in the narrow_activate
tests.
This commit is contained in:
Steve Howell 2019-01-08 00:26:02 +00:00 committed by Tim Abbott
parent 9a30b51e6e
commit 6f8da1bb27
7 changed files with 22 additions and 28 deletions

View File

@ -423,8 +423,7 @@ run_test('insert_message', () => {
assert.equal(message_store.get(new_message.id), undefined);
helper.redirect('activity', 'process_loaded_messages');
helper.redirect('message_util', 'add_messages');
helper.redirect('message_util', 'insert_new_messages');
helper.redirect('message_util', 'add_new_messages');
helper.redirect('notifications', 'received_messages');
helper.redirect('resize', 'resize_page_components');
helper.redirect('stream_list', 'update_streams_sidebar');
@ -440,8 +439,8 @@ run_test('insert_message', () => {
// the code invokes various objects when a new message
// comes in:
assert.deepEqual(helper.events, [
'message_util.add_messages',
'message_util.add_messages',
'message_util.add_new_messages',
'message_util.add_new_messages',
'activity.process_loaded_messages',
'unread_ui.update_unread_counts',
'resize.resize_page_components',
@ -474,7 +473,7 @@ run_test('insert_message', () => {
These are reflected by the following calls:
stream_list.update_streams_sidebar
message_util.add_messages
message_util.add_new_messages
activity.process_loaded_messages
For now, though, let's focus on another side effect

View File

@ -101,9 +101,9 @@ function config_process_results(messages) {
assert.deepEqual(arg, messages);
};
message_util.add_messages = function (new_messages, msg_list, opts) {
message_util.add_old_messages = function (new_messages, msg_list) {
assert.deepEqual(new_messages, messages);
msg_list.add_messages(new_messages, opts);
msg_list.add_messages(new_messages);
};
activity.process_loaded_messages = function (arg) {

View File

@ -87,12 +87,6 @@ function test_helper() {
blueslip.debug = noop;
message_util.add_messages = (messages, target_list, opts) => {
// The real function here doesn't do any more than this
// that we care about here.
target_list.add_messages(messages, opts);
};
return {
clear: () => {
events = [];

View File

@ -38,11 +38,7 @@ function maybe_add_narrowed_messages(messages, msg_list) {
// message. Arguably, it's counterproductive complexity.
new_messages = _.map(new_messages, message_store.add_message_metadata);
message_util.add_messages(
new_messages,
msg_list,
{messages_are_new: true}
);
message_util.add_new_messages(new_messages, msg_list);
unread_ops.process_visible();
notifications.notify_messages_outside_current_search(elsewhere_messages);
},
@ -66,22 +62,22 @@ exports.insert_new_messages = function insert_new_messages(messages, locally_ech
// message_list.all is a data-only list that we use to populate
// other lists, so we always update this
message_util.add_messages(messages, message_list.all, {messages_are_new: true});
message_util.add_new_messages(messages, message_list.all);
if (narrow_state.active()) {
// We do this NOW even though the home view is not active,
// because we want the home view to load fast later.
message_util.add_messages(messages, home_msg_list, {messages_are_new: true});
message_util.add_new_messages(messages, home_msg_list);
if (narrow_state.filter().can_apply_locally()) {
message_util.add_messages(messages, message_list.narrowed, {messages_are_new: true});
message_util.add_new_messages(messages, message_list.narrowed);
} else {
// if we cannot apply locally, we have to wait for this callback to happen to notify
maybe_add_narrowed_messages(messages, message_list.narrowed);
}
} else {
// we're in the home view, so update its list
message_util.add_messages(messages, home_msg_list, {messages_are_new: true});
message_util.add_new_messages(messages, home_msg_list);
}

View File

@ -43,11 +43,11 @@ function process_result(data, opts) {
// the message_list.all as well, as the home_msg_list is reconstructed
// from message_list.all.
if (opts.msg_list === home_msg_list) {
message_util.add_messages(messages, message_list.all, {messages_are_new: false});
message_util.add_old_messages(messages, message_list.all);
}
if (messages.length !== 0) {
message_util.add_messages(messages, opts.msg_list, {messages_are_new: false});
message_util.add_old_messages(messages, opts.msg_list);
}
activity.process_loaded_messages(messages);

View File

@ -8,17 +8,22 @@ exports.do_unread_count_updates = function do_unread_count_updates(messages) {
resize.resize_page_components();
};
exports.add_messages = function add_messages(messages, msg_list, opts) {
function add_messages(messages, msg_list, opts) {
if (!messages) {
return;
}
opts = _.extend({messages_are_new: false}, opts);
loading.destroy_indicator($('#page_loading_indicator'));
$('#first_run_message').remove();
msg_list.add_messages(messages, opts);
}
exports.add_old_messages = function (messages, msg_list) {
add_messages(messages, msg_list, {messages_are_new: false});
};
exports.add_new_messages = function (messages, msg_list) {
add_messages(messages, msg_list, {messages_are_new: true});
};

View File

@ -21,7 +21,7 @@ exports.update_in_home_view = function (sub, value) {
home_msg_list.clear({clear_selected_id: false});
// Recreate the home_msg_list with the newly filtered message_list.all
message_util.add_messages(message_list.all.all_messages(), home_msg_list);
message_util.add_old_messages(message_list.all.all_messages(), home_msg_list);
// Ensure we're still at the same scroll position
if (overlays.is_active()) {