From 82b833257b31df95609aa21e5e1cce7e7a2cbb6f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 14 Jul 2022 13:16:41 -1000 Subject: [PATCH 1/4] Remove fetchPersonalDetails() Fix tests Remove unused import Clean up network and timers fix style --- .../Navigation/AppNavigator/AuthScreens.js | 20 ------ src/libs/actions/App.js | 1 - src/libs/actions/PersonalDetails.js | 33 --------- tests/actions/ReportTest.js | 69 +++++++++++++++---- tests/utils/TestHelper.js | 28 -------- 5 files changed, 54 insertions(+), 97 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 02340ac16738..4d5ae9ee3e77 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -31,14 +31,11 @@ import MainDrawerNavigator from './MainDrawerNavigator'; // Modal Stack Navigators import * as ModalStackNavigators from './ModalStackNavigators'; import SCREENS from '../../../SCREENS'; -import Timers from '../../Timers'; import ValidateLoginPage from '../../../pages/ValidateLoginPage'; import defaultScreenOptions from './defaultScreenOptions'; import * as App from '../../actions/App'; import * as Session from '../../actions/Session'; import LogOutPreviousUserPage from '../../../pages/LogOutPreviousUserPage'; -import networkPropTypes from '../../../components/networkPropTypes'; -import {withNetwork} from '../../../components/OnyxProvider'; Onyx.connect({ key: ONYXKEYS.MY_PERSONAL_DETAILS, @@ -75,9 +72,6 @@ const modalScreenListeners = { }; const propTypes = { - /** Information about the network */ - network: networkPropTypes.isRequired, - ...windowDimensionsPropTypes, }; @@ -109,19 +103,6 @@ class AuthScreens extends React.Component { App.fixAccountAndReloadData(); App.setUpPoliciesAndNavigate(this.props.session); - - // Refresh the personal details, timezone and betas every 30 minutes - // There is no pusher event that sends updated personal details data yet - // See https://github.com/Expensify/ReactNativeChat/issues/468 - this.interval = Timers.register(setInterval(() => { - if (this.props.network.isOffline) { - return; - } - PersonalDetails.fetchPersonalDetails(); - User.getUserDetails(); - User.getBetas(); - }, 1000 * 60 * 30)); - Timing.end(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); const searchShortcutConfig = CONST.KEYBOARD_SHORTCUTS.SEARCH; @@ -321,7 +302,6 @@ class AuthScreens extends React.Component { AuthScreens.propTypes = propTypes; export default compose( withWindowDimensions, - withNetwork(), withOnyx({ session: { key: ONYXKEYS.SESSION, diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 89c59870ac4f..c5c0dadce87f 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -102,7 +102,6 @@ function getAppData(shouldSyncPolicyList = true) { // We should update the syncing indicator when personal details and reports are both done fetching. return Promise.all([ - PersonalDetails.fetchPersonalDetails(), Report.fetchAllReports(true, true), ]); } diff --git a/src/libs/actions/PersonalDetails.js b/src/libs/actions/PersonalDetails.js index cc7add84320e..7c454d19bf24 100644 --- a/src/libs/actions/PersonalDetails.js +++ b/src/libs/actions/PersonalDetails.js @@ -116,38 +116,6 @@ function formatPersonalDetails(personalDetailsList) { return formattedResult; } -/** - * Get the personal details for our organization - * @returns {Promise} - */ -function fetchPersonalDetails() { - return DeprecatedAPI.Get({ - returnValueList: 'personalDetailsList', - }) - .then((data) => { - let myPersonalDetails = {}; - - // If personalDetailsList does not have the current user ensure we initialize their details with an empty - // object at least - const personalDetailsList = _.isEmpty(data.personalDetailsList) ? {} : data.personalDetailsList; - if (!personalDetailsList[currentUserEmail]) { - personalDetailsList[currentUserEmail] = {}; - } - - const allPersonalDetails = formatPersonalDetails(personalDetailsList); - Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, allPersonalDetails); - - myPersonalDetails = allPersonalDetails[currentUserEmail]; - - // Add the first and last name to the current user's MY_PERSONAL_DETAILS key - myPersonalDetails.firstName = lodashGet(data.personalDetailsList, [currentUserEmail, 'firstName'], ''); - myPersonalDetails.lastName = lodashGet(data.personalDetailsList, [currentUserEmail, 'lastName'], ''); - - // Set my personal details so they can be easily accessed and subscribed to on their own key - Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, myPersonalDetails); - }); -} - /** * Gets the first and last name from the user's personal details. * If the login is the same as the displayName, then they don't exist, @@ -371,7 +339,6 @@ function deleteAvatar(defaultAvatarURL) { } export { - fetchPersonalDetails, formatPersonalDetails, getFromReportParticipants, getDisplayName, diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 77d19c5eaa1a..9743505a273a 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -16,6 +16,7 @@ import * as TestHelper from '../utils/TestHelper'; import Log from '../../src/libs/Log'; import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; import * as User from '../../src/libs/actions/User'; +import * as ReportUtils from '../../src/libs/ReportUtils'; describe('actions/Report', () => { beforeAll(() => { @@ -81,14 +82,27 @@ describe('actions/Report', () => { User.subscribeToUserEvents(); return waitForPromisesToResolve(); }) - .then(() => TestHelper.fetchPersonalDetailsForTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, { - [TEST_USER_LOGIN]: { + .then(() => { + const avatar = ReportUtils.getDefaultAvatar(TEST_USER_LOGIN); + const details = { accountID: TEST_USER_ACCOUNT_ID, - email: TEST_USER_LOGIN, + login: TEST_USER_LOGIN, + avatar, + displayName: 'Test User', firstName: 'Test', lastName: 'User', - }, - })) + pronouns: '', + timezone: CONST.DEFAULT_TIME_ZONE, + payPalMeAddress: '', + phoneNumber: '', + avatarHighResolution: avatar, + }; + Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); + Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { + [TEST_USER_LOGIN]: details, + }); + return waitForPromisesToResolve(); + }) .then(() => { // This is a fire and forget response, but once it completes we should be able to verify that we // have an "optimistic" report action in Onyx. @@ -183,14 +197,27 @@ describe('actions/Report', () => { // GIVEN a test user with initial data return TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN) - .then(() => TestHelper.fetchPersonalDetailsForTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, { - [TEST_USER_LOGIN]: { + .then(() => { + const avatar = ReportUtils.getDefaultAvatar(TEST_USER_LOGIN); + const details = { accountID: TEST_USER_ACCOUNT_ID, - email: TEST_USER_LOGIN, + login: TEST_USER_LOGIN, + avatar, + displayName: TEST_USER_LOGIN, firstName: 'Test', lastName: 'User', - }, - })) + pronouns: '', + timezone: CONST.DEFAULT_TIME_ZONE, + payPalMeAddress: '', + phoneNumber: '', + avatarHighResolution: avatar, + }; + Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); + Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { + [TEST_USER_LOGIN]: details, + }); + return waitForPromisesToResolve(); + }) .then(() => { global.fetch = TestHelper.getGlobalFetchMock(); @@ -243,14 +270,26 @@ describe('actions/Report', () => { User.subscribeToUserEvents(); return waitForPromisesToResolve(); }) - .then(() => TestHelper.fetchPersonalDetailsForTestUser(USER_1_ACCOUNT_ID, USER_1_LOGIN, { - [USER_1_LOGIN]: { + .then(() => { + const avatar = ReportUtils.getDefaultAvatar(USER_1_LOGIN); + const details = { accountID: USER_1_ACCOUNT_ID, - email: USER_1_LOGIN, + login: USER_1_LOGIN, + avatar, + displayName: USER_1_LOGIN, firstName: 'Test', lastName: 'User', - }, - })) + pronouns: '', + timezone: CONST.DEFAULT_TIME_ZONE, + payPalMeAddress: '', + phoneNumber: '', + avatarHighResolution: avatar, + }; + Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); + Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { + [USER_1_LOGIN]: details, + }); + }) .then(() => { // When a Pusher event is handled for a new report comment channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ diff --git a/tests/utils/TestHelper.js b/tests/utils/TestHelper.js index 9cee105e64b7..48cdb9f9f3bf 100644 --- a/tests/utils/TestHelper.js +++ b/tests/utils/TestHelper.js @@ -1,5 +1,4 @@ import * as Session from '../../src/libs/actions/Session'; -import * as PersonalDetails from '../../src/libs/actions/PersonalDetails'; import HttpUtils from '../../src/libs/HttpUtils'; import waitForPromisesToResolve from './waitForPromisesToResolve'; @@ -51,32 +50,6 @@ function signInWithTestUser(accountID = 1, login = 'test@user.com', password = ' }); } -/** - * Fetch and set personal details with provided personalDetailsList - * - * @param {Number} accountID - * @param {String} email - * @param {Object} personalDetailsList - * @returns {Promise} - */ -function fetchPersonalDetailsForTestUser(accountID, email, personalDetailsList) { - const originalXHR = HttpUtils.xhr; - HttpUtils.xhr = jest.fn(); - - HttpUtils.xhr - .mockImplementationOnce(() => Promise.resolve({ - accountID, - email, - personalDetailsList, - })); - - PersonalDetails.fetchPersonalDetails(); - return waitForPromisesToResolve() - .then(() => { - HttpUtils.xhr = originalXHR; - }); -} - /** * Use for situations where fetch() is required. * @@ -101,5 +74,4 @@ function getGlobalFetchMock() { export { getGlobalFetchMock, signInWithTestUser, - fetchPersonalDetailsForTestUser, }; From 5853f8830bb5b110fd4d3fb5dc1ecbb899e697ce Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 18 Jul 2022 09:43:35 -1000 Subject: [PATCH 2/4] remove random MY_PERSONAL_DETAILS keys --- src/libs/actions/Report.js | 5 ----- tests/actions/ReportTest.js | 3 --- 2 files changed, 8 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index c14279085fbf..f851f172e111 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -982,11 +982,6 @@ function addActions(reportID, text = '', file) { if (DateUtils.canUpdateTimezone()) { const timezone = DateUtils.getCurrentTimezone(); parameters.timezone = JSON.stringify(timezone); - optimisticData.push({ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: ONYXKEYS.MY_PERSONAL_DETAILS, - value: {timezone}, - }); optimisticData.push({ onyxMethod: CONST.ONYX.METHOD.MERGE, key: ONYXKEYS.PERSONAL_DETAILS, diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 9743505a273a..66b4238ee336 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -97,7 +97,6 @@ describe('actions/Report', () => { phoneNumber: '', avatarHighResolution: avatar, }; - Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { [TEST_USER_LOGIN]: details, }); @@ -212,7 +211,6 @@ describe('actions/Report', () => { phoneNumber: '', avatarHighResolution: avatar, }; - Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { [TEST_USER_LOGIN]: details, }); @@ -285,7 +283,6 @@ describe('actions/Report', () => { phoneNumber: '', avatarHighResolution: avatar, }; - Onyx.merge(ONYXKEYS.MY_PERSONAL_DETAILS, details); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { [USER_1_LOGIN]: details, }); From 0737143c0ab20a16edb7c9a869f25748d053fba0 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 18 Jul 2022 09:52:18 -1000 Subject: [PATCH 3/4] Remove PersonalDetails import --- src/libs/actions/App.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/App.js b/src/libs/actions/App.js index 02e7e37c335b..f1e5426deb8d 100644 --- a/src/libs/actions/App.js +++ b/src/libs/actions/App.js @@ -9,7 +9,6 @@ import CONST from '../../CONST'; import Log from '../Log'; import Performance from '../Performance'; import Timing from './Timing'; -import * as PersonalDetails from './PersonalDetails'; import * as Report from './Report'; import * as BankAccounts from './BankAccounts'; import * as Policy from './Policy'; From ee0dd77fe87a3302e42008de13f7abd0e6975df6 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 19 Jul 2022 09:21:40 -1000 Subject: [PATCH 4/4] DRY up tests. Create TestHelper.setPersonalDetails() --- tests/actions/ReportTest.js | 63 ++----------------------------------- tests/utils/TestHelper.js | 31 ++++++++++++++++++ 2 files changed, 34 insertions(+), 60 deletions(-) diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index 66b4238ee336..ce8e41f18122 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -16,7 +16,6 @@ import * as TestHelper from '../utils/TestHelper'; import Log from '../../src/libs/Log'; import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; import * as User from '../../src/libs/actions/User'; -import * as ReportUtils from '../../src/libs/ReportUtils'; describe('actions/Report', () => { beforeAll(() => { @@ -82,26 +81,7 @@ describe('actions/Report', () => { User.subscribeToUserEvents(); return waitForPromisesToResolve(); }) - .then(() => { - const avatar = ReportUtils.getDefaultAvatar(TEST_USER_LOGIN); - const details = { - accountID: TEST_USER_ACCOUNT_ID, - login: TEST_USER_LOGIN, - avatar, - displayName: 'Test User', - firstName: 'Test', - lastName: 'User', - pronouns: '', - timezone: CONST.DEFAULT_TIME_ZONE, - payPalMeAddress: '', - phoneNumber: '', - avatarHighResolution: avatar, - }; - Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { - [TEST_USER_LOGIN]: details, - }); - return waitForPromisesToResolve(); - }) + .then(() => TestHelper.setPersonalDetails(TEST_USER_LOGIN, TEST_USER_ACCOUNT_ID)) .then(() => { // This is a fire and forget response, but once it completes we should be able to verify that we // have an "optimistic" report action in Onyx. @@ -196,26 +176,7 @@ describe('actions/Report', () => { // GIVEN a test user with initial data return TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN) - .then(() => { - const avatar = ReportUtils.getDefaultAvatar(TEST_USER_LOGIN); - const details = { - accountID: TEST_USER_ACCOUNT_ID, - login: TEST_USER_LOGIN, - avatar, - displayName: TEST_USER_LOGIN, - firstName: 'Test', - lastName: 'User', - pronouns: '', - timezone: CONST.DEFAULT_TIME_ZONE, - payPalMeAddress: '', - phoneNumber: '', - avatarHighResolution: avatar, - }; - Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { - [TEST_USER_LOGIN]: details, - }); - return waitForPromisesToResolve(); - }) + .then(() => TestHelper.setPersonalDetails(TEST_USER_LOGIN, TEST_USER_ACCOUNT_ID)) .then(() => { global.fetch = TestHelper.getGlobalFetchMock(); @@ -268,25 +229,7 @@ describe('actions/Report', () => { User.subscribeToUserEvents(); return waitForPromisesToResolve(); }) - .then(() => { - const avatar = ReportUtils.getDefaultAvatar(USER_1_LOGIN); - const details = { - accountID: USER_1_ACCOUNT_ID, - login: USER_1_LOGIN, - avatar, - displayName: USER_1_LOGIN, - firstName: 'Test', - lastName: 'User', - pronouns: '', - timezone: CONST.DEFAULT_TIME_ZONE, - payPalMeAddress: '', - phoneNumber: '', - avatarHighResolution: avatar, - }; - Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { - [USER_1_LOGIN]: details, - }); - }) + .then(() => TestHelper.setPersonalDetails(USER_1_LOGIN, USER_1_ACCOUNT_ID)) .then(() => { // When a Pusher event is handled for a new report comment channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ diff --git a/tests/utils/TestHelper.js b/tests/utils/TestHelper.js index 48cdb9f9f3bf..d619e9d72ab2 100644 --- a/tests/utils/TestHelper.js +++ b/tests/utils/TestHelper.js @@ -1,6 +1,10 @@ +import Onyx from 'react-native-onyx'; +import CONST from '../../src/CONST'; import * as Session from '../../src/libs/actions/Session'; import HttpUtils from '../../src/libs/HttpUtils'; +import ONYXKEYS from '../../src/ONYXKEYS'; import waitForPromisesToResolve from './waitForPromisesToResolve'; +import * as ReportUtils from '../../src/libs/ReportUtils'; /** * Simulate signing in and make sure all API calls in this flow succeed. Every time we add @@ -71,7 +75,34 @@ function getGlobalFetchMock() { }); } +/** + * @param {String} login + * @param {Number} accountID + * @returns {Promise} + */ +function setPersonalDetails(login, accountID) { + const avatar = ReportUtils.getDefaultAvatar(login); + const details = { + accountID, + login, + avatar, + displayName: 'Test User', + firstName: 'Test', + lastName: 'User', + pronouns: '', + timezone: CONST.DEFAULT_TIME_ZONE, + payPalMeAddress: '', + phoneNumber: '', + avatarHighResolution: avatar, + }; + Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, { + [login]: details, + }); + return waitForPromisesToResolve(); +} + export { getGlobalFetchMock, signInWithTestUser, + setPersonalDetails, };