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 zulip#8958

zerver/tests/test_narrow.py

@@ -1736,7 +1736,7 @@ def test_find_first_unread_anchor(self) -> None:
         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 @@ def test_find_first_unread_anchor(self) -> None:
         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 @@ def test_find_first_unread_anchor(self) -> None:
         )
         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 @@ def test_use_first_unread_anchor_with_muted_topics(self) -> None:
         # 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:

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