Skip to content

Commit

Permalink
url, internalLinks: Pipe realm as URL to isUrlOnRealm, `getPathsFro…
Browse files Browse the repository at this point in the history
…mUrl`.

As in the previous and next commits, stringify the realm exactly
where we need it as a string.

This slightly expands the area of our codebase in which it's easy to
see that the realm is well-formed data.
  • Loading branch information
chrisbobbe committed Sep 21, 2020
1 parent 91b8573 commit 4207e2e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 130 deletions.
6 changes: 3 additions & 3 deletions src/message/messagesActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ export const messageLinkPress = (href: string) => async (
const auth = getAuth(state);
const usersById = getUsersById(state);
const streamsById = getStreamsById(state);
const narrow = getNarrowFromLink(href, auth.realm.toString(), usersById, streamsById);
const narrow = getNarrowFromLink(href, auth.realm, usersById, streamsById);
if (narrow) {
const anchor = getMessageIdFromLink(href, auth.realm.toString());
const anchor = getMessageIdFromLink(href, auth.realm);
dispatch(doNarrow(narrow, anchor));
} else if (!isUrlOnRealm(href, auth.realm.toString())) {
} else if (!isUrlOnRealm(href, auth.realm)) {
openLink(href);
} else {
const url =
Expand Down
153 changes: 46 additions & 107 deletions src/utils/__tests__/internalLinks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,148 +12,95 @@ import {
} from '../internalLinks';
import * as eg from '../../__tests__/lib/exampleData';

const realm = new URL('https://example.com');

describe('isInternalLink', () => {
test('when link is external, return false', () => {
expect(isInternalLink('https://example.com', 'https://another.com')).toBe(false);
expect(isInternalLink('https://example.com', new URL('https://another.com'))).toBe(false);
});

test('when link is internal, but not in app, return false', () => {
expect(isInternalLink('https://example.com/user_uploads', 'https://example.com')).toBe(false);
expect(isInternalLink('https://example.com/user_uploads', realm)).toBe(false);
});

test('when link is internal and in app, return true', () => {
expect(isInternalLink('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
true,
);
expect(isInternalLink('https://example.com/#narrow/stream/jest', realm)).toBe(true);
});

test('when link is relative and in app, return true', () => {
expect(isInternalLink('#narrow/stream/jest/topic/topic1', 'https://example.com')).toBe(true);
expect(isInternalLink('/#narrow/stream/jest', 'https://example.com')).toBe(true);
expect(isInternalLink('#narrow/stream/jest/topic/topic1', realm)).toBe(true);
expect(isInternalLink('/#narrow/stream/jest', realm)).toBe(true);
});

test('links including IDs are also recognized', () => {
expect(isInternalLink('#narrow/stream/123-jest/topic/topic1', 'https://example.com')).toBe(
true,
);
expect(isInternalLink('/#narrow/stream/123-jest', 'https://example.com')).toBe(true);
expect(isInternalLink('/#narrow/pm-with/123-mark', 'https://example.com')).toBe(true);
expect(isInternalLink('#narrow/stream/123-jest/topic/topic1', realm)).toBe(true);
expect(isInternalLink('/#narrow/stream/123-jest', realm)).toBe(true);
expect(isInternalLink('/#narrow/pm-with/123-mark', realm)).toBe(true);
});
});

describe('isMessageLink', () => {
test('only in-app link containing "near/<message-id>" is a message link', () => {
expect(isMessageLink('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
false,
);
expect(isMessageLink('https://example.com/#narrow/#near/1', 'https://example.com')).toBe(true);
expect(isMessageLink('https://example.com/#narrow/stream/jest', realm)).toBe(false);
expect(isMessageLink('https://example.com/#narrow/#near/1', realm)).toBe(true);
});
});

describe('getLinkType', () => {
test('links to a different domain are of "external" type', () => {
expect(getLinkType('https://google.com/some-path', 'https://example.com')).toBe('external');
expect(getLinkType('https://google.com/some-path', realm)).toBe('external');
});

test('only in-app link containing "stream" is a stream link', () => {
expect(
getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
).toBe('pm');
expect(getLinkType('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
'stream',
);
expect(getLinkType('https://example.com/#narrow/stream/stream/', 'https://example.com')).toBe(
'stream',
);
expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
expect(getLinkType('https://example.com/#narrow/stream/jest', realm)).toBe('stream');
expect(getLinkType('https://example.com/#narrow/stream/stream/', realm)).toBe('stream');
});

test('when a url is not a topic narrow return false', () => {
expect(
getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
).toBe('pm');
expect(getLinkType('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
'stream',
);
expect(
getLinkType(
'https://example.com/#narrow/stream/stream/topic/topic/near/',
'https://example.com',
),
).toBe('home');
expect(getLinkType('https://example.com/#narrow/stream/topic/', 'https://example.com')).toBe(
'stream',
expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
expect(getLinkType('https://example.com/#narrow/stream/jest', realm)).toBe('stream');
expect(getLinkType('https://example.com/#narrow/stream/stream/topic/topic/near/', realm)).toBe(
'home',
);
expect(getLinkType('https://example.com/#narrow/stream/topic/', realm)).toBe('stream');
});

test('when a url is a topic narrow return true', () => {
expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
expect(
getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
).toBe('topic');
expect(
getLinkType(
'https://example.com/#narrow/stream/mobile/subject/topic/near/378333',
'https://example.com',
),
).toBe('topic');
expect(
getLinkType('https://example.com/#narrow/stream/mobile/topic/topic/', 'https://example.com'),
).toBe('topic');
expect(
getLinkType(
'https://example.com/#narrow/stream/stream/topic/topic/near/1',
'https://example.com',
),
getLinkType('https://example.com/#narrow/stream/mobile/subject/topic/near/378333', realm),
).toBe('topic');
expect(getLinkType('https://example.com/#narrow/stream/mobile/topic/topic/', realm)).toBe(
'topic',
);
expect(getLinkType('https://example.com/#narrow/stream/stream/topic/topic/near/1', realm)).toBe(
'topic',
);
expect(
getLinkType(
'https://example.com/#narrow/stream/stream/subject/topic/near/1',
'https://example.com',
),
getLinkType('https://example.com/#narrow/stream/stream/subject/topic/near/1', realm),
).toBe('topic');

expect(getLinkType('/#narrow/stream/stream/subject/topic', 'https://example.com')).toBe(
'topic',
);
expect(getLinkType('/#narrow/stream/stream/subject/topic', realm)).toBe('topic');
});

test('only in-app link containing "pm-with" is a group link', () => {
expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group', realm)).toBe('pm');
expect(getLinkType('https://example.com/#narrow/pm-with/1,2-group/near/1', realm)).toBe('pm');
expect(
getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
).toBe('topic');
expect(
getLinkType('https://example.com/#narrow/pm-with/1,2-group', 'https://example.com'),
).toBe('pm');
expect(
getLinkType('https://example.com/#narrow/pm-with/1,2-group/near/1', 'https://example.com'),
).toBe('pm');
expect(
getLinkType(
'https://example.com/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3',
'https://example.com',
),
getLinkType('https://example.com/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', realm),
).toBe('pm');
});

test('only in-app link containing "is" is a special link', () => {
expect(
getLinkType('https://example.com/#narrow/stream/jest/topic/test', 'https://example.com'),
).toBe('topic');
expect(getLinkType('https://example.com/#narrow/is/private', 'https://example.com')).toBe(
'special',
);
expect(getLinkType('https://example.com/#narrow/is/starred', 'https://example.com')).toBe(
'special',
);
expect(getLinkType('https://example.com/#narrow/is/mentioned', 'https://example.com')).toBe(
'special',
);
expect(getLinkType('https://example.com/#narrow/is/men', 'https://example.com')).toBe('home');
expect(getLinkType('https://example.com/#narrow/is/men/stream', 'https://example.com')).toBe(
'home',
);
expect(getLinkType('https://example.com/#narrow/are/men/stream', 'https://example.com')).toBe(
'home',
);
expect(getLinkType('https://example.com/#narrow/stream/jest/topic/test', realm)).toBe('topic');
expect(getLinkType('https://example.com/#narrow/is/private', realm)).toBe('special');
expect(getLinkType('https://example.com/#narrow/is/starred', realm)).toBe('special');
expect(getLinkType('https://example.com/#narrow/is/mentioned', realm)).toBe('special');
expect(getLinkType('https://example.com/#narrow/is/men', realm)).toBe('home');
expect(getLinkType('https://example.com/#narrow/is/men/stream', realm)).toBe('home');
expect(getLinkType('https://example.com/#narrow/are/men/stream', realm)).toBe('home');
});
});

Expand Down Expand Up @@ -189,7 +136,7 @@ describe('getNarrowFromLink', () => {
const get = (url, streams = []) =>
getNarrowFromLink(
url,
'https://example.com',
new URL('https://example.com'),
usersById,
new Map(streams.map(s => [s.stream_id, s])),
);
Expand Down Expand Up @@ -343,26 +290,18 @@ describe('getNarrowFromLink', () => {

describe('getMessageIdFromLink', () => {
test('not message link', () => {
expect(
getMessageIdFromLink('https://example.com/#narrow/is/private', 'https://example.com'),
).toBe(0);
expect(getMessageIdFromLink('https://example.com/#narrow/is/private', realm)).toBe(0);
});

test('when link is a group link, return anchor message id', () => {
expect(
getMessageIdFromLink(
'https://example.com/#narrow/pm-with/1,3-group/near/1/',
'https://example.com',
),
getMessageIdFromLink('https://example.com/#narrow/pm-with/1,3-group/near/1/', realm),
).toBe(1);
});

test('when link is a topic link, return anchor message id', () => {
expect(
getMessageIdFromLink(
'https://example.com/#narrow/stream/jest/topic/test/near/1',
'https://example.com',
),
getMessageIdFromLink('https://example.com/#narrow/stream/jest/topic/test/near/1', realm),
).toBe(1);
});
});
14 changes: 7 additions & 7 deletions src/utils/__tests__/url-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,20 @@ describe('getResource', () => {
});

describe('isUrlOnRealm', () => {
const realm = new URL('https://example.com');

test('when link is on realm, return true', () => {
expect(isUrlOnRealm('/#narrow/stream/jest', 'https://example.com')).toBe(true);
expect(isUrlOnRealm('/#narrow/stream/jest', realm)).toBe(true);

expect(isUrlOnRealm('https://example.com/#narrow/stream/jest', 'https://example.com')).toBe(
true,
);
expect(isUrlOnRealm('https://example.com/#narrow/stream/jest', realm)).toBe(true);

expect(isUrlOnRealm('#narrow/#near/1', 'https://example.com')).toBe(true);
expect(isUrlOnRealm('#narrow/#near/1', realm)).toBe(true);
});

test('when link is on not realm, return false', () => {
expect(isUrlOnRealm('https://demo.example.com', 'https://example.com')).toBe(false);
expect(isUrlOnRealm('https://demo.example.com', realm)).toBe(false);

expect(isUrlOnRealm('www.google.com', 'https://example.com')).toBe(false);
expect(isUrlOnRealm('www.google.com', realm)).toBe(false);
});
});

Expand Down
20 changes: 12 additions & 8 deletions src/utils/internalLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import type { Narrow, Stream, User } from '../types';
import { topicNarrow, streamNarrow, groupNarrow, specialNarrow } from './narrow';
import { isUrlOnRealm } from './url';

const getPathsFromUrl = (url: string = '', realm: string) => {
// TODO: Work out what this does, write a jsdoc for its interface, and
// reimplement using URL object (not just for the realm)
const getPathsFromUrl = (url: string = '', realm: URL) => {
const paths = url
.split(realm)
.split(realm.toString())
.pop()
.split('#narrow/')
.pop()
Expand All @@ -20,16 +22,18 @@ const getPathsFromUrl = (url: string = '', realm: string) => {
};

/** PRIVATE -- exported only for tests. */
export const isInternalLink = (url: string, realm: string): boolean =>
isUrlOnRealm(url, realm) ? /^(\/#narrow|#narrow)/i.test(url.split(realm).pop()) : false;
export const isInternalLink = (url: string, realm: URL): boolean =>
isUrlOnRealm(url, realm)
? /^(\/#narrow|#narrow)/i.test(url.split(realm.toString()).pop())
: false;

/** PRIVATE -- exported only for tests. */
export const isMessageLink = (url: string, realm: string): boolean =>
export const isMessageLink = (url: string, realm: URL): boolean =>
isInternalLink(url, realm) && url.includes('near');

type LinkType = 'external' | 'home' | 'pm' | 'topic' | 'stream' | 'special';

export const getLinkType = (url: string, realm: string): LinkType => {
export const getLinkType = (url: string, realm: URL): LinkType => {
if (!isInternalLink(url, realm)) {
return 'external';
}
Expand Down Expand Up @@ -115,7 +119,7 @@ const parsePmOperand = (operand, usersById) => {

export const getNarrowFromLink = (
url: string,
realm: string,
realm: URL,
usersById: Map<number, User>,
streamsById: Map<number, Stream>,
): Narrow | null => {
Expand All @@ -141,7 +145,7 @@ export const getNarrowFromLink = (
}
};

export const getMessageIdFromLink = (url: string, realm: string): number => {
export const getMessageIdFromLink = (url: string, realm: URL): number => {
const paths = getPathsFromUrl(url, realm);

return isMessageLink(url, realm) ? parseInt(paths[paths.lastIndexOf('near') + 1], 10) : 0;
Expand Down
10 changes: 5 additions & 5 deletions src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ export const tryParseUrl = (url: string, base?: string | URL): URL | void => {
}
};

export const isUrlOnRealm = (url: string = '', realm: string): boolean =>
url.startsWith('/') || url.startsWith(realm) || !/^(http|www.)/i.test(url);
// TODO: Work out what this does, write a jsdoc for its interface, and
// reimplement using URL object (not just for the realm)
export const isUrlOnRealm = (url: string = '', realm: URL): boolean =>
url.startsWith('/') || url.startsWith(realm.toString()) || !/^(http|www.)/i.test(url);

const getResourceWithAuth = (uri: string, auth: Auth) => ({
uri: new URL(uri, auth.realm).toString(),
Expand All @@ -56,9 +58,7 @@ export const getResource = (
uri: string,
auth: Auth,
): {| uri: string, headers?: { [string]: string } |} =>
isUrlOnRealm(uri, auth.realm.toString())
? getResourceWithAuth(uri, auth)
: getResourceNoAuth(uri);
isUrlOnRealm(uri, auth.realm) ? getResourceWithAuth(uri, auth) : getResourceNoAuth(uri);

export type Protocol = 'https://' | 'http://';

Expand Down

0 comments on commit 4207e2e

Please sign in to comment.