search: Remove buggy double-call of set_message_booleans.

In a refactor last fall, we changed `set_message_booleans` to mutate
state (specifically, destroying msg.flags in favor of setting
properties like `msg.unread`).  This was fine for most code paths, but
the maybe_add_narrowed_messages code path called
`message_store.add_message_metadata` twice (once after talking to the
server to find out whether the messages go into the current narrow),
and so when we extracted set_message_booleans from that, the second
call didn't properly short-circuit.

We fix this by just removing the second call, and also add a comment
warning about the add_message_metadata call there as being dangerous.

Fixes #8184.
This commit is contained in:
Tim Abbott 2018-04-28 14:32:37 -07:00
parent e087be6630
commit 9729b1a4ad
1 changed files with 7 additions and 1 deletions

View File

@ -31,8 +31,14 @@ function maybe_add_narrowed_messages(messages, msg_list, messages_are_new) {
}
});
_.each(new_messages, message_store.set_message_booleans);
// This second call to add_message_metadata in the
// insert_new_messages code path helps in very rare race
// conditions, where e.g. the current user's name was
// edited in between when they sent the message and when
// we hear back from the server and can echo the new
// message. Arguably, it's counterproductive complexity.
new_messages = _.map(new_messages, message_store.add_message_metadata);
message_util.add_messages(
new_messages,
msg_list,