Merge pull request #6033 from Expensify/vit-refactoringOfBadSmells

Refactor the WorkspaceSettingsPage
Expensify · Oct 26, 2021 · 1870921 · 1870921
2 parents b75a4ba + a0507a9
commit 1870921
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 42 deletions.
diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js
@@ -278,22 +278,6 @@ function invite(logins, welcomeNote, policyID) {
         });
 }
 
-/**
- * @param {Object} file
- * @returns {Promise}
- */
-function uploadAvatar(file) {
-    return API.User_UploadAvatar({file})
-        .then((response) => {
-            if (response.jsonCode !== 200) {
-                // Let the component handle the issue.
-                throw new Error();
-            }
-
-            return response.s3url;
-        });
-}
-
 /**
  * Sets local values for the policy
  * @param {String} policyID
@@ -311,6 +295,7 @@ function updateLocalPolicyValues(policyID, values) {
  * @param {Boolean} [shouldGrowl]
  */
 function update(policyID, values, shouldGrowl = false) {
+    updateLocalPolicyValues(policyID, {isPolicyUpdating: true});
     API.UpdatePolicy({policyID, value: JSON.stringify(values), lastModified: null})
         .then((policyResponse) => {
             if (policyResponse.jsonCode !== 200) {
@@ -330,6 +315,29 @@ function update(policyID, values, shouldGrowl = false) {
         });
 }
 
+/**
+ * Uploads the avatar image to S3 bucket and updates the policy with new avatarURL
+ *
+ * @param {String} policyID
+ * @param {Object} file
+ */
+function uploadAvatar(policyID, file) {
+    updateLocalPolicyValues(policyID, {isAvatarUploading: true});
+    API.User_UploadAvatar({file})
+        .then((response) => {
+            if (response.jsonCode === 200) {
+                // Update the policy with the new avatarURL as soon as we get it
+                Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL: response.s3url, isAvatarUploading: false});
+                update(policyID, {avatarURL: response.s3url}, true);
+                return;
+            }
+
+            Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false});
+            const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage');
+            Growl.error(errorMessage, 5000);
+        });
+}
+
 /**
  * @param {String} policyID
  * @param {Object} errors
@@ -354,7 +362,6 @@ export {
     create,
     uploadAvatar,
     update,
-    updateLocalPolicyValues,
     setWorkspaceErrors,
     hideWorkspaceAlertMessage,
 };
diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js
@@ -18,7 +18,6 @@ import Icon from '../../components/Icon';
 import {Workspace} from '../../components/Icon/Expensicons';
 import AvatarWithImagePicker from '../../components/AvatarWithImagePicker';
 import defaultTheme from '../../styles/themes/default';
-import Growl from '../../libs/Growl';
 import CONST from '../../CONST';
 import ExpensiPicker from '../../components/ExpensiPicker';
 import {getCurrencyList} from '../../libs/actions/PersonalDetails';
@@ -58,7 +57,6 @@ class WorkspaceSettingsPage extends React.Component {
 
         this.state = {
             name: props.policy.name,
-            avatarURL: props.policy.avatarURL,
             previewAvatarURL: props.policy.avatarURL,
             currency: props.policy.outputCurrency,
         };
@@ -68,17 +66,12 @@ class WorkspaceSettingsPage extends React.Component {
         this.removeAvatar = this.removeAvatar.bind(this);
         this.getCurrencyItems = this.getCurrencyItems.bind(this);
         this.validate = this.validate.bind(this);
-        this.uploadAvatarPromise = Promise.resolve();
     }
 
     componentDidMount() {
         getCurrencyList();
     }
 
-    componentWillUnmount() {
-        Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: false});
-    }
-
     /**
      * @returns {Object[]}
      */
@@ -91,40 +84,31 @@ class WorkspaceSettingsPage extends React.Component {
     }
 
     removeAvatar() {
-        this.setState({previewAvatarURL: '', avatarURL: ''});
+        this.setState({previewAvatarURL: ''});
+        Policy.update(this.props.policy.id, {avatarURL: ''}, true);
     }
 
     /**
      * @param {Object} image
      * @param {String} image.uri
      */
     uploadAvatar(image) {
-        Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: true});
+        if (this.props.policy.isAvatarUploading) {
+            return;
+        }
         this.setState({previewAvatarURL: image.uri});
-
-        // Store the upload avatar promise so we can wait for it to finish before updating the policy
-        this.uploadAvatarPromise = Policy.uploadAvatar(image).then(url => new Promise((resolve) => {
-            this.setState({avatarURL: url}, resolve);
-        })).catch(() => {
-            this.setState({previewAvatarURL: ''});
-            Growl.error(this.props.translate('workspace.editor.avatarUploadFailureMessage'));
-        }).finally(() => Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: false}));
+        Policy.uploadAvatar(this.props.policy.id, image);
     }
 
     submit() {
-        if (this.props.policy.isAvatarUploading || !this.validate()) {
+        if (this.props.policy.isPolicyUpdating || !this.validate()) {
             return;
         }
         const name = this.state.name.trim();
-        const avatarURL = this.state.avatarURL;
         const outputCurrency = this.state.currency;
-        Policy.updateLocalPolicyValues(this.props.policy.id, {name, avatarURL, outputCurrency});
 
-        // Wait for the upload avatar promise to finish before updating the policy
-        this.uploadAvatarPromise.then(() => {
-            Policy.update(this.props.policy.id, {name, avatarURL, outputCurrency});
-        });
-        Growl.success(this.props.translate('workspace.common.growlMessageOnSave'));
+        // Send the API call with new settings values, the avatar has been updated when uploaded
+        Policy.update(this.props.policy.id, {name, outputCurrency}, true);
     }
 
     validate() {