From eb89a1fc4e78fd595edc3f05b8526789c47aa5c2 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 27 Oct 2021 15:13:40 -1000 Subject: [PATCH 01/13] Fix changePassword --- src/libs/actions/User.js | 9 +++++---- src/pages/settings/PasswordPage.js | 9 ++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index 3021af64d784..ab4af9219a73 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -40,7 +40,7 @@ Onyx.connect({ * @param {String} password * @returns {Promise} */ -function changePassword(oldPassword, password) { +function changePasswordAndNavigate(oldPassword, password) { Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); return API.ChangePassword({oldPassword, password}) @@ -48,12 +48,13 @@ function changePassword(oldPassword, password) { if (response.jsonCode !== 200) { const error = lodashGet(response, 'message', 'Unable to change password. Please try again.'); Onyx.merge(ONYXKEYS.ACCOUNT, {error}); + return; } - return response; + + Navigation.navigate(ROUTES.SETTINGS); }) - .finally((response) => { + .finally(() => { Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); - return response; }); } diff --git a/src/pages/settings/PasswordPage.js b/src/pages/settings/PasswordPage.js index 1551257028fd..940ba49218fb 100755 --- a/src/pages/settings/PasswordPage.js +++ b/src/pages/settings/PasswordPage.js @@ -13,7 +13,7 @@ import styles from '../../styles/styles'; import ONYXKEYS from '../../ONYXKEYS'; import CONST from '../../CONST'; import Button from '../../components/Button'; -import {changePassword} from '../../libs/actions/User'; +import * as User from '../../libs/actions/User'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import compose from '../../libs/compose'; import KeyboardAvoidingView from '../../components/KeyboardAvoidingView'; @@ -94,12 +94,7 @@ class PasswordPage extends Component { handleChangePassword() { - changePassword(this.state.currentPassword, this.state.newPassword) - .then((response) => { - if (response.jsonCode === 200) { - Navigation.navigate(ROUTES.SETTINGS); - } - }); + User.changePasswordAndNavigate(this.state.currentPassword, this.state.newPassword); } doPasswordsMatch() { From 8c922c45889042ac648dabbf98ca388821393940 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 27 Oct 2021 15:16:08 -1000 Subject: [PATCH 02/13] fix secondary login --- src/libs/actions/User.js | 26 ++++++++++----------- src/pages/settings/AddSecondaryLoginPage.js | 11 +++------ 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index ab4af9219a73..17f8aced5946 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -133,7 +133,7 @@ function setExpensifyNewsStatus(subscribed) { * @param {String} password * @returns {Promise} */ -function setSecondaryLogin(login, password) { +function setSecondaryLoginAndNavigate(login, password) { Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); return API.User_SecondaryLogin_Send({ @@ -143,20 +143,20 @@ function setSecondaryLogin(login, password) { if (response.jsonCode === 200) { const loginList = _.where(response.loginList, {partnerName: 'expensify.com'}); Onyx.merge(ONYXKEYS.USER, {loginList}); - } else { - let error = lodashGet(response, 'message', 'Unable to add secondary login. Please try again.'); + Navigation.navigate(ROUTES.SETTINGS_PROFILE); + return; + } - // Replace error with a friendlier message - if (error.includes('already belongs to an existing Expensify account.')) { - error = 'This login already belongs to an existing Expensify account.'; - } + let error = lodashGet(response, 'message', 'Unable to add secondary login. Please try again.'); - Onyx.merge(ONYXKEYS.USER, {error}); + // Replace error with a friendlier message + if (error.includes('already belongs to an existing Expensify account.')) { + error = 'This login already belongs to an existing Expensify account.'; } - return response; - }).finally((response) => { + + Onyx.merge(ONYXKEYS.USER, {error}); + }).finally(() => { Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); - return response; }); } @@ -300,12 +300,12 @@ function clearUserErrorMessage() { } export { - changePassword, + changePasswordAndNavigate, getBetas, getUserDetails, resendValidateCode, setExpensifyNewsStatus, - setSecondaryLogin, + setSecondaryLoginAndNavigate, validateLogin, isBlockedFromConcierge, getDomainInfo, diff --git a/src/pages/settings/AddSecondaryLoginPage.js b/src/pages/settings/AddSecondaryLoginPage.js index 5f71550833e3..904b872a7a5b 100755 --- a/src/pages/settings/AddSecondaryLoginPage.js +++ b/src/pages/settings/AddSecondaryLoginPage.js @@ -9,7 +9,7 @@ import Navigation from '../../libs/Navigation/Navigation'; import ScreenWrapper from '../../components/ScreenWrapper'; import Text from '../../components/Text'; import styles from '../../styles/styles'; -import {clearUserErrorMessage, setSecondaryLogin} from '../../libs/actions/User'; +import * as User from '../../libs/actions/User'; import ONYXKEYS from '../../ONYXKEYS'; import Button from '../../components/Button'; import ROUTES from '../../ROUTES'; @@ -59,7 +59,7 @@ class AddSecondaryLoginPage extends Component { } componentWillUnmount() { - clearUserErrorMessage(); + User.clearUserErrorMessage(); } onSecondaryLoginChange(login) { @@ -75,12 +75,7 @@ class AddSecondaryLoginPage extends Component { * Add a secondary login to a user's account */ submitForm() { - setSecondaryLogin(this.state.login, this.state.password) - .then((response) => { - if (response.jsonCode === 200) { - Navigation.navigate(ROUTES.SETTINGS_PROFILE); - } - }); + User.setSecondaryLoginAndNavigate(this.state.login, this.state.password); } /** From 3e56ee8d300c4325693b81cb3e74c7cc5f167205 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 27 Oct 2021 15:28:26 -1000 Subject: [PATCH 03/13] Fix up validateEmail action --- src/libs/actions/Session.js | 45 ++++++++++++++++++++++++++++++++ src/pages/SetPasswordPage.js | 50 +++++++++++------------------------- 2 files changed, 60 insertions(+), 35 deletions(-) diff --git a/src/libs/actions/Session.js b/src/libs/actions/Session.js index ee32e642fe77..0b195508d9ef 100644 --- a/src/libs/actions/Session.js +++ b/src/libs/actions/Session.js @@ -371,6 +371,50 @@ function clearAccountMessages() { Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''}); } +function changePasswordAndSignIn(responseValidate, password) { + API.ChangePassword({ + authToken: responseValidate.authToken, + password, + }) + .then((responsePassword) => { + if (responsePassword.jsonCode === 200) { + signIn(password); + return; + } + + Onyx.merge(ONYXKEYS.SESSION, {error: 'setPasswordPage.passwordNotSet'}); + }); +} + +/** + * @param {String} accountID + * @param {String} validateCode + * @param {String} password + */ +function validateEmail(accountID, validateCode, password) { + API.ValidateEmail({ + accountID, + validateCode, + }) + .then((responseValidate) => { + if (responseValidate.jsonCode === 200) { + return changePasswordAndSignIn(responseValidate, password); + } + + if (responseValidate.title === CONST.PASSWORD_PAGE.ERROR.ALREADY_VALIDATED) { + // If the email is already validated, set the password using the validate code + setPassword( + password, + validateCode, + accountID, + ); + return; + } + + Onyx.merge(ONYXKEYS.SESSION, {error: 'setPasswordPage.accountNotValidated'}); + }); +} + export { continueSessionFromECom, fetchAccountDetails, @@ -384,4 +428,5 @@ export { clearSignInData, cleanupSession, clearAccountMessages, + validateEmail, }; diff --git a/src/pages/SetPasswordPage.js b/src/pages/SetPasswordPage.js index 8e934e76b178..dd366d86d0dd 100755 --- a/src/pages/SetPasswordPage.js +++ b/src/pages/SetPasswordPage.js @@ -8,7 +8,7 @@ import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import lodashGet from 'lodash/get'; import styles from '../styles/styles'; -import {setPassword, signIn} from '../libs/actions/Session'; +import * as Session from '../libs/actions/Session'; import ONYXKEYS from '../ONYXKEYS'; import Button from '../components/Button'; import SignInPageLayout from './signin/SignInPageLayout'; @@ -16,8 +16,6 @@ import withLocalize, {withLocalizePropTypes} from '../components/withLocalize'; import compose from '../libs/compose'; import NewPasswordForm from './settings/NewPasswordForm'; import Text from '../components/Text'; -import * as API from '../libs/API'; -import CONST from '../CONST'; const propTypes = { /* Onyx Props */ @@ -40,6 +38,12 @@ const propTypes = { password: PropTypes.string, }), + /** Session object */ + session: PropTypes.shape({ + /** An error message to display to the user */ + error: PropTypes.string, + }), + /** The accountID and validateCode are passed via the URL */ route: PropTypes.shape({ // The name of the route @@ -67,6 +71,9 @@ const defaultProps = { route: { params: {}, }, + session: { + error: '', + }, }; class SetPasswordPage extends Component { @@ -78,7 +85,6 @@ class SetPasswordPage extends Component { this.state = { password: '', isFormValid: false, - error: '', }; } @@ -91,40 +97,13 @@ class SetPasswordPage extends Component { if (!this.state.isFormValid) { return; } - API.ValidateEmail({ - accountID, - validateCode, - }).then((responseValidate) => { - if (responseValidate.jsonCode === 200) { - API.ChangePassword({ - authToken: responseValidate.authToken, - password: this.state.password, - }).then((responsePassword) => { - if (responsePassword.jsonCode === 200) { - signIn(this.state.password); - } else { - this.setState({ - error: this.props.translate('setPasswordPage.passwordNotSet'), - }); - } - }); - } else if (responseValidate.title === CONST.PASSWORD_PAGE.ERROR.ALREADY_VALIDATED) { - // If the email is already validated, set the password using the validate code - setPassword( - this.state.password, - lodashGet(this.props.route, 'params.validateCode', ''), - lodashGet(this.props.route, 'params.accountID', ''), - ); - } else { - this.setState({ - error: this.props.translate('setPasswordPage.accountNotValidated'), - }); - } - }); + + Session.validateEmail(accountID, validateCode, this.state.password); } render() { - const error = this.state.error || this.props.account.error; + const sessionError = this.props.translate(this.props.session.error); + const error = sessionError || this.props.account.error; return ( Date: Wed, 27 Oct 2021 15:41:33 -1000 Subject: [PATCH 04/13] Fix RequestCallPage chains --- src/ONYXKEYS.js | 3 +++ src/libs/actions/Inbox.js | 37 ++++++++++++++++++++++++++++++++--- src/libs/actions/Session.js | 11 ++++++++--- src/pages/RequestCallPage.js | 38 +++++++++++++++++++++++------------- src/pages/SetPasswordPage.js | 5 ++++- 5 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/ONYXKEYS.js b/src/ONYXKEYS.js index ece47440dabe..ef0210f57fbc 100755 --- a/src/ONYXKEYS.js +++ b/src/ONYXKEYS.js @@ -140,4 +140,7 @@ export default { // Stores values for the add debit card form ADD_DEBIT_CARD_FORM: 'addDebitCardForm', + + // Stores values for the request call form + REQUEST_CALL_FORM: 'requestCallForm', }; diff --git a/src/libs/actions/Inbox.js b/src/libs/actions/Inbox.js index 4623bb260ed7..af6fe1843837 100644 --- a/src/libs/actions/Inbox.js +++ b/src/libs/actions/Inbox.js @@ -1,13 +1,44 @@ +import Onyx from 'react-native-onyx'; +import CONST from '../../CONST'; +import ONYXKEYS from '../../ONYXKEYS'; import {Inbox_CallUser} from '../API'; +import Growl from '../Growl'; +import {translateLocal} from '../translate'; +import * as Report from './Report'; -function requestInboxCall(taskID, policyID, firstName, lastName, phoneNumber) { - return Inbox_CallUser({ +/** + * @param {Object} params + * @param {String} taskID + * @param {String} policyID + * @param {String} firstName + * @param {String} lastName + * @param {String} phoneNumber + * @param {String} email + */ +function requestInboxCall({ + taskID, policyID, firstName, lastName, phoneNumber, email, +}) { + Inbox_CallUser({ policyID, firstName, lastName, phoneNumber, taskID, - }); + }) + .then((result) => { + Onyx.merge(ONYXKEYS.REQUEST_CALL_FORM, {loading: true}); + if (result.jsonCode === 200) { + Growl.success(translateLocal('requestCallPage.growlMessageOnSave')); + Report.fetchOrCreateChatReport([email, CONST.EMAIL.CONCIERGE], true); + return; + } + + // Phone number validation is handled by the API + Growl.error(result.message, 3000); + }) + .finally(() => { + Onyx.merge(ONYXKEYS.REQUEST_CALL_FORM, {loading: false}); + }); } // eslint-disable-next-line import/prefer-default-export diff --git a/src/libs/actions/Session.js b/src/libs/actions/Session.js index 0b195508d9ef..9ad4516d01b0 100644 --- a/src/libs/actions/Session.js +++ b/src/libs/actions/Session.js @@ -371,9 +371,13 @@ function clearAccountMessages() { Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''}); } -function changePasswordAndSignIn(responseValidate, password) { +/** + * @param {String} authToken + * @param {String} password + */ +function changePasswordAndSignIn(authToken, password) { API.ChangePassword({ - authToken: responseValidate.authToken, + authToken, password, }) .then((responsePassword) => { @@ -398,7 +402,8 @@ function validateEmail(accountID, validateCode, password) { }) .then((responseValidate) => { if (responseValidate.jsonCode === 200) { - return changePasswordAndSignIn(responseValidate, password); + changePasswordAndSignIn(responseValidate.authToken, password); + return; } if (responseValidate.title === CONST.PASSWORD_PAGE.ERROR.ALREADY_VALIDATED) { diff --git a/src/pages/RequestCallPage.js b/src/pages/RequestCallPage.js index 1af5d569ae07..da1ff795564e 100644 --- a/src/pages/RequestCallPage.js +++ b/src/pages/RequestCallPage.js @@ -62,6 +62,16 @@ const propTypes = { taskID: PropTypes.string, }), }).isRequired, + + requestCallForm: PropTypes.shape({ + loading: PropTypes.bool, + }), +}; + +const defaultProps = { + requestCallForm: { + loading: false, + }, }; class RequestCallPage extends Component { @@ -75,7 +85,6 @@ class RequestCallPage extends Component { phoneNumber: this.getPhoneNumber(props.user.loginList) || '', lastNameError: '', phoneNumberError: '', - isLoading: false, }; this.onSubmit = this.onSubmit.bind(this); @@ -96,19 +105,15 @@ class RequestCallPage extends Component { Growl.error(this.props.translate('requestCallPage.growlMessageNoPersonalPolicy'), 3000); return; } - this.setState({isLoading: true}); - requestInboxCall(this.props.route.params.taskID, personalPolicy.id, this.state.firstName, this.state.lastName, this.state.phoneNumber) - .then((result) => { - this.setState({isLoading: false}); - if (result.jsonCode === 200) { - Growl.success(this.props.translate('requestCallPage.growlMessageOnSave')); - fetchOrCreateChatReport([this.props.session.email, CONST.EMAIL.CONCIERGE], true); - return; - } - // Phone number validation is handled by the API - Growl.error(result.message, 3000); - }); + requestInboxCall({ + taskID: this.props.route.params.taskID, + policyID: personalPolicy.id, + firstName: this.state.firstName, + lastName: this.state.lastName, + phoneNumber: this.state.phoneNumber, + email: this.props.session.email, + }); } /** @@ -243,7 +248,7 @@ class RequestCallPage extends Component { onPress={this.onSubmit} style={[styles.w100]} text={this.props.translate('requestCallPage.callMe')} - isLoading={this.state.isLoading} + isLoading={this.props.requestCallForm.loading} /> @@ -253,6 +258,7 @@ class RequestCallPage extends Component { } RequestCallPage.propTypes = propTypes; +RequestCallPage.defaultProps = defaultProps; export default compose( withLocalize, @@ -269,5 +275,9 @@ export default compose( policies: { key: ONYXKEYS.COLLECTION.POLICY, }, + requestCallForm: { + key: ONYXKEYS.REQUEST_CALL_FORM, + initWithStoredValues: false, + }, }), )(RequestCallPage); diff --git a/src/pages/SetPasswordPage.js b/src/pages/SetPasswordPage.js index dd366d86d0dd..8173ffd61bba 100755 --- a/src/pages/SetPasswordPage.js +++ b/src/pages/SetPasswordPage.js @@ -147,6 +147,9 @@ export default compose( withOnyx({ credentials: {key: ONYXKEYS.CREDENTIALS}, account: {key: ONYXKEYS.ACCOUNT}, - session: {key: ONYXKEYS.SESSION}, + session: { + key: ONYXKEYS.SESSION, + initWithStoredValues: false, + }, }), )(SetPasswordPage); From 902177c6232876db026e7fbce8cb407551bdea7f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 27 Oct 2021 19:13:58 -1000 Subject: [PATCH 05/13] Fix up ReportActionsView and add IS_LOADING_REPORT_ACTIONS key --- src/ONYXKEYS.js | 3 ++ src/libs/actions/Report.js | 13 +++++ src/pages/home/report/ReportActionsView.js | 59 +++++++++++----------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/ONYXKEYS.js b/src/ONYXKEYS.js index ef0210f57fbc..05637dbb0eef 100755 --- a/src/ONYXKEYS.js +++ b/src/ONYXKEYS.js @@ -143,4 +143,7 @@ export default { // Stores values for the request call form REQUEST_CALL_FORM: 'requestCallForm', + + // Are report actions loading? + IS_LOADING_REPORT_ACTIONS: 'isLoadingReportActions', }; diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 4d8567ff67c1..37834692d8b7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -909,6 +909,18 @@ function fetchActions(reportID, offset) { }); } +/** + * Get the actions of a report + * + * @param {Number} reportID + * @param {Number} [offset] + */ +function fetchActionsWithLoadingState(reportID, offset) { + Onyx.set(ONYXKEYS.IS_LOADING_REPORT_ACTIONS, true); + fetchActions(reportID, offset) + .finally(() => Onyx.set(ONYXKEYS.IS_LOADING_REPORT_ACTIONS, false)); +} + /** * Get all of our reports * @@ -1416,4 +1428,5 @@ export { syncChatAndIOUReports, navigateToConciergeChat, handleInaccessibleReport, + fetchActionsWithLoadingState, }; diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 7dcc30391857..a32586f9ec64 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -5,18 +5,12 @@ import { AppState, ActivityIndicator, } from 'react-native'; +import {withOnyx} from 'react-native-onyx'; import PropTypes from 'prop-types'; import _ from 'underscore'; import lodashGet from 'lodash/get'; import Text from '../../../components/Text'; -import { - fetchActions, - fetchChatReportsByIDs, - updateLastReadActionID, - setNewMarkerPosition, - subscribeToReportTypingEvents, - unsubscribeFromReportChannel, -} from '../../../libs/actions/Report'; +import * as Report from '../../../libs/actions/Report'; import ReportActionItem from './ReportActionItem'; import styles from '../../../styles/styles'; import reportActionPropTypes from './reportActionPropTypes'; @@ -37,6 +31,7 @@ import PopoverReportActionContextMenu from './ContextMenu/PopoverReportActionCon import variables from '../../../styles/variables'; import MarkerBadge from './MarkerBadge'; import Performance from '../../../libs/Performance'; +import ONYXKEYS from '../../../ONYXKEYS'; const propTypes = { /** The ID of the report actions will be created for */ @@ -68,6 +63,9 @@ const propTypes = { email: PropTypes.string, }), + /** Are we loading more report actions? */ + isLoadingReportActions: PropTypes.bool, + ...windowDimensionsPropTypes, ...withDrawerPropTypes, ...withLocalizePropTypes, @@ -81,6 +79,7 @@ const defaultProps = { }, reportActions: {}, session: {}, + isLoadingReportActions: false, }; class ReportActionsView extends React.Component { @@ -98,7 +97,6 @@ class ReportActionsView extends React.Component { this.didLayout = false; this.state = { - isLoadingMoreChats: false, isMarkerActive: false, localUnreadActionCount: this.props.report.unreadActionCount, }; @@ -121,9 +119,9 @@ class ReportActionsView extends React.Component { // If the reportID is not found then we have either not loaded this chat or the user is unable to access it. // We will attempt to fetch it and redirect if still not accessible. if (!this.props.report.reportID) { - fetchChatReportsByIDs([this.props.reportID], true); + Report.fetchChatReportsByIDs([this.props.reportID], true); } - subscribeToReportTypingEvents(this.props.reportID); + Report.subscribeToReportTypingEvents(this.props.reportID); this.keyboardEvent = Keyboard.addListener('keyboardDidShow', () => { if (ReportActionComposeFocusManager.isFocused()) { this.scrollToListBottom(); @@ -132,11 +130,11 @@ class ReportActionsView extends React.Component { // Only mark as read if the report is open if (!this.props.isDrawerOpen) { - updateLastReadActionID(this.props.reportID); + Report.updateLastReadActionID(this.props.reportID); } this.updateUnreadIndicatorPosition(this.props.report.unreadActionCount); - fetchActions(this.props.reportID); + Report.fetchActions(this.props.reportID); } shouldComponentUpdate(nextProps, nextState) { @@ -151,7 +149,7 @@ class ReportActionsView extends React.Component { return true; } - if (nextState.isLoadingMoreChats !== this.state.isLoadingMoreChats) { + if (nextProps.isLoadingReportActions !== this.props.isLoadingReportActions) { return true; } @@ -198,7 +196,7 @@ class ReportActionsView extends React.Component { // When the last action changes, record the max action // This will make the unread indicator go away if you receive comments in the same chat you're looking at if (shouldRecordMaxAction) { - updateLastReadActionID(this.props.reportID); + Report.updateLastReadActionID(this.props.reportID); } if (lodashGet(lastAction, 'actorEmail', '') !== lodashGet(this.props.session, 'email', '')) { @@ -215,7 +213,7 @@ class ReportActionsView extends React.Component { prevProps.isDrawerOpen !== this.props.isDrawerOpen || prevProps.isSmallScreenWidth !== this.props.isSmallScreenWidth )) { - updateLastReadActionID(this.props.reportID); + Report.updateLastReadActionID(this.props.reportID); this.updateUnreadIndicatorPosition(this.props.report.unreadActionCount); } } @@ -227,7 +225,7 @@ class ReportActionsView extends React.Component { AppState.removeEventListener('change', this.onVisibilityChange); - unsubscribeFromReportChannel(this.props.reportID); + Report.unsubscribeFromReportChannel(this.props.reportID); } /** @@ -236,7 +234,7 @@ class ReportActionsView extends React.Component { onVisibilityChange() { // only mark as read if visible AND report is open. if (Visibility.isVisible() && !this.props.isDrawerOpen) { - updateLastReadActionID(this.props.reportID); + Report.updateLastReadActionID(this.props.reportID); } } @@ -257,7 +255,7 @@ class ReportActionsView extends React.Component { */ loadMoreChats() { // Only fetch more if we are not already fetching so that we don't initiate duplicate requests. - if (this.state.isLoadingMoreChats) { + if (this.props.isLoadingReportActions) { return; } @@ -270,13 +268,10 @@ class ReportActionsView extends React.Component { return; } - this.setState({isLoadingMoreChats: true}, () => { - // Retrieve the next REPORT.ACTIONS.LIMIT sized page of comments, unless we're near the beginning, in which - // case just get everything starting from 0. - const offset = Math.max(minSequenceNumber - CONST.REPORT.ACTIONS.LIMIT, 0); - fetchActions(this.props.reportID, offset) - .then(() => this.setState({isLoadingMoreChats: false})); - }); + // Retrieve the next REPORT.ACTIONS.LIMIT sized page of comments, unless we're near the beginning, in which + // case just get everything starting from 0. + const offset = Math.max(minSequenceNumber - CONST.REPORT.ACTIONS.LIMIT, 0); + Report.fetchActionsWithLoadingState(this.props.reportID, offset); } /** @@ -363,7 +358,7 @@ class ReportActionsView extends React.Component { */ scrollToListBottom() { scrollToBottom(); - updateLastReadActionID(this.props.reportID); + Report.updateLastReadActionID(this.props.reportID); } /** @@ -375,7 +370,7 @@ class ReportActionsView extends React.Component { // We determine the last read action by deducting the number of unread actions from the total number. // Then, we add 1 because we want the New marker displayed over the oldest unread sequence. const oldestUnreadSequenceNumber = unreadActionCount === 0 ? 0 : (this.props.report.maxSequenceNumber - unreadActionCount) + 1; - setNewMarkerPosition(this.props.reportID, oldestUnreadSequenceNumber); + Report.setNewMarkerPosition(this.props.reportID, oldestUnreadSequenceNumber); } @@ -544,7 +539,7 @@ class ReportActionsView extends React.Component { initialNumToRender={this.calculateInitialNumToRender()} onEndReached={this.loadMoreChats} onEndReachedThreshold={0.75} - ListFooterComponent={this.state.isLoadingMoreChats + ListFooterComponent={this.props.isLoadingReportActions ? : null} keyboardShouldPersistTaps="handled" @@ -566,4 +561,10 @@ export default compose( withWindowDimensions, withDrawerState, withLocalize, + withOnyx({ + isLoadingReportActions: { + key: ONYXKEYS.IS_LOADING_REPORT_ACTIONS, + initWithStoredValues: false, + }, + }), )(ReportActionsView); From 677b0eb1e8635de8ec1770e304024d237ec14395 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 27 Oct 2021 19:39:19 -1000 Subject: [PATCH 06/13] Add some context to README about promise handling in React views --- README.md | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index c8b326c9d56d..fdda90955400 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,7 @@ Our React Native Android app now uses the `Hermes` JS engine which requires your --- -# Structure of the app -These are the main pieces of the application. +# App structure and conventions ## Onyx This is a persistent storage solution wrapped in a Pub/Sub library. In general that means: @@ -153,7 +152,35 @@ This layer is solely responsible for: - Reflecting exactly the data that is in persistent storage by using `withOnyx()` to bind to Onyx data. - Taking user input and passing it to an action -**Note:** As a convention, the UI layer should never interact with device storage directly or call `Onyx.set()` or `Onyx.merge()`. Use an action! For example, check out this action that is signing in the user [here](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/pages/signin/PasswordForm.js#L78-L78). That action will then call `Onyx.merge()` to [set default data and a loading state, then make an API request, and set the response with another `Onyx.merge()`](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/libs/actions/Session.js#L228-L247). Keeping our `Onyx.merge()` out of the view layer and in actions helps organize things as all interactions with device storage and API handling happen in the same place. +As a convention, the UI layer should never interact with device storage directly or call `Onyx.set()` or `Onyx.merge()`. Use an action! For example, check out this action that is signing in the user [here](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/pages/signin/PasswordForm.js#L78-L78). That action will then call `Onyx.merge()` to [set default data and a loading state, then make an API request, and set the response with another `Onyx.merge()`](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/libs/actions/Session.js#L228-L247). Keeping our `Onyx.merge()` out of the view layer and in actions helps organize things as all interactions with device storage and API handling happen in the same place. + +In addition, actions that are called from inside views should not ever use the `.then()` method to set loading/error states, navigate or do any additional data processing. All of this stuff should ideally go into `Onyx` and be fed back to the component via `withOnyx()`. Design your actions so they clearly describe what they will do and encapsulate all their logic in that action. + +```javascript +// Bad +fetchData() { + this.setState({loading: true}); + fetchSomeData() + .then((response) => { + if (result.jsonCode !== 200) { + this.setState({error: response.message}); + return; + } + + Navigation.navigate(ROUTES.SOMEWHERE); + }) + .finally(() => { + this.setState({loading: false}); + }); +} + +// Good +fetchData() { + fetchDataAndNavigateSomewhere(); +} +``` + +Within our `fetchDataAndNavigateSomewhere()` action we will set our loading and error states and navigation logic. ## Directory structure Almost all the code is located in the `src` folder, inside it there's some organization, we chose to name directories that are @@ -219,9 +246,9 @@ export default withOnyx({ ``` ## Things to know or brush up on before jumping into the code -1. The major difference between React-Native and React are the [components](https://reactnative.dev/docs/components-and-apis) that are used in the `render()` method. Everything else is exactly the same. If you learn React, you've already learned 98% of React-Native. -1. The application uses [React-Router](https://reactrouter.com/native/guides/quick-start) for navigating between parts of the app. -1. [Higher Order Components](https://reactjs.org/docs/higher-order-components.html) are used to connect React components to persistent storage via Onyx. +1. The major difference between React Native and React are the [components](https://reactnative.dev/docs/components-and-apis) that are used in the `render()` method. Everything else is exactly the same. If you learn React, you've already learned 98% of React-Native. +1. The application uses [`react-navigation`](https://reactnavigation.org/) for navigating between parts of the app. +1. [Higher Order Components](https://reactjs.org/docs/higher-order-components.html) are used to connect React components to persistent storage via [`react-native-onyx`](https://github.com/Expensify/react-native-onyx). ---- @@ -252,7 +279,7 @@ This application is built with the following principles. - In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`. 1. **Actions manage Onyx Data** - When data needs to be written to or read from the server, this is done through Actions only. - - Public action methods should never return anything (not data or a promise). This is done to ensure that action methods can be called in parallel with no dependency on other methods (see discussion above). + - Public action methods should ideally never return anything (not data or a promise). This is done to ensure that action methods can be called in parallel with no dependency on other methods (see discussion above). - Actions should favor using `Onyx.merge()` over `Onyx.set()` so that other values in an object aren't completely overwritten. - Views should not call `Onyx.merge()` or `Onyx.set()` directly and should call an action instead. - In general, the operations that happen inside an action should be done in parallel and not in sequence (eg. don't use the promise of one Onyx method to trigger a second Onyx method). Onyx is built so that every operation is done in parallel and it doesn't matter what order they finish in. XHRs on the other hand need to be handled in sequence with promise chains in order to access and act upon the response. From 01488960295010c8cad49a9b574b38ff472c126d Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 29 Oct 2021 12:39:19 -1000 Subject: [PATCH 07/13] remove debugger --- src/libs/actions/Inbox.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Inbox.js b/src/libs/actions/Inbox.js index af6fe1843837..3fb1e3b1f712 100644 --- a/src/libs/actions/Inbox.js +++ b/src/libs/actions/Inbox.js @@ -18,6 +18,7 @@ import * as Report from './Report'; function requestInboxCall({ taskID, policyID, firstName, lastName, phoneNumber, email, }) { + Onyx.merge(ONYXKEYS.REQUEST_CALL_FORM, {loading: true}); Inbox_CallUser({ policyID, firstName, @@ -26,7 +27,6 @@ function requestInboxCall({ taskID, }) .then((result) => { - Onyx.merge(ONYXKEYS.REQUEST_CALL_FORM, {loading: true}); if (result.jsonCode === 200) { Growl.success(translateLocal('requestCallPage.growlMessageOnSave')); Report.fetchOrCreateChatReport([email, CONST.EMAIL.CONCIERGE], true); From eca670edc367885450b9bbe59094546db833c2bf Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 29 Oct 2021 13:12:20 -1000 Subject: [PATCH 08/13] loading for account --- src/libs/actions/Session.js | 10 +++------- src/pages/SetPasswordPage.js | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/Session.js b/src/libs/actions/Session.js index 9ad4516d01b0..46350db67d6d 100644 --- a/src/libs/actions/Session.js +++ b/src/libs/actions/Session.js @@ -293,8 +293,6 @@ function resetPassword() { * @param {String} accountID */ function setPassword(password, validateCode, accountID) { - Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); - API.SetPassword({ password, validateCode, @@ -313,9 +311,6 @@ function setPassword(password, validateCode, accountID) { if (response.title === CONST.PASSWORD_PAGE.ERROR.VALIDATE_CODE_FAILED) { Onyx.merge(ONYXKEYS.ACCOUNT, {error: translateLocal('setPasswordPage.accountNotValidated')}); } - }) - .finally(() => { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); }); } @@ -396,14 +391,15 @@ function changePasswordAndSignIn(authToken, password) { * @param {String} password */ function validateEmail(accountID, validateCode, password) { + Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); + API.ValidateEmail({ accountID, validateCode, }) .then((responseValidate) => { if (responseValidate.jsonCode === 200) { - changePasswordAndSignIn(responseValidate.authToken, password); - return; + return changePasswordAndSignIn(responseValidate.authToken, password); } if (responseValidate.title === CONST.PASSWORD_PAGE.ERROR.ALREADY_VALIDATED) { diff --git a/src/pages/SetPasswordPage.js b/src/pages/SetPasswordPage.js index 8173ffd61bba..b4b29f17bb92 100755 --- a/src/pages/SetPasswordPage.js +++ b/src/pages/SetPasswordPage.js @@ -102,7 +102,7 @@ class SetPasswordPage extends Component { } render() { - const sessionError = this.props.translate(this.props.session.error); + const sessionError = this.props.session.error && this.props.translate(this.props.session.error); const error = sessionError || this.props.account.error; return ( From 3ac982db0e11e5df8d5c5302da7a71a6d6501f23 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 29 Oct 2021 13:13:41 -1000 Subject: [PATCH 09/13] make less changes --- src/libs/actions/Session.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Session.js b/src/libs/actions/Session.js index 46350db67d6d..9ad4516d01b0 100644 --- a/src/libs/actions/Session.js +++ b/src/libs/actions/Session.js @@ -293,6 +293,8 @@ function resetPassword() { * @param {String} accountID */ function setPassword(password, validateCode, accountID) { + Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, loading: true}); + API.SetPassword({ password, validateCode, @@ -311,6 +313,9 @@ function setPassword(password, validateCode, accountID) { if (response.title === CONST.PASSWORD_PAGE.ERROR.VALIDATE_CODE_FAILED) { Onyx.merge(ONYXKEYS.ACCOUNT, {error: translateLocal('setPasswordPage.accountNotValidated')}); } + }) + .finally(() => { + Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); }); } @@ -391,15 +396,14 @@ function changePasswordAndSignIn(authToken, password) { * @param {String} password */ function validateEmail(accountID, validateCode, password) { - Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); - API.ValidateEmail({ accountID, validateCode, }) .then((responseValidate) => { if (responseValidate.jsonCode === 200) { - return changePasswordAndSignIn(responseValidate.authToken, password); + changePasswordAndSignIn(responseValidate.authToken, password); + return; } if (responseValidate.title === CONST.PASSWORD_PAGE.ERROR.ALREADY_VALIDATED) { From 69eec26e946c4af777930c2ec43d7dcd0c27ec5e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 29 Oct 2021 14:19:29 -1000 Subject: [PATCH 10/13] Hmm React Native is sorta hard. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fdda90955400..8e7ece9db6f7 100644 --- a/README.md +++ b/README.md @@ -246,7 +246,7 @@ export default withOnyx({ ``` ## Things to know or brush up on before jumping into the code -1. The major difference between React Native and React are the [components](https://reactnative.dev/docs/components-and-apis) that are used in the `render()` method. Everything else is exactly the same. If you learn React, you've already learned 98% of React-Native. +1. The major difference between React Native and React are the [components](https://reactnative.dev/docs/components-and-apis) that are used in the `render()` method. Everything else is exactly the same. Any React skills you have can be applied to React Native. 1. The application uses [`react-navigation`](https://reactnavigation.org/) for navigating between parts of the app. 1. [Higher Order Components](https://reactjs.org/docs/higher-order-components.html) are used to connect React components to persistent storage via [`react-native-onyx`](https://github.com/Expensify/react-native-onyx). From 16cded81c227dc381a14ec923c9d6febde773d97 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 29 Oct 2021 14:22:55 -1000 Subject: [PATCH 11/13] Add missing doc --- src/pages/RequestCallPage.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/RequestCallPage.js b/src/pages/RequestCallPage.js index da1ff795564e..de3f9243828c 100644 --- a/src/pages/RequestCallPage.js +++ b/src/pages/RequestCallPage.js @@ -63,6 +63,7 @@ const propTypes = { }), }).isRequired, + /** Used to track state for the request call form */ requestCallForm: PropTypes.shape({ loading: PropTypes.bool, }), From d858f364449f99dfacbb2a5b4aee3951e5e972f8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 1 Nov 2021 10:13:49 -1000 Subject: [PATCH 12/13] adapt example to the sign in case --- README.md | 50 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 8e7ece9db6f7..61acfb547f83 100644 --- a/README.md +++ b/README.md @@ -152,22 +152,51 @@ This layer is solely responsible for: - Reflecting exactly the data that is in persistent storage by using `withOnyx()` to bind to Onyx data. - Taking user input and passing it to an action -As a convention, the UI layer should never interact with device storage directly or call `Onyx.set()` or `Onyx.merge()`. Use an action! For example, check out this action that is signing in the user [here](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/pages/signin/PasswordForm.js#L78-L78). That action will then call `Onyx.merge()` to [set default data and a loading state, then make an API request, and set the response with another `Onyx.merge()`](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/libs/actions/Session.js#L228-L247). Keeping our `Onyx.merge()` out of the view layer and in actions helps organize things as all interactions with device storage and API handling happen in the same place. +As a convention, the UI layer should never interact with device storage directly or call `Onyx.set()` or `Onyx.merge()`. Use an action! For example, check out this action that is signing in the user [here](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/pages/signin/PasswordForm.js#L78-L78). -In addition, actions that are called from inside views should not ever use the `.then()` method to set loading/error states, navigate or do any additional data processing. All of this stuff should ideally go into `Onyx` and be fed back to the component via `withOnyx()`. Design your actions so they clearly describe what they will do and encapsulate all their logic in that action. +```js +validateAndSubmitForm() { + // validate... + signIn(this.state.password, this.state.twoFactorAuthCode); +} +``` + +That action will then call `Onyx.merge()` to [set default data and a loading state, then make an API request, and set the response with another `Onyx.merge()`](https://github.com/Expensify/App/blob/919c890cc391ad38b670ca1b266c114c8b3c3285/src/libs/actions/Session.js#L228-L247). + +```js +function signIn(password, twoFactorAuthCode) { + Onyx.merge(ONYXKEYS.ACCOUNT, {loading: true}); + API.Authenticate({ + ...defaultParams, + password, + twoFactorAuthCode, + }) + .then((response) => { + Onyx.merge(ONYXKEYS.SESSION, {authToken: response.authToken}); + }) + .catch((error) => { + Onyx.merge(ONYXKEYS.ACCOUNT, {error: error.message}); + }) + .finally(() => { + Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false}); + }); +} +``` + +Keeping our `Onyx.merge()` out of the view layer and in actions helps organize things as all interactions with device storage and API handling happen in the same place. In addition, actions that are called from inside views should not ever use the `.then()` method to set loading/error states, navigate or do any additional data processing. All of this stuff should ideally go into `Onyx` and be fed back to the component via `withOnyx()`. Design your actions so they clearly describe what they will do and encapsulate all their logic in that action. ```javascript // Bad -fetchData() { +validateAndSubmitForm() { + // validate... this.setState({loading: true}); - fetchSomeData() + signIn() .then((response) => { - if (result.jsonCode !== 200) { - this.setState({error: response.message}); + if (result.jsonCode === 200) { return; } - Navigation.navigate(ROUTES.SOMEWHERE); + this.setState({error: response.message}); }) .finally(() => { this.setState({loading: false}); @@ -175,13 +204,12 @@ fetchData() { } // Good -fetchData() { - fetchDataAndNavigateSomewhere(); +validateAndSubmitForm() { + // validate... + signIn(); } ``` -Within our `fetchDataAndNavigateSomewhere()` action we will set our loading and error states and navigation logic. - ## Directory structure Almost all the code is located in the `src` folder, inside it there's some organization, we chose to name directories that are created to house a collection of items in plural form and using camelCase (eg: pages, libs, etc), the main ones we have for now are: From 7509fad6612b67fb37d9c0fa81e3e8dbca1bb942 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 1 Nov 2021 19:19:48 -1000 Subject: [PATCH 13/13] clarify when it is ok to return a promise from an action for now --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 61acfb547f83..7d3a95744c55 100644 --- a/README.md +++ b/README.md @@ -307,7 +307,7 @@ This application is built with the following principles. - In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`. 1. **Actions manage Onyx Data** - When data needs to be written to or read from the server, this is done through Actions only. - - Public action methods should ideally never return anything (not data or a promise). This is done to ensure that action methods can be called in parallel with no dependency on other methods (see discussion above). + - Action methods should only have return values (data or a promise) if they are called by other actions. This is done to encourage that action methods can be called in parallel with no dependency on other methods (see discussion above). - Actions should favor using `Onyx.merge()` over `Onyx.set()` so that other values in an object aren't completely overwritten. - Views should not call `Onyx.merge()` or `Onyx.set()` directly and should call an action instead. - In general, the operations that happen inside an action should be done in parallel and not in sequence (eg. don't use the promise of one Onyx method to trigger a second Onyx method). Onyx is built so that every operation is done in parallel and it doesn't matter what order they finish in. XHRs on the other hand need to be handled in sequence with promise chains in order to access and act upon the response.