msglist: Follow /with/ links through message moves

lib/api/model/narrow.dart

@@ -6,18 +6,41 @@ part 'narrow.g.dart';
 
 typedef ApiNarrow = List<ApiNarrowElement>;
 
-/// Resolve any [ApiNarrowDm] elements appropriately.
+/// Adapt the given narrow to be sent to the given Zulip server version.
 ///
-/// This encapsulates a server-feature check.
-ApiNarrow resolveDmElements(ApiNarrow narrow, int zulipFeatureLevel) {
-  if (!narrow.any((element) => element is ApiNarrowDm)) {
+/// Any elements that take a different name on old vs. new servers
+/// will be resolved to the specific name to use.
+/// Any elements that are unknown to old servers and can
+/// reasonably be omitted will be omitted.
+ApiNarrow resolveApiNarrowForServer(ApiNarrow narrow, int zulipFeatureLevel) {
+  final supportsOperatorDm = zulipFeatureLevel >= 177; // TODO(server-7)
+  final supportsOperatorWith = zulipFeatureLevel >= 271; // TODO(server-9)
+
+  bool hasDmElement = false;
+  bool hasWithElement = false;
+  for (final element in narrow) {
+    switch (element) {
+      case ApiNarrowDm():   hasDmElement = true;
+      case ApiNarrowWith(): hasWithElement = true;
+      default:
+    }
+  }
+  if (!(hasDmElement || (hasWithElement && !supportsOperatorWith))) {
     return narrow;
   }
-  final supportsOperatorDm = zulipFeatureLevel >= 177; // TODO(server-7)
-  return narrow.map((element) => switch (element) {
-    ApiNarrowDm() => element.resolve(legacy: !supportsOperatorDm),
-    _             => element,
-  }).toList();
+
+  final result = <ApiNarrowElement>[];
+  for (final element in narrow) {
+    switch (element) {
+      case ApiNarrowDm():
+        result.add(element.resolve(legacy: !supportsOperatorDm));
+      case ApiNarrowWith() when !supportsOperatorWith:
+        break; // drop unsupported element
+      default:
+        result.add(element);
+    }
+  }
+  return result;
 }
 
 /// An element in the list representing a narrow in the Zulip API.
@@ -102,7 +125,7 @@ class ApiNarrowTopic extends ApiNarrowElement {
 /// and more generally its [operator] getter must not be called.
 /// Instead, call [resolve] and use the object it returns.
 ///
-/// If part of [ApiNarrow] use [resolveDmElements].
+/// If part of [ApiNarrow] use [resolveApiNarrowForServer].
 class ApiNarrowDm extends ApiNarrowElement {
   @override String get operator {
     assert(false,
@@ -150,6 +173,22 @@ class ApiNarrowPmWith extends ApiNarrowDm {
   ApiNarrowPmWith._(super.operand, {super.negated});
 }
 
+/// An [ApiNarrowElement] with the 'with' operator.
+///
+/// If part of [ApiNarrow] use [resolveApiNarrowForServer].
+class ApiNarrowWith extends ApiNarrowElement {
+  @override String get operator => 'with';
+
+  @override final int operand;
+
+  ApiNarrowWith(this.operand, {super.negated});
+
+  factory ApiNarrowWith.fromJson(Map<String, dynamic> json) => ApiNarrowWith(
+    json['operand'] as int,
+    negated: json['negated'] as bool? ?? false,
+  );
+}
+
 class ApiNarrowIs extends ApiNarrowElement {
   @override String get operator => 'is';
 

lib/api/route/messages.dart

@@ -91,7 +91,7 @@ Future<GetMessagesResult> getMessages(ApiConnection connection, {
   // bool? useFirstUnreadAnchor // omitted because deprecated
 }) {
   return connection.get('getMessages', GetMessagesResult.fromJson, 'messages', {
-    'narrow': resolveDmElements(narrow, connection.zulipFeatureLevel!),
+    'narrow': resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!),
     'anchor': RawParameter(anchor.toJson()),
     if (includeAnchor != null) 'include_anchor': includeAnchor,
     'num_before': numBefore,
@@ -400,7 +400,7 @@ Future<UpdateMessageFlagsForNarrowResult> updateMessageFlagsForNarrow(ApiConnect
     if (includeAnchor != null) 'include_anchor': includeAnchor,
     'num_before': numBefore,
     'num_after': numAfter,
-    'narrow': resolveDmElements(narrow, connection.zulipFeatureLevel!),
+    'narrow': resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!),
     'op': RawParameter(op.toJson()),
     'flag': RawParameter(flag.toJson()),
   });

lib/model/internal_link.dart

@@ -59,7 +59,7 @@ String? decodeHashComponent(String str) {
 // you do so by passing the `anchor` param.
 Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) {
   // TODO(server-7)
-  final apiNarrow = resolveDmElements(
+  final apiNarrow = resolveApiNarrowForServer(
     narrow.apiEncode(), store.connection.zulipFeatureLevel!);
   final fragment = StringBuffer('narrow');
   for (ApiNarrowElement element in apiNarrow) {
@@ -86,6 +86,8 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) {
         fragment.write('${element.operand.join(',')}-$suffix');
       case ApiNarrowDm():
         assert(false, 'ApiNarrowDm should have been resolved');
+      case ApiNarrowWith():
+        fragment.write(element.operand.toString());
       case ApiNarrowIs():
         fragment.write(element.operand.toString());
       case ApiNarrowMessageId():
@@ -160,6 +162,7 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
   ApiNarrowStream? streamElement;
   ApiNarrowTopic? topicElement;
   ApiNarrowDm? dmElement;
+  ApiNarrowWith? withElement;
   Set<IsOperand> isElementOperands = {};
 
   for (var i = 0; i < segments.length; i += 2) {
@@ -188,12 +191,17 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
         if (dmIds == null) return null;
         dmElement = ApiNarrowDm(dmIds, negated: negated);
 
+      case _NarrowOperator.with_:
+        if (withElement != null) return null;
+        final messageId = int.tryParse(operand, radix: 10);
+        if (messageId == null) return null;
+        withElement = ApiNarrowWith(messageId);
+
       case _NarrowOperator.is_:
         // It is fine to have duplicates of the same [IsOperand].
         isElementOperands.add(IsOperand.fromRawString(operand));
 
       case _NarrowOperator.near: // TODO(#82): support for near
-      case _NarrowOperator.with_: // TODO(#683): support for with
         continue;
 
       case _NarrowOperator.unknown:
@@ -202,7 +210,9 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
   }
 
   if (isElementOperands.isNotEmpty) {
-    if (streamElement != null || topicElement != null || dmElement != null) return null;
+    if (streamElement != null || topicElement != null || dmElement != null || withElement != null) {
+      return null;
+    }
     if (isElementOperands.length > 1) return null;
     switch (isElementOperands.single) {
       case IsOperand.mentioned:
@@ -219,13 +229,14 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
         return null;
     }
   } else if (dmElement != null) {
-    if (streamElement != null || topicElement != null) return null;
+    if (streamElement != null || topicElement != null || withElement != null) return null;
     return DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId);
   } else if (streamElement != null) {
     final streamId = streamElement.operand;
     if (topicElement != null) {
-      return TopicNarrow(streamId, topicElement.operand);
+      return TopicNarrow(streamId, topicElement.operand, with_: withElement?.operand);
     } else {
+      if (withElement != null) return null;
       return ChannelNarrow(streamId);
     }
   }

lib/model/message_list.dart

@@ -511,6 +511,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
       numAfter: 0,
     );
     if (this.generation > generation) return;
+    _adjustNarrowForTopicPermalink(result.messages.firstOrNull);
     store.reconcileMessages(result.messages);
     store.recentSenders.handleMessages(result.messages); // TODO(#824)
     for (final message in result.messages) {
@@ -524,12 +525,47 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     notifyListeners();
   }
 
+  /// Update [narrow] for the result of a "with" narrow (topic permalink) fetch.
+  ///
+  /// To avoid an extra round trip, the server handles [ApiNarrowWith]
+  /// by returning results from the indicated message's current stream/topic
+  /// (if the user has access),
+  /// even if that differs from the narrow's stream/topic filters
+  /// because the message was moved.
+  ///
+  /// If such a "redirect" happened, this helper updates the stream and topic
+  /// in [narrow] to match the message's current conversation.
+  /// It also removes the "with" component from [narrow]
+  /// whether or not a redirect happened.
+  ///
+  /// See API doc:
+  ///   https://zulip.com/api/construct-narrow#message-ids
+  void _adjustNarrowForTopicPermalink(Message? someFetchedMessageOrNull) {
+    final narrow = this.narrow;
+    if (narrow is! TopicNarrow || narrow.with_ == null) return;
+
+    switch (someFetchedMessageOrNull) {
+      case null:
+        // This can't be a redirect; a redirect can't produce an empty result.
+        // (The server only redirects if the message is accessible to the user,
+        // and if it is, it'll appear in the result, making it non-empty.)
+        this.narrow = narrow.sansWith();
+      case StreamMessage():
+        this.narrow = TopicNarrow.ofMessage(someFetchedMessageOrNull);
+      case DmMessage(): // TODO(log)
+        assert(false);
+    }
+  }
+
   /// Fetch the next batch of older messages, if applicable.
   Future<void> fetchOlder() async {
     if (haveOldest) return;
     if (fetchingOlder) return;
     if (fetchOlderCoolingDown) return;
     assert(fetched);
+    assert(narrow is! TopicNarrow
+      // We only intend to send "with" in [fetchInitial]; see there.
+      || (narrow as TopicNarrow).with_ == null);
     assert(messages.isNotEmpty);
     _fetchingOlder = true;
     _updateEndMarkers();

lib/model/narrow.dart

@@ -92,14 +92,17 @@ class ChannelNarrow extends Narrow {
 }
 
 class TopicNarrow extends Narrow implements SendableNarrow {
-  const TopicNarrow(this.streamId, this.topic);
+  const TopicNarrow(this.streamId, this.topic, {this.with_});
 
   factory TopicNarrow.ofMessage(StreamMessage message) {
     return TopicNarrow(message.streamId, message.topic);
   }
 
   final int streamId;
   final TopicName topic;
+  final int? with_;
+
+  TopicNarrow sansWith() => TopicNarrow(streamId, topic);
 
   @override
   bool containsMessage(Message message) {
@@ -108,22 +111,33 @@ class TopicNarrow extends Narrow implements SendableNarrow {
   }
 
   @override
-  ApiNarrow apiEncode() => [ApiNarrowStream(streamId), ApiNarrowTopic(topic)];
+  ApiNarrow apiEncode() => [
+    ApiNarrowStream(streamId),
+    ApiNarrowTopic(topic),
+    if (with_ != null) ApiNarrowWith(with_!),
+  ];
 
   @override
   StreamDestination get destination => StreamDestination(streamId, topic);
 
   @override
-  String toString() => 'TopicNarrow($streamId, ${topic.displayName})';
+  String toString() {
+    final fields = [
+      streamId.toString(),
+      topic.displayName,
+      if (with_ != null) 'with: ${with_!}',
+    ];
+    return 'TopicNarrow(${fields.join(', ')})';
+  }
 
   @override
   bool operator ==(Object other) {
     if (other is! TopicNarrow) return false;
-    return other.streamId == streamId && other.topic == topic;
+    return other.streamId == streamId && other.topic == topic && other.with_ == with_;
   }
 
   @override
-  int get hashCode => Object.hash('TopicNarrow', streamId, topic);
+  int get hashCode => Object.hash('TopicNarrow', streamId, topic, with_);
 }
 
 /// The narrow for a direct-message conversation.

lib/widgets/message_list.dart

@@ -503,8 +503,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
 
   void _modelChanged() {
     if (model!.narrow != widget.narrow) {
-      // A message move event occurred, where propagate mode is
-      // [PropagateMode.changeAll] or [PropagateMode.changeLater].
+      // Either:
+      // - A message move event occurred, where propagate mode is
+      //   [PropagateMode.changeAll] or [PropagateMode.changeLater]. Or:
+      // - We fetched a "with" / topic-permalink narrow, and the response
+      //   redirected us to the new location of the operand message ID.
       widget.onNarrowChanged(model!.narrow);
     }
     setState(() {

test/api/model/narrow_test.dart

@@ -0,0 +1,4 @@
+void main() {
+  // resolveApiNarrowForServer is covered in test/api/route/messages_test.dart,
+  // in the ApiNarrow.toJson test.
+}

test/api/route/messages_test.dart

@@ -169,13 +169,20 @@ void main() {
     });
   });
 
-  test('Narrow.toJson', () {
+  test('ApiNarrow.toJson', () {
     return FakeApiConnection.with_((connection) async {
       void checkNarrow(ApiNarrow narrow, String expected) {
-        narrow = resolveDmElements(narrow, connection.zulipFeatureLevel!);
+        narrow = resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!);
         check(jsonEncode(narrow)).equals(expected);
       }
 
+      checkNarrow(const MentionsNarrow().apiEncode(), jsonEncode([
+        {'operator': 'is', 'operand': 'mentioned'},
+      ]));
+      checkNarrow(const StarredMessagesNarrow().apiEncode(), jsonEncode([
+        {'operator': 'is', 'operand': 'starred'},
+      ]));
+
       checkNarrow(const CombinedFeedNarrow().apiEncode(), jsonEncode([]));
       checkNarrow(const ChannelNarrow(12).apiEncode(), jsonEncode([
         {'operator': 'stream', 'operand': 12},
@@ -184,21 +191,43 @@ void main() {
         {'operator': 'stream', 'operand': 12},
         {'operator': 'topic', 'operand': 'stuff'},
       ]));
-      checkNarrow(const MentionsNarrow().apiEncode(), jsonEncode([
-        {'operator': 'is', 'operand': 'mentioned'},
+      checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
+        {'operator': 'stream', 'operand': 12},
+        {'operator': 'topic', 'operand': 'stuff'},
+        {'operator': 'with', 'operand': 1},
       ]));
-      checkNarrow(const StarredMessagesNarrow().apiEncode(), jsonEncode([
-        {'operator': 'is', 'operand': 'starred'},
+      checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
+        {'operator': 'dm', 'operand': [123, 234]},
+      ]));
+      checkNarrow([ApiNarrowDm([123, 234]), ApiNarrowWith(1)], jsonEncode([
+        {'operator': 'dm', 'operand': [123, 234]},
+        {'operator': 'with', 'operand': 1},
       ]));
 
+      connection.zulipFeatureLevel = 270;
+      checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
+        {'operator': 'stream', 'operand': 12},
+        {'operator': 'topic', 'operand': 'stuff'},
+      ]));
       checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
         {'operator': 'dm', 'operand': [123, 234]},
       ]));
+      checkNarrow([ApiNarrowDm([123, 234]), ApiNarrowWith(1)], jsonEncode([
+        {'operator': 'dm', 'operand': [123, 234]},
+      ]));
 
       connection.zulipFeatureLevel = 176;
+      checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
+        {'operator': 'stream', 'operand': 12},
+        {'operator': 'topic', 'operand': 'stuff'},
+      ]));
       checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
         {'operator': 'pm-with', 'operand': [123, 234]},
       ]));
+      checkNarrow([ApiNarrowDm([123, 234]), ApiNarrowWith(1)], jsonEncode([
+        {'operator': 'pm-with', 'operand': [123, 234]},
+      ]));
+
       connection.zulipFeatureLevel = eg.futureZulipFeatureLevel;
     });
   });
@@ -259,7 +288,7 @@ void main() {
       });
     });
 
-    test('narrow uses resolveDmElements to encode', () {
+    test('narrow uses resolveApiNarrowForServer to encode', () {
       return FakeApiConnection.with_(zulipFeatureLevel: 176, (connection) async {
         connection.prepare(json: fakeResult.toJson());
         await checkGetMessages(connection,
@@ -707,7 +736,7 @@ void main() {
       });
     });
 
-    test('narrow uses resolveDmElements to encode', () {
+    test('narrow uses resolveApiNarrowForServer to encode', () {
       return FakeApiConnection.with_(zulipFeatureLevel: 176, (connection) async {
         connection.prepare(json: mkResult(foundOldest: true).toJson());
         await checkUpdateMessageFlagsForNarrow(connection,