From 6adcacca67c847048a9d67dd12308dc06060d134 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 14:30:37 +0100 Subject: [PATCH 01/24] Refactor the api call --- src/libs/actions/Policy.js | 14 +++++++++++--- src/pages/workspace/WorkspaceSettingsPage.js | 14 +++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 2f9c36eeedb3..fcc5af523207 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -279,10 +279,12 @@ function invite(logins, welcomeNote, policyID) { } /** + * Uploads the image of Avatar to S3 bucket and sets the url locally + * + * @param {String} policyID * @param {Object} file - * @returns {Promise} */ -function uploadAvatar(file) { +function uploadAvatar(poliycID, file) { return API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode !== 200) { @@ -293,7 +295,13 @@ function uploadAvatar(file) { } return response.s3url; - }); + }) + .then(avatarURL => updateLocalPolicyValues(policyID, {avatarURL, isAvatarUploading: false})) + .catch(() => { + updateLocalPolicyValues(policyID, {isAvatarUploading: false}); + const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); + Growl.error(errorMessage, 5000); + }) } /** diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 017a153990dc..09ce4002ba9e 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -96,25 +96,17 @@ class WorkspaceSettingsPage extends React.Component { uploadAvatar(image) { Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: true}); 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(() => { - 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() { 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}); + Policy.updateLocalPolicyValues(this.props.policy.id, {name, 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}); - }); + Policy.update(this.props.policy.id, {name, avatarURL, outputCurrency}); Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); } From 3aaefdeec457d48e80804ccf14aa9db6752da490 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 14:32:28 +0100 Subject: [PATCH 02/24] Remove whitespace --- src/libs/actions/Policy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index fcc5af523207..6b82a65ed112 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -280,11 +280,11 @@ function invite(logins, welcomeNote, policyID) { /** * Uploads the image of Avatar to S3 bucket and sets the url locally - * + * * @param {String} policyID * @param {Object} file */ -function uploadAvatar(poliycID, file) { +function uploadAvatar(policyID, file) { return API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode !== 200) { From 1b6a89cef4fa935e4e69572b3f2c124639fc8100 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 14:32:48 +0100 Subject: [PATCH 03/24] Improve fundtion docs --- src/libs/actions/Policy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 6b82a65ed112..20209eeb53b6 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -279,7 +279,7 @@ function invite(logins, welcomeNote, policyID) { } /** - * Uploads the image of Avatar to S3 bucket and sets the url locally + * Uploads the avatar image to S3 bucket and sets the url locally * * @param {String} policyID * @param {Object} file From 0d21cc08f37d06037697e6f041fce495fd180895 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 14:43:30 +0100 Subject: [PATCH 04/24] Remove the avatarURL from state and use onyx one --- src/pages/workspace/WorkspaceSettingsPage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 09ce4002ba9e..b561c2827879 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -58,7 +58,6 @@ class WorkspaceSettingsPage extends React.Component { this.state = { name: props.policy.name, - avatarURL: props.policy.avatarURL, previewAvatarURL: props.policy.avatarURL, currency: props.policy.outputCurrency, }; @@ -101,10 +100,11 @@ class WorkspaceSettingsPage extends React.Component { submit() { const name = this.state.name.trim(); - const avatarURL = this.state.avatarURL; + const avatarURL = this.props.policy.avatarURL; const outputCurrency = this.state.currency; Policy.updateLocalPolicyValues(this.props.policy.id, {name, outputCurrency}); + console.log("avatarURL: ", avatarURL) // Wait for the upload avatar promise to finish before updating the policy Policy.update(this.props.policy.id, {name, avatarURL, outputCurrency}); Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); From e69d4c17079eb8454e706b03af571c2cfda0a4b9 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 14:52:23 +0100 Subject: [PATCH 05/24] Update the remove function to reflect we are now taking the avatarURL from ONYX --- src/pages/workspace/WorkspaceSettingsPage.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index b561c2827879..8d569f58dd79 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -85,7 +85,9 @@ class WorkspaceSettingsPage extends React.Component { } removeAvatar() { - this.setState({previewAvatarURL: '', avatarURL: ''}); + this.setState({previewAvatarURL: ''}); + Policy.updateLocalPolicyValues(this.props.policy.id, {avatarURL: ''}); + Policy.update(this.props.policy.id, {avatarURL: ''}); } /** From 5e77872cdcf8398d5e7d991db3a84719330cb16b Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:31:56 +0100 Subject: [PATCH 06/24] Refactor the updateAvatar API call to update the Policy on success --- src/libs/actions/Policy.js | 5 ++++- src/pages/workspace/WorkspaceSettingsPage.js | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 20209eeb53b6..ed83b57e1899 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -296,7 +296,10 @@ function uploadAvatar(policyID, file) { return response.s3url; }) - .then(avatarURL => updateLocalPolicyValues(policyID, {avatarURL, isAvatarUploading: false})) + .then((avatarURL) => { + updateLocalPolicyValues(policyID, {avatarURL, isAvatarUploading: false}) + update(policyID, {avatarURL}); + }) .catch(() => { updateLocalPolicyValues(policyID, {isAvatarUploading: false}); const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 8d569f58dd79..f17c0e63bef5 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -102,13 +102,11 @@ class WorkspaceSettingsPage extends React.Component { submit() { const name = this.state.name.trim(); - const avatarURL = this.props.policy.avatarURL; const outputCurrency = this.state.currency; Policy.updateLocalPolicyValues(this.props.policy.id, {name, outputCurrency}); - console.log("avatarURL: ", avatarURL) // Wait for the upload avatar promise to finish before updating the policy - Policy.update(this.props.policy.id, {name, avatarURL, outputCurrency}); + Policy.update(this.props.policy.id, {name, outputCurrency}); Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); } From ae3df644425f2817b9e3576ffa96a6f974b03c2a Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:35:19 +0100 Subject: [PATCH 07/24] Add explanatory comment to the updateAvatar action --- src/libs/actions/Policy.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index ed83b57e1899..3fb96a53add6 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -297,6 +297,7 @@ function uploadAvatar(policyID, file) { return response.s3url; }) .then((avatarURL) => { + // Update the policy with the new avatarURL as soon as we get it updateLocalPolicyValues(policyID, {avatarURL, isAvatarUploading: false}) update(policyID, {avatarURL}); }) From 73becf0cc520506bacd369f73a1de7bbd8e83135 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:35:32 +0100 Subject: [PATCH 08/24] Remove unused promise --- src/pages/workspace/WorkspaceSettingsPage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index f17c0e63bef5..66f5c3ce1895 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -66,7 +66,6 @@ class WorkspaceSettingsPage extends React.Component { this.uploadAvatar = this.uploadAvatar.bind(this); this.removeAvatar = this.removeAvatar.bind(this); this.getCurrencyItems = this.getCurrencyItems.bind(this); - this.uploadAvatarPromise = Promise.resolve(); } componentDidMount() { From 5647b80d86aa398d08844193213fe0ec60c1f697 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:38:05 +0100 Subject: [PATCH 09/24] Improve some comments --- src/libs/actions/Policy.js | 2 +- src/pages/workspace/WorkspaceSettingsPage.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 3fb96a53add6..22a5d0840f5c 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -279,7 +279,7 @@ function invite(logins, welcomeNote, policyID) { } /** - * Uploads the avatar image to S3 bucket and sets the url locally + * Uploads the avatar image to S3 bucket and updates the policy with new avatarURL * * @param {String} policyID * @param {Object} file diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 66f5c3ce1895..b7c213c017e0 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -94,8 +94,8 @@ class WorkspaceSettingsPage extends React.Component { * @param {String} image.uri */ uploadAvatar(image) { - Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: true}); this.setState({previewAvatarURL: image.uri}); + Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: true}); Policy.uploadAvatar(this.props.policy.id, image); } @@ -104,7 +104,7 @@ class WorkspaceSettingsPage extends React.Component { const outputCurrency = this.state.currency; Policy.updateLocalPolicyValues(this.props.policy.id, {name, outputCurrency}); - // Wait for the upload avatar promise to finish before updating the policy + // Send the API call with new settings values, the avatar has been updated when uploaded Policy.update(this.props.policy.id, {name, outputCurrency}); Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); } From f078b767d21b1b89d55f9dd6a3476195af7308b4 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:42:19 +0100 Subject: [PATCH 10/24] Use onyx.merge in the policy actions file --- src/libs/actions/Policy.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 22a5d0840f5c..f49770c9e158 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -297,12 +297,12 @@ function uploadAvatar(policyID, file) { return response.s3url; }) .then((avatarURL) => { - // Update the policy with the new avatarURL as soon as we get it - updateLocalPolicyValues(policyID, {avatarURL, isAvatarUploading: false}) + // Update the policy with the new avatarURL as soon as we get it + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); update(policyID, {avatarURL}); }) .catch(() => { - updateLocalPolicyValues(policyID, {isAvatarUploading: false}); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); Growl.error(errorMessage, 5000); }) From aa20ef1074e01848cc984c23a3e604ba69a835e8 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 18:49:05 +0100 Subject: [PATCH 11/24] Move the updateAvatar function after update function to satisfy linter --- src/libs/actions/Policy.js | 60 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index f49770c9e158..c202063f56c1 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -278,6 +278,36 @@ function invite(logins, welcomeNote, policyID) { }); } +/** + * Sets the name of the policy + * + * @param {String} policyID + * @param {Object} values + * @param {Boolean} [shouldGrowl] + */ +function update(policyID, values, shouldGrowl = false) { + API.UpdatePolicy({policyID, value: JSON.stringify(values), lastModified: null}) + .then((policyResponse) => { + if (policyResponse.jsonCode !== 200) { + // Show the user feedback + const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); + Growl.error(errorMessage, 5000); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); + return; + } + + const updatedValues = {...values, ...{isPolicyUpdating: false}}; + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, updatedValues); + if (shouldGrowl) { + Growl.show(translateLocal('workspace.common.growlMessageOnSave'), CONST.GROWL.SUCCESS, 3000); + } + }).catch(() => { + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); + const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); + Growl.error(errorMessage, 5000); + }); +} + /** * Uploads the avatar image to S3 bucket and updates the policy with new avatarURL * @@ -305,36 +335,6 @@ function uploadAvatar(policyID, file) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); Growl.error(errorMessage, 5000); - }) -} - -/** - * Sets the name of the policy - * - * @param {String} policyID - * @param {Object} values - * @param {Boolean} [shouldGrowl] - */ -function update(policyID, values, shouldGrowl = false) { - API.UpdatePolicy({policyID, value: JSON.stringify(values), lastModified: null}) - .then((policyResponse) => { - if (policyResponse.jsonCode !== 200) { - // Show the user feedback - const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); - Growl.error(errorMessage, 5000); - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); - return; - } - - const updatedValues = {...values, ...{isPolicyUpdating: false}}; - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, updatedValues); - if (shouldGrowl) { - Growl.show(translateLocal('workspace.common.growlMessageOnSave'), CONST.GROWL.SUCCESS, 3000); - } - }).catch(() => { - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); - const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); - Growl.error(errorMessage, 5000); }); } From 5b7e995039f2cd5295ae58bb5c2d1e3ed292dbd3 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 19:14:34 +0100 Subject: [PATCH 12/24] Remove the return from updateAvatar funtion and throw error if upload failed --- src/libs/actions/Policy.js | 44 ++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index c202063f56c1..63ade3f199f8 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -287,25 +287,25 @@ function invite(logins, welcomeNote, policyID) { */ function update(policyID, values, shouldGrowl = false) { API.UpdatePolicy({policyID, value: JSON.stringify(values), lastModified: null}) - .then((policyResponse) => { - if (policyResponse.jsonCode !== 200) { - // Show the user feedback + .then((policyResponse) => { + if (policyResponse.jsonCode !== 200) { + // Show the user feedback + const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); + Growl.error(errorMessage, 5000); + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); + return; + } + + const updatedValues = {...values, ...{isPolicyUpdating: false}}; + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, updatedValues); + if (shouldGrowl) { + Growl.show(translateLocal('workspace.common.growlMessageOnSave'), CONST.GROWL.SUCCESS, 3000); + } + }).catch(() => { + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); Growl.error(errorMessage, 5000); - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); - return; - } - - const updatedValues = {...values, ...{isPolicyUpdating: false}}; - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, updatedValues); - if (shouldGrowl) { - Growl.show(translateLocal('workspace.common.growlMessageOnSave'), CONST.GROWL.SUCCESS, 3000); - } - }).catch(() => { - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isPolicyUpdating: false}); - const errorMessage = translateLocal('workspace.editor.genericFailureMessage'); - Growl.error(errorMessage, 5000); - }); + }); } /** @@ -315,13 +315,12 @@ function update(policyID, values, shouldGrowl = false) { * @param {Object} file */ function uploadAvatar(policyID, file) { - return API.User_UploadAvatar({file}) + API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode !== 200) { // Show the user feedback const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); - Growl.error(errorMessage, 5000); - return; + throw new Error(`${errorMessage}`); } return response.s3url; @@ -331,10 +330,9 @@ function uploadAvatar(policyID, file) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); update(policyID, {avatarURL}); }) - .catch(() => { + .catch((error) => { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); - const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); - Growl.error(errorMessage, 5000); + Growl.error(error, 5000); }); } From 9e5683797486ffc37201b008faa25ac031a25303 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 19:27:30 +0100 Subject: [PATCH 13/24] Add growl success message on avatar upload --- src/libs/actions/Policy.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 63ade3f199f8..123a1cd5619d 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -329,6 +329,7 @@ function uploadAvatar(policyID, file) { // Update the policy with the new avatarURL as soon as we get it Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); update(policyID, {avatarURL}); + Growl.success(translateLocal('workspace.common.growlMessageOnSave')); }) .catch((error) => { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); From 79c9f9f28b4b05821231eb3b1654d1d11eb882dd Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 19:53:49 +0100 Subject: [PATCH 14/24] Use shouldGrowl parameter of update policy function --- src/libs/actions/Policy.js | 3 +-- src/pages/workspace/WorkspaceSettingsPage.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 123a1cd5619d..ad3a6893c567 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -328,8 +328,7 @@ function uploadAvatar(policyID, file) { .then((avatarURL) => { // Update the policy with the new avatarURL as soon as we get it Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); - update(policyID, {avatarURL}); - Growl.success(translateLocal('workspace.common.growlMessageOnSave')); + update(policyID, {avatarURL}, true); }) .catch((error) => { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index b7c213c017e0..7efb46730c2b 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -105,7 +105,7 @@ class WorkspaceSettingsPage extends React.Component { Policy.updateLocalPolicyValues(this.props.policy.id, {name, outputCurrency}); // Send the API call with new settings values, the avatar has been updated when uploaded - Policy.update(this.props.policy.id, {name, outputCurrency}); + Policy.update(this.props.policy.id, {name, outputCurrency}, true); Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); } From 4072f2919c729a55c2f27d35ad7f7e4e8dbb3af1 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 19:54:21 +0100 Subject: [PATCH 15/24] Remove unnecessary growl call --- src/pages/workspace/WorkspaceSettingsPage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 7efb46730c2b..ece98164f89a 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -106,7 +106,6 @@ class WorkspaceSettingsPage extends React.Component { // Send the API call with new settings values, the avatar has been updated when uploaded Policy.update(this.props.policy.id, {name, outputCurrency}, true); - Growl.success(this.props.translate('workspace.common.growlMessageOnSave')); } render() { From 24d2dc710d4d9bcc447a94df797c5c37f4f54039 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 19:56:59 +0100 Subject: [PATCH 16/24] Use shouldGrowl parameter of when removing the avatar too --- src/pages/workspace/WorkspaceSettingsPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index ece98164f89a..28b982612155 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -86,7 +86,7 @@ class WorkspaceSettingsPage extends React.Component { removeAvatar() { this.setState({previewAvatarURL: ''}); Policy.updateLocalPolicyValues(this.props.policy.id, {avatarURL: ''}); - Policy.update(this.props.policy.id, {avatarURL: ''}); + Policy.update(this.props.policy.id, {avatarURL: ''}, true); } /** From 0ef12a9b16ef0c68addd9d02dde6751e63976c78 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 20:03:22 +0100 Subject: [PATCH 17/24] Dont pass specific error message in updateAvatar function --- src/libs/actions/Policy.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index ad3a6893c567..e85b5b550864 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -318,9 +318,8 @@ function uploadAvatar(policyID, file) { API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode !== 200) { - // Show the user feedback - const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); - throw new Error(`${errorMessage}`); + // Show the user error feedback + throw new Error(); } return response.s3url; @@ -330,9 +329,10 @@ function uploadAvatar(policyID, file) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); update(policyID, {avatarURL}, true); }) - .catch((error) => { + .catch(() => { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); - Growl.error(error, 5000); + const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); + Growl.error(errorMessage, 5000); }); } From 3d83845ceac289a6d4ea56a1f50dd02d0ae4dd45 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Mon, 25 Oct 2021 20:03:36 +0100 Subject: [PATCH 18/24] Remove growl --- src/pages/workspace/WorkspaceSettingsPage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 28b982612155..db1a93e4aaf6 100644 --- 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'; From 184ed36dbb53d1f772abfb7a8c7d91825d504381 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 12:37:38 +0100 Subject: [PATCH 19/24] Simplofy the updateAvatar funtion --- src/libs/actions/Policy.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 5f236b4ecd30..7a16d324a2c4 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -314,19 +314,13 @@ function update(policyID, values, shouldGrowl = false) { function uploadAvatar(policyID, file) { API.User_UploadAvatar({file}) .then((response) => { - if (response.jsonCode !== 200) { - // Show the user error feedback - throw new Error(); + 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; } - return response.s3url; - }) - .then((avatarURL) => { - // Update the policy with the new avatarURL as soon as we get it - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {avatarURL, isAvatarUploading: false}); - update(policyID, {avatarURL}, true); - }) - .catch(() => { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {isAvatarUploading: false}); const errorMessage = translateLocal('workspace.editor.avatarUploadFailureMessage'); Growl.error(errorMessage, 5000); From 0f0d5dc978a42c59c846397eaffae1b4d392f05a Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 12:54:40 +0100 Subject: [PATCH 20/24] Refactor to have updateLocalPolicyValues as internal function --- src/libs/actions/Policy.js | 1 + src/pages/workspace/WorkspaceSettingsPage.js | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 7a16d324a2c4..488792210a58 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -312,6 +312,7 @@ function update(policyID, values, shouldGrowl = false) { * @param {Object} file */ function uploadAvatar(policyID, file) { + updateLocalPolicyValues(policyID, {isPolicyUpdating: true}); API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode === 200) { diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index 02477160b196..f28a36451780 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -89,7 +89,6 @@ class WorkspaceSettingsPage extends React.Component { removeAvatar() { this.setState({previewAvatarURL: ''}); - Policy.updateLocalPolicyValues(this.props.policy.id, {avatarURL: ''}); Policy.update(this.props.policy.id, {avatarURL: ''}, true); } @@ -99,7 +98,6 @@ class WorkspaceSettingsPage extends React.Component { */ uploadAvatar(image) { this.setState({previewAvatarURL: image.uri}); - Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: true}); Policy.uploadAvatar(this.props.policy.id, image); } @@ -109,7 +107,6 @@ class WorkspaceSettingsPage extends React.Component { } const name = this.state.name.trim(); const outputCurrency = this.state.currency; - Policy.updateLocalPolicyValues(this.props.policy.id, {name, outputCurrency}); // Send the API call with new settings values, the avatar has been updated when uploaded Policy.update(this.props.policy.id, {name, outputCurrency}, true); From 45d259eb9a532433f18a82af1f5cae7e5b611677 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 13:01:20 +0100 Subject: [PATCH 21/24] Do not allow to submit the changes when upload is already happening --- src/libs/actions/Policy.js | 3 ++- src/pages/workspace/WorkspaceSettingsPage.js | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index 488792210a58..d58a2921190b 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -286,6 +286,7 @@ function invite(logins, welcomeNote, policyID) { * @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) { @@ -312,7 +313,7 @@ function update(policyID, values, shouldGrowl = false) { * @param {Object} file */ function uploadAvatar(policyID, file) { - updateLocalPolicyValues(policyID, {isPolicyUpdating: true}); + updateLocalPolicyValues(policyID, {isAvatarUploading: true}); API.User_UploadAvatar({file}) .then((response) => { if (response.jsonCode === 200) { diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js index f28a36451780..3ada85ede357 100644 --- a/src/pages/workspace/WorkspaceSettingsPage.js +++ b/src/pages/workspace/WorkspaceSettingsPage.js @@ -72,10 +72,6 @@ class WorkspaceSettingsPage extends React.Component { getCurrencyList(); } - componentWillUnmount() { - Policy.updateLocalPolicyValues(this.props.policy.id, {isAvatarUploading: false}); - } - /** * @returns {Object[]} */ @@ -97,12 +93,15 @@ class WorkspaceSettingsPage extends React.Component { * @param {String} image.uri */ uploadAvatar(image) { + if (this.props.policy.isAvatarUploading) { + return; + } this.setState({previewAvatarURL: image.uri}); 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(); From 93ce5b01b7ebb1c61d98f0e2fe651b5a7853dfab Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 13:01:49 +0100 Subject: [PATCH 22/24] Do not export updateLocalPolicyValues from the Policy action --- src/libs/actions/Policy.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index d58a2921190b..adb04755f5ed 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -362,7 +362,6 @@ export { create, uploadAvatar, update, - updateLocalPolicyValues, setWorkspaceErrors, hideWorkspaceAlertMessage, }; From f0759cefb341d5fc3f81f1949cde839862bbb246 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 13:04:44 +0100 Subject: [PATCH 23/24] Reorder the functions to satisfy linter --- src/libs/actions/Policy.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index adb04755f5ed..ab66f57f43b5 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -278,6 +278,15 @@ function invite(logins, welcomeNote, policyID) { }); } +/** + * Sets local values for the policy + * @param {String} policyID + * @param {Object} values + */ + function updateLocalPolicyValues(policyID, values) { + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, values); +} + /** * Sets the name of the policy * @@ -329,15 +338,6 @@ function uploadAvatar(policyID, file) { }); } -/** - * Sets local values for the policy - * @param {String} policyID - * @param {Object} values - */ -function updateLocalPolicyValues(policyID, values) { - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, values); -} - /** * @param {String} policyID * @param {Object} errors From a0507a9b707dbfee66f1fba9cb6cd42f0f8758ee Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Tue, 26 Oct 2021 13:08:28 +0100 Subject: [PATCH 24/24] Fix linter --- src/libs/actions/Policy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js index ab66f57f43b5..fa6ff2b74df5 100644 --- a/src/libs/actions/Policy.js +++ b/src/libs/actions/Policy.js @@ -283,7 +283,7 @@ function invite(logins, welcomeNote, policyID) { * @param {String} policyID * @param {Object} values */ - function updateLocalPolicyValues(policyID, values) { +function updateLocalPolicyValues(policyID, values) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, values); }