Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Link Navigation relative to current Environment #24483

Merged
merged 7 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions __mocks__/react-native.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// eslint-disable-next-line no-restricted-imports
import * as ReactNative from 'react-native';
import _ from 'underscore';
import CONST from '../src/CONST';

jest.doMock('react-native', () => {
let url = 'https://new.expensify.com/';
Expand All @@ -15,7 +14,12 @@ jest.doMock('react-native', () => {
// runs against index.native.js source and so anything that is testing a component reliant on withWindowDimensions()
// would be most commonly assumed to be on a mobile phone vs. a tablet or desktop style view. This behavior can be
// overridden by explicitly setting the dimensions inside a test via Dimensions.set()
let dimensions = CONST.TESTING.SCREEN_SIZE.SMALL;
let dimensions = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to make these changes because this mock file was importing CONST file which was in turn importing src/libs/Url.js file in which we have imported the polyfill. Since the polyfill import internally imports RN packages and since we are in test environment the packages were being used before mocking them (which is done later in this file) and the test suite broke.To fix this we have multiple options:

  1. We can import the polyfill in the root App.js file as mentioned in the usage guidelines (https://github.com/charpeni/react-native-url-polyfill) but it will then override URL and URLSearchParams classes in the entire project.I actually committed that change in the previous commit and everything was working fine including the test cases.Also these 2 classes are not used much so the chance of breaking something is low and since the overrides basically extends the default RN classes so I think we are fine using this option too.

  2. Remove the import from the test suite before the mocking happens.In this option I had 2 further options:

    • Remove the Url.js import from CONST.js as it was imported to use the addTrailingForwardSlash just at one place in the file.Also importing lib files in CONST file does not make sense.
    • The other option was to remove the CONST file import in this file and that's what I have done now.

Let me know what you think is the best way forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the CONST file import in this file

Current solution you did works for me.
@francoisl what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah works for me too.

width: 300,
height: 700,
scale: 1,
fontScale: 1,
};

return Object.setPrototypeOf(
{
Expand Down
58 changes: 58 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
"react-native-screens": "3.21.0",
"react-native-svg": "^13.9.0",
"react-native-tab-view": "^3.5.2",
"react-native-url-polyfill": "^2.0.0",
"react-native-view-shot": "^3.6.0",
"react-native-vision-camera": "^2.15.4",
"react-native-web-linear-gradient": "^1.1.2",
Expand Down
2 changes: 1 addition & 1 deletion src/CONFIG.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export default {
},
CAPTURE_METRICS: lodashGet(Config, 'CAPTURE_METRICS', 'false') === 'true',
ONYX_METRICS: lodashGet(Config, 'ONYX_METRICS', 'false') === 'true',
DEV_PORT: process.env.PORT || 8080,
DEV_PORT: process.env.PORT || 8082,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated since we now run dev on 8082 so fallback port should be 8082.

E2E_TESTING: lodashGet(Config, 'E2E_TESTING', 'false') === 'true',
SEND_CRASH_REPORTS: lodashGet(Config, 'SEND_CRASH_REPORTS', 'false') === 'true',
IS_USING_WEB_PROXY: getPlatform() === 'web' && useWebProxy,
Expand Down
10 changes: 0 additions & 10 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -1311,16 +1311,6 @@ const CONST = {
INCORRECT_PASSWORD: 2,
},
},
TESTING: {
SCREEN_SIZE: {
SMALL: {
width: 300,
height: 700,
scale: 1,
fontScale: 1,
},
},
},
API_REQUEST_TYPE: {
READ: 'read',
WRITE: 'write',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,23 @@ import AnchorForAttachmentsOnly from '../../AnchorForAttachmentsOnly';
import * as Url from '../../../libs/Url';
import ROUTES from '../../../ROUTES';
import tryResolveUrlFromApiRoot from '../../../libs/tryResolveUrlFromApiRoot';
import useEnvironment from '../../../hooks/useEnvironment';

function AnchorRenderer(props) {
const htmlAttribs = props.tnode.attributes;

const {environmentURL} = useEnvironment();
// An auth token is needed to download Expensify chat attachments
const isAttachment = Boolean(htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]);
const displayName = lodashGet(props.tnode, 'domNode.children[0].data', '');
const parentStyle = lodashGet(props.tnode, 'parent.styles.nativeTextRet', {});
const attrHref = htmlAttribs.href || '';
const attrPath = lodashGet(Url.getURLObject(attrHref), 'path', '').replace('/', '');
const attrPath = Url.getPathFromURL(attrHref);
const hasSameOrigin = Url.hasSameExpensifyOrigin(attrHref, environmentURL);
const hasExpensifyOrigin = Url.hasSameExpensifyOrigin(attrHref, CONFIG.EXPENSIFY.EXPENSIFY_URL) || Url.hasSameExpensifyOrigin(attrHref, CONFIG.EXPENSIFY.STAGING_API_ROOT);
const internalNewExpensifyPath =
(Url.hasSameExpensifyOrigin(attrHref, CONST.NEW_EXPENSIFY_URL) || Url.hasSameExpensifyOrigin(attrHref, CONST.STAGING_NEW_EXPENSIFY_URL)) &&
(Url.hasSameExpensifyOrigin(attrHref, CONST.NEW_EXPENSIFY_URL) ||
Url.hasSameExpensifyOrigin(attrHref, CONST.STAGING_NEW_EXPENSIFY_URL) ||
attrHref.startsWith(CONST.DEV_NEW_EXPENSIFY_URL)) &&
!CONST.PATHS_TO_TREAT_AS_EXTERNAL.includes(attrPath)
? attrPath
: '';
Expand All @@ -48,7 +52,7 @@ function AnchorRenderer(props) {

// If we are handling a New Expensify link then we will assume this should be opened by the app internally. This ensures that the links are opened internally via react-navigation
// instead of in a new tab or with a page refresh (which is the default behavior of an anchor tag)
if (internalNewExpensifyPath) {
if (internalNewExpensifyPath && hasSameOrigin) {
Navigation.navigate(internalNewExpensifyPath);
return;
}
Expand Down
74 changes: 20 additions & 54 deletions src/libs/Url.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {URL_WEBSITE_REGEX} from 'expensify-common/lib/Url';

import 'react-native-url-polyfill/auto';
/**
* Add / to the end of any URL if not present
* @param {String} url
Expand All @@ -13,76 +12,43 @@ function addTrailingForwardSlash(url) {
}

/**
* Parse href to URL object
* @param {String} href
* @returns {Object}
* Get path from URL string
* @param {String} url
* @returns {String}
*/
function getURLObject(href) {
const urlRegex = new RegExp(URL_WEBSITE_REGEX, 'gi');
let match;
function getPathFromURL(url) {
try {
if (!href.startsWith('mailto:')) {
match = urlRegex.exec(href);
}
} catch (e) {
// eslint-disable-next-line no-console
console.warn('Error parsing url in Url.getURLObject', {error: e});
}
if (!match) {
return {
href: undefined,
protocol: undefined,
hostname: undefined,
path: undefined,
};
}
const baseUrl = match[0];
const protocol = match[1];
return {
href,
protocol,
hostname: baseUrl.replace(protocol, ''),
path: href.startsWith(baseUrl) ? href.replace(baseUrl, '') : '',
};
}

/**
* Determine if we should remove w3 from hostname
* E.g www.expensify.com should be the same as expensify.com
* @param {String} hostname
* @returns {Boolean}
*/
function shouldRemoveW3FromExpensifyUrl(hostname) {
// Since expensify.com.dev is accessible with and without www subdomain
if (hostname === 'www.expensify.com.dev') {
return true;
const path = new URL(url).pathname;
return path.substring(1); // Remove the leading '/'
} catch (error) {
console.error('Error parsing URL:', error);
return ''; // Return empty string for invalid URLs
}
const parts = hostname.split('.').reverse();
const subDomain = parts[2];
return subDomain === 'www';
}

/**
* Determine if two urls have the same origin
* Just care about expensify url to avoid the second-level domain (www.example.co.uk)
* @param {String} url1
* @param {String} url2
* @returns {Boolean}
*/
function hasSameExpensifyOrigin(url1, url2) {
const host1 = getURLObject(url1).hostname;
const host2 = getURLObject(url2).hostname;
if (!host1 || !host2) {
const removeW3 = (host) => host.replace(/^www\./i, '');
try {
const parsedUrl1 = new URL(url1);
const parsedUrl2 = new URL(url2);

return removeW3(parsedUrl1.host) === removeW3(parsedUrl2.host);
} catch (error) {
// Handle invalid URLs or other parsing errors
console.error('Error parsing URLs:', error);
return false;
}
const host1WithoutW3 = shouldRemoveW3FromExpensifyUrl(host1) ? host1.replace('www.', '') : host1;
const host2WithoutW3 = shouldRemoveW3FromExpensifyUrl(host2) ? host2.replace('www.', '') : host2;
return host1WithoutW3 === host2WithoutW3;
}

export {
// eslint-disable-next-line import/prefer-default-export
addTrailingForwardSlash,
hasSameExpensifyOrigin,
getURLObject,
getPathFromURL,
};
Loading
Loading