api: Start relying more on non-ancient servers

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

@@ -197,10 +197,12 @@ private fun extractIdentity(data: Map<String, String>): Identity =
         // 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")
     )
 

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

@@ -198,6 +198,7 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {
         ))
 
         /// 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"
         ))

src/__tests__/lib/exampleData.js

@@ -328,7 +328,6 @@ const messagePropertiesFromSender = (user: User) => {
   const { user_id: sender_id, email: sender_email, full_name: sender_full_name } = user;
 
   return deepFreeze({
-    sender_domain: '',
     avatar_url: user.avatar_url,
     client: 'ExampleClient',
     gravatar_hash: 'd3adb33f',
@@ -714,8 +713,8 @@ export const action = Object.freeze({
     anchor: 0,
     numBefore: 50,
     numAfter: 50,
-    foundNewest: undefined,
-    foundOldest: undefined,
+    foundNewest: false,
+    foundOldest: false,
     ownUserId: selfUser.user_id,
   }): MessageFetchCompleteAction),
   // If a given action is only relevant to a single test file, no need to

src/account/accountsSelectors.js

@@ -89,8 +89,22 @@ export const getAccount = (state: PerAccountState): Account => {
 /** The realm URL for this account. */
 export const getRealmUrl = (state: PerAccountState): URL => getAccount(state).realm;
 
-/** The Zulip server version for this account. */
-export const getZulipVersion = (state: PerAccountState): ZulipVersion => {
+/**
+ * The Zulip server version for this account.
+ *
+ * Prefer `getZulipFeatureLevel`, which is finer-grained, for most uses.
+ * This function is useful for logging, for user-facing information, and for
+ * old releases (pre-3.0) where the feature level is always 0.
+ *
+ * This function assumes we have server data for this account, and if not it
+ * may throw.  If you want to call it from a context where we may not have
+ * server data, we can fix that; see implementation comment.
+ *
+ * If using this to condition behavior where we make a request to the
+ * server, note that there's fundamentally a race; for details, see
+ * `getZulipFeatureLevel`.
+ */
+export const getServerVersion = (state: PerAccountState): ZulipVersion => {
   const { zulipVersion } = getAccount(state);
   // This invariant will hold as long as we only call this function in a
   // context where we have server data.
@@ -107,13 +121,29 @@ export const getZulipVersion = (state: PerAccountState): ZulipVersion => {
   return zulipVersion;
 };
 
-/** The Zulip server feature level for this account. */
+/**
+ * The Zulip server feature level for this account.
+ *
+ * This function assumes we have server data for this account, and if not it
+ * may throw.  If you want to call it from a context where we may not have
+ * server data, we can fix that; see implementation comment.
+ *
+ * If using this to condition behavior where we make a request to the
+ * server, note that there's fundamentally a race: the server may have been
+ * upgraded since we last heard from it.  Generally we don't worry about
+ * this because (a) usually our behavior for old servers is fine for new
+ * servers (by design in the new server, because it's the same behavior old
+ * clients will have), and then this can only be a problem on downgrade,
+ * which is rare; (b) with the exception of initial fetch, almost all our
+ * requests come while we already have a live event queue which would tell
+ * us about an upgrade, so the race is quite narrow.
+ */
 export const getZulipFeatureLevel = (state: PerAccountState): number => {
   const { zulipFeatureLevel } = getAccount(state);
   // This invariant will hold as long as we only call this function in a
   // context where we have server data.
   //
-  // TODO(#5006): Much like getZulipVersion above.  This property is just a
+  // TODO(#5006): Much like getServerVersion above.  This property is just a
   //   bit newer: b058fa266, from 2020-09.
   invariant(zulipFeatureLevel !== null, 'zulipFeatureLevel must be non-null in PerAccountState');
   return zulipFeatureLevel;
@@ -178,12 +208,3 @@ export const getAuth = (state: PerAccountState): Auth => {
 export const getIdentity: Selector<Identity> = createSelector(getAuth, auth =>
   identityOfAuth(auth),
 );
-
-/**
- * The Zulip server version for this account, or null if unknown.
- *
- * See the `zulipVersion` property of `Account` for details on how this
- * information is kept up to date.
- */
-export const getServerVersion = (state: PerAccountState): ZulipVersion | null =>
-  getAccount(state).zulipVersion;

src/actionTypes.js

@@ -221,8 +221,8 @@ export type MessageFetchCompleteAction = {|
   anchor: number,
   numBefore: number,
   numAfter: number,
-  foundNewest: boolean | void,
-  foundOldest: boolean | void,
+  foundNewest: boolean,
+  foundOldest: boolean,
   ownUserId: UserId,
 |};
 
@@ -297,8 +297,8 @@ type EventSubscriptionUpdateAction = {|
   property: string,
   value: boolean | number | string,
 
+  // TODO(server-4.0): Delete these commented-out properties.
   // name: string, // exists pre-4.0, but expected to be removed soon
-
   // email: string, // gone in 4.0; was the user's own email, so never useful
 |};
 

src/api/eventTypes.js

@@ -145,6 +145,7 @@ export type UpdateMessageFlagsEvent = $ReadOnly<{|
 
   // Servers with feature level 32+ send `op`. Servers will eventually
   // stop sending `operation`; see #4238.
+  // TODO(server-4.0): Simplify to just `op`.
   operation?: 'add' | 'remove',
   op?: 'add' | 'remove',
 
@@ -168,6 +169,7 @@ export type RestartEvent = $ReadOnly<{|
   //
   // They have the same shape and meaning as the same-named fields in
   // the /server_settings and /register responses.
+  // TODO(server-4.0): Mark these as required.
   zulip_version?: string,
   zulip_feature_level?: number,
 |}>;

src/api/initialDataTypes.js

@@ -30,10 +30,11 @@ export type InitialDataBase = $ReadOnly<{|
   // `fetch_event_types` and remove this comment.
 
   /**
-   * Added in server version 2.2, feature level 1.
+   * Added in server version 3.0, feature level 1.
    * Same meaning as in the server_settings response:
    * https://zulip.com/api/get-server-settings. See also the comment above.
    */
+  // TODO(server-3.0): Mark as required.
   zulip_feature_level?: number,
 
   /**
@@ -59,8 +60,8 @@ export type InitialDataMutedTopics = $ReadOnly<{|
   muted_topics: $ReadOnlyArray<MuteTuple>,
 |}>;
 
-/** Added in server version 4.0, feature level 48 */
 export type InitialDataMutedUsers = $ReadOnly<{|
+  /** (When absent, treat as empty.  Added in server version 4.0, feature level 48.) */
   muted_users?: $ReadOnlyArray<MutedUser>,
 |}>;
 
@@ -124,6 +125,7 @@ export type InitialDataRealmEmoji = $ReadOnly<{|
 export type RawInitialDataRealmFilters = $ReadOnly<{|
   // We still request this, since not all servers can provide the
   // newer `realm_linkifiers` format.
+  // TODO(server-4.0): Drop this.
   realm_filters?: $ReadOnlyArray<RealmFilter>,
 |}>;
 
@@ -173,14 +175,17 @@ export type InitialDataRealmUserGroups = $ReadOnly<{|
   /**
    * Absent in servers prior to v1.8.0-rc1~2711 (or thereabouts).
    */
+  // TODO(server-1.8): Mark as required.
   realm_user_groups?: $ReadOnlyArray<UserGroup>,
 |}>;
 
 export type InitialDataRecentPmConversations = $ReadOnly<{|
   // * Added in server commit 2.1-dev-384-g4c3c669b41.
+  //   TODO(server-2.1): Mark this required.  See MIN_RECENTPMS_SERVER_VERSION.
   // * `user_id` fields are sorted as of commit 2.2-dev-53-g405a529340, which
   //    was backported to 2.1.1-50-gd452ad31e0 -- meaning that they are _not_
   //    sorted in either v2.1.0 or v2.1.1.
+  // TODO(server-3.0): Simply say these are sorted.  ("2.2" became 3.0.)
   recent_private_conversations?: $ReadOnlyArray<RecentPrivateConversation>,
 |}>;
 

src/api/messages/getMessages.js

@@ -10,9 +10,9 @@ import { AvatarURL } from '../../utils/avatar';
 type ApiResponseMessages = {|
   ...$Exact<ApiResponseSuccess>,
   anchor: number,
-  found_anchor?: boolean,
-  found_newest?: boolean,
-  found_oldest?: boolean,
+  found_anchor: boolean,
+  found_newest: boolean,
+  found_oldest: boolean,
   messages: Message[],
 |};
 
@@ -25,6 +25,7 @@ type ApiResponseMessages = {|
 // We shouldn't have to rely on this format on servers at feature
 // level 2+; those newer servers include a top-level `user_id` field
 // in addition to the `user` object. See #4072.
+// TODO(server-3.0): Simplify this away.
 export type ServerReaction = $ReadOnly<{|
   ...$Diff<Reaction, {| user_id: mixed |}>,
   user: $ReadOnly<{|
@@ -83,11 +84,6 @@ const migrateResponse = (response, identity: Identity) => {
 
 /**
  * See https://zulip.com/api/get-messages
- *
- * These values exist only in Zulip 1.8 or newer:
- *   * found_anchor
- *   * found_newest
- *   * found_oldest
  */
 export default async (
   auth: Auth,

src/api/modelTypes.js

@@ -37,6 +37,7 @@ export type RealmEmojiById = $ReadOnly<{|
 /**
  * The only way servers before feature level 54 represent linkifiers.
  */
+// TODO(server-4.0): Delete this.
 export type RealmFilter = [string, string, number];
 
 /**
@@ -52,8 +53,8 @@ export type RealmFilter = [string, string, number];
 //   `realm_linkifiers` format. (When doing so, also don't forget to
 //   change various variable and type definition names to be like
 //   `realm_linkifiers`.)
-// - When we drop support for servers older than 54, we can remove all
-//   our code that knows about the `realm_filters` format.
+// - TODO(server-4.0): When we drop support for servers older than 54, we
+//   can remove all our code that knows about the `realm_filters` format.
 export type RealmLinkifier = $ReadOnly<{|
   id: number,
   pattern: string,
@@ -108,6 +109,7 @@ export type User = $ReadOnly<{|
   full_name: string,
 
   // date_joined included since commit 372e9740a (in 1.9.0)
+  // TODO(server-1.9): mark this as required
   date_joined?: string,
 
   // is_active doesn't appear in `/register` responses -- instead,
@@ -155,7 +157,7 @@ export type User = $ReadOnly<{|
   // profile_data added in commit 02b845336 (in 1.8.0);
   // see also e3aed0f7b (in 2.0.0)
   // (This one doesn't appear in `/users` responses.)
-  profile_data?: empty, // TODO describe actual type
+  profile_data?: empty, // When we need this, describe its actual type.
 |}>;
 
 /**
@@ -177,6 +179,7 @@ export type CrossRealmBot = $ReadOnly<{|
   avatar_url: AvatarURL,
 
   // date_joined included since commit 58ee3fa8c (in 1.9.0)
+  // TODO(server-1.9): mark this as required
   date_joined?: string,
 
   email: string,
@@ -351,6 +354,7 @@ export type NarrowElement =
  //  * `group-pm-with` since 2.1-dev-1813-gb338fd130
  //  * `sender` since 2.1-dev-1812-gc067c155a
  //  * `pm-with` since 2.1-dev-1350-gd7b4de234
+ // TODO(server-2.1): Stop sending stream names or user emails here.
  | {| +operator: 'stream', +operand: string | number |} // stream ID
  | {| +operator: 'pm-with', +operand: string | $ReadOnlyArray<UserId> |}
  | {| +operator: 'sender', +operand: string | UserId |}
@@ -539,9 +543,6 @@ type MessageBase = $ReadOnly<{|
   match_content?: string,
   match_subject?: string,
 
-  /** Obsolete? Gone in server commit 1.6.0~1758 . */
-  sender_domain: string,
-
   /**
    * The `flags` story is a bit complicated:
    *  * Absent in `event.message` for a `message` event... but instead
@@ -598,7 +599,7 @@ export type PmMessage = $ReadOnly<{|
   // Notes from studying the server code:
   //  * Notes are primarily from the server as of 2020-04 at cb85763c7, but
   //    this logic is very stable; confirmed all points about behavior as of
-  //    1.8.0, too.
+  //    1.8.0 (from 2018-04), too.
   //
   //  * This field is ultimately computed (for both events and /messages
   //    results) in MessageDict.hydrate_recipient_info, with most of the
@@ -725,5 +726,6 @@ export type RecentPrivateConversation = $ReadOnly<{|
   max_message_id: number,
   // When received from the server, these are guaranteed to be sorted only after
   // 2.2-dev-53-g405a529340. To be safe, we always sort them on receipt.
+  // TODO(server-3.0): Stop sorting these ourselves.  ("2.2" became 3.0.)
   user_ids: $ReadOnlyArray<UserId>,
 |}>;

src/api/registerForEvents.js

@@ -54,6 +54,8 @@ const transform = (rawInitialData: RawInitialData, auth: Auth): InitialData => (
   // Transform the newer `realm_linkifiers` format, if present, to the
   // older `realm_filters` format. We do the same transformation on
   // 'realm_linkifiers' events.
+  // TODO(server-4.0): Switch to new format, if we haven't already;
+  //   and drop conversion.
   realm_filters: rawInitialData.realm_linkifiers
     ? rawInitialData.realm_linkifiers.map(({ pattern, url_format, id }) => [
         pattern,

src/api/settings/getServerSettings.js

@@ -26,6 +26,7 @@ export type ApiResponseServerSettings = {|
   ...$Exact<ApiResponseSuccess>,
   authentication_methods: AuthenticationMethods,
   // external_authentication_methods added for server v2.1
+  // TODO(server-2.1): Mark this as required; simplify downstream.
   external_authentication_methods?: ExternalAuthenticationMethod[],
   email_auth_enabled: boolean,
   push_notifications_enabled: boolean,
@@ -36,8 +37,9 @@ export type ApiResponseServerSettings = {|
   require_email_format_usernames: boolean,
   zulip_version: string,
 
-  // zulip_feature_level added for server v2.2, feature level 1
+  // zulip_feature_level added for server v3.0, feature level 1
   // See https://zulip.com/api/get-server-settings
+  // When absent, equivalent to 0.
   zulip_feature_level?: number,
 |};
 

src/api/streams/updateStream.js

@@ -9,6 +9,7 @@ import { apiPatch } from '../apiFetch';
  */
 // TODO(#4659): Once we pass the feature level to API methods, this one
 //   should encapsulate a switch at feature level 64.  See its call sites.
+// TODO(server-4.0): Simplify that away.
 export default (
   auth: Auth,
   id: number,

src/api/subscriptions/setSubscriptionProperty.js

@@ -20,8 +20,8 @@ export default async (
         // the server the older, confusingly named property
         // 'in_home_view' with the opposite value.
         //
-        // TODO: 'is_muted' is said to be new in Zulip 2.1, released
-        // 2019-12-12. Switch to sending that, once we can.
+        // TODO(server-2.1): 'is_muted' is said to be new in Zulip 2.1,
+        //   released 2019-12-12.  Switch to sending that, once we can.
         ...(property === 'is_muted'
           ? { property: 'in_home_view', value: !value }
           : { property, value }),

src/api/transportTypes.js

@@ -61,6 +61,8 @@ export type ApiResponseSuccess = $ReadOnly<{
  * that value when connecting to old servers that don't provide an error
  * code.
  *
+ * TODO(server-1.7): Simplify this.
+ *
  * [1] Specifically at 1.7.0~2354 and ancestors, aka 9faa44af6^..709c3b50fc .
  *     See: https://github.com/zulip/zulip/commit/709c3b50fc
  *