From 56b2b838ee17cbce191f40704df4cfec710a2ece Mon Sep 17 00:00:00 2001
From: Adam Sah <140002+asah@users.noreply.github.com>
Date: Tue, 9 Aug 2022 23:20:48 +0200
Subject: [PATCH] narrow: /streams/public should not have bookends.

Fixes #18280.
---
 .../node_tests/message_list_view.js           | 22 +++++++------
 .../puppeteer_tests/message-basics.ts         | 12 +++++++
 static/js/message_list_view.js                | 31 ++++++++++---------
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/frontend_tests/node_tests/message_list_view.js b/frontend_tests/node_tests/message_list_view.js
index 712bfa4649..437afce5a7 100644
--- a/frontend_tests/node_tests/message_list_view.js
+++ b/frontend_tests/node_tests/message_list_view.js
@@ -11,8 +11,6 @@ set_global("document", "document-stub");
 
 const noop = () => {};
 
-mock_esm("../../static/js/message_lists", {home: "stub"});
-
 // timerender calls setInterval when imported
 mock_esm("../../static/js/timerender", {
     render_date(time1, time2) {
@@ -421,13 +419,19 @@ test("merge_message_groups", () => {
     }
 
     function build_list(message_groups) {
-        const list = new MessageListView(undefined, undefined, true);
-        list._message_groups = message_groups;
-        list.list = {
-            unsubscribed_bookend_content() {},
-            subscribed_bookend_content() {},
-        };
-        return list;
+        const table_name = "zfilt";
+        const filter = new Filter([{operator: "stream", operand: "foo"}]);
+
+        const list = new message_list.MessageList({
+            table_name,
+            filter,
+        });
+
+        const view = new MessageListView(list, table_name, true);
+        view._message_groups = message_groups;
+        view.list.unsubscribed_bookend_content = () => {};
+        view.list.subscribed_bookend_content = () => {};
+        return view;
     }
 
     function extract_message_ids(lst) {
diff --git a/frontend_tests/puppeteer_tests/message-basics.ts b/frontend_tests/puppeteer_tests/message-basics.ts
index ff416a7b0b..d73d72d156 100644
--- a/frontend_tests/puppeteer_tests/message-basics.ts
+++ b/frontend_tests/puppeteer_tests/message-basics.ts
@@ -458,6 +458,17 @@ async function test_users_search(page: Page): Promise<void> {
     await expect_cordelia_private_narrow(page);
 }
 
+async function test_narrow_public_streams(page: Page): Promise<void> {
+    await page.click(await get_stream_li(page, "Denmark"));
+    await page.waitForFunction(() => $(".recipient_row:visible").length >= 3);
+    assert.equal(await page.evaluate(() => $(".stream-status:visible").length), 1);
+
+    await page.goto("http://zulip.zulipdev.com:9981/#narrow/streams/public");
+    await page.waitForFunction(() => $(".recipient_row:visible").length >= 3);
+
+    assert.equal(await page.evaluate(() => $(".stream-status:visible").length), 0);
+}
+
 async function message_basic_tests(page: Page): Promise<void> {
     await common.log_in(page);
     await page.click(".top_left_all_messages");
@@ -484,6 +495,7 @@ async function message_basic_tests(page: Page): Promise<void> {
     await test_narrow_by_clicking_the_left_sidebar(page);
     await test_stream_search_filters_stream_list(page);
     await test_users_search(page);
+    await test_narrow_public_streams(page);
 }
 
 common.run_test(message_basic_tests);
diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js
index adadbbfec7..00ca019ebe 100644
--- a/static/js/message_list_view.js
+++ b/static/js/message_list_view.js
@@ -351,7 +351,18 @@ export class MessageListView {
         this._add_msg_edited_vars(message_container);
     }
 
-    add_subscription_marker(group, last_msg_container, first_msg_container) {
+    maybe_add_subscription_marker(group, last_msg_container, first_msg_container) {
+        // The `historical` flag is present on messages which were
+        // sent a time when the current user was not subscribed to the
+        // stream receiving the message.
+        //
+        // When a narrow contains only messages within a given stream,
+        // we can infer that whenever the historical flag flips
+        // between adjacent messages, the current user must have
+        // (un)subscribed in between those messages.
+        if (!this.list.data.filter.has_operator("stream")) {
+            return;
+        }
         if (last_msg_container === undefined) {
             return;
         }
@@ -431,13 +442,7 @@ export class MessageListView {
                 message_container.subscribed = false;
                 message_container.unsubscribed = false;
 
-                // This message_lists.home condition can be removed
-                // once we filter historical messages from the
-                // home view on the server side (which requires
-                // having an index on UserMessage.flags)
-                if (this.list !== message_lists.home) {
-                    this.add_subscription_marker(current_group, prev, message_container);
-                }
+                this.maybe_add_subscription_marker(current_group, prev, message_container);
 
                 if (message_container.msg.stream) {
                     message_container.stream_url = hash_util.by_stream_url(
@@ -505,13 +510,11 @@ export class MessageListView {
                 second_group.message_containers,
             );
             return true;
-            // Add a subscription marker
-        } else if (
-            this.list !== message_lists.home &&
-            last_msg_container.msg.historical !== first_msg_container.msg.historical
-        ) {
-            this.add_subscription_marker(second_group, last_msg_container, first_msg_container);
         }
+
+        // We may need to add a subscripton marker after merging the groups.
+        this.maybe_add_subscription_marker(second_group, last_msg_container, first_msg_container);
+
         return false;
     }