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.
This commit is contained in:
Swati Bhageria 2021-12-09 12:10:26 -05:00 committed by Tim Abbott
parent 93e18fe289
commit 2d766f3e78
3 changed files with 137 additions and 3 deletions

View File

@ -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`

View File

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

View File

@ -3,7 +3,13 @@
({{t "SAVING" }})
</div>
{{else}}
<div class="message_edit_notice auto-select" title="{{#tr}}Edited ({last_edit_timestr}){{/tr}}">
{{#if moved}}
<div class="message_edit_notice auto-select" title="{{#tr}}Moved ({last_edit_timestr}){{/tr}}">
({{t "MOVED" }})
</div>
{{else}}
<div class="message_edit_notice auto-select" title="{{#tr}}Edited ({last_edit_timestr}){{/tr}}">
({{t "EDITED" }})
</div>
</div>
{{/if}}
{{/if}}