From 2d766f3e7874bd46f7b3eba8c245ee176f4feaa3 Mon Sep 17 00:00:00 2001 From: Swati Bhageria Date: Thu, 9 Dec 2021 12:10:26 -0500 Subject: [PATCH] message list: Display MOVED for messages with only topic edits. This avoids the somewhat confusing visuals of showing messages as EDITED where the content had not been changed, which also obscured situations where a message had both been edited and moved. It's possible we could do better with some sort of fancier block-move visual styling, but it's a bit tricky to do well given that we support moving multiple messages at once. Fixes #20451. --- .../node_tests/message_list_view.js | 109 ++++++++++++++++++ static/js/message_list_view.js | 19 +++ static/templates/edited_notice.hbs | 12 +- 3 files changed, 137 insertions(+), 3 deletions(-) diff --git a/frontend_tests/node_tests/message_list_view.js b/frontend_tests/node_tests/message_list_view.js index 904cc410f9..54f6bf66a5 100644 --- a/frontend_tests/node_tests/message_list_view.js +++ b/frontend_tests/node_tests/message_list_view.js @@ -58,6 +58,115 @@ function test(label, f) { }); } +test("msg_moved_var", () => { + // This is a test to verify that when the stream or topic is changed + // (and the content is not), the message says "MOVED" rather than "EDITED." + // See the end of the test for the list of cases verified. + + function build_message_context(message = {}, message_context = {}) { + message_context = { + ...message_context, + }; + message_context.msg = { + last_edit_timestamp: (next_timestamp += 1), + ...message, + }; + return message_context; + } + + function build_message_group(messages) { + return {message_containers: messages}; + } + + function build_list(message_groups) { + const list = new MessageListView(undefined, undefined, true); + list._message_groups = message_groups; + return list; + } + + function assert_moved_true(message_container) { + assert.equal(message_container.moved, true); + } + function assert_moved_false(message_container) { + assert.equal(message_container.moved, false); + } + + (function test_msg_moved_var() { + const messages = [ + // no edits: Not moved. + build_message_context(), + // stream changed: Move + build_message_context({ + edit_history: [{prev_stream: "test_stream", timestamp: 1000, user_id: 1}], + }), + // topic changed: Move + build_message_context({ + edit_history: [{prev_subject: "test_topic", timestamp: 1000, user_id: 1}], + }), + // content edited: Edit + build_message_context({ + edit_history: [{prev_content: "test_content", timestamp: 1000, user_id: 1}], + }), + // stream and topic edited: Move + build_message_context({ + edit_history: [ + {prev_stream: "test_stream", timestamp: 1000, user_id: 1}, + {prev_topic: "test_topic", timestamp: 1000, user_id: 1}, + ], + }), + // topic and content changed: Edit + build_message_context({ + edit_history: [ + {prev_topic: "test_topic", timestamp: 1000, user_id: 1}, + {prev_content: "test_content", timestamp: 1001, user_id: 1}, + ], + }), + // stream and content changed: Edit + build_message_context({ + edit_history: [ + {prev_content: "test_content", timestamp: 1000, user_id: 1}, + {prev_stream: "test_stream", timestamp: 1001, user_id: 1}, + ], + }), + // topic, stream, and content changed: Edit + build_message_context({ + edit_history: [ + {prev_topic: "test_topic", timestamp: 1000, user_id: 1}, + {prev_stream: "test_stream", timestamp: 1001, user_id: 1}, + {prev_content: "test_content", timestamp: 1002, user_id: 1}, + ], + }), + ]; + + const message_group = build_message_group(messages); + const list = build_list([message_group]); + + for (const message_container of messages) { + list._maybe_format_me_message(message_container); + list._add_msg_edited_vars(message_container); + } + + const result = list._message_groups[0].message_containers; + + // no edits: false + assert_moved_false(result[0]); + // stream changed: true + assert_moved_true(result[1]); + // topic changed: true + assert_moved_true(result[2]); + // content edited: false + assert_moved_false(result[3]); + // stream and topic edited: true + assert_moved_true(result[4]); + // topic and content changed: false + assert_moved_false(result[5]); + // stream and content changed: false + assert_moved_false(result[6]); + // topic, stream, and content changed: false + assert_moved_false(result[7]); + })(); +}); + test("msg_edited_vars", () => { // This is a test to verify that only one of the three bools, // `edited_in_left_col`, `edited_alongside_sender`, `edited_status_msg` diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index 8c909cf283..de59ad72fa 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -56,6 +56,24 @@ function same_recipient(a, b) { return util.same_recipient(a.msg, b.msg); } +function message_was_only_moved(message) { + // Returns true if the message has had its stream/topic edited + // (i.e. the message was moved), but its content has not been + // edited. + let moved = false; + if (message.edit_history !== undefined) { + for (const msg of message.edit_history) { + if (msg.prev_content) { + return false; + } + if (util.get_edit_event_prev_topic(msg) || msg.prev_stream) { + moved = true; + } + } + } + return moved; +} + function render_group_display_date(group, message_container) { const time = new Date(message_container.msg.timestamp * 1000); const today = new Date(); @@ -235,6 +253,7 @@ export class MessageListView { message_container.edited_in_left_col = !include_sender && !is_hidden; message_container.edited_alongside_sender = include_sender && !status_message; message_container.edited_status_msg = include_sender && status_message; + message_container.moved = message_was_only_moved(message_container.msg); } else { delete message_container.last_edit_timestr; message_container.edited_in_left_col = false; diff --git a/static/templates/edited_notice.hbs b/static/templates/edited_notice.hbs index fcc2f08bad..41636bc46d 100644 --- a/static/templates/edited_notice.hbs +++ b/static/templates/edited_notice.hbs @@ -3,7 +3,13 @@ ({{t "SAVING" }}) {{else}} -
- ({{t "EDITED" }}) -
+{{#if moved}} +
+ ({{t "MOVED" }}) +
+{{else}} +
+ ({{t "EDITED" }}) +
+{{/if}} {{/if}}