paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-06 23:13:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

message events: Refactor remove_and_rerender codepath.

Browse Source 
The changes made in this commit are as follows:

* We remove the now unused `ui.find_message` which was added
  in commit 1666403850.

* We change the function paramter to now accept message ids
  instead of messages to eliminate redundant message ids to
  message convertion as only the id is required.

* The remove method in MessageListData did not remove the
  messages from the hash, it removed only from the items,
  this fixes it.

* This commit also fixes a bug where messages are not added
  to the current message list if an event is recieved where
  messages are moved to this current narrow.
  Only the message removal logic was present, which has been
  refactored in this commit.
This commit is contained in:
ryanreh99
2020-11-13 02:33:45 +05:30
committed by Tim Abbott
parent 4d458eeb49
commit 59e5f2d8fc
7 changed files with 38 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
frontend_tests/node_tests/message_list.js
View File

@@ -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);
});

4
frontend_tests/node_tests/message_list_data.js
View File

@@ -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]));

2
static/js/echo.js
View File

@@ -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]);
}
}

44
static/js/message_events.js
View File

@@ -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);
}
}

4
static/js/message_list.js
View File

@@ -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();
}

25
static/js/message_list_data.js
View File

@@ -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);
}
}

24
static/js/ui.js
View File

@@ -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);