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.

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

[CP-stag] Get CompanyAddress field to autofill and show errors #5800

Merged
merged 17 commits into from
Oct 13, 2021

Conversation

Luke9389
Copy link
Contributor

@Luke9389 Luke9389 commented Oct 13, 2021

Details

This PR fixes several things about the Address Picker.

This component uses a 3rd party package to populate the dropdown list for addresses. After quite a lot of wrestling we decided to just make AddressSearch a functional component because that seems to fix our problems immediately.

Fixed Issues

$ #5794
$ https://github.com/Expensify/Expensify/issues/181023

Tests / Web QA

Error reporting

  1. Sign in to New Dot with a new account
  2. Create a new workspace
  3. Connect a new bank account
  4. Routing number: 123123123
    Account Number: 1111222233331111
  5. Hit enter. When you're on the Company Information page, immediately hit enter.
  6. Verify that you see the error under the Company Address field and that the border of that field turns red.

Autofill

  1. Sign in to New Dot (new account is fine)
  2. Create a new workspace
  3. Connect a new bank account
  4. Routing number: 123123123
    Account Number: 1111222233331111
  5. Input valid information on all fields in the Company Information step (doesn't need to be real info)
  6. a) Variant 1: Refresh the page
    b) Variant 2: Hit Submit. When you're on Step 3 (Personal Information) click the back arrow to go back to Step 2 (Company Informaion)
  7. Notice that all the fields are auto-filled, including the Company Address.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

iOS

Screen Shot 2021-10-13 at 3 12 50 PM

Screen Shot 2021-10-13 at 3 14 03 PM

Android

Uploading Screen Shot 2021-10-13 at 3.26.02 PM.png…
Screen Shot 2021-10-13 at 3 27 53 PM

@Luke9389 Luke9389 requested a review from a team as a code owner October 13, 2021 02:41
@Luke9389 Luke9389 self-assigned this Oct 13, 2021
@MelvinBot MelvinBot requested review from jasperhuangg and removed request for a team October 13, 2021 02:42
@Luke9389 Luke9389 changed the title refactor to funcitonal component Get CompanyAddress field to autofill and show errors Oct 13, 2021
@Luke9389 Luke9389 changed the title Get CompanyAddress field to autofill and show errors [WIP] Get CompanyAddress field to autofill and show errors Oct 13, 2021
@Luke9389
Copy link
Contributor Author

Putting this on WIP until I've tested on all platforms

@Luke9389
Copy link
Contributor Author

I'll be back to this tomorrow. Gotta go

@Luke9389
Copy link
Contributor Author

Ok I cannot for the life of me figure out how to get rid of the line mentioned in this issue, so for now I'm gonna test this on all platforms and open up for review.
#5810

@robertjchen robertjchen requested review from robertjchen and removed request for jasperhuangg October 13, 2021 22:08
@trjExpensify
Copy link
Contributor

Can we add the CP Staging label to this, so we can get it into the launch.

@github-actions
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@Luke9389 Luke9389 changed the title [WIP] Get CompanyAddress field to autofill and show errors [CP-stag] Get CompanyAddress field to autofill and show errors Oct 13, 2021
Copy link
Contributor

@robertjchen robertjchen left a comment

Choose a reason for hiding this comment

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

Nice! 👍

@robertjchen robertjchen merged commit 89ebeaa into main Oct 13, 2021
@robertjchen robertjchen deleted the luke-company-address-autfill-and-error-message branch October 13, 2021 22:33
github-actions bot pushed a commit that referenced this pull request Oct 13, 2021
…and-error-message

[CP-stag] Get CompanyAddress field to autofill and show errors

(cherry picked from commit 89ebeaa)
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging by @robertjchen in version: 1.1.7-13 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@isagoico
Copy link

@Luke9389 I was testing this and I'm a bit confused with step 7. I think it should say Notice that all the fields are auto-filled including the Company Address. and not excluding (since the issue being fixed mentioned that the address was not auto-filling before). If this is correct, the PR is a pass.

@kevinksullivan
Copy link
Contributor

kevinksullivan commented Oct 14, 2021

@isagoico that is a typo, company address should be auto-filled. I went ahead and updated the OP. Can we check this one off?

@Luke9389
Copy link
Contributor Author

Yup my bad. That was copy pasta from the issue I made for this 😬

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.7-24 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @robertjchen in version: 1.1.7-25 🚀

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

@ogumen
Copy link

ogumen commented Oct 22, 2021

Please make sure that the dropdowns "Company type" and "Incorporation state" also display error message and their correction suggestions when no option has been selected and the form is submitted. In addition, the mandatory fields and dropdowns should be marked as required (both visually and programmatically), as mentioned in #5703 (comment)

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants