Gyamada/lg 8861 usps locations are sorted

[gina-yamada]

🎫 Ticket

LG-8861

🛠 Summary of changes

• Sort USPS facilities by ascending distance from the user's address
• Added a tests to check the order

Question for invidual doing review

• Best not to assume API will always return a distance? I can add a ternary and +1000 for the else which will move any facilities without a distance to the bottom of the list.
  Current: facilities.sort_by { |f| f[:distance].delete_suffix(' mi').to_f }
  Thoughts: facilities.sort_by { |s | s[:distance] ? s[:distance].delete_suffix(' mi').to_f : +1000 }

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Run tests in path/file: spec/services/usps_in_person_proofing/proofer_spec.rb
• Locally, set arcgis_search_enabled to false inside config/application.yml. Run through flow described above (It looks like the list is coming from ipp_pilot_usps_facitilites.json. All of the distances are null and my code is not breaking.)
• Locally, set arcgis_search_enabled to true inside config/application.yml. Edit the location distances in request_faciltities_response.json to be out of order. Run through flow described below.

Flow:

1. Go to /verify/doc_auth/welcome
2. Get started verifying your identify flow
3. Upload .yml files to fail ID verification
4. Click Try in person button
5. Type in a few characters for Enter an address to find a Post Office near you on verify/doc_auth/document_capture#location
6. When taken to /verify/doc_auth/document_capture#location you should have ordered list

👀 Screenshots

Test Results for spec/services/usps_in_person_proofing/proofer_spec.rb

My branch returned 3 different failure results so I ran the pipeline on Gitlab.
run 1: 7776 examples, 1022 failures, 7 pending
run 2: 7776 examples, 1068 failures, 7 pending
run 3: 7776 examples, 1042 failures, 7 pending
Pipeline Results on GitLab

[zachmargolis]

Turns out Ruby's String#to_f is super liberal, and is happy to parse a number out of a mixed string, maybe we drop the .delete_suffix?

 "5.111 mi".to_f
=> 5.111

Suggested change

	facilities.sort_by { |f| f[:distance].delete_suffix(' mi').to_f }
	facilities.sort_by { |f| f[:distance].to_f }

and that way if USPS ever changes units or formatting, this will continue to work

[gina-yamada]

Super liberal, thanks for the knowledge! 🥳 I made this update

[zachmargolis]

another way to test that they're sorted would be like an each_cons:

Suggested change

	previous_post_office_distance = 0
	facilities.count do |post_office|
	current_distance = post_office.distance.delete_suffix(' mi').to_f
	expect(previous_post_office_distance).to be <= current_distance
	previous_post_office_distance = current_distance
	end
	expect(facilities.count).to be > 1
	facilities.each_cons(2) do |facility_a, facility_b|
	expect(facility_a.distance).to be <= facility_b.distance
	end

[gina-yamada]

Less code and cleaner! I made this update too

[svalexander]

do we maybe want to remove the dupes before sorting - so we don't have to sort unnecessary locations?

[gina-yamada]

Yes! Good eye for detail. I made this update

[gina-yamada]

@svalexander @zachmargolis Updates have been made. Thank you both for the feedback! 🙏 Other than the ways described in my testing plan, is there a way to hit USPS api locally to test this or is the test I have written and the tinker I have done enough in your opinion? If this is enough, I can remove draft. Please resolve comments if I have fulfilled suggestions. Let me know if you'd like to see anything else changed.

[gina-yamada]

Acceptance Criteria

1. Sign in
2. Go to /verify/doc_auth/welcome
3. Get started verifying your identify flow
4. Upload .yml files to fail ID verification
5. Click Try in person button
6. Type in a few characters for Enter an address to find a Post Office near you on verify/doc_auth/document_capture#location
7. When taken to /verify/doc_auth/document_capture#location you should have ordered list of facilities with the shortest distance at the top of the list

[zachmargolis]

LGTM

[gina-yamada]

Shannon paired with me on this so we could test this out with data returned from the API. Working as we would expect. Merging this in!