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

Only accept "address" result from google API - validate address_components before using it #5854

Merged
merged 3 commits into from
Oct 14, 2021

Conversation

aldo-expensify
Copy link
Contributor

@aldo-expensify aldo-expensify commented Oct 14, 2021

Details

We were getting results that were not full addresses from the google places API. An example of this is a "premise" type of result, which has the street name and number missing. i.e. search "88 Kearney Street", there are results that will cause the bug.

Adding this new parameter will prevent us from getting this kind of result and we only get full addresses:

https://developers.google.com/maps/documentation/places/web-service/supported_types#table3

As @roryabraham pointed up here (great catch!): #5840 (comment), it is not enough to add types: 'address'. Using this we can still get results that are 'routes', which don't have a street number.

To fix this, I added the function validateAddressComponents to verify the address_components checking point (2) and (3) in @roryabraham 's suggestions #5840 (comment) (thanks again 😝 )

If we validateAddressComponents we nullify the values addressStreet, addressCity, addressState, addressZipCode so our form regular validations will trigger on submit. If we don't nullify, we would keep the old values while showing the user something different in the UI.

I didn't try to set any error from the AddressSearch because our approach so far has been that the form validation and setting errors is a responsibility of the form and not the individual input components.

I think it is acceptable to do this verification here as it can be analog to verifying the format of a date in a date input. If the date format is totally wrong, the date input component would reject the date and call on change with undefined or null.

Fixed Issues

$ #5840

Tests / QA

  1. Create a NewDot account, verify it, log in
  2. Create a workspace and start adding a VBA
  3. Use information in (3) from this SO in the first step (Connect bank account)
  4. You should be in "Company information" step
  5. Input "88 Kearney Street" in "Company address" field and click the "88 Kearny Street, San Francisco, CA, USA" result
  6. Click "Save & continue"
  7. Expected result: the "Company address" field should not have any error (is valid) Screen Shot 2021-10-14 at 12 41 50 PM
  8. Refresh the page, make sure the url is "/bank-account/company"
  9. Expected result: the "Company address" field should have the correct information (no undefined text in between)Screen Shot 2021-10-14 at 12 42 23 PM
  10. Input "25220 Quail Ridge Road, Escondido, CA 97027" and select the "Quail Ridge Road, Escondido, CA, USA" result
  11. Click "Save & continue"
  12. Expected result: the "Company address" field should have the error
    Screen Shot 2021-10-14 at 12 40 11 PM
    because the selected result is missing the street number.
  13. Refresh the page, make sure the url is "/bank-account/company"
  14. Expected result: the "Company address" field should be empty
    Screen Shot 2021-10-14 at 12 41 17 PM

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@aldo-expensify aldo-expensify changed the title [WIP] Only accept "address" result from google API [WIP] Only accept "address" result from google API - validate address_components before using it Oct 14, 2021
@aldo-expensify aldo-expensify changed the title [WIP] Only accept "address" result from google API - validate address_components before using it Only accept "address" result from google API - validate address_components before using it Oct 14, 2021
@aldo-expensify aldo-expensify marked this pull request as ready for review October 14, 2021 19:53
@aldo-expensify aldo-expensify requested a review from a team as a code owner October 14, 2021 19:53
@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team October 14, 2021 19:53
@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.

@roryabraham roryabraham merged commit b83d213 into main Oct 14, 2021
@roryabraham roryabraham deleted the aldo_only-accept-address-addresssearch branch October 14, 2021 21:32
github-actions bot pushed a commit that referenced this pull request Oct 14, 2021
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging by @roryabraham in version: 1.1.7-22 🚀

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.

@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 @roryabraham in version: 1.1.7-25 🚀

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

@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.

3 participants