Wish List - Ability to sort holdings in OPAC based on geographic proximity to user specified location

Bug #1863252 reported by Ruth Frasur Davis
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

The public catalog record page will provide an option for users to sort holdings by geographic proximity to user specified location.

Revision history for this message
Ruth Frasur Davis (redavis) wrote :

This proposed feature has been selected as a development project by the Evergreen Community Development Initiative. The ECDI has compiled and refined a set of requirements and seeks proposals including specifications and project cost/time estimates from interested and able parties to develop this feature.

Those requirements and additional information are available at https://docs.google.com/document/d/1o14B839_KeOfQntmtYDJsBqfatv55aEMF7_gqE_N2Cg

Lynn Floyd (lfloyd)
Changed in evergreen:
importance: Undecided → Wishlist
tags: added: opac
Revision history for this message
Andrea Neiman (aneiman) wrote :

ECDI has contracted with Equinox to develop this feature based on the following specifications:

https://yeti.equinoxinitiative.org/dev/public/techspecs/geosort_rev202008.pdf

Revision history for this message
Jason Etheridge (phasefx) wrote :

I've pushed a branch for this to collab/phasefx/geosort @ working/Evergreen.git

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/geosort

You need to install the following perl modules (or run Makefile.install):
Geo::Coder::Free, Geo::Coder::Google, and Geo::Coder::OSM

With stock concerto, live_t/32-geosort.t does some basic tests.

To test by hand, you need to:
1) enable the opac.use_geolocation global flag under Administration -> Server Administration -> Global Flags
2) create a service under Administration -> Server Administration -> Geographic Location Service, with the Service Code set to OSM and the Active field checked. Naming the service OSM would make sense, and let it be owned by the top of the org tree.
3) under Administration -> Local Administration -> Library Settings, set the "Geographic Location Service to use for Addresses" setting to the service created in step 2, for the whole consortium.
4) In the same interface, set the "Enable Holdings Sort by Geographic Proximity" setting to True for the whole consortium.
5) Under Administration -> Server Administration -> Organizational Units, walk through the Addresses tab for each org unit, and click Get Coordinates for each one and then Save if the Latitude & Longitude fields get filled in. Unfortunately, the 3rd party OSM service works best with zip codes and not full addresses, so you may need to just make up some decimal values for Latitude & Longitude instead.
6) in the public OPAC (log out of the staff client first if using the same browser without private windows), pull up a record details page, such as the one for "Portrait of the artist Yehudi Menuhin". In the "Sort by distance from" textbox above the copies listing, enter a zip code and click Go or press enter. Depending on the latitude/longitude values for the org units, the copies should re-sort based on "as the crow flies" distances between the orgs and the entered address. Orgs without lat/long coordinates should fall to the end of the sorting. There's a library setting for changing the unit of measurement display from kilometers to miles.

tags: added: pullrequest
Revision history for this message
Terran McCanna (tmccanna) wrote :

I'm excited about this!

But to be clear, is this fix just the 3 commits labeled "lp1863252 toward geosort" or the other two commits that are shown above that, as well?

Revision history for this message
Chris Sharp (chrissharp123) wrote :

There may be good reason not to do this, but I have pushed a branch that pulls the Geo::Coder Perl libraries from APT where possible (Geo::Coder::Free is not packaged by Debian or Ubuntu at this point so would still get installed with CPAN):

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1863252_geosort

Revision history for this message
Terran McCanna (tmccanna) wrote :

Note that when I tried installing this branch prior to Chris's change in #5, I hit errors and could not complete the upgrade script. Screenshot shows the point where it stalled.

Revision history for this message
Jason Etheridge (phasefx) wrote :

Thanks Chris, Terran! My apt-fu failed me searching for existing packages. Terran, was that with the Geo::Coder perl modules prerequisite install? I know I said No to certain tests in that step that warned they would be slow. We're going to look into making Geo::Coder::Free an optional install (particularly since it takes up a lot of space). That'll leave us with just the Debian packages, which should make things easy/robust.

tags: removed: pullrequest
Changed in evergreen:
assignee: nobody → Jason Etheridge (phasefx)
Revision history for this message
Terran McCanna (tmccanna) wrote :

I think so? Chris would understand what it was doing at the point where it got stuck better than I do.

Revision history for this message
Jason Etheridge (phasefx) wrote :

I pushed Chris's commit to the collab branch and an additional commit to make Geo::Coder::Free optional.

Changed in evergreen:
assignee: Jason Etheridge (phasefx) → nobody
tags: added: pullrequest
Revision history for this message
Jason Etheridge (phasefx) wrote :

So that makes the top 7 commits on that branch.

Revision history for this message
Andrea Neiman (aneiman) wrote :
Revision history for this message
Jason Etheridge (phasefx) wrote :

I force pushed a new version of the branch to tweak my last commit to restore Geo::Coder::Google over the Debian package for Geo::Coder::Googlev3. The former does require an API key to use, and the latter purports not to, but has been broken for two years. :-(

Changed in evergreen:
assignee: nobody → Terran McCanna (tmccanna)
Revision history for this message
Terran McCanna (tmccanna) wrote :

The sorting itself is working great - I tested it in both the TPAC and the Bootstrap OPAC.

However, the "Get Coordinates" button in the org units interface isn't working for me. (This is during feedback fest on both the pattypan and pattypan-tpac test servers, logged in as the admin account.) I changed BR4 to that of a library address in Georgia, but the Get Coordinates button isn't doing anything and I don't see anything in the browser console. I had to manually look up the latitude and longitude and add them. I tried this under both of the geolocation providers that are currently configured.

Changed in evergreen:
assignee: Terran McCanna (tmccanna) → nobody
Revision history for this message
Llewellyn Marshall (lbmarshallv) wrote :

I tried a similar test to Terran: I set BR3 with our office address, set up OSM and FREE in the geo services admin panel, enabled them in the library settings. With both services I get this error message in my Chrome console:

main.a6e62b5b283d66668032.js:1 open-ils.actor.geo.retrieve_coordinates failed! stat=500 msg= *** Call to [open-ils.actor.geo.retrieve_coordinates] failed for session [0.067704927209826021613162094940], thread trace [0]:
Exception: OpenSRF::EX::ERROR 2021-02-12T15:34:54 OpenSRF::Application /usr/local/share/perl/5.26.1/OpenSRF/Application.pm:243 System ERROR: Call to open-ils.geo for method open-ils.geo.retrieve_coordinates
 failed with exception: Exception: OpenSRF::EX::ERROR 2021-02-12T15:34:54 OpenILS::Application::AppUtils /usr/local/share/perl/5.26.1/OpenILS/Application/AppUtils.pm:201 System ERROR: Exception: OpenSRF::DomainObject::oilsMethodException 2021-02-12T15:34:54 OpenSRF::AppRequest /usr/local/share/perl/5.26.1/OpenSRF/AppSession.pm:1159 <404> Method [open-ils.geo.retrieve_coordinates] not found for OpenILS::Application::Geo

Revision history for this message
Jennifer Bruch (jbruchpails) wrote :

I tested this on pattypan server and used just a town name I found on the provided map. I was glad that I did not need to provide an exact address and the zip code worked too.

Next I logged into the staff side and changed the physical address for System 1 making sure to save the changes first.

After saving the changed address, the get coordinates function worked on the first try. So the geo services on that server is working as expected for me.

Love the distance column.

Revision history for this message
Jason Boyer (jboyer) wrote :

There seems to be an issue the first time you try to use the Get Coordinates button. If you open the Organizational Units page, click the Addresses tab, and then click Get Coordinates, you'll get an error in the browser console. If you then change org units on the left though, the button will begin working as expected.

Revision history for this message
Terran McCanna (tmccanna) wrote :

I can confirm Jason's comment in #16 that if I switch to a different org unit and then switch back to it, it then works. As soon as that is fixed, I think it is ready for a sign off!

Andrea Neiman (aneiman)
Changed in evergreen:
milestone: none → 3.7-beta
Revision history for this message
Jason Etheridge (phasefx) wrote :

Thanks! Ah, inconsistency in the data structure for that UI; sometimes it was passing an object for the org unit associated with an address, and sometimes the ID. I've tweaked the middle layer to handle both and the UI to send only the ID. I also added a toast alert for the Get Coordinates action for when and if it fails with a non-catastrophic (i.e. expected) error, such as the address not being found.

I've pushed a new branch with this fix that is also rebased against master at collab/phasefx/geosort-2021-02-19

Incidentally, I noticed another bug with the Delete button being duplicated when jumping around addresses, but this already exists in master and is unrelated to geosort.

Revision history for this message
Terran McCanna (tmccanna) wrote :

I have installed this on a test server with current master and gone through the documentation to get everything set up with my Google API Key, but the Get Coordinates button still isn't doing anything for me.

It isn't clear in the documentation which Google API it is actually calling, since Google has like, 20 map APIs. These are the ones I currently have enabled (all set up to use the same API key):

Maps JavaScript API
Geocoding API
Distance Matrix API
Geolocation API

Am I missing something else that I need to enable?

Revision history for this message
Terran McCanna (tmccanna) wrote :

More info: When I click "Get Coordinates" it shows this in the browser console, but doesn't return any error or success messages: "Net: request open-ils.actor.geo.retrieve_coordinates"

Revision history for this message
Terran McCanna (tmccanna) wrote :

(Ignore comment #20)

Revision history for this message
Terran McCanna (tmccanna) wrote :

Okay, so I had been missing the changes to opensrf.xml and opensrf_core.xml. Those are now in place and I've restarted everything, but every address I try gets a "No location returned by geographic location service" error with this in the browser console:

Object { pid: 120549, desc: "No location returned by Geographic Location Service", servertime: "Fri Feb 19 12:49:24 2021", ilsevent: "7035", stacktrace: "/usr/local/share/perl/5.26.1/OpenILS/Application/Geo.pm:196 /usr/local/share/perl/5.26.1/OpenSRF/Application.pm:628 /usr/share/perl5/Error.pm:421", textcode: "GEOCODING_LOCATION_NOT_FOUND" }

Any thoughts on why this might be failing in my test?

Revision history for this message
Terran McCanna (tmccanna) wrote :

For documentation purposes: My first Google API Key I tested with was set up to be restricted by HTTP referrers, and that does not work for this feature. I had to set up a new Google API Key that was restricted by IP address instead. Adding this to the setup documentation would save future users some time. Also, it would be helpful to indicate which of the Google APIs that the code is actually using (as of right now, Google has 17 map-related APIs). I ended up enabling the four most likely ones for testing, but I doubt all of them are needed:

Maps JavaScript API
Geocoding API
Distance Matrix API
Geolocation API

For the Bootstrap OPAC, I identified a dropped END tag that was causing an Internal Server Error in the item details page. My sign off here includes a fix for that:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mccanna/geosort-2021-02-19-signoff

Also, note that in order to properly test the sorting, I applied the fix for copy table pagination here: https://bugs.launchpad.net/evergreen/+bug/1916085

tags: added: signedoff
Revision history for this message
Andrea Neiman (aneiman) wrote :

Thanks Terran, I will add that those things to the docs for GeoSort before I submit them to the community docs.

Revision history for this message
Jason Etheridge (phasefx) wrote :

Hi Terran, could you PM me in IRC or email the address you tried? Some addresses with some services will just not return coordinates. For example, the OSM service appears to work best with just zip codes. However, I haven't seen Google fail on one.

In srfsh, you can try things more directly:

srfsh# request open-ils.geo open-ils.geo.retrieve_coordinates 1 "30016"

Received Data: {
  "longitude":"127.28711683398349",
  "latitude":"36.61658955343946"
}

------------------------------------
Request Completed Successfully
Request Time in seconds: 1.118888
------------------------------------

The first parameter is the org unit ID.

Revision history for this message
Terran McCanna (tmccanna) wrote :

Hi Jason - I got it working, but the Google API account has to either be unrestricted or restricted based on IP addresses. (See comment #23) Once I figured that out, it worked great.

Revision history for this message
Jason Etheridge (phasefx) wrote :

Awesome, thank you. Good catch on the [% END %] tag; I killed that by accident rebasing after courses were added. I'm looking at your pagination bug and then I'll push a new branch for this one.

Revision history for this message
Jason Etheridge (phasefx) wrote :

Alright, rebased against today's master, with Terran's fix, and also with the HTML escape fix removed since the real fix made it into master: collab/phasefx/geosort-2021-02-23

That would be these commits:
35f14ecae32a5503fd704dc4b7f46bac32b96bf7 lp1863252 toward geosort
87c169476f2a60169eb3eb2a3344d1b5f9de2ba9 lp1863252 toward geosort
60937f303e5c708ee89c631ed43d88818b6e738d lp1863252 toward geosort
e1385e4c959445f199d86908695de14b55fba79a fix unrelated issue that was breaking display of the copy_table
8d0b1d73cdfa25affc4c5965ab04653cf9989fd0 LP#1863252 - Use APT for Perl dependencies where possible.
acb7f2c5ce674041f1771ee100aca3c7fa26de11 lp1863252 make Geo::Coder::Free optional
8aafdf30deb85d7e8f6fb5e8cbc1768b98d31585 lp1863252 fix Get Coordinates button in org admin
27b8702b5ab9932e1521b3fba869e147f7e3934d LP1863252 Geosort - add dropped END tag

Revision history for this message
Terran McCanna (tmccanna) wrote :

I tried applying this today on current master and I'm getting conflicts. (Apologies for my lack of rebasing / merging skills.)

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Merged to master for 3.7. Thanks, All!

Changed in evergreen:
status: New → Fix Committed
assignee: Bill Erickson (berick) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.