Bug 31652 - Add geo-search
Summary: Add geo-search
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Thomas Klausner
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-29 11:38 UTC by Thomas Klausner
Modified: 2023-10-06 20:40 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 31652: Add geo-search (16.34 KB, patch)
2022-10-06 11:39 UTC, Thomas Klausner
Details | Diff | Splinter Review
Bug 31652: Add geo-search (16.31 KB, patch)
2023-07-20 13:23 UTC, Thomas Klausner
Details | Diff | Splinter Review
Bug 31652: Add geo-search (26.92 KB, patch)
2023-08-11 19:17 UTC, Thomas Klausner
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Klausner 2022-09-29 11:38:56 UTC
A customer wants to add a geo-search to Koha:

* Store lat/lng in biblio 034$s and 034$t
* Add lat/lng to ElasticSearch index as geo_point: https://www.elastic.co/guide/en/elasticsearch/reference/7.17/geo-point.html
* Add support for ElasticSearch geo-queries: https://www.elastic.co/guide/en/elasticsearch/reference/7.17/geo-queries.html
* Use Leaflet.js to render maps and pins

We (HKS3, koha-support.eu) have already implemented most of it. We will provide our changes as a patch (or a set of patches) and hope that they will be merged into the main Koha source code.

It will take a bit more time to actually provide the code, but I create this ticket now so we have an "anchor" if anybody wants to discuss this feature etc.

Greetings,
domm
Comment 1 Thomas Klausner 2022-10-06 11:39:40 UTC
Created attachment 141406 [details] [review]
Bug 31652: Add geo-search

This patch adds geosearch to Koha (using Elasticsearch 7). ElasticSearch
search_mappings get new types to store lat/lon, which can be indexed
from MARC 034$s and 034$t. There is a small change to the DB to allow a
new value in search_field.type ENUM.

The QueryBuilder is extended to allow for building advanced
ElasticSearch Querys (eg geo_distance) that cannot be represented in a
simple string query. The UI for searching (including showing the results
on a OSM/Leaflet map) is implemented in a separate plugin
(https://github.com/HKS3/HKS3GeoSearch)

Test Plan:

* make sure you're running ElasticSearch 7
  (eg via `curl http://es:9200?pretty | grep number`)
* apply patch
* Set system preference "SaerchEngine" to "Elasticsearch"
* got to a Framework, check Editor for 034$s and 034$t and save
* got to some books (in the correct framework) and enter some lat and lon into 034$s and 034$t (for example lat=48.216, lon=16.395)
* Run the elasticsearch indexer, maybe limited on the books you edited (-bn 123 -bn 456):
  misc/search_tools/rebuild_elasticsearch.pl -b -v
* You can check if the indexing worked by inspecting the document in elasticsearch:
  * get the biblionumber (eg 123)
  * curl http://es:9200/koha_kohadev_biblios/_doc/123?pretty | grep -A5 geolocation
  * You should get back a JSON fragment containing the lat/lon you stored
* You can query elasticsearch directly:
  * Run the following curl command, but adapt the value for lat/lng and/or the distance (in meters)
  * curl -X GET "http://es:9200/koha_kohadev_biblios/_search?pretty" -H 'Content-Type: application/json' -d '{"query": {"bool":{"must":{"match_all":{}},"filter":{"geo_distance":{"distance":100000,"geolocation":{"lat":48.2,"lon":16.4}}}}}}'
* To run the search via Koha, you need to either install and use https://github.com/HKS3/HKS3GeoSearch or create a handcrafted query string:
  * handcrafted query string:
    * /cgi-bin/koha/opac-search.pl?advsearch=1&idx=geolocation&q=lat:48.25+lng:18.35+distance:100km&do=Search
  * HKS3GeoSearch
    * install the plugin and enable it
    * got to OPAC / Advanced Search
    * There is a new input box "Geographic Search" where you can enter lat/long/radius
    * On the search result page a map is shown with pins for each found biblioitem

Sponsored-by: ZAMG - Zentralanstalt für Meterologie und Geodynamik, Austria - https://www.zamg.ac.at/

This is a combination of 17 commits.
Comment 2 David Nind 2023-05-02 23:25:58 UTC
The patch no longer applies 8-(...

git bz apply 31652

Bug 31652 - Add geo-search

141406 - Bug 31652: Add geo-search

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31652: Add geo-search
.git/rebase-apply/patch:323: trailing whitespace.
# can be single or multiple parameters separated by comma: kw,right-Truncation 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M       Koha/SearchEngine/Elasticsearch.pm
M       Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
M       Koha/SearchEngine/Elasticsearch/Search.pm
M       admin/searchengine/elasticsearch/field_config.yaml
M       admin/searchengine/elasticsearch/mappings.yaml
M       installer/data/mysql/kohastructure.sql
M       koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
M       opac/opac-search.pl
.git/rebase-apply/patch:323: trailing whitespace.
# can be single or multiple parameters separated by comma: kw,right-Truncation 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-search.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
Auto-merging installer/data/mysql/kohastructure.sql
CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql
Auto-merging admin/searchengine/elasticsearch/mappings.yaml
Auto-merging admin/searchengine/elasticsearch/field_config.yaml
CONFLICT (content): Merge conflict in admin/searchengine/elasticsearch/field_config.yaml
Auto-merging Koha/SearchEngine/Elasticsearch/Search.pm
Auto-merging Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
Auto-merging Koha/SearchEngine/Elasticsearch.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 31652: Add geo-search
Comment 3 Thomas Klausner 2023-07-20 13:23:26 UTC
Created attachment 153720 [details] [review]
Bug 31652: Add geo-search

This patch adds geosearch to Koha (using Elasticsearch 7). ElasticSearch
search_mappings get new types to store lat/lon, which can be indexed
from MARC 034$s and 034$t. There is a small change to the DB to allow a
new value in search_field.type ENUM.

The QueryBuilder is extended to allow for building advanced
ElasticSearch Querys (eg geo_distance) that cannot be represented in a
simple string query. The UI for searching (including showing the results
on a OSM/Leaflet map) is implemented in a separate plugin
(https://github.com/HKS3/HKS3GeoSearch)

Test Plan:

* make sure you're running ElasticSearch 7
  (eg via `curl http://es:9200?pretty | grep number`)
* apply patch
* Set system preference "SaerchEngine" to "Elasticsearch"
* got to a Framework, check Editor for 034$s and 034$t and save
* got to some books (in the correct framework) and enter some lat and lon into 034$s and 034$t (for example lat=48.216, lon=16.395)
* Run the elasticsearch indexer, maybe limited on the books you edited (-bn 123 -bn 456):
  misc/search_tools/rebuild_elasticsearch.pl -b -v
* You can check if the indexing worked by inspecting the document in elasticsearch:
  * get the biblionumber (eg 123)
  * curl http://es:9200/koha_kohadev_biblios/_doc/123?pretty | grep -A5 geolocation
  * You should get back a JSON fragment containing the lat/lon you stored
* You can query elasticsearch directly:
  * Run the following curl command, but adapt the value for lat/lng and/or the distance (in meters)
  * curl -X GET "http://es:9200/koha_kohadev_biblios/_search?pretty" -H 'Content-Type: application/json' -d '{"query": {"bool":{"must":{"match_all":{}},"filter":{"geo_distance":{"distance":100000,"geolocation":{"lat":48.2,"lon":16.4}}}}}}'
* To run the search via Koha, you need to either install and use https://github.com/HKS3/HKS3GeoSearch or create a handcrafted query string:
  * handcrafted query string:
    * /cgi-bin/koha/opac-search.pl?advsearch=1&idx=geolocation&q=lat:48.25+lng:18.35+distance:100km&do=Search
  * HKS3GeoSearch
    * install the plugin and enable it
    * got to OPAC / Advanced Search
    * There is a new input box "Geographic Search" where you can enter lat/long/radius
    * On the search result page a map is shown with pins for each found biblioitem

Sponsored-by: ZAMG - Zentralanstalt für Meterologie und Geodynamik, Austria - https://www.zamg.ac.at/

This is a combination of 17 commits.
Comment 4 Thomas Klausner 2023-07-20 13:25:18 UTC
I've updated the patch and it should apply now (again) on current master. Please try to signoff again, I'd love to have this in master before Perl Koha Conference (where I'll do a talk on this patch!)
Comment 5 David Cook 2023-07-20 23:47:01 UTC
Comment on attachment 153720 [details] [review]
Bug 31652: Add geo-search

Review of attachment 153720 [details] [review]:
-----------------------------------------------------------------

::: Koha/SearchEngine/Elasticsearch/Indexer.pm
@@ +117,4 @@
>      }
>  
>      my $documents = $self->marc_records_to_documents($records);
> +

It looks like this file might not be altered beyond whitespace? Looks like the whitespace change could be removed to simplify the overall patch.

::: Koha/SearchEngine/Elasticsearch/Search.pm
@@ +93,5 @@
>      my $elasticsearch = $self->get_elasticsearch();
> +
> +    # XXX investigate where empty query_string is coming from
> +    delete $query->{query}->{query_string} if
> +      $query->{query}->{query_string} && !%{$query->{query}->{query_string}};

Is this a pre-existing problem or introduced in this patch?
Comment 6 Mark Hofstetter 2023-08-08 07:39:45 UTC
This is/was a pre-existing problem
Comment 7 Thomas Klausner 2023-08-11 19:17:12 UTC
Created attachment 154388 [details] [review]
Bug 31652: Add geo-search

This patch adds geosearch to Koha (using Elasticsearch 7). ElasticSearch
search_mappings get new types to store lat/lon, which can be indexed
from MARC 034$s and 034$t. There is a small change to the DB to allow a
new value in search_field.type ENUM.

The QueryBuilder is extended to allow for building advanced
ElasticSearch Querys (eg geo_distance) that cannot be represented in a
simple string query.

We added a new SystemPreference `GeoSearchEnabled`. If it is set to
`true`, we show a new advanced search field that allows for input of lat/lng
and distance (radius). On the search result page, a we render a map
using leaflet.js with pins for each found item. Therefor we also added a
new api endpoint that returns lat/lng for each biblio in the selection.

Test Plan:

* make sure you're running ElasticSearch 7
  (eg via `curl http://es:9200?pretty | grep number`)
* apply patch
* Set system preference "SearchEngine" to "Elasticsearch"
* got to a Framework, check Editor for 034$s and 034$t and save
* got to some books (in the correct framework) and enter some lat and lon into 034$s and 034$t (for example lat=48.216, lon=16.395)
* Run the elasticsearch indexer, maybe limited on the books you edited (-bn 123 -bn 456):
  misc/search_tools/rebuild_elasticsearch.pl -b -v
* You can check if the indexing worked by inspecting the document in elasticsearch:
  * get the biblionumber (eg 123)
  * curl http://es:9200/koha_kohadev_biblios/_doc/123?pretty | grep -A5 geolocation
  * You should get back a JSON fragment containing the lat/lon you stored
* You can query elasticsearch directly:
  * Run the following curl command, but adapt the value for lat/lng and/or the distance (in meters)
  * curl -X GET "http://es:9200/koha_kohadev_biblios/_search?pretty" -H 'Content-Type: application/json' -d '{"query": {"bool":{"must":{"match_all":{}},"filter":{"geo_distance":{"distance":100000,"geolocation":{"lat":48.2,"lon":16.4}}}}}}'
* To run the search via Koha:
  * Set system preference "GeoSearchEnabled" to "true"
  * got to OPAC / Advanced Search
  * There is a new input box "Geographic Search" where you can enter lat/long/radius, for example lat=48.2, lng=16.3, distance=100km
  * On the search result page a map is shown with pins for each found biblioitem

Sponsored-by: ZAMG - Zentralanstalt für Meterologie und Geodynamik, Austria - https://www.zamg.ac.at/

Authors: Mark Hofstetter <mark@hofstetter.at>, Thomas Klausner <domm@plix.at>
Comment 8 Thomas Klausner 2023-08-11 19:40:30 UTC
The latest patch has some warnings/errors reported from koha-qa.pl. Plus I'm not totally happy with the way the search params are handled. So I guess it will need a bit more work.
Comment 9 Lucas Gass 2023-08-14 17:01:50 UTC
(In reply to Thomas Klausner from comment #8)
> The latest patch has some warnings/errors reported from koha-qa.pl. Plus I'm
> not totally happy with the way the search params are handled. So I guess it
> will need a bit more work.

Based on this I am setting this to FQA, to get it out of the 'Needs signoff' queue
Comment 10 Thomas Klausner 2023-08-21 14:36:34 UTC
Here's some old-ish work on improving ElasticSearch, which could be used/integrated here (as pointed out by Jonathan during PerlKohaCon)

https://github.com/fredericd/Koha/compare/master...newelastic
Comment 11 Nick Clemens 2023-08-22 15:21:50 UTC
(In reply to Thomas Klausner from comment #10)
> Here's some old-ish work on improving ElasticSearch, which could be
> used/integrated here (as pointed out by Jonathan during PerlKohaCon)
> 
> https://github.com/fredericd/Koha/compare/master...newelastic

This should go onto a new bug - if we want to change searching at large let's do that, then base this on top of that work
Comment 12 Fridolin Somers 2023-08-28 18:55:59 UTC
Is is necessary to use external server for static files ?

https://unpkg.com/leaflet@1.9.1/dist/leaflet.css
https://unpkg.com/leaflet@1.9.1/dist/leaflet.js

Can't we embed in sources ?
Comment 13 Mark Hofstetter 2023-08-31 07:36:30 UTC
I don't see a problem in embedding the files if that is "the prefered way of doing things"
Comment 14 Fridolin Somers 2023-08-31 22:21:48 UTC
Lets see what QA says
Comment 15 David Cook 2023-09-01 00:05:04 UTC
Comment on attachment 154388 [details] [review]
Bug 31652: Add geo-search

Review of attachment 154388 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
@@ +592,5 @@
>          [% Asset.js("js/openlibrary.js") | $raw %]
>      [% END %]
> +    [% IF ( GeoSearchEnabled ) %]
> +        <link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.1/dist/leaflet.css" integrity="sha256-sA+zWATbFveLLNqWO2gtiw3HL/lh1giY/Inf1BJ0z14=" crossorigin=""/>
> +        <script src="https://unpkg.com/leaflet@1.9.1/dist/leaflet.js" integrity="sha256-NDI0K41gVbWqfkkaHj15IzU7PtMoelkzyKp8TOaFQ3s=" crossorigin=""></script>

As Frido suggested, we don't link out to externally hosted static files. Lots of reasons for that.

Koha already has leaflet.js and leaflet.css although a much older version: 1.0.3+ed36a04

I'd suggest opening a separate issue to upgrade Leaflet and then having that be a dependency of this issue.
Comment 16 Katrin Fischer 2023-09-10 22:55:44 UTC
I think we should definitely not link to external files but keep shipping our own. I believe updating the ones we ship would be a good first step here and hopefully not too hard. I think maybe they are used for Mana?