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
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.
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
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.
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 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?
This is/was a pre-existing problem
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>
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.
(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
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
(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
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 ?
I don't see a problem in embedding the files if that is "the prefered way of doing things"
Lets see what QA says
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.
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?