internalLinks [nfc]: Dissolve getLinkType / LinkType indirection

This puts the logic to recognize a "stream link" right next to the
logic for building a stream narrow, and likewise for topic narrows,
etc.

Now it's quicker to read what `Narrow` we construct for a given
`url` param. We don't have to click through `getLinkType` and wonder
whether anything surprising is done at that layer.

Author: chrisbobbe

src/utils/__tests__/internalLinks-test.js

@@ -6,17 +6,27 @@ import invariant from 'invariant';
 /* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "check", "expectStream"] }] */
 
 import type { Stream } from '../../types';
-import { streamNarrow, topicNarrow, pmNarrowFromUsersUnsafe, STARRED_NARROW } from '../narrow';
+import {
+  streamNarrow,
+  topicNarrow,
+  pmNarrowFromUsersUnsafe,
+  STARRED_NARROW,
+  isPmNarrow,
+  isStreamNarrow,
+  isTopicNarrow,
+  isSpecialNarrow,
+} from '../narrow';
 import {
   isNarrowLink,
-  getLinkType,
   getNarrowFromLink,
   getNearOperandFromLink,
   decodeHashComponent,
 } from '../internalLinks';
 import * as eg from '../../__tests__/lib/exampleData';
 import { isUrlRelative } from '../url';
-import type { LinkType } from '../internalLinks';
+import { getStreamsById, getStreamsByName } from '../../subscriptions/subscriptionSelectors';
+import { getOwnUserId } from '../../users/userSelectors';
+import type { Narrow } from '../narrow';
 
 const realm = new URL('https://example.com');
 const urlOnRealm = relativeUrl => {
@@ -121,22 +131,44 @@ describe('isNarrowLink', () => {
   }
 });
 
-describe('getLinkType', () => {
-  const mkCheck = (expected: LinkType) => hash => {
-    test(`${hash} - should return ${expected}`, () => {
-      expect(getLinkType(new URL(hash, realm), realm)).toBe(expected);
-    });
+// TODO: Combine / dedupe with "getNarrowFromLink (part 2)" tests, below
+describe('getNarrowFromLink (part 1)', () => {
+  const mkCheck = (narrowExpectation: (Narrow => boolean) | null) => hash => {
+    const streams = [
+      eg.makeStream({ name: 'jest' }),
+      eg.makeStream({ name: 'stream' }),
+      eg.makeStream({ name: 'topic' }),
+      eg.makeStream({ name: 'mobile' }),
+    ];
+    const baseState = eg.reduxStatePlus({ streams });
+    const narrow = getNarrowFromLink(
+      new URL(hash, realm),
+      realm,
+      getStreamsById(baseState),
+      getStreamsByName(baseState),
+      getOwnUserId(baseState),
+    );
+
+    if (typeof narrowExpectation === 'function') {
+      test(`${hash} - ${narrowExpectation.name} should be true`, () => {
+        expect(narrow && narrowExpectation(narrow)).toBeTrue();
+      });
+    } else {
+      test(`${hash} - should return null`, () => {
+        expect(narrow).toBeNull();
+      });
+    }
   };
 
   describe('link containing "stream" is a stream link', () => {
-    const check = mkCheck('stream');
+    const check = mkCheck(isStreamNarrow);
     ['/#narrow/stream/jest', '/#narrow/stream/stream/', '/#narrow/stream/topic/'].forEach(hash =>
       check(hash),
     );
   });
 
   describe('link containing "topic" is a topic link', () => {
-    const check = mkCheck('topic');
+    const check = mkCheck(isTopicNarrow);
     [
       '/#narrow/stream/jest/topic/test',
       '/#narrow/stream/mobile/subject/topic/near/378333',
@@ -148,7 +180,7 @@ describe('getLinkType', () => {
   });
 
   describe('link containing "pm-with" is a PM link', () => {
-    const check = mkCheck('pm');
+    const check = mkCheck(isPmNarrow);
     [
       '/#narrow/pm-with/1,2-group',
       '/#narrow/pm-with/1,2-group/near/1',
@@ -157,14 +189,14 @@ describe('getLinkType', () => {
   });
 
   describe('link containing "is" with valid operand is a special link', () => {
-    const check = mkCheck('special');
+    const check = mkCheck(isSpecialNarrow);
     ['/#narrow/is/private', '/#narrow/is/starred', '/#narrow/is/mentioned'].forEach(hash =>
       check(hash),
     );
   });
 
-  describe('unexpected link shape gives "home"', () => {
-    const check = mkCheck('home');
+  describe('unexpected link shape gives null', () => {
+    const check = mkCheck(null);
     [
       // `near` with no operand
       '/#narrow/stream/stream/topic/topic/near/',
@@ -198,7 +230,8 @@ describe('decodeHashComponent', () => {
   });
 });
 
-describe('getNarrowFromLink', () => {
+// TODO: Combine / dedupe with "getNarrowFromLink (part 1)" tests above
+describe('getNarrowFromLink (part 2)', () => {
   const [userB, userC] = [eg.makeUser(), eg.makeUser()];
 
   const streamGeneral = eg.makeStream({ name: 'general' });

src/utils/internalLinks.js

@@ -5,7 +5,6 @@ import { makeUserId } from '../api/idTypes';
 import type { Narrow, Stream, UserId, Message, Outbox, PmMessage, PmOutbox } from '../types';
 import { topicNarrow, streamNarrow, specialNarrow, pmNarrowFromRecipients } from './narrow';
 import { pmKeyRecipientsFromIds, recipientsOfPrivateMessage } from './recipient';
-import { ensureUnreachable } from '../generics';
 import { isUrlOnRealm } from './url';
 
 /**
@@ -48,51 +47,6 @@ export const isNarrowLink = (url: URL, realm: URL): boolean =>
   && url.search === ''
   && /^#narrow\//i.test(url.hash);
 
-/* PRIVATE: Exported only for tests. */
-export type LinkType = 'home' | 'pm' | 'topic' | 'stream' | 'special';
-
-/**
- * PRIVATE -- exported only for tests.
- *
- * The passed `url` must appear to be a link to a Zulip narrow on the given
- * `realm`. In particular, `isNarrowLink(url, realm)` must be true.
- */
-// TODO: Work out what this does, write a jsdoc for its interface, and
-// reimplement using URL object (not just for the realm)
-export const getLinkType = (url: URL, realm: URL): LinkType => {
-  // isNarrowLink(…) is true, by jsdoc, so this call is OK.
-  const hashSegments = getHashSegmentsFromNarrowLink(url, realm);
-
-  if (
-    (hashSegments.length === 2 && hashSegments[0] === 'pm-with')
-    || (hashSegments.length === 4 && hashSegments[0] === 'pm-with' && hashSegments[2] === 'near')
-  ) {
-    return 'pm';
-  }
-
-  if (
-    (hashSegments.length === 4 || hashSegments.length === 6)
-    && hashSegments[0] === 'stream'
-    && (hashSegments[2] === 'subject' || hashSegments[2] === 'topic')
-  ) {
-    return 'topic';
-  }
-
-  if (hashSegments.length === 2 && hashSegments[0] === 'stream') {
-    return 'stream';
-  }
-
-  if (
-    hashSegments.length === 2
-    && hashSegments[0] === 'is'
-    && /^(private|starred|mentioned)/i.test(hashSegments[1])
-  ) {
-    return 'special';
-  }
-
-  return 'home';
-};
-
 /** Decode a dot-encoded string. */
 // The Zulip webapp uses this encoding in narrow-links:
 // https://github.com/zulip/zulip/blob/1577662a6/static/js/hash_util.js#L18-L25
@@ -167,41 +121,49 @@ export const getNarrowFromLink = (
     return null;
   }
 
-  const type = getLinkType(url, realm);
-
   // isNarrowLink(…) is true, by early return above, so this call is OK.
   const hashSegments = getHashSegmentsFromNarrowLink(url, realm);
 
-  switch (type) {
-    case 'pm': {
-      // 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
-      //   group PM conversation excludes self, so it's unusable for anyone
-      //   else.  In particular this will foil you if, say, you try to give
-      //   someone else in the conversation a link to a particular message.
-      const ids = parsePmOperand(hashSegments[1]);
-      return pmNarrowFromRecipients(pmKeyRecipientsFromIds(ids, ownUserId));
-    }
-    case 'topic': {
-      const stream = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
-      return stream && topicNarrow(stream.stream_id, parseTopicOperand(hashSegments[3]));
-    }
-    case 'stream': {
-      const stream = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
-      return stream && streamNarrow(stream.stream_id);
-    }
-    case 'special':
-      try {
-        return specialNarrow(hashSegments[1]);
-      } catch {
-        return null;
-      }
-    case 'home':
-      return null; // TODO(?): Give HOME_NARROW
-    default:
-      ensureUnreachable(type);
+  if (
+    (hashSegments.length === 2 && hashSegments[0] === 'pm-with')
+    || (hashSegments.length === 4 && hashSegments[0] === 'pm-with' && hashSegments[2] === 'near')
+  ) {
+    // 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
+    //   group PM conversation excludes self, so it's unusable for anyone
+    //   else.  In particular this will foil you if, say, you try to give
+    //   someone else in the conversation a link to a particular message.
+    const ids = parsePmOperand(hashSegments[1]);
+    return pmNarrowFromRecipients(pmKeyRecipientsFromIds(ids, ownUserId));
+  }
+
+  if (
+    (hashSegments.length === 4 || hashSegments.length === 6)
+    && hashSegments[0] === 'stream'
+    && (hashSegments[2] === 'subject' || hashSegments[2] === 'topic')
+  ) {
+    const stream = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
+    return stream && topicNarrow(stream.stream_id, parseTopicOperand(hashSegments[3]));
+  }
+
+  if (hashSegments.length === 2 && hashSegments[0] === 'stream') {
+    const stream = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
+    return stream && streamNarrow(stream.stream_id);
+  }
+
+  if (
+    hashSegments.length === 2
+    && hashSegments[0] === 'is'
+    && /^(private|starred|mentioned)/i.test(hashSegments[1])
+  ) {
+    try {
+      return specialNarrow(hashSegments[1]);
+    } catch {
       return null;
+    }
   }
+
+  return null; // TODO(?) Give HOME_NARROW
 };
 
 /**