Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove confirm modals from the VBA flow #5311

Merged
merged 8 commits into from
Sep 17, 2021
Merged

Remove confirm modals from the VBA flow #5311

merged 8 commits into from
Sep 17, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Sep 16, 2021 •
edited
Loading

Details

Fixed Issues

$ Expensify/Expensify#176726

Tests / QA Steps

bank-account/new / Plaid flow

  1. Navigate to /bank-account/new
  2. Select "Log into your bank"
  3. Enter plaid test credentials (Bank: Chase, User: user_good, Pass: pass_good) Note: This will only work on native when on the staging secure server which we can switch on in settings/preferences.
  4. When on this screen verify that you can press "Save & Continue" and the button is not disabled.
  5. Press the button
  6. Verify that a message appears above the button
  7. Verify that the Choose an account field is highlighted in red

2021-09-16_11-13-14

bank-account/new / Add Manual flow

  1. Navigate to /bank-account/new
  2. Select "Connect manually"
  3. Verify the "Save & Continue" button is not disabled
  4. De-select the "I accept" checkbox so it is not checked
  5. Press the "Save & Continue" button
  6. Verify that a message appears above the button
  7. Verify that the checkbox is outlined in red border
  8. Enter plaid test account info (Routing: 011401533, Account: 1111222233331111) Note: This will only work on native when on the staging secure server which we can switch on in settings/preferences.
  9. Check the checkbox
  10. Verify the red border is cleared
  11. Press "Save & Continue"
  12. Verify we advance to the next step

2021-09-16_11-14-42

bank-account/company

  1. Verify Save & Continue button is not disabled
  2. Press the button
  3. Verify all fields are highlighted in red (Note: Not all fields will have messages under them).
  4. Enter information into all the fields
  5. Verify the errors are cleared from all fields
  6. Press "Save & Continue"

2021-09-16_11-19-15

bank-account/personal-information

  1. Verify the "Save & Continue" button is not disabled
  2. Press the button
  3. Verify the message appears above the submit button
  4. Press the "fix the errors" link and verify the view scrolls to the top
  5. Verify the Checkbox is highlighted and a message displayed under it.
  6. Check the checkbox and verify the error clears
  7. Sumbit the form again and verify that other missing fields are highlighted
  8. Fill out all fields except the "State" and "City" fields
  9. Submit the form
  10. Verify error appears above subimit button + State selector is highlighted + text appears below the field
  11. Select a state and verify the error clears
  12. Submit again
  13. Verify error text appears above the button + error text appears for the City field
  14. Enter a city and verify the error clears
  15. Submit again

2021-09-16_11-18-43

bank-account/contract

  1. Verify the "Save & Continue" button is not disabled
  2. Check "I own more than 25% of..."
  3. Press the button
  4. Verify error appears above submit button + terms and conditions checkbox is highlighted + message displayed under
  5. Check the box and verify the error clears
  6. Submit again
  7. Verify the "certify true and accurate" error appears + checkbox is highlighted.
  8. Check the box and verify the error clears

2021-09-16_11-19-33

bank-account/validate

  1. Create a new account on a non public domain
  2. Create a workspace and navigate to it
  3. Press Get Started button
  4. Move through the entire VBA flow to create an account in a Pending state that needs the 3 validation amounts
  5. Enter the amounts wrong
  6. Verify that a error appears above submit button instructing the user of the incorrect amounts
  7. Enter the correct amounts

2021-09-16_11-19-52

2021-09-16_11-31-53

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

2021-09-16_12-54-33
2021-09-16_12-54-49
2021-09-16_12-55-11
2021-09-16_12-56-10
![2021-09-16_12-56-45](https://user-images.githubusercontent.com/32969087/133696007-cb8b1cdf-3786-439b-99f9-a896001
2021-09-16_12-56-59
680f2.png)
2021-09-16_12-57-18

Android

2021-09-16_13-33-53
2021-09-16_13-33-33
2021-09-16_13-32-20
2021-09-16_13-25-43
2021-09-16_13-25-14
2021-09-16_13-24-53

@marcaaron marcaaron self-assigned this Sep 16, 2021
@marcaaron
Copy link
Contributor Author

This is testing well for me on iOS so I'm going to take it off draft.

@marcaaron marcaaron marked this pull request as ready for review September 16, 2021 23:04
@marcaaron marcaaron requested a review from a team as a code owner September 16, 2021 23:04
@MelvinBot MelvinBot requested review from AndrewGable and removed request for a team September 16, 2021 23:05
ctkochan22
ctkochan22 previously approved these changes Sep 17, 2021
View reviewed changes
aldo-expensify
aldo-expensify previously approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on web, code looks good to me!

@luacmartins
Copy link
Contributor

luacmartins commented Sep 17, 2021 •
edited
Loading

Haven't looked at the code yet, but ran some tests.

I had issues with the personal-information step on web. Fields displayed errors seemingly out of order. Is this expected?

web.mov

Also in the contract step, when pressing the Save & continue button without any boxes checked, I only got one error at a time instead of errors for both required checkboxes in one go, like in previous steps.

@marcaaron
Copy link
Contributor Author

I had issues with the personal-information step on web. Fields displayed errors seemingly out of order. Is this expected?

Yeah this is expected for now. We haven't yet updated these steps so they will show multiple errors at once.

Also in the contract step, when pressing the Save & continue button without any boxes checked, I only got one error at a time instead of errors for both required checkboxes in one go, like in previous steps.

Same deal. We still have to fix those screens up, but will do it post launch.

luacmartins
luacmartins reviewed Sep 17, 2021
View reviewed changes
Copy link
Contributor

@luacmartins luacmartins left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let a small comment. Other than that, tests well on all platforms! Great work!

AndrewGable
AndrewGable previously approved these changes Sep 17, 2021
View reviewed changes
@marcaaron
Copy link
Contributor Author

Thanks for catching that one @luacmartins. Updated.

luacmartins
luacmartins previously approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marcaaron
Copy link
Contributor Author

Fixed conflicts

@AndrewGable AndrewGable merged commit 6bc010b into main Sep 17, 2021
@AndrewGable AndrewGable deleted the marcaaron-removeConfirmModals branch September 17, 2021 18:02
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify OSBotify mentioned this pull request Sep 20, 2021
Closed
31 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @AndrewGable in version: 1.0.99-5 🚀

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

@isagoico
Copy link

We were able to experience the same behaviour in the Personal Information step here #5311 (comment). Marking the PR as pass since it is expected behaviour.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.0-2 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrewGable AndrewGable AndrewGable approved these changes

@luacmartins luacmartins luacmartins approved these changes

@ctkochan22 ctkochan22 ctkochan22 left review comments

@aldo-expensify aldo-expensify aldo-expensify left review comments

Assignees

@marcaaron marcaaron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@marcaaron @luacmartins @OSBotify @isagoico @AndrewGable @ctkochan22 @aldo-expensify