From bf14f4cd7b2ddc2bd01ac5b64a7250745d11c2d3 Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Tue, 12 Feb 2019 07:55:26 +0530 Subject: [PATCH] notification: Show wrong narrow notification for non locally echoed message. Show "sent to different narrow" notification and other such notification by notifications.notify_local_mixes for non locally echoed message sent by current client. With significant new comments added by tabbott. Fixes: #11488. --- frontend_tests/node_tests/server_events.js | 1 + static/js/message_events.js | 3 +++ static/js/notifications.js | 9 ++++++++- static/js/server_events.js | 18 +++++++++++++++++- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/frontend_tests/node_tests/server_events.js b/frontend_tests/node_tests/server_events.js index 225aae2a73..f635ae994e 100644 --- a/frontend_tests/node_tests/server_events.js +++ b/frontend_tests/node_tests/server_events.js @@ -9,6 +9,7 @@ global.stub_out_jquery(); zrequire('message_store'); zrequire('server_events_dispatch'); zrequire('server_events'); +zrequire('sent_messages'); set_global('blueslip', global.make_zblueslip()); set_global('channel', {}); diff --git a/static/js/message_events.js b/static/js/message_events.js index 4f0c69089f..e12f697410 100644 --- a/static/js/message_events.js +++ b/static/js/message_events.js @@ -85,6 +85,9 @@ exports.insert_new_messages = function insert_new_messages(messages, sent_by_thi if (sent_by_this_client) { var need_user_to_scroll = render_info && render_info.need_user_to_scroll; + // sent_by_this_client will be true if ANY of the messages + // were sent by this client; notifications.notify_local_mixes + // will filter out any not sent by us. notifications.notify_local_mixes(messages, need_user_to_scroll); } diff --git a/static/js/notifications.js b/static/js/notifications.js index 785bb5108e..6df04e19af 100644 --- a/static/js/notifications.js +++ b/static/js/notifications.js @@ -598,11 +598,18 @@ exports.notify_local_mixes = function (messages, need_user_to_scroll) { checkable locally, so we may want to execute this code earlier in the codepath at some point and possibly punt on local rendering. + + Possible cleanup: Arguably, we should call this function + unconditionally and just check if message.local_id is in + sent_messages.messages here. */ _.each(messages, function (message) { if (!people.is_my_user_id(message.sender_id)) { - blueslip.warn('We did not expect messages sent by others to get here'); + // This can happen if the client is offline for a while + // around the time this client sends a message; see the + // caller of message_events.insert_new_messages. + blueslip.info('Slightly unexpected: A message not sent by us batches with those that were.'); return; } diff --git a/static/js/server_events.js b/static/js/server_events.js index d8ade3d8c4..a97ef32254 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -104,7 +104,23 @@ function get_events_success(events) { try { messages = echo.process_from_server(messages); _.each(messages, message_store.set_message_booleans); - message_events.insert_new_messages(messages); + var sent_by_this_client = false; + _.each(messages, function (msg) { + var msg_state = sent_messages.messages[msg.local_id]; + if (msg_state) { + // Almost every time, this message will be the + // only one in messages, because multiple messages + // being returned by get_events usually only + // happens when a client is offline, but we know + // this client just sent a message in this batch + // of events. But in any case, + // insert_new_messages handles multiple messages, + // only one of which was sent by this client, + // correctly. + sent_by_this_client = true; + } + }); + message_events.insert_new_messages(messages, sent_by_this_client); } catch (ex2) { blueslip.error('Failed to insert new messages\n' + blueslip.exception_msg(ex2),