[HOLD for payment 2024-07-24] [$250] Workspace - Bank account personal info isn't accepting characters with umlaut as name

[lanitochka17]

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

---

Version Number: 1.4.85-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in with a new expensifail account
2. Create a workspace
3. Enable Workflows
4. Navigate to workspace settings - Workflows
5. Click on "Connect bank account"
6. Choose the manual option
7. Input "011401533" for Routing Number
8. Input "1111222233331111" for Account Number
9. Input "Ádám" for first name
10. Input "Horváth for last name

Expected Result:

Characters with umlaut should be accepted

Actual Result:

Bank account personal info isn't accepting characters with umlaut as name. Please enter a valid name error appears. These characters are accepted in the Save the world - teacher and principal flows

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6517259_1718718248651.bandicam_2024-06-18_15-34-26-002.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01000955591143b611
• Upwork Job ID: 1805814229750460922
• Last Price Increase: 2024-06-26
• Automatic offers:
• suneox | Reviewer | 102945796
• eucool | Contributor | 102945799

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot]

@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01000955591143b611

[JmillsExpensify]

Opening up to community

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @suneox (External)

[dominictb]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Bank account personal info isn't accepting characters with umlaut as name. Please enter a valid name error appears. These characters are accepted in the Save the world - teacher and principal flows

What is the root cause of that problem?

We don't allow users to enter the accented characters in

App/src/libs/ValidationUtils.ts

Line 352 in 9862fdb

const hasAccentedChars = !!name.match(CONST.REGEX.ACCENT_LATIN_CHARS);

What changes do you think we should make in order to solve the problem?

Remove the above condition

What alternative solutions did you explore? (Optional)

We can update the isValidLegalName based on BE side.

[eucool]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Bank account personal information doesn't accept umlaut characters.

What is the root cause of that problem?

We have a check in place which checks for isValidLegalName, this will not allow umlaut.

This is the RCA.

What changes do you think we should make in order to solve the problem?

We should use isValidPersonName instead here, this will reject special characters and accept umlaut.

so we need to update the following block of code:

App/src/pages/ReimbursementAccount/PersonalInfo/substeps/FullName.tsx

Lines 42 to 49 in 57e5d0e

	if (values.firstName && !ValidationUtils.isValidLegalName(values.firstName)) {
	errors.firstName = translate('bankAccount.error.firstName');
	}
	if (values.lastName && !ValidationUtils.isValidLegalName(values.lastName)) {
	errors.lastName = translate('bankAccount.error.lastName');
	}
	return errors;

At both the places above, we will use isValidPersonName util function:

if (values.firstName && !ValidationUtils. isValidPersonName(values.firstName)) {
                errors.firstName = translate('bankAccount.error.firstName');
            }

            if (values.lastName && !ValidationUtils. isValidPersonName(values.lastName)) {
                errors.lastName = translate('bankAccount.error.lastName');
            }

What alternative solutions did you explore? (Optional)

[suneox]

Thanks for @dominictb & @eucool proposals. The RCA is correct and I have some feedback:

About @dominictb proposal to remove the condition check hasAccentedChars in the function isValidLegalName can works but it will raise regression in other places because that function allows accents/diacritics characters, but APIs from other places do not support accents/diacritics.

Data test:

Việt Nam: Công Phạm
Hungary: Horváth Ádám
France: Crème Brûlée
Spain: José Rodríguez
Portugal: João Silva
Germany: Müller Schön
Turkey: Ayşe Yılmaz

About @eucool solution, it seems like can work if we only update for Bank account personal info screen

We use isValidLegalName at a lot of places where the behaviour to reject umlaut is required so instead of updating that util, we should instead use isValidPersonName:

I don't think we need to update all of them will also raise regression with the data test above

Next step: Waiting @dominictb & @eucool update there proposal

[eucool]

@suneox what are the next steps here I proposed that we should use isValidPersonName in the bank account personal name page instead of the current isValidLegalName , what should I update in my proposal confused a little

I don't think we need to update all of them will also raise regression with the data test above

Yes is agree I just mentioned it for the additional point, the changes in my solution are simple:

App/src/pages/ReimbursementAccount/PersonalInfo/substeps/FullName.tsx

Lines 42 to 49 in 57e5d0e

	if (values.firstName && !ValidationUtils.isValidLegalName(values.firstName)) {
	errors.firstName = translate('bankAccount.error.firstName');
	}
	if (values.lastName && !ValidationUtils.isValidLegalName(values.lastName)) {
	errors.lastName = translate('bankAccount.error.lastName');
	}
	return errors;

At both the places above, we will use isValidPersonName util function:

if (values.firstName && !ValidationUtils. isValidPersonName(values.firstName)) {
                errors.firstName = translate('bankAccount.error.firstName');
            }

            if (values.lastName && !ValidationUtils. isValidPersonName(values.lastName)) {
                errors.lastName = translate('bankAccount.error.lastName');
            }

[suneox]

what should I update in my proposal confused a little

@eucool Ah, You should update your proposal with more detail as mentioned above and update unnecessary content, because the internal reviewer will review your solutions again instead of reading the entire conversation to get the context.

[eucool]

Updated proposal

Updated proposal to remove unwanted content and added some code

[suneox]

@eucool proposal LGTM we can go with this solution.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @eucool 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fixes issue: Bank account personal info isn't accepting characters with umlaut as name #44689

If no regressions arise, payment will be issued on 2024-07-24. 🎊

For reference, here are some details about the assignees on this issue:

• @suneox requires payment automatic offer (Reviewer)
• @eucool requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@suneox] The PR that introduced the bug has been identified. Link to the PR:
• [@suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@suneox] Determine if we should create a regression test for this bug.
• [@suneox] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[suneox]

• [@suneox] The PR that introduced the bug has been identified. Link to the PR: I don't think there's a PR responsible for it, as this is an update validation rule and previous rule also match with BE API
• [@suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• [@suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [@suneox] Determine if we should create a regression test for this bug: N/A

[JmillsExpensify]

Payment summary:

• Contributor: @eucool $250
• Reviewer: @suneox $250

[JmillsExpensify]

All contributors paid via Upwork and checklist complete. Closing this one!