message-edit: Use last_moved_timestamp for edited/moved indicators.

Use both the last_moved_timestamp and last_edit_timestamp to show
edited and moved indicators/tooltips in the message list view of
the web app, instead of parsing the message edit history array.

We still maintain and build the message edit history array as it's
used for calculating the narrow terms when there is a near operator
and a message has been moved to a different channel or topic.

Updates the tooltip for message edit indicators to include both
the moved and edited time if a message has been both moved and
edited.

web/src/message_edit_history.ts

@@ -332,8 +332,11 @@ export function initialize(): void {
             }
 
             if (
-                realm.realm_message_edit_history_visibility_policy !==
-                message_edit_history_visibility_policy_values.never.code
+                realm.realm_message_edit_history_visibility_policy ===
+                    message_edit_history_visibility_policy_values.always.code ||
+                (realm.realm_message_edit_history_visibility_policy ===
+                    message_edit_history_visibility_policy_values.moves_only.code &&
+                    message.last_moved_timestamp !== undefined)
             ) {
                 fetch_and_render_message_history(message);
                 $("#message-history-overlay .exit-sign").trigger("focus");

web/src/message_events.ts

@@ -3,6 +3,8 @@ import _ from "lodash";
 import assert from "minimalistic-assert";
 import {z} from "zod";
 
+import * as resolved_topic from "../shared/src/resolved_topic.ts";
+
 import * as activity from "./activity.ts";
 import * as alert_words from "./alert_words.ts";
 import * as channel from "./channel.ts";
@@ -375,6 +377,16 @@ export function insert_new_messages(
     return messages;
 }
 
+function topic_resolve_toggled(new_topic: string, original_topic: string): boolean {
+    if (resolved_topic.is_resolved(new_topic) && new_topic.slice(2) === original_topic) {
+        return true;
+    }
+    if (resolved_topic.is_resolved(original_topic) && original_topic.slice(2) === new_topic) {
+        return true;
+    }
+    return false;
+}
+
 export function update_messages(events: UpdateMessageEvent[]): void {
     const messages_to_rerender: Message[] = [];
     let changed_narrow = false;
@@ -582,7 +594,15 @@ export function update_messages(events: UpdateMessageEvent[]): void {
                         ...moved_message.edit_history,
                     ];
                 }
-                moved_message.last_edit_timestamp = event.edit_timestamp;
+
+                if (stream_changed) {
+                    moved_message.last_moved_timestamp = event.edit_timestamp;
+                } else if (topic_edited) {
+                    assert(new_topic !== undefined);
+                    if (!topic_resolve_toggled(new_topic, orig_topic)) {
+                        moved_message.last_moved_timestamp = event.edit_timestamp;
+                    }
+                }
 
                 // Update the unread counts; again, this must be called
                 // before we modify the topic field on the message.
@@ -753,7 +773,7 @@ export function update_messages(events: UpdateMessageEvent[]): void {
             // flag is used to indicated update_message events that are
             // triggered by server latency optimizations, not user
             // interactions; these should not generate edit history updates.
-            if (!event.rendering_only) {
+            if (!event.rendering_only && any_message_content_edited) {
                 anchor_message.last_edit_timestamp = event.edit_timestamp;
             }
 

web/src/message_list_tooltips.ts

@@ -384,20 +384,36 @@ export function initialize(): void {
             if (!message_container.modified) {
                 return false;
             }
-            // We know the message has been modified, so we either have a timestamp
+            let edited_time_string = "";
+            let moved_time_string = "";
+            if (message_container.edited) {
+                // We know the message has been edited, so we either have a timestamp
                 // from the server or from a local edit.
                 assert(message_container.last_edit_timestamp !== undefined);
-            const last_modified_time_string = get_time_string(
-                message_container.last_edit_timestamp,
-            );
+                edited_time_string = get_time_string(message_container.last_edit_timestamp);
+            }
+            if (message_container.moved) {
+                // We know the message has been moved, so we have a timestamp from
+                // the server.
+                assert(message_container.last_moved_timestamp !== undefined);
+                moved_time_string = get_time_string(message_container.last_moved_timestamp);
+            }
+            const edit_history_access =
+                realm.realm_message_edit_history_visibility_policy ===
+                message_edit_history_visibility_policy_values.always.code;
+            const message_moved_and_move_history_access =
+                realm.realm_message_edit_history_visibility_policy ===
+                    message_edit_history_visibility_policy_values.moves_only.code &&
+                message_container.moved;
             instance.setContent(
                 parse_html(
                     render_message_edit_notice_tooltip({
                         moved: message_container.moved,
-                        last_edit_timestr: last_modified_time_string,
-                        realm_message_edit_history_is_visible:
-                            realm.realm_message_edit_history_visibility_policy !==
-                            message_edit_history_visibility_policy_values.never.code,
+                        edited: message_container.edited,
+                        edited_time_string,
+                        moved_time_string,
+                        edit_history_access,
+                        message_moved_and_move_history_access,
                     }),
                 ),
             );

web/src/message_list_view.ts

@@ -55,11 +55,13 @@ export type MessageContainer = {
     include_sender: boolean;
     is_hidden: boolean;
     last_edit_timestamp: number | undefined;
+    last_moved_timestamp: number | undefined;
     mention_classname: string | undefined;
     message_edit_notices_in_left_col: boolean;
     message_edit_notices_alongside_sender: boolean;
     message_edit_notices_for_status_message: boolean;
     modified: boolean;
+    edited: boolean;
     moved: boolean;
     msg: Message;
     sender_is_bot: boolean;
@@ -164,69 +166,6 @@ function same_recipient(a: MessageContainer | undefined, b: MessageContainer | u
     return util.same_recipient(a.msg, b.msg);
 }
 
-function analyze_edit_history(
-    message: Message,
-    last_edit_timestamp: number | undefined,
-): {
-    edited: boolean;
-    moved: boolean;
-    resolve_toggled: boolean;
-} {
-    // Returns a dict of booleans that describe the message's history:
-    //   * edited: if the message has had its content edited
-    //   * moved: if the message has had its stream/topic edited
-    //   * resolve_toggled: if the message has had a topic resolve/unresolve edit
-    let edited = false;
-    let moved = false;
-    let resolve_toggled = false;
-
-    if (message.edit_history !== undefined) {
-        for (const edit_history_event of message.edit_history) {
-            if (edit_history_event.prev_content) {
-                edited = true;
-            }
-
-            if (edit_history_event.prev_stream) {
-                moved = true;
-            }
-
-            if (edit_history_event.prev_topic !== undefined) {
-                // TODO: Possibly this assert could be removed if we tightened the type
-                // on edit history elements such that a `prev_topic` being present means a
-                // `topic` element is.
-                assert(edit_history_event.topic !== undefined);
-                // We know it has a topic edit. Now we need to determine if
-                // it was a true move or a resolve/unresolve.
-                if (
-                    resolved_topic.is_resolved(edit_history_event.topic) &&
-                    edit_history_event.topic.slice(2) === edit_history_event.prev_topic
-                ) {
-                    // Resolved.
-                    resolve_toggled = true;
-                    continue;
-                }
-                if (
-                    resolved_topic.is_resolved(edit_history_event.prev_topic) &&
-                    edit_history_event.prev_topic.slice(2) === edit_history_event.topic
-                ) {
-                    // Unresolved.
-                    resolve_toggled = true;
-                    continue;
-                }
-                // Otherwise, it is a real topic rename/move.
-                moved = true;
-            }
-        }
-    } else if (last_edit_timestamp !== undefined) {
-        // When the edit_history is disabled for the organization, we do not receive the edit_history
-        // variable in the message object. In this case, we will check if the last_edit_timestamp is
-        // available or not. Since we don't have the edit_history, we can't determine if the message
-        // was moved or edited. Therefore, we simply mark the messages as edited.
-        edited = true;
-    }
-    return {edited, moved, resolve_toggled};
-}
-
 function get_group_display_date(message: Message, display_year: boolean): string {
     const time = new Date(message.timestamp * 1000);
     const date_element = timerender.render_date(time, display_year);
@@ -592,8 +531,10 @@ export class MessageListView {
         );
     }
 
-    _get_message_edited_vars(message: Message): {
+    _get_message_edited_and_moved_vars(message: Message): {
         last_edit_timestamp: number | undefined;
+        last_moved_timestamp: number | undefined;
+        edited: boolean;
         moved: boolean;
         modified: boolean;
     } {
@@ -603,25 +544,14 @@ export class MessageListView {
         } else {
             last_edit_timestamp = message.last_edit_timestamp;
         }
-        const edit_history_details = analyze_edit_history(message, last_edit_timestamp);
-
-        if (!last_edit_timestamp || !(edit_history_details.moved || edit_history_details.edited)) {
-            // For messages whose edit history at most includes
-            // resolving topics, we don't display an EDITED/MOVED
-            // notice at all. (The message actions popover will still
-            // display an edit history option, so you can see when it
-            // was marked as resolved if you need to).
-            return {
-                last_edit_timestamp: undefined,
-                moved: false,
-                modified: false,
-            };
-        }
+        const last_moved_timestamp = message.last_moved_timestamp;
 
         return {
             last_edit_timestamp,
-            moved: edit_history_details.moved && !edit_history_details.edited,
-            modified: true,
+            last_moved_timestamp,
+            edited: last_edit_timestamp !== undefined,
+            moved: last_moved_timestamp !== undefined,
+            modified: last_edit_timestamp !== undefined || last_moved_timestamp !== undefined,
         };
     }
 
@@ -646,6 +576,8 @@ export class MessageListView {
         include_sender: boolean;
         status_message: string | false;
         last_edit_timestamp: number | undefined;
+        last_moved_timestamp: number | undefined;
+        edited: boolean;
         moved: boolean;
         modified: boolean;
     } {
@@ -745,7 +677,7 @@ export class MessageListView {
             mention_classname,
             include_sender,
             ...this._maybe_get_me_message(is_hidden, message),
-            ...this._get_message_edited_vars(message),
+            ...this._get_message_edited_and_moved_vars(message),
         };
     }
 

web/src/message_store.ts

@@ -74,6 +74,7 @@ export const raw_message_schema = z.intersection(
             id: z.number(),
             is_me_message: z.boolean(),
             last_edit_timestamp: z.optional(z.number()),
+            last_moved_timestamp: z.optional(z.number()),
             reactions: z.array(message_reaction_schema),
             recipient_id: z.number(),
             sender_email: z.string(),

web/src/server_message.ts

@@ -49,6 +49,7 @@ export const server_message_schema = z
         id: z.number(),
         is_me_message: z.boolean(),
         last_edit_timestamp: z.number().optional(),
+        last_moved_timestamp: z.number().optional(),
         reactions: message_reaction_schema,
         recipient_id: z.number(),
         sender_email: z.string(),

web/templates/edited_notice.hbs

@@ -4,13 +4,13 @@
         <div class="message_edit_notice">
             {{t "SAVING"}}
         </div>
-    {{else if moved}}
-        <div class="message_edit_notice">
-            {{t "MOVED"}}
-        </div>
-    {{else}}
+    {{else if edited}}
         <div class="message_edit_notice">
             {{t "EDITED"}}
         </div>
+    {{else}}
+        <div class="message_edit_notice">
+            {{t "MOVED"}}
+        </div>
     {{/if}}
 {{/if}}

web/templates/message_edit_notice_tooltip.hbs

@@ -1,15 +1,30 @@
 <div>
-    {{#if realm_message_edit_history_is_visible}}
+    {{#if edit_history_access}}
+        {{#if edited}}
+            {{#if moved}}
+            <div>{{t "View edit and move history"}}</div>
+            {{else}}
             <div>{{t "View edit history"}}</div>
             {{/if}}
-    <div class="tooltip-inner-content italic">
-        {{#if moved}}
-        {{t 'Last moved {last_edit_timestr}.'}}
-        {{else}}
-        {{t 'Last edited {last_edit_timestr}.'}}
+        {{else if moved}}
+            <div>{{t "View move history"}}</div>
         {{/if}}
+    {{else if message_moved_and_move_history_access}}
+        <div>{{t "View move history"}}</div>
+    {{/if}}
+    {{#if edited}}
+        <div class="tooltip-inner-content italic">
+            {{t 'Last edited {edited_time_string}.'}}
         </div>
+    {{/if}}
+    {{#if moved}}
+    <div class="tooltip-inner-content italic">
+        {{t 'Last moved {moved_time_string}.'}}
+    </div>
+    {{/if}}
 </div>
-{{#if realm_message_edit_history_is_visible}}
+{{#if edit_history_access}}
+{{tooltip_hotkey_hints "Shift" "H"}}
+{{else if message_moved_and_move_history_access}}
 {{tooltip_hotkey_hints "Shift" "H"}}
 {{/if}}

web/tests/message_list_view.test.cjs

@@ -45,27 +45,9 @@ function test(label, f) {
     });
 }
 
-test("msg_moved_var", () => {
+test("msg_edited_and_moved_vars", () => {
     // 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,
-        };
-        if ("edit_history" in message) {
-            message_context.msg = {
-                last_edit_timestamp: (next_timestamp += 1),
-                ...message,
-            };
-        } else {
-            message_context.msg = {
-                ...message,
-            };
-        }
-        return message_context;
-    }
 
     function build_message_group(messages) {
         return {message_containers: messages};
@@ -83,85 +65,29 @@ test("msg_moved_var", () => {
         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 edit history: NO LABEL
-            build_message_context({}),
-            // stream changed: MOVED
-            build_message_context({
-                edit_history: [{prev_stream: 1, timestamp: 1000, user_id: 1}],
-            }),
-            // topic changed (not resolved/unresolved): MOVED
-            build_message_context({
-                edit_history: [
-                    {prev_topic: "test_topic", topic: "new_topic", timestamp: 1000, user_id: 1},
-                ],
-            }),
-            // content edited: EDITED
-            build_message_context({
-                edit_history: [{prev_content: "test_content", timestamp: 1000, user_id: 1}],
-            }),
-            // stream and topic edited: MOVED
-            build_message_context({
-                edit_history: [
+            // no edit or moved timestamps
+            {msg: {}},
+            // edit timestamp: EDITED
             {
-                        prev_stream: 1,
-                        prev_topic: "test_topic",
-                        topic: "new_topic",
-                        timestamp: 1000,
-                        user_id: 1,
+                msg: {
+                    last_edit_timestamp: (next_timestamp += 1),
                 },
-                ],
-            }),
-            // topic and content changed: EDITED
-            build_message_context({
-                edit_history: [
+            },
+            // moved timestamp: MOVED
             {
-                        prev_topic: "test_topic",
-                        topic: "new_topic",
-                        prev_content: "test_content",
-                        timestamp: 1000,
-                        user_id: 1,
+                msg: {
+                    last_moved_timestamp: (next_timestamp += 1),
+                },
+            },
+            // both edit and moved timestamp: EDITED
+            {
+                msg: {
+                    last_edit_timestamp: (next_timestamp += 1),
+                    last_moved_timestamp: (next_timestamp += 1),
+                },
             },
-                ],
-            }),
-            // only topic resolved: NO LABEL
-            build_message_context({
-                edit_history: [
-                    {prev_topic: "test_topic", topic: "✔ test_topic", timestamp: 1000, user_id: 1},
-                ],
-            }),
-            // only topic unresolved: NO LABEL
-            build_message_context({
-                edit_history: [
-                    {prev_topic: "✔ test_topic", topic: "test_topic", timestamp: 1000, user_id: 1},
-                ],
-            }),
-            // multiple edit history logs, with at least one content edit: EDITED
-            build_message_context({
-                edit_history: [
-                    {prev_stream: 1, timestamp: 1000, user_id: 1},
-                    {prev_topic: "old_topic", topic: "test_topic", timestamp: 1001, user_id: 1},
-                    {prev_content: "test_content", timestamp: 1002, user_id: 1},
-                    {prev_topic: "test_topic", topic: "✔ test_topic", timestamp: 1003, user_id: 1},
-                ],
-            }),
-            // multiple edit history logs with no content edit: MOVED
-            build_message_context({
-                edit_history: [
-                    {prev_stream: 1, timestamp: 1000, user_id: 1},
-                    {prev_topic: "old_topic", topic: "test_topic", timestamp: 1001, user_id: 1},
-                    {prev_topic: "test_topic", topic: "✔ test_topic", timestamp: 1002, user_id: 1},
-                    {prev_topic: "✔ test_topic", topic: "test_topic", timestamp: 1003, user_id: 1},
-                ],
-            }),
         ];
 
         const message_group = build_message_group(messages);
@@ -171,32 +97,28 @@ test("msg_moved_var", () => {
             Object.assign(
                 message_container,
                 list._maybe_get_me_message(message_container.is_hidden, message_container.msg),
-                list._get_message_edited_vars(message_container.msg),
+                list._get_message_edited_and_moved_vars(message_container.msg),
             );
         }
 
         const result = list._message_groups[0].message_containers;
 
-        // no edit history: 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]);
-        // only topic resolved: false
-        assert_moved_false(result[6]);
-        // only topic unresolved: false
-        assert_moved_false(result[7]);
-        // multiple edits with content edit: false
-        assert_moved_false(result[8]);
-        // multiple edits without content edit: true
-        assert_moved_true(result[9]);
+        // no edit or moved timestamps
+        assert.equal(result[0].edited, false);
+        assert.equal(result[0].moved, false);
+        assert.equal(result[0].modified, false);
+        // edit timestamp: EDITED
+        assert.equal(result[1].edited, true);
+        assert.equal(result[1].moved, false);
+        assert.equal(result[1].modified, true);
+        // moved timestamp: MOVED
+        assert.equal(result[2].edited, false);
+        assert.equal(result[2].moved, true);
+        assert.equal(result[2].modified, true);
+        // both edit and moved timestamp: EDITED
+        assert.equal(result[3].edited, true);
+        assert.equal(result[3].moved, true);
+        assert.equal(result[3].modified, true);
     })();
 });
 
@@ -283,7 +205,7 @@ test("message_edited_vars", () => {
             Object.assign(
                 message_container,
                 list._maybe_get_me_message(message_container.is_hidden, message_container.msg),
-                list._get_message_edited_vars(message_container.msg),
+                list._get_message_edited_and_moved_vars(message_container.msg),
             );
         }
 
@@ -368,7 +290,7 @@ test("muted_message_vars", () => {
         ];
         const message_group = build_message_group(messages);
         const list = build_list([message_group]);
-        list._get_message_edited_vars = noop;
+        list._get_message_edited_and_moved_vars = noop;
 
         // Sender is not muted.
         let result = calculate_variables(list, messages);