api: Remove legacy getMessageCompat, relying on server 5+, FL 120+

See "Feature level 120" from Zulip API changelog: https://zulip.com/api/changelog See also: 631f4d6 Signed-off-by: Zixuan James Li <[email protected]>
zulip · Aug 23, 2024 · de9d000 · de9d000
1 parent 4f7c773
commit de9d000
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 171 deletions.
diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart
@@ -1,64 +1,16 @@
 import 'package:json_annotation/json_annotation.dart';
 
 import '../core.dart';
-import '../exception.dart';
 import '../model/model.dart';
 import '../model/narrow.dart';
 
 part 'messages.g.dart';
 
-/// Convenience function to get a single message from any server.
-///
-/// This encapsulates a server-feature check.
-///
-/// Gives null if the server reports that the message doesn't exist.
-// TODO(server-5) Simplify this away; just use getMessage.
-Future<Message?> getMessageCompat(ApiConnection connection, {
-  required int messageId,
-  bool? applyMarkdown,
-}) async {
-  final useLegacyApi = connection.zulipFeatureLevel! < 120;
-  if (useLegacyApi) {
-    final response = await getMessages(connection,
-      narrow: [ApiNarrowMessageId(messageId)],
-      anchor: NumericAnchor(messageId),
-      numBefore: 0,
-      numAfter: 0,
-      applyMarkdown: applyMarkdown,
-
-      // Hard-code this param to `true`, as the new single-message API
-      // effectively does:
-      //   https://chat.zulip.org/#narrow/stream/378-api-design/topic/.60client_gravatar.60.20in.20.60messages.2F.7Bmessage_id.7D.60/near/1418337
-      clientGravatar: true,
-    );
-    return response.messages.firstOrNull;
-  } else {
-    try {
-      final response = await getMessage(connection,
-        messageId: messageId,
-        applyMarkdown: applyMarkdown,
-      );
-      return response.message;
-    } on ZulipApiException catch (e) {
-      if (e.code == 'BAD_REQUEST') {
-        // Servers use this code when the message doesn't exist, according to
-        // the example in the doc.
-        return null;
-      }
-      rethrow;
-    }
-  }
-}
-
 /// https://zulip.com/api/get-message
-///
-/// This binding only supports feature levels 120+.
-// TODO(server-5) remove FL 120+ mention in doc, and the related `assert`
 Future<GetMessageResult> getMessage(ApiConnection connection, {
   required int messageId,
   bool? applyMarkdown,
 }) {
-  assert(connection.zulipFeatureLevel! >= 120);
   return connection.get('getMessage', GetMessageResult.fromJson, 'messages/$messageId', {
     if (applyMarkdown != null) 'apply_markdown': applyMarkdown,
   });

diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart
@@ -190,17 +190,20 @@ Future<String?> fetchRawContentWithFeedback({
     //   On final failure or success, auto-dismiss the snackbar.
     final zulipLocalizations = ZulipLocalizations.of(context);
     try {
-      fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(context).connection,
+      fetchedMessage = (await getMessage(PerAccountStoreWidget.of(context).connection,
         messageId: messageId,
         applyMarkdown: false,
-      );
-      if (fetchedMessage == null) {
-        errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist;
-      }
+      )).message;
     } catch (e) {
       switch (e) {
         case ZulipApiException():
-          errorMessage = e.message;
+          if (e.code == 'BAD_REQUEST') {
+            // Servers use this code when the message doesn't exist, according
+            // to the example in the doc.
+            errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist;
+          } else {
+            errorMessage = e.message;
+          }
         // TODO specific messages for common errors, like network errors
         //   (support with reusable code)
         default:

diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart
@@ -15,114 +15,6 @@ import '../fake_api.dart';
 import 'route_checks.dart';
 
 void main() {
-  group('getMessageCompat', () {
-    Future<Message?> checkGetMessageCompat(FakeApiConnection connection, {
-      required bool expectLegacy,
-      required int messageId,
-      bool? applyMarkdown,
-    }) async {
-      final result = await getMessageCompat(connection,
-        messageId: messageId,
-        applyMarkdown: applyMarkdown,
-      );
-      if (expectLegacy) {
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('GET')
-          ..url.path.equals('/api/v1/messages')
-          ..url.queryParameters.deepEquals({
-            'narrow': jsonEncode([ApiNarrowMessageId(messageId)]),
-            'anchor': messageId.toString(),
-            'num_before': '0',
-            'num_after': '0',
-            if (applyMarkdown != null) 'apply_markdown': applyMarkdown.toString(),
-            'client_gravatar': 'true',
-          });
-      } else {
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('GET')
-          ..url.path.equals('/api/v1/messages/$messageId')
-          ..url.queryParameters.deepEquals({
-            if (applyMarkdown != null) 'apply_markdown': applyMarkdown.toString(),
-          });
-      }
-      return result;
-    }
-
-    test('modern; message found', () {
-      return FakeApiConnection.with_((connection) async {
-        final message = eg.streamMessage();
-        final fakeResult = GetMessageResult(message: message);
-        connection.prepare(json: fakeResult.toJson());
-        final result = await checkGetMessageCompat(connection,
-          expectLegacy: false,
-          messageId: message.id,
-          applyMarkdown: true,
-        );
-        check(result).isNotNull().jsonEquals(message);
-      });
-    });
-
-    test('modern; message not found', () {
-      return FakeApiConnection.with_((connection) async {
-        final message = eg.streamMessage();
-        final fakeResponseJson = {
-          'code': 'BAD_REQUEST',
-          'msg': 'Invalid message(s)',
-          'result': 'error',
-        };
-        connection.prepare(httpStatus: 400, json: fakeResponseJson);
-        final result = await checkGetMessageCompat(connection,
-          expectLegacy: false,
-          messageId: message.id,
-          applyMarkdown: true,
-        );
-        check(result).isNull();
-      });
-    });
-
-    test('legacy; message found', () {
-      return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async {
-        final message = eg.streamMessage();
-        final fakeResult = GetMessagesResult(
-          anchor: message.id,
-          foundNewest: false,
-          foundOldest: false,
-          foundAnchor: true,
-          historyLimited: false,
-          messages: [message],
-        );
-        connection.prepare(json: fakeResult.toJson());
-        final result = await checkGetMessageCompat(connection,
-          expectLegacy: true,
-          messageId: message.id,
-          applyMarkdown: true,
-        );
-        check(result).isNotNull().jsonEquals(message);
-      });
-    });
-
-    test('legacy; message not found', () {
-      return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async {
-        final message = eg.streamMessage();
-        final fakeResult = GetMessagesResult(
-          anchor: message.id,
-          foundNewest: false,
-          foundOldest: false,
-          foundAnchor: false,
-          historyLimited: false,
-          messages: [],
-        );
-        connection.prepare(json: fakeResult.toJson());
-        final result = await checkGetMessageCompat(connection,
-          expectLegacy: true,
-          messageId: message.id,
-          applyMarkdown: true,
-        );
-        check(result).isNull();
-      });
-    });
-  });
-
   group('getMessage', () {
     Future<GetMessageResult> checkGetMessage(
       FakeApiConnection connection, {
@@ -162,15 +54,6 @@ void main() {
           expected: {'apply_markdown': 'false'});
       });
     });
-
-    test('Throws assertion error when FL <120', () {
-      return FakeApiConnection.with_(zulipFeatureLevel: 119, (connection) async {
-        connection.prepare(json: fakeResult.toJson());
-        check(() => getMessage(connection,
-          messageId: 1,
-        )).throws<AssertionError>();
-      });
-    });
   });
 
   test('Narrow.toJson', () {