department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
281 stars 197 forks source link

Use server distance for sort order #30212

Open mmiddaugh opened 2 years ago

mmiddaugh commented 2 years ago

Issue Description

In #29942, we addressed the number of search results per page and lettering conventions from one page to the next. The overlapping radius scenario may be due to a small shift in the map occurring between queries for a page of search results, which slightly alters the computed distance.

During grooming of #29942 we determined that we would return to this item and consider approaches to getting a full set of data to avoid this risk.

Steps to reproduce:

Reference for computing distance

Dev notes

Query 1: https://api.va.gov/facilities_api/v1/va?bbox[]=-74.6996&bbox[]=39.9001&bbox[]=-73.1996&bbox[]=41.4001&type=health&page=1&per_page=20&mobile=false&radius=16&latitude=40.6501&longitude=-73.9496

Query 2: https://api.va.gov/facilities_api/v1/va?bbox[]=-74.6996&bbox[]=39.9001&bbox[]=-73.1996&bbox[]=41.4001&type=health&page=2&per_page=20&mobile=false&radius=16&latitude=40.6501&longitude=-73.9496

https://developer.va.gov/explore/facilities/docs/facilities?version=current A query by latitude and longitude returns all facilities in the system, sorted by distance from that location.

Link to action for sort order


Tasks

Acceptance Criteria

mpelzsherman commented 2 years ago

Current method for computing distance:

We compute the lat/long center from MapBox forwardGeocode call, which takes the string entered by the user in the location field and returns a center and a bounding box.

We compute the geographic distance from this center point to each facility returned from the search using a mathematical function that accounts for the curvature of the Earth.

When we paginate, the map center point is not recalculated.

The overlap in distance is the result of overlapping data we receive from Lighthouse.

For example, for New York, VA Health, All services:

First page:

https://dev-api.va.gov/facilities_api/v1/va?bbox[]=-74.7366&bbox[]=39.9806&bbox[]=-73.2366&bbox[]=41.4806&type=health&page=1&per_page=10&mobile=false&radius=42&latitude=40.7306&longitude=-73.9866

Second page:

https://dev-api.va.gov/facilities_api/v1/va?bbox[]=-74.7366&bbox[]=39.9806&bbox[]=-73.2366&bbox[]=41.4806&type=health&page=2&per_page=10&mobile=false&radius=42&latitude=40.7306&longitude=-73.9866

The only different in these queries is the page number.

However, the fartherst result on page 1, Yonkers VA Clinic, is indeed 14.7 miles from the center of New York. And the closest result on page 2, Staten Island Community VA Clinic, is indeed only 13 miles from the center of New York.

Options:

1) fetch all pages, then sort and paginate client side. 2) try switching to lat/long only (no bounding box) and see if the results are accurate enough to use server-side sorting.

I’m not sure #2 would work for PPMS though, and for the mashups we have to re-sort the results anyway.

mmiddaugh commented 2 years ago

19065

Summary of engineering conversation 10/6/21

Background info:

  1. We are moving toward talking directly from FE -> Lighthouse (for better performance)
  2. PPMS queries will continue to go through Vets API
  3. PPMS paginated data still has duplication issues
  4. Lighthouse queries cannot have both bbox and lat/long; we must pick one. Use of bbox may also affect sorting. We should use lat/long only. (Lighthouse does not support radius; results are sorted by distance.)
  5. Lighthouse will return distance if we send lat/long, but it comes back in metadata.

Tasks:

  1. Vets-API needs to be changed to convert lat/long to address for PPMS (BE)
  2. Convert Lighthouse queries to use lat/long only (no bbox) (FE)
  3. Once we do #2, we can remove address from query (already being ignored) (FE)
  4. Need to decide whether we can consume Lighthouse and PPMS distance data:
    • Is it driving distance or geo-distance?
    • Which one do we want? (Dave question?)
  5. Depending on answer to #4:
    • Expose/consume “miles” attribute from PPMS (BE/FE)
    • Expose/consume “distance” from Lighthouse (BE/FE)
mmiddaugh commented 2 years ago

This should be resolved by #31527

mpelzsherman commented 2 years ago

I can no longer reproduce the overlapping data issue described in this ticket. I’m not sure why - maybe the Lighthouse data changed?

mmiddaugh commented 2 years ago

This task is being returned to the backlog due to usability issues discovered when the results are unlimited by radius or bounding box. Can return to this work when Lighthouse adds a radius parameter.

mmiddaugh commented 2 years ago

We were able to see that the overlapping sort order is resolved by the change to send lat/long but we are unable to enable this change in Prod due to usability issues caused by the unbounded data set.

Agile6MSkinner commented 1 month ago

@mmiddaugh this is the last issue standing in Epic #19065. What are your thoughts on it?