From 50f5cf9ad818b2dfc8dc34f47d7d286a96250c37 Mon Sep 17 00:00:00 2001
From: evykassirer <evy@zulip.com>
Date: Tue, 28 May 2024 13:07:25 -0700
Subject: [PATCH] message booleans: Return new message instead of mutating a
 raw message.

---
 web/src/message_helper.js       |  2 +-
 web/src/message_store.ts        | 44 ++++++++++++++++++++++-----------
 web/tests/example2.test.js      |  7 +++---
 web/tests/message_store.test.js |  4 +--
 4 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/web/src/message_helper.js b/web/src/message_helper.js
index d25fe99827..2044dc3d50 100644
--- a/web/src/message_helper.js
+++ b/web/src/message_helper.js
@@ -23,7 +23,7 @@ export function process_new_message(message) {
         return cached_msg;
     }
 
-    message_store.set_message_booleans(message);
+    message = message_store.set_message_booleans(message);
     message.sent_by_me = people.is_current_user(message.sender_email);
 
     people.extract_people_from_message(message);
diff --git a/web/src/message_store.ts b/web/src/message_store.ts
index 46ef2b1ed9..76cd571892 100644
--- a/web/src/message_store.ts
+++ b/web/src/message_store.ts
@@ -1,3 +1,5 @@
+import _ from "lodash";
+
 import * as blueslip from "./blueslip";
 import * as people from "./people";
 import type {RawReaction} from "./reactions";
@@ -193,30 +195,44 @@ export function get_pm_full_names(user_ids: number[]): string {
     return names.join(", ");
 }
 
-export function set_message_booleans(message: Message): void {
+export function set_message_booleans(message: RawMessage): MessageWithBooleans {
     const flags = message.flags ?? [];
 
     function convert_flag(flag_name: string): boolean {
         return flags.includes(flag_name);
     }
 
-    message.unread = !convert_flag("read");
-    message.historical = convert_flag("historical");
-    message.starred = convert_flag("starred");
-    message.mentioned =
-        convert_flag("mentioned") ||
-        convert_flag("stream_wildcard_mentioned") ||
-        convert_flag("topic_wildcard_mentioned");
-    message.mentioned_me_directly = convert_flag("mentioned");
-    message.stream_wildcard_mentioned = convert_flag("stream_wildcard_mentioned");
-    message.topic_wildcard_mentioned = convert_flag("topic_wildcard_mentioned");
-    message.collapsed = convert_flag("collapsed");
-    message.alerted = convert_flag("has_alert_word");
+    const converted_flags = {
+        unread: !convert_flag("read"),
+        historical: convert_flag("historical"),
+        starred: convert_flag("starred"),
+        mentioned:
+            convert_flag("mentioned") ||
+            convert_flag("stream_wildcard_mentioned") ||
+            convert_flag("topic_wildcard_mentioned"),
+        mentioned_me_directly: convert_flag("mentioned"),
+        stream_wildcard_mentioned: convert_flag("stream_wildcard_mentioned"),
+        topic_wildcard_mentioned: convert_flag("topic_wildcard_mentioned"),
+        collapsed: convert_flag("collapsed"),
+        alerted: convert_flag("has_alert_word"),
+    };
 
     // Once we have set boolean flags here, the `flags` attribute is
     // just a distraction, so we delete it.  (All the downstream code
     // uses booleans.)
-    delete message.flags;
+
+    // We have to return these separately because of how the `MessageWithBooleans`
+    // type is set up.
+    if (message.type === "private") {
+        return {
+            ..._.omit(message, "flags"),
+            ...converted_flags,
+        };
+    }
+    return {
+        ..._.omit(message, "flags"),
+        ...converted_flags,
+    };
 }
 
 export function update_booleans(message: Message, flags: string[]): void {
diff --git a/web/tests/example2.test.js b/web/tests/example2.test.js
index c304130849..4bfa7daed1 100644
--- a/web/tests/example2.test.js
+++ b/web/tests/example2.test.js
@@ -70,8 +70,7 @@ run_test("message_store", () => {
     assert.equal(message_store.get(in_message.id), undefined);
     message_helper.process_new_message(in_message);
     const message = message_store.get(in_message.id);
-    assert.equal(in_message.alerted, true);
-    assert.equal(message, in_message);
+    assert.equal(message.alerted, true);
 
     // There are more side effects.
     const topic_names = stream_topic_history.get_recent_topic_names(denmark_stream.stream_id);
@@ -91,8 +90,8 @@ run_test("unread", () => {
 
     assert.equal(unread.num_unread_for_topic(stream_id, topic_name), 0);
 
-    const in_message = {...messages.isaac_to_denmark_stream};
-    message_store.set_message_booleans(in_message);
+    let in_message = {...messages.isaac_to_denmark_stream};
+    in_message = message_store.set_message_booleans(in_message);
 
     unread.process_loaded_messages([in_message]);
     assert.equal(unread.num_unread_for_topic(stream_id, topic_name), 1);
diff --git a/web/tests/message_store.test.js b/web/tests/message_store.test.js
index 691b89bff3..f644fd1d30 100644
--- a/web/tests/message_store.test.js
+++ b/web/tests/message_store.test.js
@@ -160,9 +160,9 @@ test("message_booleans_parity", () => {
     // This test asserts that both have identical behavior for the
     // flags common between them.
     const assert_bool_match = (flags, expected_message) => {
-        const set_message = {topic: "set_message_booleans", flags};
+        let set_message = {topic: "set_message_booleans", flags};
         const update_message = {topic: "update_booleans"};
-        message_store.set_message_booleans(set_message);
+        set_message = message_store.set_message_booleans(set_message);
         message_store.update_booleans(update_message, flags);
         for (const key of Object.keys(expected_message)) {
             assert.equal(