[HOLD for payment 2021-11-04] Edit Workspace - Saving an empty name shows infinite loading in button +$250 N6hold bonus

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Navigate to settings
2. Click on a workspace
3. Click on the workspace avatar to edit
4. Delete the name and save

Expected Result:

There should be an error and the button should be enabled so the user can enter and save the new name.

Actual Result:

An error and a infinite loading is displayed in the button.

Workaround:

User has to refresh the page to remove the infinite loading and enable the button.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.96-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1631214975132400

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[parasharrajat]

Proposal

1. We should update the Validation isButtonDisabled so that button is disabled when the name is empty and is not an empty string.

   App/src/pages/workspace/WorkspaceEditorPage.js

   Line 97 in 6cdfd4b

   const isButtonDisabled = policy.isAvatarUploading

const isButtonDisabled = policy.isAvatarUploading
            || (this.state.avatarURL === this.props.policy.avatarURL
            && this.state.name === this.props.policy.name)
            || _.isEmpty(this.state.name.trim());

2. Then we have to propagate the update API error to the main Promise.
   

   App/src/pages/workspace/WorkspaceEditorPage.js

   Line 79 in 6cdfd4b

   update(policyID, {name, avatarURL});

a. we should return a promise from update function and then return that promise in this then callback so that catch block can be triggered when update fails. With respect to this we should also throw the err in the catch block of the update Promise.
b. Or, we can simply reset the loader in the catch block here

App/src/libs/actions/Policy.js

Line 256 in 6cdfd4b

}).catch(() => {

.

            updateLocalPolicyValues(this.props.policy.id, {isPolicyUpdating: false});

I think it is good that there are fewer side-effects so I suggest 2.a.

[MelvinBot]

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[laurenreidexpensify]

Adding to Upwork now.

[johnmlee101]

@parasharrajat your proposal looks good!

[laurenreidexpensify]

https://www.upwork.com/jobs/~013b1b10c80cb7c1b2 will hire @parasharrajat now

[parasharrajat]

There have been recent changes to the Form design style guide. cc @Jag96. I would like to ask the same for this component.

1. Instead of disabling the button, should we enable the button and show the error on blur and submit if the field is required?

[parasharrajat]

@Jag96 Any input? Thanks for looking into the request.

[Jag96]

@parasharrajat let's ensure the button is always enabled and that the error message shows up on submit if the field is required. We can leave out the onBlur for now, per this message.

[parasharrajat]

On it. Thanks. @Jag96

But but...
we have to check where if the name or avatar does not change button remains disabled. How should we handle this?

I think for this case button should be disabled. What do you say?

[kadiealexander]

Hi @parasharrajat, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

[laurenreidexpensify]

Still on Hold

[laurenreidexpensify]

Still on hold

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[laurenreidexpensify]

Still on N6 Hold

[parasharrajat]

@johnmlee101 I think this should not be on n6-hold. Also, I see that the page is already changed but there is a bug in that code which this PR is trying to solve too.

So if you roll the dice here, I will update the PR and ready it for merge.

[laurenreidexpensify]

@johnmlee101 as the N6 hold is lifted can you confirm if Rajat can move forward on the PR?

[laurenreidexpensify]

@johnmlee101 bump

[parasharrajat]

@laurenreidexpensify PR is already ready for review and just need to be reviewed.

[johnmlee101]

Yep! @parasharrajat go ahead!

[johnmlee101]

I'l review now

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.10-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• chg: Workspace Edit Page #5271

If no regressions arise, payment will be issued on 2021-11-04. 🎊

[laurenreidexpensify]

Issuing payment now @parasharrajat

$250 - bug report
$250 - issue deploy
$250 - N6 Hold