internalLinks [nfc]: Bring an isNarrowLink check out to caller

Also rename the function and add a note in the jsdoc, to make it
clear that it's meant to operate on "narrow links", i.e., those that
isNarrowLink gives `true` for.

While we're at it, write the rest of the jsdoc.

Author: chrisbobbe

src/message/messagesActions.js

@@ -3,7 +3,11 @@ import * as logging from '../utils/logging';
 import * as NavigationService from '../nav/NavigationService';
 import type { GetText, Message, Narrow, ThunkAction } from '../types';
 import { getAuth, getRealm, getMessages, getZulipFeatureLevel } from '../selectors';
-import { getNearOperandFromLink, getNarrowFromLink } from '../utils/internalLinks';
+import {
+  getNearOperandFromLink,
+  getNarrowFromNarrowLink,
+  isNarrowLink,
+} from '../utils/internalLinks';
 import { openLinkWithUserPreference } from '../utils/openLink';
 import { navigateToChat } from '../nav/navActions';
 import { FIRST_UNREAD_ANCHOR } from '../anchor';
@@ -232,7 +236,9 @@ export const messageLinkPress =
       return;
     }
 
-    const narrow = getNarrowFromLink(parsedUrl, auth.realm, streamsById, streamsByName, ownUserId);
+    const narrow = isNarrowLink(parsedUrl, auth.realm)
+      ? getNarrowFromNarrowLink(parsedUrl, auth.realm, streamsById, streamsByName, ownUserId)
+      : null;
 
     if (narrow) {
       // This call is OK: `narrow` is truthy, so isNarrowLink(…) was true.

src/utils/__tests__/internalLinks-test.js

@@ -18,7 +18,7 @@ import {
 } from '../narrow';
 import {
   isNarrowLink,
-  getNarrowFromLink,
+  getNarrowFromNarrowLink,
   getNearOperandFromLink,
   decodeHashComponent,
 } from '../internalLinks';
@@ -131,8 +131,8 @@ describe('isNarrowLink', () => {
   }
 });
 
-// TODO: Combine / dedupe with "getNarrowFromLink (part 2)" tests, below
-describe('getNarrowFromLink (part 1)', () => {
+// TODO: Combine / dedupe with "getNarrowFromNarrowLink (part 2)" tests, below
+describe('getNarrowFromNarrowLink (part 1)', () => {
   const mkCheck = (narrowExpectation: (Narrow => boolean) | null) => hash => {
     const streams = [
       eg.makeStream({ name: 'jest' }),
@@ -141,7 +141,7 @@ describe('getNarrowFromLink (part 1)', () => {
       eg.makeStream({ name: 'mobile' }),
     ];
     const baseState = eg.reduxStatePlus({ streams });
-    const narrow = getNarrowFromLink(
+    const narrow = getNarrowFromNarrowLink(
       new URL(hash, realm),
       realm,
       getStreamsById(baseState),
@@ -232,26 +232,22 @@ describe('decodeHashComponent', () => {
   });
 });
 
-// TODO: Combine / dedupe with "getNarrowFromLink (part 1)" tests above
-describe('getNarrowFromLink (part 2)', () => {
+// TODO: Combine / dedupe with "getNarrowFromNarrowLink (part 1)" tests above
+describe('getNarrowFromNarrowLink (part 2)', () => {
   const [userB, userC] = [eg.makeUser(), eg.makeUser()];
 
   const streamGeneral = eg.makeStream({ name: 'general' });
 
   // TODO: Take URL object instead of string
   const get = (url, streams: $ReadOnlyArray<Stream>) =>
-    getNarrowFromLink(
+    getNarrowFromNarrowLink(
       new URL(url, 'https://example.com'),
       new URL('https://example.com'),
       new Map(streams.map(s => [s.stream_id, s])),
       new Map(streams.map(s => [s.name, s])),
       eg.selfUser.user_id,
     );
 
-  test('on link to realm domain but not narrow: return null', () => {
-    expect(get('https://example.com/user_uploads', [])).toEqual(null);
-  });
-
   describe('on stream links', () => {
     const expectStream = (operand, streams, expectedStream: null | Stream) => {
       expect(get(`#narrow/stream/${operand}`, streams)).toEqual(

src/utils/internalLinks.js

@@ -34,8 +34,6 @@ const getHashSegmentsFromNarrowLink = (url: URL, realm: URL) => {
 };
 
 /**
- * PRIVATE -- exported only for tests.
- *
  * Test for a link to a Zulip narrow on the given realm.
  *
  * True just if the given URL appears to be a link to a Zulip narrow on the
@@ -119,20 +117,30 @@ const parsePmOperand = operand => {
 };
 
 /**
- * TODO write jsdoc
+ * Gives a Narrow object for the given narrow link, or `null`.
+ *
+ * Returns null if any of the operator/operand pairs are invalid.
+ *
+ * Since narrow links can combine operators in ways our Narrow type can't
+ * represent, this can also return null for valid narrow links.
+ *
+ * This can also return null for some valid narrow links that our Narrow
+ * type *could* accurately represent. We should try to understand these
+ * better, but some kinds will be rare, even unheard-of:
+ *   #narrow/topic/mobile.20releases/stream/1-announce (stream/topic reversed)
+ *   #narrow/stream/1-announce/stream/1-announce (duplicated operator)
+ *
+ * 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.
  */
-export const getNarrowFromLink = (
+export const getNarrowFromNarrowLink = (
   url: URL,
   realm: URL,
   streamsById: Map<number, Stream>,
   streamsByName: Map<string, Stream>,
   ownUserId: UserId,
 ): Narrow | null => {
-  if (!isNarrowLink(url, realm)) {
-    return null;
-  }
-
-  // isNarrowLink(…) is true, by early return above, so this call is OK.
+  // isNarrowLink(…) is true, by jsdoc, so this call is OK.
   const hashSegments = getHashSegmentsFromNarrowLink(url, realm);
 
   if (