Replace growl errors by modal in VBA flow

[aldo-expensify]

Replacing all growls in the VBA flow by the modal.

The error message coming from the backend is being shown in the ConfirmModal now.

Errors messages associated with inputs where we don't handle highlighting yet, like checkboxes, are shown in the ConfirmModal.

Error modal generic message: 'Please double check any highlighted field and try again'

Fixed Issues

$ #4785

Tests / QA Steps

Company Step: /bank-account/company

1. Navigate to /bank-account/company
2. Fill all inputs, input an invalid 'Zip code' (less than 6 numbers)
3. Click 'Save & continue'
4. Expected results: errors in inputs validated in the front (i.e. zip code, tax id, etc) should be highlighted in the form and a generic message should be shown in a modal.
5. Input a 'Zip Code' of 6 numbers (i.e. 123123) and input an invalid phone number (i.e. 123).
6. Click 'Save & continue'
7. Expected results: The phone number is a field validated in the backend, we will display a specific message in the error modal.

ACH Contract / Beneficial Owners Step: /bank-account/contract

1. Navigate to /bank-account/contract
2. Check I accept the terms and conditions and I certify that the information provided is true and accurate
3. Check Somebody else owns more than 25% of XXX, this will add a form to input a beneficial owner's information. Don't fill any of the beneficial owner data.
4. Click 'Save & continue'
5. Expected result: Highlighted invalid field (frontend validation) with error modal with generic message

Requestor Step: /bank-account/requestor

1. Navigate to /bank-account/requestor
2. Fill all inputs
3. Input a date with invalid format in 'Date of birth', in example: asd
4. Expected result: 'Date of birth' field is highlighted with an error, we get the error modal with the generic message (this validation happened in the frontend).
5. Input a date rejected by the backend in the 'Date of birth' , in example: 2020-10-10
6. Expected result: The error modal displays a specific error about the 'Date of birth' being incorrect. No field is highlighted.

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

[marcaaron]

Good start. I left a few ideas for improvements.

[marcaaron]

Changes look perfect! 🏆

I think the one thing left to resolve is that there are some places where the confirm modal title does not entirely make sense. Trying to get a reply to the thread here now

[aldo-expensify]

think the one thing left to resolve is that there are some places where the confirm modal title does not entirely make sense. Trying to get a reply to the thread here now

To handle this, would it be fine to include an extra parameter on the showBankAccountErrorModal like:

function showBankAccountErrorModal(errorModalMessage = null, errorModalTitle = null) {
    Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {isErrorModalVisible: true, errorModalMessage, errorModalTitle});
}

Use it in the display modal like:

<ConfirmModal
  title={lodashGet(this.props, 'reimbursementAccount.errorModalTitle', this.props.translate('companyStep.confirmModalTitle'))}

Then, when we set the error from the server and want a custom title, we do:

showBankAccountErrorModal(error, translateLocal('bankAccount.error.serverErrorModalTitle'));

[marcaaron]

To handle this, would it be fine to include an extra parameter on the showBankAccountErrorModal
Then, when we set the error from the server and want a custom title

Let's wait for feedback first? I haven't seen much discussion about whether these particular errors should have custom titles.

[aldo-expensify]

Let's wait for feedback first? I haven't seen much discussion about whether these particular errors should have custom titles.

Hey @marcaaron, considering we are going to use the Oops header, does the proposed way of handling it above work for you?

[marcaaron]

Hey @marcaaron, considering we are going to use the Oops header, does the proposed way of handling it above work for you?

Ah no, I think we should make all the headers say "Oops" if that's acceptable?
I'm not sure there is a need to get very custom on these headers and it is always best to do the simplest thing.

[aldo-expensify]

Ah no, I think we should make all the headers say "Oops" if that's acceptable?
I'm not sure there is a need to get very custom on these headers and it is always best to do the simplest thing.

Okk, I think I was miss-understanding that we wanted different headers. I will change the generic header of the error modal to "Oops" then!

[aldo-expensify]

Update:

• Replaced error modal header with "Oops" (EN) / "Ups" (ES)
• Moved translation keys related to error modal from companyStep into bankAccount

[marcaaron]

Looks perfect there are just two small issues where unnecessary new lines are added. But other than than great work.

[aldo-expensify]

Update: removed empty lines

[marcaaron]

Looks good! Thanks for the changes!

Approving the code. But we still need to clean up the PR description because there is nothing listed for QA Steps. We need to put something there so that QA knows what to test (and also figure out what QA is able to test or not). Otherwise, they will inevitably ping you (or me since I reviewed) and come asking if there is QA to do or not :)

Also, we should test this on all platforms to make sure the changes are safe. There's checkboxes listed so that we test on every platform. But if there's a good reason not to then we can mention it somewhere.

[marcaaron]

Oh and also update any screenshots since they are now out of date I think.

[MitchExpensify]

Flo is likely offline for the night. Anyone else we can request? @marcaaron

[marcaaron]

@MitchExpensify the comments are for @aldo-expensify 😄

[aldo-expensify]

Looks good! Thanks for the changes!

Approving the code. But we still need to clean up the PR description because there is nothing listed for QA Steps. We need to put something there so that QA knows what to test (and also figure out what QA is able to test or not). Otherwise, they will inevitably ping you (or me since I reviewed) and come asking if there is QA to do or not :)

Also, we should test this on all platforms to make sure the changes are safe. There's checkboxes listed so that we test on every platform. But if there's a good reason not to then we can mention it somewhere.

I'll start cleaning this up now (QA steps, outdated pictures), thanks for the reminder!

[aldo-expensify]

Updated the Tests section and merged it with QA into Tests / QA Steps. Tested locally in Web (mobile/desktop) and Android.

There is one thing in the bank-account/contract step that looks like a bug, but it could also be that it just doesn't work well in the development environment. I'll test it in main branch, it feels unrelated to what we are doing.

In the /bank-account/contract step, add a beneficial owner and put a birth date that is wrong for the backend validation: 2020-10-10, we get not very descriptive message in the error modal. If we try a second time with the same data, it flashes back to the empty form and then success:

Screen.Recording.2021-09-01.at.5.13.40.PM.mov

[marcaaron]

There is one thing in the bank-account/contract step that looks like a bug, but it could also be that it just doesn't work well in the development environment. I'll test it in main branch, it feels unrelated to what we are doing.

Nice! That was just called out by @JmillsExpensify here. I think we'll need to create a follow up issue to look into it.

If anything else pops up that you notice we can check in the free plan channel to see if @MitchExpensify & @kevinksullivan are aware of the issue yet.

[MitchExpensify]

Nice one @aldo-expensify!

I'll create a follow up issue to change the error copy we use when the age entered is under our required 18 years old

[OSBotify]

🚀 Deployed to staging by @marcaaron in version: 1.0.91-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by @roryabraham in version: 1.0.92-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅