paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 21:43:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

message_events: Change narrow after updating local data.

Browse Source 
On update_message events, we were changing narrow before we
locally updated the data, this resulted in a weird mismatch
between locally available data and that fetched from the server.
Ideally, we should not be requesting any data from the server
in most scenarios since the messages for new narrow is
locally available.

As a result, the new narrow didn't have any messages other than
a breadcrumb message. To fix this, we change narrow post
locally updating the data.

The original bug was not exactly reproduced, but a similar version
of it was simulated and was found to be fixed.

Tweaked by tabbott to preserve an optimization.
This commit is contained in:
Aman Agrawal
2020-06-14 13:43:14 +05:30
committed by Tim Abbott
parent e60771020a
commit 8a163840f4
1 changed files with 65 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
static/js/message_events.js
View File

@@ -170,6 +170,56 @@ 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);
if (msg === undefined) {
continue;
}
// Remove the recent topics entry for the old topics;
// must be called before we call set_message_topic.
stream_topic_history.remove_message({
stream_id: msg.stream_id,
topic_name: msg.topic,
});
// Update the unread counts; again, this must be called
// before we modify the topic field on the message.
unread.update_unread_topics(msg, event);
// Now edit the attributes of our message object.
if (topic_edited) {
msg.topic = new_topic;
msg.topic_links = event.topic_links;
}
if (stream_changed) {
const new_stream_name = stream_data.get_sub_by_id(new_stream_id).name;
msg.stream_id = event.new_stream_id;
msg.stream = new_stream_name;
msg.display_recipient = new_stream_name;
}
// Add the recent topics entry for the new stream/topics.
stream_topic_history.add_message({
stream_id: msg.stream_id,
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: id});
}
}
}
if (going_forward_change) {
// This logic is a bit awkward. What we're trying to
// accomplish is two things:
@@ -210,7 +260,10 @@ exports.update_messages = function update_messages(events) {
});
changed_narrow = true;
}
// NOTE: We should always be changing narrows after we finish
// updating the local data and UI. This avoids conflict
// with data fetched from the server (which is already updated)
// when we move to new narrow and what data is locally available.
if (changed_narrow) {
const operators = new_filter.operators();
const opts = {
@@ -218,63 +271,24 @@ exports.update_messages = function update_messages(events) {
then_select_id: current_selected_id,
};
narrow.activate(operators, opts);
changed_narrow = true;
}
}
}
}
const messages_to_rerender = [];
for (const id of event.message_ids) {
const msg = message_store.get(id);
if (msg === undefined) {
continue;
}
// Remove the recent topics entry for the old topics;
// must be called before we call set_message_topic.
stream_topic_history.remove_message({
stream_id: msg.stream_id,
topic_name: msg.topic,
});
// Update the unread counts; again, this must be called
// before we modify the topic field on the message.
unread.update_unread_topics(msg, event);
// Now edit the attributes of our message object.
if (topic_edited) {
msg.topic = new_topic;
msg.topic_links = event.topic_links;
}
if (stream_changed) {
const new_stream_name = stream_data.get_sub_by_id(new_stream_id).name;
msg.stream_id = event.new_stream_id;
msg.stream = new_stream_name;
msg.display_recipient = new_stream_name;
}
// Add the recent topics entry for the new stream/topics.
stream_topic_history.add_message({
stream_id: msg.stream_id,
topic_name: msg.topic,
message_id: msg.id,
});
if (!changed_narrow && 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: id});
}
}
}
// Ensure messages that are no longer part of this narrow
// are deleted from the message_list.
//
// Even if we end up renarrowing, the message_list_data
// part of this is important for non-rendering message
// lists, so we do this unconditionally. Most correctly,
// 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 (event.orig_content !== undefined) {
if (page_params.realm_allow_edit_history) {