From 36844418e936950f1f205d7dc16b6e533c498e45 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Thu, 5 Apr 2018 16:32:30 -0400
Subject: [PATCH] bug fix: Respect include_history for certain queries.

For certain queries where both include_history and
use_first_unread_anchor are set to True, we were excluding
historical rows.  Now we only use the use_first_unread_anchor
flag to filter rows that we use to find the anchor, without
having it filter the actual search results.

The bug went unreported for a long time, because it only
affected mobile users who had newly subscribed to streams.

Note that we make a small change to the test called
test_use_first_unread_anchor_with_muted_topics, which has
a very scary comment about being "arcane" and "be
absolutely sure you know what you're doing."  I think it's
fine.

Also, the new test code would fail before this fix, so it
should help prevent future regressions.

Fixes #8958
---
 zerver/tests/test_narrow.py | 32 ++++++++++++++++++++++++++++----
 zerver/views/messages.py    |  4 ++--
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/zerver/tests/test_narrow.py b/zerver/tests/test_narrow.py
index c8fa06f375..5b5cb44ad7 100644
--- a/zerver/tests/test_narrow.py
+++ b/zerver/tests/test_narrow.py
@@ -1736,7 +1736,7 @@ class GetOldMessagesTest(ZulipTestCase):
         self.make_stream('England')
 
         # Send a few messages that Hamlet won't have UserMessage rows for.
-        self.send_stream_message(cordelia.email, 'England')
+        unsub_message_id = self.send_stream_message(cordelia.email, 'England')
         self.send_personal_message(cordelia.email, othello.email)
 
         self.subscribe(hamlet, 'England')
@@ -1747,13 +1747,13 @@ class GetOldMessagesTest(ZulipTestCase):
         set_topic_mutes(hamlet, muted_topics)
 
         # send a muted message
-        self.send_stream_message(cordelia.email, 'England', topic_name='muted')
+        muted_message_id = self.send_stream_message(cordelia.email, 'England', topic_name='muted')
 
         # finally send Hamlet a "normal" message
         first_message_id = self.send_stream_message(cordelia.email, 'England')
 
         # send a few more messages
-        self.send_stream_message(cordelia.email, 'England')
+        extra_message_id = self.send_stream_message(cordelia.email, 'England')
         self.send_personal_message(cordelia.email, hamlet.email)
 
         sa_conn = get_sqlalchemy_connection()
@@ -1767,6 +1767,30 @@ class GetOldMessagesTest(ZulipTestCase):
         )
         self.assertEqual(anchor, first_message_id)
 
+        # With the same data setup, we now want to test that a reasonable
+        # search still gets the first message sent to Hamlet (before he
+        # subscribed) and other recent messages to the stream.
+        query_params = dict(
+            use_first_unread_anchor='true',
+            anchor=0,
+            num_before=10,
+            num_after=10,
+            narrow='[["stream", "England"]]'
+        )
+        request = POSTRequestMock(query_params, user_profile)
+
+        payload = get_messages_backend(request, user_profile)
+        result = ujson.loads(payload.content)
+        self.assertEqual(result['anchor'], first_message_id)
+        self.assertEqual(result['found_newest'], True)
+        self.assertEqual(result['found_oldest'], True)
+
+        messages = result['messages']
+        self.assertEqual(
+            {msg['id'] for msg in messages},
+            {unsub_message_id, muted_message_id, first_message_id, extra_message_id}
+        )
+
     def test_use_first_unread_anchor_with_some_unread_messages(self) -> None:
         user_profile = self.example_user('hamlet')
 
@@ -1947,7 +1971,7 @@ class GetOldMessagesTest(ZulipTestCase):
         # the `message_id = LARGER_THAN_MAX_MESSAGE_ID` hack.
         queries = [q for q in all_queries if '/* get_messages */' in q['sql']]
         self.assertEqual(len(queries), 1)
-        self.assertIn('AND message_id = %d' % (LARGER_THAN_MAX_MESSAGE_ID,),
+        self.assertIn('AND zerver_message.id = %d' % (LARGER_THAN_MAX_MESSAGE_ID,),
                       queries[0]['sql'])
 
     def test_exclude_muting_conditions(self) -> None:
diff --git a/zerver/views/messages.py b/zerver/views/messages.py
index b39df2fc44..0261b5ff49 100644
--- a/zerver/views/messages.py
+++ b/zerver/views/messages.py
@@ -692,7 +692,7 @@ def get_messages_backend(request: HttpRequest, user_profile: UserProfile,
                          apply_markdown: bool=REQ(validator=check_bool, default=True)) -> HttpResponse:
     include_history = ok_to_include_history(narrow, user_profile)
 
-    if include_history and not use_first_unread_anchor:
+    if include_history:
         # The initial query in this case doesn't use `zerver_usermessage`,
         # and isn't yet limited to messages the user is entitled to see!
         #
@@ -701,7 +701,7 @@ def get_messages_backend(request: HttpRequest, user_profile: UserProfile,
         # See `ok_to_include_history` for details.
         need_message = True
         need_user_message = False
-    elif narrow is None and not use_first_unread_anchor:
+    elif narrow is None:
         # We need to limit to messages the user has received, but we don't actually
         # need any fields from Message
         need_message = False