From a5330bc08f4a001f31543ba20533fdab113d9bb7 Mon Sep 17 00:00:00 2001 From: Amit Uttam Date: Mon, 13 Jan 2020 16:28:14 -0300 Subject: [PATCH] MM-21369 Require server/site URL for deep links (#3770) **Old (only worked without serverURL or siteURL)** Beta * `mattermost-beta:///channels/` * `mattermost-beta:///pl/` Release * `mattermost-mobile:///channels/` * `mattermost-mobile:///pl/` **New working deep link patterns** Beta * `mattermost-beta:///channels/` * `mattermost-beta:///pl/` * `mattermost-beta://https:///channels/` * `mattermost-beta://https:///pl/` Note: Transport protocol (http, https, etc.) is optional. Release * `mattermost-mobile:///channels/` * `mattermost-mobile:///pl/` * `mattermost-mobile://https:///channels/` * `mattermost-mobile://https:///pl/` Note: Transport protocol (http, https, etc.) is optional. --- .../markdown/markdown_link/markdown_link.js | 17 +++++++++++-- app/components/post_list/post_list.js | 20 +++++++++++++++- app/components/post_list/post_list.test.js | 7 ++++-- app/utils/url.js | 10 ++++---- app/utils/url.test.js | 24 ++++++++++++++++++- assets/base/i18n/en.json | 2 ++ fastlane/Fastfile | 8 ------- 7 files changed, 70 insertions(+), 18 deletions(-) diff --git a/app/components/markdown/markdown_link/markdown_link.js b/app/components/markdown/markdown_link/markdown_link.js index 3aa9ff310..a01117972 100644 --- a/app/components/markdown/markdown_link/markdown_link.js +++ b/app/components/markdown/markdown_link/markdown_link.js @@ -3,7 +3,7 @@ import React, {Children, PureComponent} from 'react'; import PropTypes from 'prop-types'; -import {Clipboard, Linking, Text} from 'react-native'; +import {Alert, Clipboard, Linking, Text} from 'react-native'; import urlParse from 'url-parse'; import {intlShape} from 'react-intl'; @@ -52,7 +52,8 @@ export default class MarkdownLink extends PureComponent { serverUrl = await getCurrentServerUrl(); } - const match = matchDeepLink(url, serverUrl, siteURL); + const match = matchDeepLink(url, serverURL, siteURL); + if (match) { if (match.type === DeepLinkTypes.CHANNEL) { this.props.actions.handleSelectChannelByName(match.channelName, match.teamName); @@ -63,6 +64,18 @@ export default class MarkdownLink extends PureComponent { Linking.canOpenURL(url).then((supported) => { if (supported) { Linking.openURL(url); + } else { + const {formatMessage} = this.context.intl; + Alert.alert( + formatMessage({ + id: 'mobile.server_link.error.title', + defaultMessage: 'Link Error', + }), + formatMessage({ + id: 'mobile.server_link.error.text', + defaultMessage: 'The link could not be found on this server.', + }), + ); } }); } diff --git a/app/components/post_list/post_list.js b/app/components/post_list/post_list.js index 7cc13f5be..cc396ba68 100644 --- a/app/components/post_list/post_list.js +++ b/app/components/post_list/post_list.js @@ -3,7 +3,8 @@ import React, {PureComponent} from 'react'; import PropTypes from 'prop-types'; -import {FlatList, RefreshControl, StyleSheet} from 'react-native'; +import {Alert, FlatList, RefreshControl, StyleSheet} from 'react-native'; +import {intlShape} from 'react-intl'; import EventEmitter from 'mattermost-redux/utils/event_emitter'; import * as PostListUtils from 'mattermost-redux/utils/post_list'; @@ -78,6 +79,10 @@ export default class PostList extends PureComponent { postIds: [], }; + static contextTypes = { + intl: intlShape.isRequired, + }; + constructor(props) { super(props); @@ -163,12 +168,25 @@ export default class PostList extends PureComponent { const {serverURL, siteURL} = this.props; const match = matchDeepLink(url, serverURL, siteURL); + if (match) { if (match.type === DeepLinkTypes.CHANNEL) { this.props.actions.handleSelectChannelByName(match.channelName, match.teamName); } else if (match.type === DeepLinkTypes.PERMALINK) { this.handlePermalinkPress(match.postId, match.teamName); } + } else { + const {formatMessage} = this.context.intl; + Alert.alert( + formatMessage({ + id: 'mobile.server_link.error.title', + defaultMessage: 'Link Error', + }), + formatMessage({ + id: 'mobile.server_link.error.text', + defaultMessage: 'The link could not be found on this server.', + }), + ); } }; diff --git a/app/components/post_list/post_list.test.js b/app/components/post_list/post_list.test.js index ef3aa620d..3ea3f4774 100644 --- a/app/components/post_list/post_list.test.js +++ b/app/components/post_list/post_list.test.js @@ -10,9 +10,12 @@ import * as NavigationActions from 'app/actions/navigation'; import PostList from './post_list'; jest.useFakeTimers(); +jest.mock('react-intl'); describe('PostList', () => { const serverURL = 'https://server-url.fake'; + const deeplinkRoot = 'mattermost-beta://server-url.fake'; + const baseProps = { actions: { handleSelectChannelByName: jest.fn(), @@ -31,8 +34,8 @@ describe('PostList', () => { }; const deepLinks = { - permalink: serverURL + '/team-name/pl/pl-id', - channel: serverURL + '/team-name/channels/channel-name', + permalink: deeplinkRoot + '/team-name/pl/pl-id', + channel: deeplinkRoot + '/team-name/channels/channel-name', }; test('should match snapshot', () => { diff --git a/app/utils/url.js b/app/utils/url.js index da2baf0ed..da62951ca 100644 --- a/app/utils/url.js +++ b/app/utils/url.js @@ -9,7 +9,6 @@ import {Files} from 'mattermost-redux/constants'; import {DeepLinkTypes} from 'app/constants'; const ytRegex = /(?:http|https):\/\/(?:www\.|m\.)?(?:(?:youtube\.com\/(?:(?:v\/)|(?:(?:watch|embed\/watch)(?:\/|.*v=))|(?:embed\/)|(?:user\/[^/]+\/u\/[0-9]\/)))|(?:youtu\.be\/))([^#&?]*)/; -const APP_SCHEME = 'mattermost-beta'; export function isValidUrl(url = '') { const regex = /^https?:\/\//i; @@ -104,15 +103,18 @@ export function matchDeepLink(url, serverURL, siteURL) { return null; } - const linkRoot = `(?:${escapeRegex(APP_SCHEME)}:\\/|${escapeRegex(serverURL)}|${escapeRegex(siteURL)})?`; + const serverURLWithoutProtocol = removeProtocol(serverURL); + const siteURLWithoutProtocol = removeProtocol(siteURL); - let match = new RegExp('^' + linkRoot + '\\/([^\\/]+)\\/channels\\/(\\S+)').exec(url); + const linkRoot = `(?:${escapeRegex(serverURLWithoutProtocol || siteURLWithoutProtocol)})`; + + let match = new RegExp(linkRoot + '\\/([^\\/]+)\\/channels\\/(\\S+)').exec(url); if (match) { return {type: DeepLinkTypes.CHANNEL, teamName: match[1], channelName: match[2]}; } - match = new RegExp('^' + linkRoot + '\\/([^\\/]+)\\/pl\\/(\\w+)').exec(url); + match = new RegExp(linkRoot + '\\/([^\\/]+)\\/pl\\/(\\w+)').exec(url); if (match) { return {type: DeepLinkTypes.PERMALINK, teamName: match[1], postId: match[2]}; } diff --git a/app/utils/url.test.js b/app/utils/url.test.js index e7f6670b9..b29a15f3a 100644 --- a/app/utils/url.test.js +++ b/app/utils/url.test.js @@ -87,17 +87,39 @@ describe('UrlUtils', () => { }); }); + describe('removeProtocol', () => { + const tests = [ + {name: 'should return url without http protocol prefix', url: 'http://localhost:8065', expected: 'localhost:8065'}, + {name: 'should return url without https protocol prefix', url: 'https://localhost:8065', expected: 'localhost:8065'}, + {name: 'should return null', url: '', expected: ''}, + {name: 'should return url without arbitrary protocol prefix', url: 'udp://localhost:8065', expected: 'localhost:8065'}, + ]; + + for (const test of tests) { + const {name, url, expected} = test; + + it(name, () => { + expect(UrlUtils.removeProtocol(url)).toEqual(expected); + }); + } + }); + describe('matchDeepLink', () => { const SITE_URL = 'http://localhost:8065'; const SERVER_URL = 'http://localhost:8065'; + const DEEPLINK_URL_ROOT = 'mattermost-beta://localhost:8065'; + const tests = [ {name: 'should return null if all inputs are empty', input: {url: '', serverURL: '', siteURL: ''}, expected: null}, {name: 'should return null if any of the input is null', input: {url: '', serverURL: '', siteURL: null}, expected: null}, {name: 'should return null if any of the input is null', input: {url: '', serverURL: null, siteURL: ''}, expected: null}, {name: 'should return null if any of the input is null', input: {url: null, serverURL: '', siteURL: ''}, expected: null}, - {name: 'should return null for not supported link', input: {url: 'https://mattermost.com', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: null}, + {name: 'should return null for not supported link', input: {url: 'https://otherserver.com', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: null}, + {name: 'should return null despite url subset match', input: {url: 'http://myserver.com', serverURL: 'http://myserver.co'}, expected: null}, {name: 'should match channel link', input: {url: SITE_URL + '/ad-1/channels/town-square', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: {channelName: 'town-square', teamName: 'ad-1', type: 'channel'}}, {name: 'should match permalink', input: {url: SITE_URL + '/ad-1/pl/qe93kkfd7783iqwuwfcwcxbsgy', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: {postId: 'qe93kkfd7783iqwuwfcwcxbsgy', teamName: 'ad-1', type: 'permalink'}}, + {name: 'should match channel link with deeplink prefix', input: {url: DEEPLINK_URL_ROOT + '/ad-1/channels/town-square', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: {channelName: 'town-square', teamName: 'ad-1', type: 'channel'}}, + {name: 'should match permalink with depplink prefix', input: {url: DEEPLINK_URL_ROOT + '/ad-1/pl/qe93kkfd7783iqwuwfcwcxbsgy', serverURL: SERVER_URL, siteURL: SITE_URL}, expected: {postId: 'qe93kkfd7783iqwuwfcwcxbsgy', teamName: 'ad-1', type: 'permalink'}}, ]; for (const test of tests) { diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 6d785ed68..a138192e2 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -443,6 +443,8 @@ "mobile.select_team.guest_cant_join_team": "Your guest account has no teams or channels assigned. Please contact an administrator.", "mobile.select_team.join_open": "Open teams you can join", "mobile.select_team.no_teams": "There are no available teams for you to join.", + "mobile.server_link.error.text": "The link could not be found on this server.", + "mobile.server_link.error.title": "Link Error", "mobile.server_upgrade.button": "OK", "mobile.server_upgrade.description": "\nA server upgrade is required to use the Mattermost app. Please ask your System Administrator for details.\n", "mobile.server_upgrade.title": "Server upgrade required", diff --git a/fastlane/Fastfile b/fastlane/Fastfile index f7e360ee3..a0f2bc595 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -167,14 +167,6 @@ lane :configure do # Save the config.json file save_config_json('../dist/assets/config.json', json) - # Set deep link prefix for URL lookups - app_scheme = ENV['APP_SCHEME'] || 'mattermost-beta' - find_replace_string( - path_to_file: './app/utils/url.js', - old_string: "APP_SCHEME = 'mattermost-beta'", - new_string: "APP_SCHEME = '#{app_scheme}'", - ) - configured = true end