messages: Simplify API for use_first_unread_anchor.

Now that we have the type situation of having anchor support passing a
string, this is a much more natural way to implement
use_first_unread_anchor.

We still support the old interface to avoid breaking compatibility
with legacy versions of the mobile apps.

frontend_tests/node_tests/narrow_activate.js

@@ -188,7 +188,6 @@ run_test('basics', () => {
             cont: opts.cont,
             pre_scroll_cont: opts.pre_scroll_cont,
             anchor: 1000,
-            use_first_unread_anchor: false,
         });
     };
 

static/js/message_fetch.js

@@ -162,9 +162,6 @@ exports.load_messages = function (opts) {
     if (opts.msg_list === home_msg_list && page_params.narrow_stream !== undefined) {
         data.narrow = JSON.stringify(page_params.narrow);
     }
-    if (opts.use_first_unread_anchor) {
-        data.use_first_unread_anchor = true;
-    }
 
     if (opts.num_before > 0) {
         opts.msg_list.fetch_status.start_older_batch();
@@ -226,7 +223,6 @@ exports.load_messages_for_narrow = function (opts) {
         num_before: consts.narrow_before,
         num_after: consts.narrow_after,
         msg_list: msg_list,
-        use_first_unread_anchor: opts.use_first_unread_anchor,
         pre_scroll_cont: opts.pre_scroll_cont,
         cont: function () {
             message_scroll.hide_indicators();

static/js/narrow.js

@@ -222,7 +222,6 @@ exports.activate = function (raw_operators, opts) {
 
     (function fetch_messages() {
         let anchor;
-        let use_first_unread;
 
         // Either we're trying to center the narrow around a
         // particular message ID (which could be max_int), or we're
@@ -230,15 +229,12 @@ exports.activate = function (raw_operators, opts) {
         // unread message is, and center the narrow around that.
         if (id_info.final_select_id !== undefined) {
             anchor = id_info.final_select_id;
-            use_first_unread = false;
         } else {
-            anchor = -1;
+            anchor = "first_unread";
-            use_first_unread = true;
         }
 
         message_fetch.load_messages_for_narrow({
             anchor: anchor,
-            use_first_unread_anchor: use_first_unread,
             cont: function () {
                 if (!select_immediately) {
                     exports.update_selection({

zerver/tests/test_messages.py

@@ -1371,7 +1371,7 @@ class MessageDictTest(ZulipTestCase):
             '/json/messages?use_first_unread_anchor=false&num_before=1&num_after=1')
 
         self.assert_json_error(
-            result, "Missing 'anchor' argument (or set 'use_first_unread_anchor'=True).")
+            result, "Missing 'anchor' argument.")
 
     def test_invalid_anchor(self) -> None:
         self.login(self.example_email("hamlet"))

zerver/tests/test_narrow.py

@@ -2447,8 +2447,7 @@ class GetOldMessagesTest(ZulipTestCase):
         # 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="first_unread",
-            anchor=0,
             num_before=10,
             num_after=10,
             narrow='[["stream", "England"]]'
@@ -2484,8 +2483,7 @@ class GetOldMessagesTest(ZulipTestCase):
         self.send_personal_message(self.example_email("othello"), self.example_email("iago"))
 
         query_params = dict(
-            use_first_unread_anchor='true',
+            anchor="first_unread",
-            anchor=0,
             num_before=10,
             num_after=10,
             narrow='[]'
@@ -2532,8 +2530,7 @@ class GetOldMessagesTest(ZulipTestCase):
         self.send_personal_message(self.example_email("othello"), self.example_email("iago"))
 
         query_params = dict(
-            use_first_unread_anchor='true',
+            anchor="first_unread",
-            anchor=0,
             num_before=10,
             num_after=10,
             narrow='[]'
@@ -2563,8 +2560,7 @@ class GetOldMessagesTest(ZulipTestCase):
         user_profile = self.example_user('hamlet')
 
         query_params = dict(
-            use_first_unread_anchor='true',
+            anchor="first_unread",
-            anchor=0,
             num_before=10,
             num_after=10,
             narrow='[]'
@@ -2616,8 +2612,7 @@ class GetOldMessagesTest(ZulipTestCase):
         set_topic_mutes(user_profile, muted_topics)
 
         query_params = dict(
-            use_first_unread_anchor='true',
+            anchor="first_unread",
-            anchor=0,
             num_before=0,
             num_after=0,
             narrow='[["stream", "Scotland"]]'

zerver/tests/test_unread.py

@@ -110,12 +110,18 @@ class PointerTest(ZulipTestCase):
 
         # If we call get_messages with use_first_unread_anchor=True, we
         # should get the message we just sent
+        messages_response = self.get_messages_response(
+            anchor="first_unread", num_before=0, num_after=1)
+        self.assertEqual(messages_response['messages'][0]['id'], new_message_id)
+        self.assertEqual(messages_response['anchor'], new_message_id)
+
+        # Test with the old way of expressing use_first_unread_anchor=True
         messages_response = self.get_messages_response(
             anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
         self.assertEqual(messages_response['messages'][0]['id'], new_message_id)
         self.assertEqual(messages_response['anchor'], new_message_id)
 
-        # We want to get the message_id of an arbitrar old message. We can
+        # We want to get the message_id of an arbitrary old message. We can
         # call get_messages with use_first_unread_anchor=False and simply
         # save the first message we're returned.
         messages = self.get_messages(
@@ -145,7 +151,7 @@ class PointerTest(ZulipTestCase):
         # Now if we call get_messages with use_first_unread_anchor=True,
         # we should get the old message we just set to unread
         messages_response = self.get_messages_response(
-            anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+            anchor="first_unread", num_before=0, num_after=1)
         self.assertEqual(messages_response['messages'][0]['id'], old_message_id)
         self.assertEqual(messages_response['anchor'], old_message_id)
 
@@ -167,7 +173,7 @@ class PointerTest(ZulipTestCase):
         # we should not get the old unread message (because it's before the
         # pointer), and instead should get the newly sent unread message
         messages_response = self.get_messages_response(
-            anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+            anchor="first_unread", num_before=0, num_after=1)
         self.assertEqual(messages_response['messages'][0]['id'], new_message_id)
         self.assertEqual(messages_response['anchor'], new_message_id)
 
@@ -182,25 +188,25 @@ class PointerTest(ZulipTestCase):
                                                   "test")
 
         messages_response = self.get_messages_response(
-            anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+            anchor="first_unread", num_before=0, num_after=1)
         self.assertEqual(messages_response['messages'][0]['id'], new_message_id)
         self.assertEqual(messages_response['anchor'], new_message_id)
 
         with mock.patch('zerver.views.messages.get_first_visible_message_id', return_value=new_message_id):
             messages_response = self.get_messages_response(
-                anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+                anchor="first_unread", num_before=0, num_after=1)
         self.assertEqual(messages_response['messages'][0]['id'], new_message_id)
         self.assertEqual(messages_response['anchor'], new_message_id)
 
         with mock.patch('zerver.views.messages.get_first_visible_message_id', return_value=new_message_id + 1):
             messages_reponse = self.get_messages_response(
-                anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+                anchor="first_unread", num_before=0, num_after=1)
         self.assert_length(messages_reponse['messages'], 0)
         self.assertIn('anchor', messages_reponse)
 
         with mock.patch('zerver.views.messages.get_first_visible_message_id', return_value=new_message_id - 1):
             messages = self.get_messages(
-                anchor=0, num_before=0, num_after=1, use_first_unread_anchor=True)
+                anchor="first_unread", num_before=0, num_after=1)
         self.assert_length(messages, 1)
 
 class UnreadCountTests(ZulipTestCase):

zerver/views/messages.py

@@ -764,13 +764,29 @@ def zcommand_backend(request: HttpRequest, user_profile: UserProfile,
                      command: str=REQ('command')) -> HttpResponse:
     return json_success(process_zcommands(command, user_profile))
 
-def parse_anchor_value(anchor_val: Optional[str]) -> Optional[int]:
+def parse_anchor_value(anchor_val: Optional[str],
-    if anchor_val is None:
+                       use_first_unread_anchor: bool) -> Optional[int]:
+    """Given the anchor and use_first_unread_anchor parameters passed by
+    the client, computes what anchor value the client requested,
+    handling backwards-compatibility and the various string-valued
+    fields.  We encode use_first_unread_anchor as anchor=None.
+    """
+    if use_first_unread_anchor:
+        # Backwards-compatibility: Before we added support for the
+        # special string-typed anchor values, clients would pass
+        # anchor=None and use_first_unread_anchor=True to indicate
+        # what is now expressed as anchor="first_unread".
         return None
+    if anchor_val is None:
+        # Throw an exception if neither an anchor argument not
+        # use_first_unread_anchor was specified.
+        raise JsonableError(_("Missing 'anchor' argument."))
     if anchor_val == "oldest":
         return 0
     if anchor_val == "newest":
         return LARGER_THAN_MAX_MESSAGE_ID
+    if anchor_val == "first_unread":
+        return None
     try:
         # We don't use `.isnumeric()` to support negative numbers for
         # anchor.  We don't recommend it in the API (if you want the
@@ -791,12 +807,11 @@ def get_messages_backend(request: HttpRequest, user_profile: UserProfile,
                          num_before: int=REQ(converter=to_non_negative_int),
                          num_after: int=REQ(converter=to_non_negative_int),
                          narrow: OptionalNarrowListT=REQ('narrow', converter=narrow_parameter, default=None),
-                         use_first_unread_anchor: bool=REQ(validator=check_bool, default=False),
+                         use_first_unread_anchor_val: bool=REQ('use_first_unread_anchor',
+                                                               validator=check_bool, default=False),
                          client_gravatar: bool=REQ(validator=check_bool, default=False),
                          apply_markdown: bool=REQ(validator=check_bool, default=True)) -> HttpResponse:
-    anchor = parse_anchor_value(anchor_val)
+    anchor = parse_anchor_value(anchor_val, use_first_unread_anchor_val)
-    if anchor is None and not use_first_unread_anchor:
-        return json_error(_("Missing 'anchor' argument (or set 'use_first_unread_anchor'=True)."))
     if num_before + num_after > MAX_MESSAGES_PER_FETCH:
         return json_error(_("Too many messages requested (maximum %s).")
                           % (MAX_MESSAGES_PER_FETCH,))
@@ -850,7 +865,8 @@ def get_messages_backend(request: HttpRequest, user_profile: UserProfile,
 
     sa_conn = get_sqlalchemy_connection()
 
-    if use_first_unread_anchor:
+    if anchor is None:
+        # The use_first_unread_anchor code path
         anchor = find_first_unread_anchor(
             sa_conn,
             user_profile,