api: Remove backward-compat code for mark-read protocol in FL <155

This code had a switch/case on the Narrow type, so I discovered it
while implementing keyword-search narrows.

We support Zulip Server 7 and later (see README) and refuse to
connect to older servers. Since we haven't been using this protocol
for servers FL 155+, this should be NFC for all servers we connect
to, as long as we know their feature level accurately.

Related: #992

Author: chrisbobbe

lib/api/route/messages.dart

@@ -392,9 +392,6 @@ class UpdateMessageFlagsResult {
 }
 
 /// https://zulip.com/api/update-message-flags-for-narrow
-///
-/// This binding only supports feature levels 155+.
-// TODO(server-6) remove FL 155+ mention in doc, and the related `assert`
 Future<UpdateMessageFlagsForNarrowResult> updateMessageFlagsForNarrow(ApiConnection connection, {
   required Anchor anchor,
   bool? includeAnchor,
@@ -404,7 +401,6 @@ Future<UpdateMessageFlagsForNarrowResult> updateMessageFlagsForNarrow(ApiConnect
   required UpdateMessageFlagsOp op,
   required MessageFlag flag,
 }) {
-  assert(connection.zulipFeatureLevel! >= 155);
   return connection.post('updateMessageFlagsForNarrow', UpdateMessageFlagsForNarrowResult.fromJson, 'messages/flags/narrow', {
     'anchor': RawParameter(anchor.toJson()),
     if (includeAnchor != null) 'include_anchor': includeAnchor,
@@ -439,63 +435,3 @@ class UpdateMessageFlagsForNarrowResult {
 
   Map<String, dynamic> toJson() => _$UpdateMessageFlagsForNarrowResultToJson(this);
 }
-
-/// https://zulip.com/api/mark-all-as-read
-///
-/// This binding is deprecated, in FL 155+ use
-/// [updateMessageFlagsForNarrow] instead.
-// TODO(server-6): Remove as deprecated by updateMessageFlagsForNarrow
-//
-// For FL < 153 this call was atomic on the server and would
-// not mark any messages as read if it timed out.
-// From FL 153 and onward the server started processing
-// in batches so progress could still be made in the event
-// of a timeout interruption. Thus, in FL 153 this call
-// started returning `result: partially_completed` and
-// `code: REQUEST_TIMEOUT` for timeouts.
-//
-// In FL 211 the `partially_completed` variant of
-// `result` was removed, the string `code` field also
-// removed, and a boolean `complete` field introduced.
-//
-// For full support of this endpoint we would need three
-// variants of the return structure based on feature
-// level (`{}`, `{code: string}`, and `{complete: bool}`)
-// as well as handling of `partially_completed` variant
-// of `result` in `lib/api/core.dart`. For simplicity we
-// ignore these return values.
-//
-// We don't use this method for FL 155+ (it is replaced
-// by `updateMessageFlagsForNarrow`) so there are only
-// two versions (FL 153 and FL 154) affected.
-Future<void> markAllAsRead(ApiConnection connection) {
-  return connection.post('markAllAsRead', (_) {}, 'mark_all_as_read', {});
-}
-
-/// https://zulip.com/api/mark-stream-as-read
-///
-/// This binding is deprecated, in FL 155+ use
-/// [updateMessageFlagsForNarrow] instead.
-// TODO(server-6): Remove as deprecated by updateMessageFlagsForNarrow
-Future<void> markStreamAsRead(ApiConnection connection, {
-  required int streamId,
-}) {
-  return connection.post('markStreamAsRead', (_) {}, 'mark_stream_as_read', {
-    'stream_id': streamId,
-  });
-}
-
-/// https://zulip.com/api/mark-topic-as-read
-///
-/// This binding is deprecated, in FL 155+ use
-/// [updateMessageFlagsForNarrow] instead.
-// TODO(server-6): Remove as deprecated by updateMessageFlagsForNarrow
-Future<void> markTopicAsRead(ApiConnection connection, {
-  required int streamId,
-  required TopicName topicName,
-}) {
-  return connection.post('markTopicAsRead', (_) {}, 'mark_topic_as_read', {
-    'stream_id': streamId,
-    'topic_name': RawParameter(topicName.apiName),
-  });
-}

lib/model/unreads.dart

@@ -441,22 +441,20 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier {
     notifyListeners();
   }
 
-  /// To be called on success of a mark-all-as-read task in the modern protocol.
+  /// To be called on success of a mark-all-as-read task.
   ///
   /// When the user successfully marks all messages as read,
   /// there can't possibly be ancient unreads we don't know about.
   /// So this updates [oldUnreadsMissing] to false and calls [notifyListeners].
   ///
-  /// When we use POST /messages/flags/narrow (FL 155+) for mark-all-as-read,
-  /// we don't expect to get a mark-as-read event with `all: true`,
+  /// We don't expect to get a mark-as-read event with `all: true`,
   /// even on completion of the last batch of unreads.
-  /// If we did get an event with `all: true` (as we do in the legacy mark-all-
+  /// If we did get an event with `all: true` (as we did in a legacy mark-all-
   /// as-read protocol), this would be handled naturally, in
   /// [handleUpdateMessageFlagsEvent].
   ///
   /// Discussion:
   ///   <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680275>
-  // TODO(server-6) Delete mentions of legacy protocol.
   void handleAllMessagesReadSuccess() {
     oldUnreadsMissing = false;
 

lib/widgets/actions.dart

@@ -26,25 +26,7 @@ abstract final class ZulipAction {
   /// This is mostly a wrapper around [updateMessageFlagsStartingFromAnchor];
   /// for details on the UI feedback, see there.
   static Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
-    final store = PerAccountStoreWidget.of(context);
     final zulipLocalizations = ZulipLocalizations.of(context);
-    final useLegacy = store.zulipFeatureLevel < 155; // TODO(server-6)
-    if (useLegacy) {
-      try {
-        await _legacyMarkNarrowAsRead(context, narrow);
-        return;
-      } catch (e) {
-        if (!context.mounted) return;
-        final message = switch (e) {
-          ZulipApiException() => zulipLocalizations.errorServerMessage(e.message),
-          _ => e.toString(), // TODO(#741): extract user-facing message better
-        };
-        showErrorDialog(context: context,
-          title: zulipLocalizations.errorMarkAsReadFailedTitle,
-          message: message);
-        return;
-      }
-    }
 
     final didPass = await updateMessageFlagsStartingFromAnchor(
       context: context,
@@ -208,39 +190,6 @@ abstract final class ZulipAction {
     }
   }
 
-  static Future<void> _legacyMarkNarrowAsRead(BuildContext context, Narrow narrow) async {
-    final store = PerAccountStoreWidget.of(context);
-    final connection = store.connection;
-    switch (narrow) {
-      case CombinedFeedNarrow():
-        await markAllAsRead(connection);
-      case ChannelNarrow(:final streamId):
-        await markStreamAsRead(connection, streamId: streamId);
-      case TopicNarrow(:final streamId, :final topic):
-        await markTopicAsRead(connection, streamId: streamId, topicName: topic);
-      case DmNarrow():
-        final unreadDms = store.unreads.dms[narrow];
-        // Silently ignore this race-condition as the outcome
-        // (no unreads in this narrow) was the desired end-state
-        // of pushing the button.
-        if (unreadDms == null) return;
-        await updateMessageFlags(connection,
-          messages: unreadDms,
-          op: UpdateMessageFlagsOp.add,
-          flag: MessageFlag.read);
-      case MentionsNarrow():
-        final unreadMentions = store.unreads.mentions.toList();
-        if (unreadMentions.isEmpty) return;
-        await updateMessageFlags(connection,
-          messages: unreadMentions,
-          op: UpdateMessageFlagsOp.add,
-          flag: MessageFlag.read);
-      case StarredMessagesNarrow():
-        // TODO: Implement unreads handling.
-        return;
-    }
-  }
-
   /// Fetch and return the raw Markdown content for [messageId],
   /// showing an error dialog on failure.
   static Future<String?> fetchRawContentWithFeedback({

test/api/route/messages_test.dart

@@ -829,76 +829,4 @@ void main() {
       });
     });
   });
-
-  group('markAllAsRead', () {
-    Future<void> checkMarkAllAsRead(
-      FakeApiConnection connection, {
-      required Map<String, String> expected,
-    }) async {
-      connection.prepare(json: {});
-      await markAllAsRead(connection);
-      check(connection.lastRequest).isA<http.Request>()
-        ..method.equals('POST')
-        ..url.path.equals('/api/v1/mark_all_as_read')
-        ..bodyFields.deepEquals(expected);
-    }
-
-    test('smoke', () {
-      return FakeApiConnection.with_((connection) async {
-        await checkMarkAllAsRead(connection, expected: {});
-      });
-    });
-  });
-
-  group('markStreamAsRead', () {
-    Future<void> checkMarkStreamAsRead(
-      FakeApiConnection connection, {
-      required int streamId,
-      required Map<String, String> expected,
-    }) async {
-      connection.prepare(json: {});
-      await markStreamAsRead(connection, streamId: streamId);
-      check(connection.lastRequest).isA<http.Request>()
-        ..method.equals('POST')
-        ..url.path.equals('/api/v1/mark_stream_as_read')
-        ..bodyFields.deepEquals(expected);
-    }
-
-    test('smoke', () {
-      return FakeApiConnection.with_((connection) async {
-        await checkMarkStreamAsRead(connection,
-          streamId: 10,
-          expected: {'stream_id': '10'});
-      });
-    });
-  });
-
-  group('markTopicAsRead', () {
-    Future<void> checkMarkTopicAsRead(
-      FakeApiConnection connection, {
-      required int streamId,
-      required String topicName,
-      required Map<String, String> expected,
-    }) async {
-      connection.prepare(json: {});
-      await markTopicAsRead(connection,
-        streamId: streamId, topicName: eg.t(topicName));
-      check(connection.lastRequest).isA<http.Request>()
-        ..method.equals('POST')
-        ..url.path.equals('/api/v1/mark_topic_as_read')
-        ..bodyFields.deepEquals(expected);
-    }
-
-    test('smoke', () {
-      return FakeApiConnection.with_((connection) async {
-        await checkMarkTopicAsRead(connection,
-          streamId: 10,
-          topicName: 'topic',
-          expected: {
-            'stream_id': '10',
-            'topic_name': 'topic',
-          });
-      });
-    });
-  });
 }

test/widgets/actions_test.dart

@@ -21,7 +21,6 @@ import '../api/fake_api.dart';
 import '../example_data.dart' as eg;
 import '../flutter_checks.dart';
 import '../model/binding.dart';
-import '../model/unreads_checks.dart';
 import '../stdlib_checks.dart';
 import '../test_clipboard.dart';
 import 'dialog_checks.dart';
@@ -119,106 +118,6 @@ void main() {
         await future;
         check(store.unreads.oldUnreadsMissing).isFalse();
       });
-
-      testWidgets('CombinedFeedNarrow on legacy server', (tester) async {
-        const narrow = CombinedFeedNarrow();
-        await prepare(tester);
-        // Might as well test with oldUnreadsMissing: true.
-        store.unreads.oldUnreadsMissing = true;
-
-        connection.zulipFeatureLevel = 154;
-        connection.prepare(json: {});
-        final future = ZulipAction.markNarrowAsRead(context, narrow);
-        await tester.pump(Duration.zero);
-        await future;
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('POST')
-          ..url.path.equals('/api/v1/mark_all_as_read')
-          ..bodyFields.deepEquals({});
-
-        // Check that [Unreads.handleAllMessagesReadSuccess] wasn't called;
-        // in the legacy protocol, that'd be redundant with the mark-read event.
-        check(store.unreads).oldUnreadsMissing.isTrue();
-      });
-
-      testWidgets('ChannelNarrow on legacy server', (tester) async {
-        final stream = eg.stream();
-        final narrow = ChannelNarrow(stream.streamId);
-        await prepare(tester);
-        connection.zulipFeatureLevel = 154;
-        connection.prepare(json: {});
-        final future = ZulipAction.markNarrowAsRead(context, narrow);
-        await tester.pump(Duration.zero);
-        await future;
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('POST')
-          ..url.path.equals('/api/v1/mark_stream_as_read')
-          ..bodyFields.deepEquals({
-              'stream_id': stream.streamId.toString(),
-            });
-      });
-
-      testWidgets('TopicNarrow on legacy server', (tester) async {
-        final narrow = TopicNarrow.ofMessage(eg.streamMessage());
-        await prepare(tester);
-        connection.zulipFeatureLevel = 154;
-        connection.prepare(json: {});
-        final future = ZulipAction.markNarrowAsRead(context, narrow);
-        await tester.pump(Duration.zero);
-        await future;
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('POST')
-          ..url.path.equals('/api/v1/mark_topic_as_read')
-          ..bodyFields.deepEquals({
-              'stream_id': narrow.streamId.toString(),
-              'topic_name': narrow.topic,
-            });
-      });
-
-      testWidgets('DmNarrow on legacy server', (tester) async {
-        final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
-        final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
-        final unreadMsgs = eg.unreadMsgs(dms: [
-          UnreadDmSnapshot(otherUserId: eg.otherUser.userId,
-            unreadMessageIds: [message.id]),
-        ]);
-        await prepare(tester, unreadMsgs: unreadMsgs);
-        connection.zulipFeatureLevel = 154;
-        connection.prepare(json:
-          UpdateMessageFlagsResult(messages: [message.id]).toJson());
-        final future = ZulipAction.markNarrowAsRead(context, narrow);
-        await tester.pump(Duration.zero);
-        await future;
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('POST')
-          ..url.path.equals('/api/v1/messages/flags')
-          ..bodyFields.deepEquals({
-              'messages': jsonEncode([message.id]),
-              'op': 'add',
-              'flag': 'read',
-            });
-      });
-
-      testWidgets('MentionsNarrow on legacy server', (tester) async {
-        const narrow = MentionsNarrow();
-        final message = eg.streamMessage(flags: [MessageFlag.mentioned]);
-        final unreadMsgs = eg.unreadMsgs(mentions: [message.id]);
-        await prepare(tester, unreadMsgs: unreadMsgs);
-        connection.zulipFeatureLevel = 154;
-        connection.prepare(json:
-          UpdateMessageFlagsResult(messages: [message.id]).toJson());
-        final future = ZulipAction.markNarrowAsRead(context, narrow);
-        await tester.pump(Duration.zero);
-        await future;
-        check(connection.lastRequest).isA<http.Request>()
-          ..method.equals('POST')
-          ..url.path.equals('/api/v1/messages/flags')
-          ..bodyFields.deepEquals({
-              'messages': jsonEncode([message.id]),
-              'op': 'add',
-              'flag': 'read',
-            });
-      });
     });
 
     group('updateMessageFlagsStartingFromAnchor', () {