[HOLD for payment 2022-01-19] When setting a custom pronoun that is over the character limit, the API throws an error but the user isn't informed - Reported by: @anthony-hull

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Navigate to Profile settings
2. On the pronouns dropdown select "Self Select"
3. Enter a really long pronoun >50 characters
4. Press save

Expected Result:

There's an error letting the user know that the pronouns should be under a certain amount of characters.

Actual Result:

Nothing happens, changes are not saved and no error is displayed.

Workaround:

N/A

Platform:

Where is this issue occurring?

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

Version Number: 1.1.16-9

Reproducible in staging?: Yes
Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

New.Expensify.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: @anthony-hull
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1637837622005000

View all open jobs on GitHub

[MelvinBot]

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

[kakajann]

Proposal

1. Add pronounsLength after lastNameLength (EN and ES)
   

   App/src/languages/en.js

   Lines 396 to 401 in d0d1d58

   	personalDetails: {
   	error: {
   	firstNameLength: 'First name shouldn\'t be longer than 50 characters',
   	lastNameLength: 'Last name shouldn\'t be longer than 50 characters',
   	},
   	},

pronounsLength: 'Pronouns shouldn\'t be longer than 50 characters',

2. Create a new condition after this
   

   App/src/libs/actions/PersonalDetails.js

   Lines 253 to 255 in d0d1d58

   	} else if (response.jsonCode === 401) {
   	Growl.error(Localize.translateLocal('personalDetails.error.lastNameLength'), 3000);
   	}

   
   The condition should be:

else if (response.jsonCode === 666) {
    Growl.error(Localize.translateLocal('personalDetails.error.pronounsLength'), 3000);
}

Result

Screen.Recording.2021-11-26.at.5.22.05.AM.mov

[anthony-hull]

Should we even attempt the API call if we know it will fail?
Maybe we should consider doing a check for character length client side and putting a contextual error on the input?
We could also lock the save button until the error state is resolved.

[anthony-hull]

or even better use:
https://reactnative.dev/docs/textinput#maxlength on the input element to stop flickering when user tries to exceed the limit.

[MelvinBot]

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

[kakajann]

Yes we can use maxLength for the input, alongside the API response error handling

Or we can add pronouns validation to this function

App/src/pages/settings/Profile/ProfilePage.js

Lines 161 to 169 in d2eadd4

	validateInputs() {
	const {firstNameError, lastNameError} = PersonalDetails.getFirstAndLastNameErrors(this.state.firstName, this.state.lastName);
	this.setState({
	firstNameError,
	lastNameError,
	});
	return _.isEmpty(firstNameError) && _.isEmpty(lastNameError);
	}

it will be something like this:

const {pronounceError} = PersonalDetails.getPronounsError(this.state.hasSelfSelectedPronouns, this.state.pronouns);

this.setState({
    firstNameError,
    lastNameError,
    pronounceError,
});

[anthony-hull]

Reading into it more I think it's bad practice to limit the text with maxLength, you don't want to interrupt the user mid flow. Let them get it all down and then fix the limit after. Putting it into the validate function and putting the error into the input context like above is probably best.

I'm still not convinced it's worth putting in error handling logic from the API for a state the user shouldn't be able to get to once there it client side validation. You shouldn't be allowed to submit the form if it fails validation.

I'd like to work on this issue. I think your suggested approach is a good one, I would do the same, copying the implementation of the first and last name validation and errors.

[kadiealexander]

Upwork job here: https://www.upwork.com/jobs/~0179b0bb6fe5bcfda3

[MelvinBot]

Triggered auto assignment to @parasharrajat (Exported)

[MelvinBot]

Triggered auto assignment to @Julesssss (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[kadiealexander]

@Julesssss and @parasharrajat, would you guys mind please reviewing @anthony-hull's proposal above? :)

[parasharrajat]

I don't see any formal proposal(exact approach) from @anthony-hull. I liked @kakajann #6492 (comment) and he posted first. I also think we won't need API checks after putting in this code. @anthony-hull If you can think of a better approach then I would like to hear about it as you raised the issue.

🎀 👀 🎀

[anthony-hull]

sure I'm working on this today and tomorrow.
Apologies I had some down time after a vaccine.

[kadiealexander]

No worries, thanks for the update! I hope you're feeling better.

[MelvinBot]

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

[kadiealexander]

Reassigning this as I'm ooo until 10th Jan, thanks @bfitzexpensify!!

[anthony-hull]

have a great break!

[parasharrajat]

@anthony-hull When can we expect the PR for this issue?

[anthony-hull]

I will finish the PR tomorrow. I just finished the first pass of coding the change.
Please could I have a translation of:
Exceeds the max length of ${limit} characters

[iwiznia]

Supera el límite de ${limit} caracteres

[bfitzexpensify]

Progress being made in #6899, Melv

[anthony-hull]

this should be merging today 🤞

[anthony-hull]

has been approved, waiting for it to merge

[botify]

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

• implement character limit checking for custom pronouns #6899

If no regressions arise, payment will be issued on 2022-01-19. 🎊

[bfitzexpensify]

No regressions — @anthony-hull, ended the contract and paid you $500 (for reporting + fix). Thanks!

@parasharrajat, sent you an offer here for C+ review.

[bfitzexpensify]

All wrapped up.