LG-7719: IPP PO Search: findAddressCandidates setup

[allthesignals]

🎫 Ticket

Link to the relevant ticket.

🛠 Summary of changes

Adds a new method wrapping findAddressCandidates API endpoint.

This method accepts a "magic_key" returned from #suggest.

There are many other ways to query findAddressCandidates, and the method can be expanded to be more flexible, but this is the current path for geocoding.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Automated test coverage
• Manual checks

🚀 Notes for Deployment

Still need to add the long-lived API token to deploy environments.

[aduth]

👍

[aduth]

Minor: Do we need to extract this property? Or is it redundant if we could construct it with the other properties (street_address, city, state, zip_code)

[allthesignals]

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.

[aduth]

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 😄

[allthesignals]

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 😂

[aduth]

Should we include some error cases here? Maybe as shared_examples or helper method if we're expecting it to behave the same across the two API calls.

[allthesignals]

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 shared_examples ("webauthn setup" for example)...

[aduth]

(Meant to approve with comments)

[sheldon-b]

Looks good! Left a couple minor comments

[allthesignals]

Note:

One thing I changed up in the mapping: for the sake of simplicity, I made location into an array, which orders as [x,y], according to the GeoJSON spec as well as how ESRI/ArcGIS represents coordinates.

[zachmargolis]

I have spent a decent amount of time with GeoJSON and always confuse myself with the nameless array syntax, could we make a Location struct with named latitude, longitude members (could also be a follow-up for a future PR where we actually drop pins on a map, but wanted to flag this now)

[allthesignals]

Sure thing! Yeah, you're probably right. I'll go ahead and do it for this PR.

GeoJSON describes an order for coordinates: they should go, in order: [longitude, latitude, elevation]... Historically, the order of coordinates is usually “latitude, longitude”, and many people will assume that this is the case universally. Long hours have been wasted discussing which is better... applications have tended to use latitude, longitude order

So, there's widespread ambiguity there, and I've been bit by mixing it up in the past, so explicit it is...

[allthesignals]

@zachmargolis note

[allthesignals]

@zachmargolis not sure whether this is the best way to assert for this?

[zachmargolis]

This is probably fine. If you wanted to remove that long namespace, you could assert on .as_json instead of .to_h, and compare string keys:

diff --git a/spec/services/arcgis_api/geocoder_spec.rb b/spec/services/arcgis_api/geocoder_spec.rb
index 2763bca4f..9b9040256 100644
--- a/spec/services/arcgis_api/geocoder_spec.rb
+++ b/spec/services/arcgis_api/geocoder_spec.rb
@@ -40,14 +40,14 @@ RSpec.describe ArcgisApi::Geocoder do
 
       suggestions = subject.find_address_candidates('abc123')
 
-      expect(suggestions.first.to_h).to eq(
+      expect(suggestions.first.as_json).to eq(
         {
-          address: '100 Main Ave, La Grande, Oregon, 97850',
-          location: ArcgisApi::Geocoder::Location.new(longitude: -118.10754025791812, latitude: 45.328271485226445),
-          street_address: '100 Main Ave',
-          city: 'La Grande',
-          state: 'OR',
-          zip_code: '97850',
+          '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

[zachmargolis]

how come this shorter line had to break?

Suggested change

	latitude: candidate.dig(
	'location', 'y'
	),
	latitude: candidate.dig('location', 'y'),

[allthesignals]

Oof, not sure. Rubocop? I'll fix it in this future PR.