From ef92fcbe2b7e580df17c5fa89415bcfdbcf20f00 Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Sun, 11 Mar 2018 20:59:20 -0700
Subject: [PATCH] topic history: Fix fetching topic history of public streams.

Apparently, we did essentially all the work to support showing full
topic history to newly subscribed users from a data flow perspective,
but didn't actually enable this feature by having the topic history
endpoint grant access to historical topics.  This fixes that gap.

I'm not altogether happy with how the code and tests read for this
feature; the code itself has more duplication than I'd like, and the
tests do too, but it works.
---
 zerver/lib/actions.py         | 27 +++++++++++++++++-----
 zerver/tests/test_messages.py | 42 +++++++++++++++++++++++++++++++++++
 zerver/views/streams.py       |  1 +
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
index d13e56468f..e77ac0775a 100644
--- a/zerver/lib/actions.py
+++ b/zerver/lib/actions.py
@@ -196,8 +196,26 @@ def realm_user_count(realm: Realm) -> int:
     return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=False).count()
 
 def get_topic_history_for_stream(user_profile: UserProfile,
-                                 recipient: Recipient) -> List[Dict[str, Any]]:
-    query = '''
+                                 recipient: Recipient,
+                                 public_history: bool) -> List[Dict[str, Any]]:
+    cursor = connection.cursor()
+    if public_history:
+        query = '''
+        SELECT
+            "zerver_message"."subject" as topic,
+            max("zerver_message".id) as max_message_id
+        FROM "zerver_message"
+        WHERE (
+            "zerver_message"."recipient_id" = %s
+        )
+        GROUP BY (
+            "zerver_message"."subject"
+        )
+        ORDER BY max("zerver_message".id) DESC
+        '''
+        cursor.execute(query, [recipient.id])
+    else:
+        query = '''
         SELECT
             "zerver_message"."subject" as topic,
             max("zerver_message".id) as max_message_id
@@ -213,9 +231,8 @@ def get_topic_history_for_stream(user_profile: UserProfile,
             "zerver_message"."subject"
         )
         ORDER BY max("zerver_message".id) DESC
-    '''
-    cursor = connection.cursor()
-    cursor.execute(query, [user_profile.id, recipient.id])
+        '''
+        cursor.execute(query, [user_profile.id, recipient.id])
     rows = cursor.fetchall()
     cursor.close()
 
diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py
index 76a9cc0bf5..e82badc776 100644
--- a/zerver/tests/test_messages.py
+++ b/zerver/tests/test_messages.py
@@ -24,6 +24,7 @@ from zerver.lib.actions import (
     do_create_user,
     get_client,
     do_add_alert_words,
+    do_change_stream_invite_only,
 )
 
 from zerver.lib.message import (
@@ -87,6 +88,7 @@ class TopicHistoryTest(ZulipTestCase):
         recipient = get_stream_recipient(stream.id)
 
         def create_test_message(topic: str) -> int:
+            # TODO: Clean this up to send messages the normal way.
 
             hamlet = self.example_user('hamlet')
             message = Message.objects.create(
@@ -144,6 +146,46 @@ class TopicHistoryTest(ZulipTestCase):
             topic2_msg_id,
         ])
 
+        # Now try as cordelia, who we imagine as a totally new user in
+        # that she doesn't have UserMessage rows.  We should see the
+        # same results for a public stream.
+        self.login(self.example_email("cordelia"))
+        result = self.client_get(endpoint, dict())
+        self.assert_json_success(result)
+        history = result.json()['topics']
+
+        # We only look at the most recent three topics, because
+        # the prior fixture data may be unreliable.
+        history = history[:3]
+
+        self.assertEqual([topic['name'] for topic in history], [
+            'topic0',
+            'topic1',
+            'topic2',
+        ])
+        self.assertIn('topic0', [topic['name'] for topic in history])
+
+        self.assertEqual([topic['max_id'] for topic in history], [
+            topic0_msg_id,
+            topic1_msg_id,
+            topic2_msg_id,
+        ])
+
+        # Now make stream private, but subscribe cordelia
+        do_change_stream_invite_only(stream, True)
+        self.subscribe(self.example_user("cordelia"), stream.name)
+
+        result = self.client_get(endpoint, dict())
+        self.assert_json_success(result)
+        history = result.json()['topics']
+        history = history[:3]
+
+        # Cordelia doesn't have these recent history items when we
+        # wasn't subscribed in her results.
+        self.assertNotIn('topic0', [topic['name'] for topic in history])
+        self.assertNotIn('topic1', [topic['name'] for topic in history])
+        self.assertNotIn('topic2', [topic['name'] for topic in history])
+
     def test_bad_stream_id(self) -> None:
         email = self.example_email("iago")
         self.login(email)
diff --git a/zerver/views/streams.py b/zerver/views/streams.py
index 409b70ffdd..165bc9bece 100644
--- a/zerver/views/streams.py
+++ b/zerver/views/streams.py
@@ -440,6 +440,7 @@ def get_topics_backend(request: HttpRequest, user_profile: UserProfile,
     result = get_topic_history_for_stream(
         user_profile=user_profile,
         recipient=recipient,
+        public_history=not stream.invite_only,
     )
 
     return json_success(dict(topics=result))