[wip] Cut various support for ancient servers

android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt

@@ -24,7 +24,7 @@ data class Identity(
 
     /// This user's ID within the server.  Useful mainly in the case where
     /// the user has multiple accounts in the same org.
-    val userId: Int?,
+    val userId: Int,
 )
 
 /**
@@ -120,7 +120,7 @@ data class MessageFcmMessage(
         // NOTE: Keep the JS-side type definition in sync with this code.
         buildArray { list ->
             list.add("realm_uri" to identity.realmUri.toString())
-            identity.userId?.let { list.add("user_id" to it) }
+            list.add("user_id" to identity.userId)
             when (recipient) {
                 is Recipient.Stream -> {
                     list.add("recipient_type" to "stream")
@@ -183,9 +183,6 @@ data class RemoveFcmMessage(
     companion object {
         fun fromFcmData(data: Map<String, String>): RemoveFcmMessage {
             val messageIds = HashSet<Int>()
-            data["zulip_message_id"]?.parseInt("zulip_message_id")?.let {
-                messageIds.add(it)
-            }
             data["zulip_message_ids"]?.parseCommaSeparatedInts("zulip_message_ids")?.let {
                 messageIds.addAll(it)
             }
@@ -206,18 +203,8 @@ private fun extractIdentity(data: Map<String, String>): Identity =
         // `realm_uri` was added in server version 1.9.0
         realmUri = data.require("realm_uri").parseUrl("realm_uri"),
 
-        // Server versions from 1.6.0 through 2.0.0 (and possibly earlier
-        // and later) send the user's email address, as `user`.  We *could*
-        // use this as a substitute for `user_id` when that's missing...
-        // but it'd be inherently buggy, and the bug it'd introduce seems
-        // likely to affect more users than the bug it'd fix.  So just ignore.
-        // TODO(server-2.0): Delete this comment.
-        // (data["user"] ignored)
-
-        // `user_id` was added in server version 2.1.0 (released 2019-12-12;
-        // commit 447a517e6, PR #12172.)
-        // TODO(server-2.1): Require this.
-        userId = data["user_id"]?.parseInt("user_id")
+        // `user_id` was added in server version 2.1.0.
+        userId = data.require("user_id").parseInt("user_id")
     )
 
 private fun Map<String, String>.require(key: String): String =

android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt

@@ -179,7 +179,6 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
             streamName = Example.stream["stream"]!!,
             topic = Example.stream["topic"]!!
         ))
-        expect.that(parse(Example.pm.minus("user_id")).identity.userId).isNull()
     }
 
     @Test
@@ -192,8 +191,6 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
             "stream_name" to Example.stream["stream"]!!,
             "topic" to Example.stream["topic"]!!,
         )
-        expect.that(dataForOpen(Example.stream.minus("user_id")))
-            .isEqualTo(baseExpected.minus("user_id"))
         expect.that(dataForOpen(Example.stream.minus("stream_id")))
             .isEqualTo(baseExpected.minus("stream_id"))
     }
@@ -215,6 +212,7 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
         assertParseFails(Example.pm.minus("realm_uri"))
         assertParseFails(Example.pm.plus("realm_uri" to "zulip.example.com"))
         assertParseFails(Example.pm.plus("realm_uri" to "/examplecorp"))
+        assertParseFails(Example.pm.minus("user_id"))
 
         assertParseFails(Example.stream.minus("recipient_type"))
         assertParseFails(Example.stream.plus("stream_id" to "12,34"))
@@ -250,20 +248,14 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {
             "event" to "remove"
         ))
 
-        /// The Zulip server before v2.0 sends this form (plus some irrelevant fields).
-        // TODO(server-2.0): Drop this, and the logic it tests.
-        val singular = base.plus(sequenceOf(
-            "zulip_message_id" to "123"
-        ))
-
         /// Zulip servers starting at v2.0 (released 2019-02-28; commit 9869153ae)
-        /// send a hybrid form.  (In practice the singular field has one of the
-        /// same IDs found in the batch.)
+        /// send a hybrid singular-plural form.  The singular field has one of the
+        /// same IDs found in the batch.
         ///
         /// We started consuming the batch field in 23.2.111 (released 2019-02-28;
         /// commit 4acd07376).
         val hybrid = base.plus(sequenceOf(
-            "zulip_message_ids" to "234,345",
+            "zulip_message_ids" to "123,234,345",
             "zulip_message_id" to "123"
         ))
 
@@ -293,20 +285,12 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {
         expect.that(parse(Example.batched)).isEqualTo(
             RemoveFcmMessage(Example.identity, setOf(123, 234, 345))
         )
-        expect.that(parse(Example.singular)).isEqualTo(
-            RemoveFcmMessage(Example.identity, setOf(123))
-        )
-        expect.that(parse(Example.singular.minus("zulip_message_id"))).isEqualTo(
+        expect.that(parse(Example.base)).isEqualTo(
             // This doesn't seem very useful to send, but harmless.
             RemoveFcmMessage(Example.identity, setOf())
         )
     }
 
-    @Test
-    fun `optional fields missing cause no error`() {
-        expect.that(parse(Example.hybrid.minus("user_id")).identity.userId).isNull()
-    }
-
     @Test
     fun `parse failures on malformed data`() {
         assertParseFails(Example.hybrid.minus("server"))
@@ -316,15 +300,7 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {
         assertParseFails(Example.hybrid.minus("realm_uri"))
         assertParseFails(Example.hybrid.plus("realm_uri" to "zulip.example.com"))
         assertParseFails(Example.hybrid.plus("realm_uri" to "/examplecorp"))
-
-        for (badInt in sequenceOf(
-            "12,34",
-            "abc",
-            ""
-        )) {
-            assertParseFails(Example.singular.plus("zulip_message_id" to badInt))
-            assertParseFails(Example.hybrid.plus("zulip_message_id" to badInt))
-        }
+        assertParseFails(Example.hybrid.minus("user_id"))
 
         for (badIntList in sequenceOf(
             "abc,34",

src/action-sheets/index.js

@@ -343,7 +343,7 @@ const toggleResolveTopic = async ({ auth, streamId, topic, _, streams, zulipFeat
 
   await api.updateMessage(auth, messageId, {
     propagate_mode: 'change_all',
-    subject: newTopic,
+    topic: newTopic,
     ...(zulipFeatureLevel >= 9 && {
       send_notification_to_old_thread: false,
       send_notification_to_new_thread: true,

src/api/initialDataTypes.js

@@ -663,14 +663,9 @@ export type InitialDataUserStatus = $ReadOnly<{|
    * status is the zero status), the server may omit that user's record
    * entirely.
    *
-   * New in Zulip 2.0.  Older servers don't send this, so it's natural
-   * to treat them as if all users have the zero status; and that gives
-   * correct behavior for this feature.
-   *
    * See UserStatusEvent for the event that carries updates to this data.
    */
-  // TODO(server-2.0): Make required.
-  user_status?: $ReadOnly<{|
+  user_status: $ReadOnly<{|
     // Keys are UserId encoded as strings (just because JS objects are
     // string-keyed).
     [userId: string]: UserStatusUpdate,

src/api/messages/sendMessage.js

@@ -1,25 +1,26 @@
 /* @flow strict-local */
 
 import type { ApiResponse, Auth } from '../transportTypes';
+import type { UserId } from '../idTypes';
 import { apiPost } from '../apiFetch';
 
+type CommonParams = {|
+  content: string,
+  localId?: number,
+  eventQueueId?: string,
+|};
+
 /** See https://zulip.com/api/send-message */
 export default async (
   auth: Auth,
-  params: {|
-    type: 'private' | 'stream',
-    to: string,
-    // TODO(server-2.0): Say "topic", not "subject"
-    subject?: string,
-    content: string,
-    localId?: number,
-    eventQueueId?: string,
-  |},
+  params:
+    | {| ...CommonParams, type: 'stream', to: number, topic: string |}
+    | {| ...CommonParams, type: 'private', to: $ReadOnlyArray<UserId> |},
 ): Promise<ApiResponse> =>
   apiPost(auth, 'messages', {
     type: params.type,
-    to: params.to,
-    subject: params.subject,
+    to: JSON.stringify(params.to),
+    topic: (params: { +topic?: string, ... }).topic,
     content: params.content,
     local_id: params.localId,
     queue_id: params.eventQueueId,

src/api/messages/updateMessage.js

@@ -17,9 +17,7 @@ export default async (
   auth: Auth,
   messageId: number,
   params: $ReadOnly<{|
-    // TODO(server-2.0): Say "topic", not "subject"
-    subject?: string,
-
+    topic?: string,
     propagate_mode?: PropagateMode,
     content?: string,
 

src/api/modelTypes.js

@@ -1029,6 +1029,9 @@ export type PmMessage = $ReadOnly<{|
    *
    * The ordering is less well specified; if it matters, sort first.
    */
+  // TODO: We frequently do `.map(r => r.id)` on this, via `recipientIdsOfPrivateMessage`.
+  //   Instead of making a new array every time, it'd be good to do that
+  //   once, up front, at the edge.
   display_recipient: $ReadOnlyArray<PmRecipientUser>,
 
   /**
@@ -1062,6 +1065,7 @@ export type StreamMessage = $ReadOnly<{|
    *   https://chat.zulip.org/#narrow/stream/19-documentation/topic/.60orig_subject.60.20in.20.60update_message.60.20events/near/1112709
    * (see point 4). We assume this has always been the case.
    */
+  // Still "subject", as of 2022: https://github.com/zulip/zulip/issues/1192
   subject: string,
 
   // We don't actually use this property.  If and when we do, we'll probably

src/api/notificationTypes.js

@@ -32,9 +32,7 @@ type BaseData = {|
    *
    * (This lets us distinguish different accounts in the same realm.)
    */
-  // added 2.1-dev-540-g447a517e6f, release 2.1.0+
-  // TODO(server-2.1): Mark required; simplify comment.
-  +user_id?: UserId,
+  +user_id: UserId,
 
   // The rest of the data is about the Zulip message we're being notified
   // about.

src/api/typing.js

@@ -1,12 +1,17 @@
 /* @flow strict-local */
 import type { ApiResponse, Auth } from './transportTypes';
+import type { UserId } from './idTypes';
 import { apiPost } from './apiFetch';
 
 type TypingOperation = 'start' | 'stop';
 
 /** See https://zulip.com/api/set-typing-status */
-export default (auth: Auth, recipients: string, operation: TypingOperation): Promise<ApiResponse> =>
+export default (
+  auth: Auth,
+  recipients: $ReadOnlyArray<UserId>,
+  operation: TypingOperation,
+): Promise<ApiResponse> =>
   apiPost(auth, 'typing', {
-    to: recipients,
+    to: JSON.stringify(recipients),
     op: operation,
   });

src/chat/ChatScreen.js

@@ -201,7 +201,7 @@ export default function ChatScreen(props: Props): Node {
         );
 
         if ((content !== undefined && content !== '') || (topic !== undefined && topic !== '')) {
-          api.updateMessage(auth, editMessage.id, { content, subject: topic }).catch(error => {
+          api.updateMessage(auth, editMessage.id, { content, topic }).catch(error => {
             showErrorAlert(_('Failed to edit message'), error.message);
           });
         }