compose: Have narrowLink take an optional nearMessageId

Author: chrisbobbe

lib/model/compose.dart

@@ -117,7 +117,22 @@ String encodeHashComponent(String str) {
 }
 
 /// A URL to the given [Narrow], on `store`'s realm.
-Uri narrowLink(PerAccountStore store, Narrow narrow) {
+///
+/// To include /near/{messageId} in the link, pass a non-null [nearMessageId].
+// Why take [nearMessageId] in a param, instead of looking for it in [narrow]?
+//
+// A reasonable question: after all, the "near" part of a near link (e.g., for
+// quote-and-reply) does take the same form as other operator/operand pairs
+// that we represent with [ApiNarrowElement]s, like "/stream/48-mobile".
+//
+// But unlike those other elements, we choose not to give the "near" element
+// an [ApiNarrowElement] representation, because it doesn't have quite that role:
+// it says where to look in a list of messages, but it doesn't filter the list down.
+// In fact, from a brief look at server code, it seems to be *ignored*
+// if you include it in the `narrow` param in get-messages requests.
+// When you want to point the server to a location in a message list, you
+// you do so by passing the `anchor` param.
+Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) {
   final apiNarrow = narrow.apiEncode();
   final fragment = StringBuffer('narrow');
   for (ApiNarrowElement element in apiNarrow) {
@@ -153,8 +168,10 @@ Uri narrowLink(PerAccountStore store, Narrow narrow) {
         fragment.write(element.operand.toString());
     }
   }
+
+  if (nearMessageId != null) {
+    fragment.write('/near/$nearMessageId');
+  }
+
   return store.account.realmUrl.replace(fragment: fragment.toString());
 }
-
-// TODO more, like /near links to messages in conversations
-//   (also to be used in quote-and-reply)

test/model/compose_test.dart

@@ -233,22 +233,27 @@ hello
   group('narrowLink', () {
     test('AllMessagesNarrow', () {
       final store = eg.store();
-      check(narrowLink(store, const AllMessagesNarrow())).equals(store.account.realmUrl.resolve('#narrow'));
+      check(narrowLink(store, const AllMessagesNarrow()))
+        .equals(store.account.realmUrl.resolve('#narrow'));
+      check(narrowLink(store, const AllMessagesNarrow(), nearMessageId: 1))
+        .equals(store.account.realmUrl.resolve('#narrow/near/1'));
     });
 
     test('StreamNarrow / TopicNarrow', () {
       void checkNarrow(String expectedFragment, {
         required int streamId,
         required String name,
         String? topic,
+        int? nearMessageId,
       }) {
         assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment');
         final store = eg.store();
         store.addStream(eg.stream(streamId: streamId, name: name));
         final narrow = topic == null
           ? StreamNarrow(streamId)
           : TopicNarrow(streamId, topic);
-        check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment));
+        check(narrowLink(store, narrow, nearMessageId: nearMessageId))
+          .equals(store.account.realmUrl.resolve(expectedFragment));
       }
 
       checkNarrow(streamId: 1,   name: 'announce',       '#narrow/stream/1-announce');
@@ -257,26 +262,32 @@ hello
       checkNarrow(streamId: 415, name: 'chat.zulip.org', '#narrow/stream/415-chat.2Ezulip.2Eorg');
       checkNarrow(streamId: 419, name: 'français',       '#narrow/stream/419-fran.C3.A7ais');
       checkNarrow(streamId: 403, name: 'Hshs[™~}(.',     '#narrow/stream/403-Hshs.5B.E2.84.A2~.7D.28.2E');
+      checkNarrow(streamId: 60,  name: 'twitter', nearMessageId: 1570686, '#narrow/stream/60-twitter/near/1570686');
 
       checkNarrow(streamId: 48,  name: 'mobile', topic: 'Welcome screen UI',
                   '#narrow/stream/48-mobile/topic/Welcome.20screen.20UI');
       checkNarrow(streamId: 243, name: 'mobile-team', topic: 'Podfile.lock clash #F92',
                   '#narrow/stream/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92');
       checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"',
                   '#narrow/stream/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22');
+      checkNarrow(streamId: 42,  name: 'Outreachy 2016-2017', topic: '2017-18 Stream?', nearMessageId: 302690,
+                  '#narrow/stream/42-Outreachy-2016-2017/topic/2017-18.20Stream.3F/near/302690');
     });
 
     test('DmNarrow', () {
       void checkNarrow(String expectedFragment, String legacyExpectedFragment, {
         required List<int> allRecipientIds,
         required int selfUserId,
+        int? nearMessageId,
       }) {
         assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment');
         final store = eg.store();
         final narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: selfUserId);
-        check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment));
+        check(narrowLink(store, narrow, nearMessageId: nearMessageId))
+          .equals(store.account.realmUrl.resolve(expectedFragment));
         store.connection.zulipFeatureLevel = 176;
-        check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(legacyExpectedFragment));
+        check(narrowLink(store, narrow, nearMessageId: nearMessageId))
+          .equals(store.account.realmUrl.resolve(legacyExpectedFragment));
       }
 
       checkNarrow(allRecipientIds: [1], selfUserId: 1,
@@ -291,6 +302,9 @@ hello
       checkNarrow(allRecipientIds: [1, 2, 3, 4], selfUserId: 4,
         '#narrow/dm/1,2,3,4-group',
         '#narrow/pm-with/1,2,3,4-group');
+      checkNarrow(allRecipientIds: [1, 2], selfUserId: 1, nearMessageId: 12345,
+        '#narrow/dm/1,2-dm/near/12345',
+        '#narrow/pm-with/1,2-pm/near/12345');
     });
 
     // TODO other Narrow subclasses as we add them: