diff --git a/frontend_tests/node_tests/message_list.js b/frontend_tests/node_tests/message_list.js index 97d377a6cf..955cd38bd4 100644 --- a/frontend_tests/node_tests/message_list.js +++ b/frontend_tests/node_tests/message_list.js @@ -113,7 +113,7 @@ run_test("basics", () => { list.view.clear_table = function () {}; - list.remove_and_rerender([{id: 60}]); + list.remove_and_rerender([60]); const removed = list.all_messages().filter((msg) => msg.id !== 60); assert.deepEqual(list.all_messages(), removed); @@ -410,7 +410,8 @@ run_test("add_remove_rerender", () => { global.with_stub((stub) => { list.rerender = stub.f; - list.remove_and_rerender(messages); + const message_ids = messages.map((msg) => msg.id); + list.remove_and_rerender(message_ids); assert.equal(stub.num_calls, 1); assert.equal(list.num_items(), 0); }); diff --git a/frontend_tests/node_tests/message_list_data.js b/frontend_tests/node_tests/message_list_data.js index cea13f3fe6..88c57fe096 100644 --- a/frontend_tests/node_tests/message_list_data.js +++ b/frontend_tests/node_tests/message_list_data.js @@ -63,7 +63,7 @@ run_test("basics", () => { assert.equal(mld.selected_id(), 50); assert.equal(mld.selected_idx(), 8); - mld.remove([mld.get(50)]); + mld.remove([50]); assert_contents(mld, [10, 15, 20, 25, 30, 35, 40, 45, 60, 70]); mld.update_items_for_muting(); @@ -116,7 +116,7 @@ run_test("muting enabled", () => { mld.update_items_for_muting(); assert.deepEqual(mld._items, [mld.get(35)]); - mld.remove(make_msgs([35, 15])); + mld.remove([35, 15]); assert_contents(mld, []); assert.deepEqual(mld._all_items, make_msgs([25, 45])); diff --git a/static/js/echo.js b/static/js/echo.js index 0d86406bb6..2d970fcaba 100644 --- a/static/js/echo.js +++ b/static/js/echo.js @@ -362,7 +362,7 @@ exports.message_send_error = function (message_id, error_response) { function abort_message(message) { // Remove in all lists in which it exists for (const msg_list of [message_list.all, home_msg_list, current_msg_list]) { - msg_list.remove_and_rerender([message]); + msg_list.remove_and_rerender([message.id]); } } diff --git a/static/js/message_events.js b/static/js/message_events.js index 9e27bd3246..917965c80d 100644 --- a/static/js/message_events.js +++ b/static/js/message_events.js @@ -161,8 +161,12 @@ exports.update_messages = function update_messages(events) { const compose_stream_name = compose_state.stream_name(); const orig_topic = util.get_edit_event_orig_topic(event); + const current_filter = narrow_state.filter(); const current_selected_id = current_msg_list.selected_id(); const selection_changed_topic = event.message_ids.includes(current_selected_id); + const event_messages = event.message_ids.map((id) => message_store.get(id)); + // The event.message_ids received from the server are not in sorted order. + event_messages.sort((a, b) => a.id - b.id); if (going_forward_change && stream_name && compose_stream_name) { if (stream_name.toLowerCase() === compose_stream_name.toLowerCase()) { @@ -174,10 +178,7 @@ exports.update_messages = function update_messages(events) { } } - const current_filter = narrow_state.filter(); - const messages_to_rerender = []; - for (const id of event.message_ids) { - const msg = message_store.get(id); + for (const msg of event_messages) { if (msg === undefined) { continue; } @@ -221,21 +222,6 @@ exports.update_messages = function update_messages(events) { topic_name: msg.topic, message_id: msg.id, }); - - if ( - current_filter && - current_filter.can_apply_locally() && - !current_filter.predicate()(msg) - ) { - // This topic edit makes this message leave the - // current narrow, which is not being changed as - // part of processing this event. So we should - // remove the message from the current/narrowed message list. - const cur_row = current_msg_list.get_row(id); - if (cur_row !== undefined) { - messages_to_rerender.push({id}); - } - } } if (going_forward_change) { @@ -294,8 +280,9 @@ exports.update_messages = function update_messages(events) { } } - // Ensure messages that are no longer part of this narrow - // are deleted from the message_list. + // Ensure messages that are no longer part of this + // narrow are deleted and messages that are now part + // of this narrow are added to the message_list. // // Even if we end up renarrowing, the message_list_data // part of this is important for non-rendering message @@ -303,8 +290,19 @@ exports.update_messages = function update_messages(events) { // this should be a loop over all valid message_list_data // objects, without the rerender (which will naturally // happen in the following code). - if (!changed_narrow && messages_to_rerender.length > 0) { - current_msg_list.remove_and_rerender(messages_to_rerender); + if (!changed_narrow) { + let message_ids_to_remove = []; + if (current_filter && current_filter.can_apply_locally()) { + const predicate = current_filter.predicate(); + message_ids_to_remove = event_messages.filter((msg) => !predicate(msg)); + message_ids_to_remove = message_ids_to_remove.map((msg) => msg.id); + } + // We filter out messages that do not belong to the message + // list and then pass these to the remove messages codepath. + // While we can pass all our messages to the add messages + // codepath as the filtering is done within the method. + current_msg_list.remove_and_rerender(message_ids_to_remove); + current_msg_list.add_messages(event_messages); } } diff --git a/static/js/message_list.js b/static/js/message_list.js index a7a731161b..5db04b810c 100644 --- a/static/js/message_list.js +++ b/static/js/message_list.js @@ -295,8 +295,8 @@ class MessageList { return render_info; } - remove_and_rerender(messages) { - this.data.remove(messages); + remove_and_rerender(message_ids) { + this.data.remove(message_ids); this.rerender(); } diff --git a/static/js/message_list_data.js b/static/js/message_list_data.js index 78ed2896e1..0c3f34389e 100644 --- a/static/js/message_list_data.js +++ b/static/js/message_list_data.js @@ -297,26 +297,17 @@ class MessageListData { return viewable_messages; } - remove(messages) { - for (const message of messages) { - const stored_message = this._hash.get(message.id); - if (stored_message !== undefined) { - this._hash.delete(stored_message); - } - this._local_only.delete(message.id); + remove(message_ids) { + const msg_ids_to_remove = new Set(message_ids); + for (const id of msg_ids_to_remove) { + this._hash.delete(id); + this._local_only.delete(id); } - const msg_ids_to_remove = new Set(); - - for (const message of messages) { - msg_ids_to_remove.add(message.id); - } - - this._items = this._items.filter((message) => !msg_ids_to_remove.has(message.id)); + const remove_messages = (msg) => !msg_ids_to_remove.has(msg.id); + this._items = this._items.filter(remove_messages); if (this.muting_enabled) { - this._all_items = this._all_items.filter( - (message) => !msg_ids_to_remove.has(message.id), - ); + this._all_items = this._all_items.filter(remove_messages); } } diff --git a/static/js/ui.js b/static/js/ui.js index a20d80981f..aedf9ab901 100644 --- a/static/js/ui.js +++ b/static/js/ui.js @@ -76,21 +76,6 @@ exports.show_error_for_unsupported_platform = function () { } }; -exports.find_message = function (message_id) { - // Try to find the message object. It might be in the narrow list - // (if it was loaded when narrowed), or only in the message_list.all - // (if received from the server while in a different narrow) - let message; - - for (const msg_list of [message_list.all, home_msg_list, message_list.narrowed]) { - if (msg_list !== undefined && message === undefined) { - message = msg_list.get(message_id); - } - } - - return message; -}; - exports.update_starred_view = function (message_id, new_value) { const starred = new_value; @@ -121,18 +106,11 @@ exports.show_message_failed = function (message_id, failed_msg) { }; exports.remove_messages = function (message_ids) { - const msg_ids_to_rerender = []; for (const list of [message_list.all, home_msg_list, message_list.narrowed]) { if (list === undefined) { continue; } - for (const message_id of message_ids) { - const row = list.get_row(message_id); - if (row !== undefined) { - msg_ids_to_rerender.push({id: message_id}); - } - } - list.remove_and_rerender(msg_ids_to_rerender); + list.remove_and_rerender(message_ids); } recent_senders.update_topics_of_deleted_message_ids(message_ids); recent_topics.update_topics_of_deleted_message_ids(message_ids);