Skip to content

LG-8563: Restrict ArcGIS API address results to certain types#7753

Merged
allthesignals merged 13 commits intomainfrom
wmg/8563-cross-streets
Feb 2, 2023
Merged

LG-8563: Restrict ArcGIS API address results to certain types#7753
allthesignals merged 13 commits intomainfrom
wmg/8563-cross-streets

Conversation

@allthesignals
Copy link
Contributor

🎫 Ticket

🛠 Summary of changes

Before:
Geocoder class was asking ArcGIS API for addresses of any kind of address. This caused errors when ArcGIS would find a valid cross street, for example, but USPS's API does not understand cross streets.

After:
Geocoder class asks ArcGIS API only for a select few types of addresses using the category parameter. Those categories are described in the Addr_type field.

Specifically, we restrict the categories only to ones we believe USPS API will understand, which are:

  • Subaddress
  • Point Address
  • Street Address
  • Street Name

USPS API does not seem to understand these types of addresses:

  • Distance Marker
  • Intersection

📜 Testing Plan

  • Automated test asserting that our request always includes the specific category params
  • Manual tests locally against ArcGIS
  • Continued observations and testing with user data

@allthesignals
Copy link
Contributor Author

Commit history looks crazy sorry!

if supported_params.empty?
raise ArgumentError, <<~MSG
Unknown parameters: #{options.except(KNOWN_FIND_ADDRESS_CANDIDATES_PARAMETERS)}.
Unknown parameters: #{options.except(*KNOWN_FIND_ADDRESS_CANDIDATES_PARAMETERS)}.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tomas-nava noticed this earlier so I'm fixing here: #7746 (comment)

@allthesignals allthesignals merged commit 0f77e85 into main Feb 2, 2023
@allthesignals allthesignals deleted the wmg/8563-cross-streets branch February 2, 2023 15:59
@aduth aduth mentioned this pull request Feb 6, 2023
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.

2 participants