-
Notifications
You must be signed in to change notification settings - Fork 166
LG-7719: IPP PO Search: findAddressCandidates setup #7199
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
Changes from all commits
ddd4275
ae5d332
0bac469
7fe8389
32db941
b449bc4
0ea3b7f
4bee11b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,23 @@ | ||||||||||
| module ArcgisApi | ||||||||||
| class Geocoder | ||||||||||
| Suggestion = Struct.new(:text, :magic_key, keyword_init: true) | ||||||||||
| AddressCandidate = Struct.new( | ||||||||||
| :address, :location, :street_address, :city, :state, :zip_code, | ||||||||||
| keyword_init: true | ||||||||||
| ) | ||||||||||
| Location = Struct.new(:latitude, :longitude, keyword_init: true) | ||||||||||
|
|
||||||||||
| # Makes HTTP request to get potential address matches | ||||||||||
| # These are option URL params that tend to apply to multiple endpoints | ||||||||||
| # https://developers.arcgis.com/rest/geocode/api-reference/geocoding-find-address-candidates.htm#ESRI_SECTION2_38613C3FCB12462CAADD55B2905140BF | ||||||||||
| COMMON_DEFAULT_PARAMETERS = { | ||||||||||
| f: 'json', | ||||||||||
| countryCode: 'USA', | ||||||||||
| category: 'address', | ||||||||||
| }.freeze | ||||||||||
|
|
||||||||||
| # Makes an HTTP request to quickly find potential address matches. Each match that is found | ||||||||||
| # will include an associated magic_key value which can later be used to get more details about | ||||||||||
| # the address using the #find_address_candidates method | ||||||||||
| # Requests text input and will only match possible addresses | ||||||||||
| # A maximum of 5 suggestions are included in the suggestions array. | ||||||||||
| # @param text [String] | ||||||||||
|
|
@@ -11,9 +26,7 @@ def suggest(text) | |||||||||
| url = "#{root_url}/suggest" | ||||||||||
| params = { | ||||||||||
| text: text, | ||||||||||
| category: 'address', | ||||||||||
| countryCode: 'USA', | ||||||||||
| f: 'json', | ||||||||||
| **COMMON_DEFAULT_PARAMETERS, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| parse_suggestions( | ||||||||||
|
|
@@ -23,6 +36,24 @@ def suggest(text) | |||||||||
| ) | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # Makes HTTP request to find an exact address using magic_key | ||||||||||
| # @param magic_key [String] a magic key value from a previous call to the #suggest method | ||||||||||
| # @return [Array<AddressCandidate>] AddressCandidates | ||||||||||
| def find_address_candidates(magic_key) | ||||||||||
| url = "#{root_url}/findAddressCandidates" | ||||||||||
| params = { | ||||||||||
| magicKey: magic_key, | ||||||||||
| outFields: 'StAddr,City,RegionAbbr,Postal', | ||||||||||
| **COMMON_DEFAULT_PARAMETERS, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| parse_address_candidates( | ||||||||||
| faraday.get(url, params) do |req| | ||||||||||
| req.options.context = { service_name: 'arcgis_geocoder_find_address_candidates' } | ||||||||||
| end.body, | ||||||||||
| ) | ||||||||||
| end | ||||||||||
|
|
||||||||||
| private | ||||||||||
|
|
||||||||||
| def root_url | ||||||||||
|
|
@@ -48,6 +79,39 @@ def request_headers | |||||||||
| end | ||||||||||
|
|
||||||||||
| def parse_suggestions(response_body) | ||||||||||
| handle_api_errors(response_body) | ||||||||||
|
|
||||||||||
| response_body['suggestions'].map do |suggestion| | ||||||||||
| Suggestion.new( | ||||||||||
| text: suggestion['text'], | ||||||||||
| magic_key: suggestion['magicKey'], | ||||||||||
| ) | ||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| def parse_address_candidates(response_body) | ||||||||||
| handle_api_errors(response_body) | ||||||||||
|
|
||||||||||
| response_body['candidates'].map do |candidate| | ||||||||||
| AddressCandidate.new( | ||||||||||
| address: candidate['address'], | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Do we need to extract this property? Or is it redundant if we could construct it with the other properties (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure yet. It could be used for display purposes later ("10 USPS locations found near ${address}"). We could construct it with the other properties, but I'm unsure what assumptions that would be making about what ESRI includes in a full "address," etc. The other properties that I extract here are going to be the precise input values for a later request to USPS PO Search endpoint.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could always add it in the future if it turns out that we need it. I'm typically in favor of YAGNI; granted, this whole thing is technically YAGNI since the UI doesn't exist yet 😄
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeahhh. I sorta started this backwards. But I'm starting on the UI piece next, which'll give us a bit more direction on what we ARE going to need 😂 |
||||||||||
| location: Location.new( | ||||||||||
| longitude: candidate.dig('location', 'x'), | ||||||||||
| latitude: candidate.dig( | ||||||||||
| 'location', 'y' | ||||||||||
| ), | ||||||||||
|
Comment on lines
+100
to
+102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how come this shorter line had to break?
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oof, not sure. Rubocop? I'll fix it in this future PR. |
||||||||||
| ), | ||||||||||
| street_address: candidate.dig('attributes', 'StAddr'), | ||||||||||
| city: candidate.dig('attributes', 'City'), | ||||||||||
| state: candidate.dig('attributes', 'RegionAbbr'), | ||||||||||
| zip_code: candidate.dig('attributes', 'Postal'), | ||||||||||
| ) | ||||||||||
| end | ||||||||||
| end | ||||||||||
|
|
||||||||||
| # handles API error state when returned as a status of 200 | ||||||||||
| # @param response_body [Hash] | ||||||||||
| def handle_api_errors(response_body) | ||||||||||
| if response_body['error'] | ||||||||||
| error_code = response_body.dig('error', 'code') | ||||||||||
|
|
||||||||||
|
|
@@ -56,13 +120,6 @@ def parse_suggestions(response_body) | |||||||||
| response_body, | ||||||||||
| ) | ||||||||||
| end | ||||||||||
|
|
||||||||||
| response_body['suggestions'].map do |suggestion| | ||||||||||
| Suggestion.new( | ||||||||||
| text: suggestion['text'], | ||||||||||
| magic_key: suggestion['magicKey'], | ||||||||||
| ) | ||||||||||
| end | ||||||||||
| end | ||||||||||
| end | ||||||||||
| end | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "error": { | ||
| "code": 400, | ||
| "extendedCode": -2147467259, | ||
| "message": "Unable to complete operation.", | ||
| "details": [ | ||
| "Something went wrong" | ||
| ] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| { | ||
| "spatialReference": { | ||
| "wkid": 4326, | ||
| "latestWkid": 4326 | ||
| }, | ||
| "candidates": [ | ||
| { | ||
| "address": "100 Main Ave, La Grande, Oregon, 97850", | ||
| "location": { | ||
| "x": -118.10754025791812, | ||
| "y": 45.328271485226445 | ||
| }, | ||
| "score": 100, | ||
| "attributes": { | ||
| "StAddr": "100 Main Ave", | ||
| "City": "La Grande", | ||
| "RegionAbbr": "OR", | ||
| "Postal": "97850" | ||
| }, | ||
| "extent": { | ||
| "xmin": -118.10854025791812, | ||
| "ymin": 45.327271485226447, | ||
| "xmax": -118.10654025791811, | ||
| "ymax": 45.329271485226442 | ||
| } | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "spatialReference": { | ||
| "wkid": 4326, | ||
| "latestWkid": 4326 | ||
| }, | ||
| "candidates": [ | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,4 +33,42 @@ | |
| ) | ||
| end | ||
| end | ||
|
|
||
| describe '#find_address_candidates' do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we include some error cases here? Maybe as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll definitely add some error cases. My first issue is that the way we deal with HTTP mocks here inhibits reusability. This might be a little more configurable with something like VCR. I'll dig around for some examples where HTTP mocks are used along with |
||
| it 'returns candidates from magic_key' do | ||
| stub_request_candidates_response | ||
|
|
||
| suggestions = subject.find_address_candidates('abc123') | ||
|
|
||
| expect(suggestions.first.as_json).to eq( | ||
| { | ||
| 'address' => '100 Main Ave, La Grande, Oregon, 97850', | ||
| 'location' => { 'longitude' => -118.10754025791812, 'latitude' => 45.328271485226445 }, | ||
| 'street_address' => '100 Main Ave', | ||
| 'city' => 'La Grande', | ||
| 'state' => 'OR', | ||
| 'zip_code' => '97850', | ||
| }, | ||
| ) | ||
| end | ||
|
|
||
| # https://developers.arcgis.com/rest/geocode/api-reference/geocoding-service-output.htm#ESRI_SECTION3_619341BEAA3A4F488FC66FAE8E479563 | ||
| it 'handles no results' do | ||
| stub_request_candidates_empty_response | ||
|
|
||
| suggestions = subject.find_address_candidates('abc123') | ||
|
|
||
| expect(suggestions).to be_empty | ||
| end | ||
|
|
||
| it 'returns an error response body but with Status coded as 200' do | ||
| stub_request_candidates_error | ||
|
|
||
| expect { subject.find_address_candidates('abc123') }.to raise_error do |error| | ||
| expect(error).to be_instance_of(Faraday::ClientError) | ||
| expect(error.message).to eq('received error code 400') | ||
| expect(error.response).to be_kind_of(Hash) | ||
| end | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zachmargolis note