Remove all promise chains in actions called from views

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,63 @@ 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).
+
+```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
+validateAndSubmitForm() {
+    // validate...
+    this.setState({loading: true});
+    signIn()
+        .then((response) => {
+            if (result.jsonCode === 200) {
+                return;
+            }
+
+            this.setState({error: response.message});
+        })
+        .finally(() => {
+            this.setState({loading: false});
+        });
+}
+
+// Good
+validateAndSubmitForm() {
+    // validate...
+    signIn();
+}
+```
 
 ## 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 +274,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. 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).
 
 ----
 
@@ -252,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 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.

src/ONYXKEYS.js

@@ -140,4 +140,10 @@ 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',
+
+    // Are report actions loading?
+    IS_LOADING_REPORT_ACTIONS: 'isLoadingReportActions',
 };

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,
+}) {
+    Onyx.merge(ONYXKEYS.REQUEST_CALL_FORM, {loading: true});
+    Inbox_CallUser({
         policyID,
         firstName,
         lastName,
         phoneNumber,
         taskID,
-    });
+    })
+        .then((result) => {
+            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

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,
 };

src/libs/actions/Session.js

@@ -371,6 +371,55 @@ function clearAccountMessages() {
     Onyx.merge(ONYXKEYS.ACCOUNT, {error: '', success: ''});
 }
 
+/**
+ * @param {String} authToken
+ * @param {String} password
+ */
+function changePasswordAndSignIn(authToken, password) {
+    API.ChangePassword({
+        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) {
+                changePasswordAndSignIn(responseValidate.authToken, password);
+                return;
+            }
+
+            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 +433,5 @@ export {
     clearSignInData,
     cleanupSession,
     clearAccountMessages,
+    validateEmail,
 };

src/libs/actions/User.js

@@ -40,20 +40,21 @@ 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})
         .then((response) => {
             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;
         });
 }
 
@@ -132,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({
@@ -142,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;
     });
 }
 
@@ -299,12 +300,12 @@ function clearUserErrorMessage() {
 }
 
 export {
-    changePassword,
+    changePasswordAndNavigate,
     getBetas,
     getUserDetails,
     resendValidateCode,
     setExpensifyNewsStatus,
-    setSecondaryLogin,
+    setSecondaryLoginAndNavigate,
     validateLogin,
     isBlockedFromConcierge,
     getDomainInfo,