msglist: Handle topic permalinks (/with/<id>) just as we do /near/ links

Explanation in a comment, which quotes Greg in #5866.

Fixes: #5866

Author: chrisbobbe

src/message/__tests__/messageActions-test.js

@@ -68,6 +68,21 @@ describe('messageActions', () => {
         pmNarrowFromUsersUnsafe([user1, user2]),
       );
     });
+
+    test('handles /with/ links', async () => {
+      const stream = eg.makeStream({ stream_id: 1, name: 'test' });
+      const user1 = eg.makeUser({ user_id: 1, full_name: 'user 1' });
+      const user2 = eg.makeUser({ user_id: 2, full_name: 'user 2' });
+      const { store } = prepare({ streams: [stream], users: [user1, user2] });
+
+      await checkLink(store, '#narrow/stream/1-test/topic/hello/with/1', topicNarrow(1, 'hello'));
+      await checkLink(store, '#narrow/dm/1-user-1/with/1', pmNarrowFromUsersUnsafe([user1]));
+      await checkLink(
+        store,
+        '#narrow/dm/1,2-group/with/1',
+        pmNarrowFromUsersUnsafe([user1, user2]),
+      );
+    });
   });
 
   describe('doNarrow', () => {

src/utils/__tests__/internalLinks-test.js

@@ -194,8 +194,10 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
     [
       '/#narrow/channel/1-jest/topic/test',
       '/#narrow/channel/4-mobile/subject/topic/near/378333',
+      '/#narrow/channel/4-mobile/subject/topic/with/378333',
       '/#narrow/channel/4-mobile/topic/topic/',
       '/#narrow/channel/3-stream/topic/topic/near/1',
+      '/#narrow/channel/3-stream/topic/topic/with/1',
     ].forEach(hash => check(hash));
   });
 
@@ -204,9 +206,12 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
     [
       '/#narrow/stream/jest/topic/test',
       '/#narrow/stream/mobile/subject/topic/near/378333',
+      '/#narrow/stream/mobile/subject/topic/with/378333',
       '/#narrow/stream/mobile/topic/topic/',
       '/#narrow/stream/stream/topic/topic/near/1',
+      '/#narrow/stream/stream/topic/topic/with/1',
       '/#narrow/stream/stream/subject/topic/near/1',
+      '/#narrow/stream/stream/subject/topic/with/1',
       '/#narrow/stream/stream/subject/topic',
     ].forEach(hash => check(hash));
   });
@@ -216,7 +221,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
     [
       '/#narrow/dm/1,2-group',
       '/#narrow/dm/1,2-group/near/1',
+      '/#narrow/dm/1,2-group/with/1',
       '/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3',
+      '/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/with/3',
     ].forEach(hash => check(hash));
   });
 
@@ -225,7 +232,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
     [
       '/#narrow/pm-with/1,2-group',
       '/#narrow/pm-with/1,2-group/near/1',
+      '/#narrow/pm-with/1,2-group/with/1',
       '/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3',
+      '/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/with/3',
     ].forEach(hash => check(hash));
   });
 
@@ -245,6 +254,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
       // `near` with no operand
       '/#narrow/channel/stream/topic/topic/near/',
 
+      // `with` with no operand
+      '/#narrow/channel/stream/topic/topic/with/',
+
       // `is` with invalid operand
       '/#narrow/is/men',
       '/#narrow/is/men/channel',
@@ -472,6 +484,31 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
       get(`https://example.com/#narrow/stream/${eg.stream.name}/subject/test/near/1`, [eg.stream]),
     ).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
   });
+
+  test('on a /with/ link', () => {
+    const ids = `${userB.user_id},${userC.user_id}`;
+    expect(get(`https://example.com/#narrow/dm/${ids}-group/with/2`, [])).toEqual(
+      pmNarrowFromUsersUnsafe([userB, userC]),
+    );
+    expect(get(`https://example.com/#narrow/pm-with/${ids}-group/with/2`, [])).toEqual(
+      pmNarrowFromUsersUnsafe([userB, userC]),
+    );
+
+    expect(
+      get(
+        `https://example.com/#narrow/channel/${eg.stream.stream_id}-${eg.stream.name}/topic/test/with/1`,
+        [eg.stream],
+      ),
+    ).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
+
+    expect(
+      get(`https://example.com/#narrow/stream/${eg.stream.name}/topic/test/with/1`, [eg.stream]),
+    ).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
+
+    expect(
+      get(`https://example.com/#narrow/stream/${eg.stream.name}/subject/test/with/1`, [eg.stream]),
+    ).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
+  });
 });
 
 describe('getNearOperandFromLink', () => {
@@ -485,13 +522,24 @@ describe('getNearOperandFromLink', () => {
     expect(getNearOperandFromLink(new URL('/#narrow/near/1', realm), realm)).toBe(1);
   });
 
+  test('`with` is the only operator', () => {
+    expect(getNearOperandFromLink(new URL('/#narrow/with/1', realm), realm)).toBe(1);
+  });
+
   test('when link is a group link, return anchor message id', () => {
     expect(getNearOperandFromLink(new URL('/#narrow/dm/1,3-group/near/1/', realm), realm)).toBe(1);
     expect(
       getNearOperandFromLink(new URL('/#narrow/pm-with/1,3-group/near/1/', realm), realm),
     ).toBe(1);
   });
 
+  test('when link is a group link with /with/, return anchor message id', () => {
+    expect(getNearOperandFromLink(new URL('/#narrow/dm/1,3-group/with/1/', realm), realm)).toBe(1);
+    expect(
+      getNearOperandFromLink(new URL('/#narrow/pm-with/1,3-group/with/1/', realm), realm),
+    ).toBe(1);
+  });
+
   test('when link is a topic link, return anchor message id', () => {
     expect(
       getNearOperandFromLink(new URL('/#narrow/channel/1-jest/topic/test/near/1', realm), realm),
@@ -500,4 +548,13 @@ describe('getNearOperandFromLink', () => {
       getNearOperandFromLink(new URL('/#narrow/stream/jest/topic/test/near/1', realm), realm),
     ).toBe(1);
   });
+
+  test('when link is a topic link with /with/, return anchor message id', () => {
+    expect(
+      getNearOperandFromLink(new URL('/#narrow/channel/1-jest/topic/test/with/1', realm), realm),
+    ).toBe(1);
+    expect(
+      getNearOperandFromLink(new URL('/#narrow/stream/jest/topic/test/with/1', realm), realm),
+    ).toBe(1);
+  });
 });

src/utils/internalLinks.js

@@ -148,7 +148,7 @@ export const getNarrowFromNarrowLink = (
     (hashSegments.length === 2 && (hashSegments[0] === 'pm-with' || hashSegments[0] === 'dm'))
     || (hashSegments.length === 4
       && (hashSegments[0] === 'pm-with' || hashSegments[0] === 'dm')
-      && hashSegments[2] === 'near')
+      && (hashSegments[2] === 'near' || hashSegments[2] === 'with'))
   ) {
     // TODO: This case is pretty useless in practice, due to basically a
     //   bug in the webapp: the URL that appears in the location bar for a
@@ -167,7 +167,7 @@ export const getNarrowFromNarrowLink = (
     || (hashSegments.length === 6
       && (hashSegments[0] === 'stream' || hashSegments[0] === 'channel')
       && (hashSegments[2] === 'subject' || hashSegments[2] === 'topic')
-      && hashSegments[4] === 'near')
+      && (hashSegments[4] === 'near' || hashSegments[4] === 'with'))
   ) {
     const streamId = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
     return streamId != null ? topicNarrow(streamId, parseTopicOperand(hashSegments[3])) : null;
@@ -219,9 +219,18 @@ export const getNearOperandFromLink = (url: URL, realm: URL): number | null => {
     (str, i) =>
       // This is a segment where we expect an operator to be specified.
       i % 2 === 0
-      // The operator is 'near' and its meaning is not negated (`str` does
-      // not start with "-").
-      && str === 'near',
+      // The operator is 'near' or 'with' and its meaning is not negated
+      // (`str` does not start with "-").
+      //
+      // Quoting Greg from #5866:
+      // > Currently the app doesn't interpret the "anchor" meaning of
+      // > `/near/` links at all (that's #3604) — we already effectively
+      // > give `/near/` links exactly the meaning that the upcoming
+      // > `/with/` links will have. So to handle the new links, we just
+      // > need to parse them and then handle them the way we already handle
+      // > `/near/` links.
+      // Which makes sense for this legacy codebase.
+      && (str === 'near' || str === 'with'),
   );
   if (nearOperatorIndex < 0) {
     return null;