Bug 27113 - Elasticsearch: Autocomplete in search
Summary: Elasticsearch: Autocomplete in search
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ivan Dziuba
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-27 20:17 UTC by Ivan Dziuba
Modified: 2021-09-04 11:48 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Text to go in the release notes:
This enhancement adds autocomplete suggestions for searches in the OPAC and staff interface, including the advanced search forms (when using Elasticsearch). This is enabled using two new system preferences: - StaffInterfaceAutocompleteElasticsearch - OPACAutocompleteElasticSearch Note: For this to work the search index needs rebuilding.
Version(s) released in:


Attachments
Bug 27113: ElasticSearch: Autocomplete in input search (48.06 KB, patch)
2020-12-03 21:26 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Result (34.12 KB, image/png)
2020-12-03 21:32 UTC, Ivan Dziuba
Details
Bug 27113: ElasticSearch: Autocomplete in input search (52.57 KB, patch)
2021-01-07 21:17 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (44.85 KB, patch)
2021-01-13 16:30 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (43.41 KB, patch)
2021-02-03 19:42 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (43.52 KB, patch)
2021-04-21 21:29 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (45.27 KB, patch)
2021-04-22 21:33 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (45.27 KB, patch)
2021-04-22 21:35 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: (follow-up) Move new CSS to main SCSS files (7.66 KB, patch)
2021-04-23 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
GIF showing the buggy behavior (249.57 KB, image/gif)
2021-04-23 12:50 UTC, Owen Leonard
Details
Bug 27113: (follow-up) Move new CSS to main SCSS files (7.78 KB, patch)
2021-04-23 14:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (45.27 KB, patch)
2021-04-23 21:24 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: (follow-up) QA Tests (6.03 KB, patch)
2021-04-23 21:24 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: (follow-up) QA Tests (10.45 KB, patch)
2021-05-10 19:27 UTC, Ivan Dziuba
Details | Diff | Splinter Review
I have changed CGI::Session->new() from CGI::Session->load() for use session. This condition - 'CGI::Session->load() or die CGI::Session->errstr();' (2.81 KB, patch)
2021-05-28 20:44 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: (follow-up) Correction with Session (1.86 KB, patch)
2021-05-28 20:49 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27113: ElasticSearch: Autocomplete in input search (45.32 KB, patch)
2021-06-03 00:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 27113: (follow-up) QA Tests (10.49 KB, patch)
2021-06-03 00:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 27113: (follow-up) Correction with Session (1.91 KB, patch)
2021-06-03 00:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 27113: (follow-up) Move new CSS to main SCSS files (7.79 KB, patch)
2021-06-03 00:51 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Dziuba 2020-11-27 20:17:39 UTC

    
Comment 1 Ivan Dziuba 2020-12-03 21:26:59 UTC Comment hidden (obsolete)
Comment 2 Ivan Dziuba 2020-12-03 21:32:38 UTC
Created attachment 114169 [details]
Result
Comment 3 Ivan Dziuba 2021-01-07 21:17:34 UTC Comment hidden (obsolete)
Comment 4 Fridolin Somers 2021-01-08 08:13:07 UTC
Whaou I like this :D

Just a question :
The patch changes some unrelated text in installer/data/mysql/mandatory/sysprefs.sql :
-('AnonymousPatron','0',NULL,'Set the identifier (borrowernumber) of the anonymous patron. Used for suggestion and checkout history privacy',''),
+('AnonymousPatron','0',NULL,'Set the identifier (borrowernumber) of the anonymous patron. Used for Suggestion and reading history privacy',''),

Looks like is it unwanted.
Comment 5 Fridolin Somers 2021-01-08 08:18:56 UTC
In koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc
 + var OL_PREVIEW = _("Preview");

Why this ?
Comment 6 Fridolin Somers 2021-01-08 08:23:46 UTC
I see :
api/elasticsearch/intranet-autocomplete.pl

This is strange, its not using the REST API right ?

Its more a perl script like :
opac/svc/elasticsearch/opac-autocomplete.pl

Should be in svc/elasticsearch I'd say.

BTW :
It is possible to use the REST API ?
It whould provide the service for both opac and intranet.

Best regards,
Comment 7 Ivan Dziuba 2021-01-08 19:54:52 UTC
Yes, I see many errors in the files. I will change it.
I would like to provide the service for both opac and intranet through REST API. But, I see many preferences for REST API:

('RESTBasicAuth'- 'If enabled, Basic authentication is enabled for the REST API.','YesNo'),
('RESTdefaultPageSize' - 'Default page size for endpoints listing objects','Integer'),
('RESTOAuth2ClientCredentials' - 'If enabled, the OAuth2 client credentials flow is enabled for the REST API.','YesNo'),
('RESTPublicAnonymousRequests' -'If enabled, the API will allow anonymous access to public routes that don\'t require authenticated access.','YesNo'),
('RESTPublicAPI' - 'If enabled, the REST API will expose the /public endpoints.','YesNo')

I am not sure that good idea for OPAC because REST API uses Auth. 
It is possible to disable authentication in the preferences for REST API, but this may have a consequences...

Do you have a solution about this?

Thanks!
Comment 8 Fridolin Somers 2021-01-11 10:57:21 UTC
I think you can provide a public API for OPAC :
https://wiki.koha-community.org/wiki/REST_api_RFCs#Unprivileged_.28public.29_routes

datas are the same as searching with OPAC website.
Comment 9 Ivan Dziuba 2021-01-11 15:26:25 UTC
Are you sure that OPAC search uses REST API? It's only GET request (with parametres 'q', 'idx', ...) for file opac-search.pl and return a results for template *-results.tt. 
I don't see here REST API.

Best regards,
Comment 10 Ivan Dziuba 2021-01-13 16:30:36 UTC Comment hidden (obsolete)
Comment 11 Fridolin Somers 2021-01-19 15:43:57 UTC
(In reply to Ivan Dziuba from comment #9)
> Are you sure that OPAC search uses REST API? It's only GET request (with
> parametres 'q', 'idx', ...) for file opac-search.pl and return a results for
> template *-results.tt. 
> I don't see here REST API.
> 
> Best regards,

No indeed, search is not actually using API.

I say your autocomplete feature should use a dedicated API service.
Comment 12 Ivan Dziuba 2021-02-03 19:42:35 UTC Comment hidden (obsolete)
Comment 13 Ivan Dziuba 2021-04-21 21:29:11 UTC
Created attachment 119979 [details] [review]
Bug 27113: ElasticSearch: Autocomplete in input search

Bug 27113: ElasticSearch: Autocomplete in input search. Fix bugs.

Bug 27113: ElasticSearch: Autocomplete in input search. Use Unicode::Normalize instead Text::Unaccent.
Comment 14 Owen Leonard 2021-04-22 12:29:28 UTC
This is very cool, and is working well in the OPAC. I don't think you need to add a separate CSS file. I think it should be incorporated into the main one. It's not a large enough addition to necessitate a separate file.

I wonder if we should have an entry at the end of the autocomplete menu every time which the user can click on to submit the string they've entered? For instance, I type "dogs," and the autocomplete shows the first 10 results. A last menu item could say, "See full search results for <dogs>" or something like that?

This patch adds the feature to the OPAC and staff interface but only adds one preference. I think it should be separated into two, one for each interface.

It doesn't work for me in the staff interface. There is an error:

"Could not compile /kohadevbox/koha/api/elasticsearch/autocomplete.pl: Can't locate Switch.pm in @INC (you may need to install the Switch module)"

There are many errors flagged by the QA tool.

There are instances of "console.log" in the code which must be removed.
Comment 15 Blou 2021-04-22 13:06:27 UTC
Good idea on the button.  But I asked Ivan to focus on feature at hand and everything else can be a new BZ.  If he gets it through, it'll be easy to add on afterward.

For the .css, you mean merging autocomplete.css into opac.css ?  Couldn't that be a RM thing at the end, if really needed?  Otherwise that makes the rebasing very painful. Ivan is certainly not at ease with that demand.
Comment 16 Owen Leonard 2021-04-22 13:48:54 UTC
(In reply to Blou from comment #15)

> For the .css, you mean merging autocomplete.css into opac.css ?

I mean adding the contents of autocomplete.css into opac.scss. I'd be happy to submit a follow-up for this part, converting the CSS to SCSS, if that would help.
Comment 17 Katrin Fischer 2021-04-22 19:34:32 UTC
(In reply to Blou from comment #15)
> Good idea on the button.  But I asked Ivan to focus on feature at hand and
> everything else can be a new BZ.  If he gets it through, it'll be easy to
> add on afterward.
> 
> For the .css, you mean merging autocomplete.css into opac.css ?  Couldn't
> that be a RM thing at the end, if really needed?  Otherwise that makes the
> rebasing very painful. Ivan is certainly not at ease with that demand.

The RM will generate the scss files, you don't need to submit those. But they won't merge CSS changes into the css files. But the risk of conflict is not usually high there.
Comment 18 Ivan Dziuba 2021-04-22 21:33:30 UTC Comment hidden (obsolete)
Comment 19 Ivan Dziuba 2021-04-22 21:35:14 UTC
Created attachment 120029 [details] [review]
Bug 27113: ElasticSearch: Autocomplete in input search

Usually the user knows only part of the title of the book or only the name of the author, etc. When he start search something Koha (ElasticSearch) predicts the rest of a word or expression which user is typing.  Autocomplete predicts that thanks to index of ElasticSearch.

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also).
Comment 20 Owen Leonard 2021-04-23 12:44:13 UTC
Created attachment 120084 [details] [review]
Bug 27113: (follow-up) Move new CSS to main SCSS files

This patch removes the separate CSS files added for the autocomplete
feature and puts them into the "main" SCSS files. I think a separate
file isn't necessary because the amount of CSS it adds is so small.

I've also tweaked the style of the autocomplete menu shown when you use
the arrow keys to navigate through the autocomplete choices. I think the
previous white-on-light-green didn't have enough contrast.

To test, follow the previous test plan, being careful to observe how it
works when the autocomplete menu has been triggered and you use the
arrow keys to navigate through the results.
Comment 21 Owen Leonard 2021-04-23 12:50:49 UTC
Created attachment 120088 [details]
GIF showing the buggy behavior

I have to mark this "Failed QA" again because the QA tool issues were not all addressed. Please run the QA tool and fix the problems it raises.

The only other thing I noticed in testing today was that there is a change in behavior of the search tabs in the staff client. The layout is off, and the focus doesn't move to the input field like it does on the other tabs. See the attached screen grab.
Comment 22 Ivan Dziuba 2021-04-23 13:51:01 UTC
(In reply to Owen Leonard from comment #21)
> Created attachment 120088 [details]
> GIF showing the buggy behavior
> 
> I have to mark this "Failed QA" again because the QA tool issues were not
> all addressed. Please run the QA tool and fix the problems it raises.
> 
> The only other thing I noticed in testing today was that there is a change
> in behavior of the search tabs in the staff client. The layout is off, and
> the focus doesn't move to the input field like it does on the other tabs.
> See the attached screen grab.

You have changed/moved CSS files and it provoke your problem with the layout (it was not good idea). Katrin Fischer gave you notice of that!  
Thanks!
Comment 23 Owen Leonard 2021-04-23 14:08:39 UTC
(In reply to Ivan Dziuba from comment #22)
> You have changed/moved CSS files and it provoke your problem with the layout

Okay, but the issue with the input field focus is the same without my follow-up.
Comment 24 Owen Leonard 2021-04-23 14:13:14 UTC
(In reply to Owen Leonard from comment #21)
> The layout is off

My mistake--I recorded that screengrab without having rebuilt the CSS from SCSS. I will modify the test plan to make that part clear.
Comment 25 Owen Leonard 2021-04-23 14:13:32 UTC
Created attachment 120109 [details] [review]
Bug 27113: (follow-up) Move new CSS to main SCSS files

This patch removes the separate CSS files added for the autocomplete
feature and puts them into the "main" SCSS files. I think a separate
file isn't necessary because the amount of CSS it adds is so small.

I've also tweaked the style of the autocomplete menu shown when you use
the arrow keys to navigate through the autocomplete choices. I think the
previous white-on-light-green didn't have enough contrast.

To test you must rebuild the OPAC and staff client CSS:
https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client

Follow the previous test plan, being careful to observe how it
works when the autocomplete menu has been triggered and you use the
arrow keys to navigate through the results.
Comment 26 Ivan Dziuba 2021-04-23 21:24:50 UTC
Created attachment 120140 [details] [review]
Bug 27113: ElasticSearch: Autocomplete in input search

Usually the user knows only part of the title of the book or only the name of the author, etc. When he start search something Koha (ElasticSearch) predicts the rest of a word or expression which user is typing.  Autocomplete predicts that thanks to index of ElasticSearch.

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also).
Comment 27 Ivan Dziuba 2021-04-23 21:24:54 UTC
Created attachment 120141 [details] [review]
Bug 27113: (follow-up) QA Tests

Bug 27113: (follow-up) QA Tests (2)
Comment 28 Katrin Fischer 2021-04-24 10:23:24 UTC
> 
> You have changed/moved CSS files and it provoke your problem with the layout
> (it was not good idea). Katrin Fischer gave you notice of that!  
> Thanks!

I think I am the one who messed up here. Owen is our GUI expert, what he says goes :) I think where I got confused was the file endings. It should have been: RM does generate .css files, but you should commit the .scss changes.

(Basically, the fiels that are minified and generated - don't submit them, but the normal ones you need to).

Owen, please feel free to correct if I got it not quite right again.
Comment 29 Ivan Dziuba 2021-05-10 19:27:38 UTC
Created attachment 120806 [details] [review]
Bug 27113: (follow-up) QA Tests

Bug 27113: (follow-up) QA Tests (2)

Bug 27113: (follow-up) Fixup warnings.

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also).
Comment 30 Ivan Dziuba 2021-05-28 20:44:37 UTC
Created attachment 121497 [details] [review]
I have changed CGI::Session->new() from CGI::Session->load() for use session. This condition - 'CGI::Session->load() or die CGI::Session->errstr();'

I am not sure that was good idea to use 'die' in this condition...What do you think? 
Thanks!


Bug 27113: (follow-up) Correction with Session

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also)
Comment 31 Ivan Dziuba 2021-05-28 20:49:07 UTC
Created attachment 121498 [details] [review]
Bug 27113: (follow-up) Correction with Session

I have changed CGI::Session->new() from CGI::Session->load() for use session. This condition - 'CGI::Session->load() or die CGI::Session->errstr();'

I am not sure that was good idea to use 'die' in this condition...What do you think?
Thanks!
Comment 32 David Nind 2021-05-29 23:14:37 UTC
Patch no longer applies 8-(...

Bug 27113 - ElasticSearch: Autocomplete in input search

120109 - Bug 27113: (follow-up) Move new CSS to main SCSS files
120140 - Bug 27113: ElasticSearch: Autocomplete in input search
120806 - Bug 27113: (follow-up) QA Tests
121498 - Bug 27113: (follow-up) Correction with Session

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 27113: (follow-up) Move new CSS to main SCSS files
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/css/elasticsearch/autocomplete.css).
error: could not build fake ancestor
Patch failed at 0001 Bug 27113: (follow-up) Move new CSS to main SCSS files
Comment 33 Ivan Dziuba 2021-05-31 16:07:58 UTC
(In reply to David Nind from comment #32)
> Patch no longer applies 8-(...
> 
> Bug 27113 - ElasticSearch: Autocomplete in input search
> 
> 120109 - Bug 27113: (follow-up) Move new CSS to main SCSS files
> 120140 - Bug 27113: ElasticSearch: Autocomplete in input search
> 120806 - Bug 27113: (follow-up) QA Tests
> 121498 - Bug 27113: (follow-up) Correction with Session
> 
> Apply? [(y)es, (n)o, (i)nteractive] y
> Applying: Bug 27113: (follow-up) Move new CSS to main SCSS files
> error: sha1 information is lacking or useless
> (koha-tmpl/intranet-tmpl/prog/css/elasticsearch/autocomplete.css).
> error: could not build fake ancestor
> Patch failed at 0001 Bug 27113: (follow-up) Move new CSS to main SCSS files

I checked it and it works :). Do you use Koha 21.05? Thanks.

Bug 27113 - ElasticSearch: Autocomplete in input search

120109 - Bug 27113: (follow-up) Move new CSS to main SCSS files
120140 - Bug 27113: ElasticSearch: Autocomplete in input search
120806 - Bug 27113: (follow-up) QA Tests
121498 - Bug 27113: (follow-up) Correction with Session

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 27113: (follow-up) Move new CSS to main SCSS files
Using index info to reconstruct a base tree...
A       koha-tmpl/intranet-tmpl/prog/css/elasticsearch/autocomplete.css
M       koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc
A       koha-tmpl/opac-tmpl/bootstrap/css/opac-elasticsearch/opac-autocomplete.css
M       koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
M       koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
Applying: Bug 27113: ElasticSearch: Autocomplete in input search
Applying: Bug 27113: (follow-up) QA Tests
Applying: Bug 27113: (follow-up) Correction with Session
Comment 34 Tomás Cohen Arazi 2021-06-01 15:53:20 UTC
I agree with Fridolin: we should think of adding API routes for this. There should be a privileged route, and an unprivileged route.
You shouldn't worry about sessions and all. Just add the required permissions to interact with the routes.
Comment 35 Blou 2021-06-01 18:14:30 UTC
I'm sorry Tomas, this really muddied the picture for Ivan, and I can't help him as I've no idea how he should proceed.

Do you mean his 
api/elasticsearch/elasticsearch.pl should be put under /v1/ and the routes added to swagger, etc...   Any wiki to help him there?

We had hope this would get into 21.05 initially, but right now it looks like Ivan will recode it for the foreseable future.  Which won't happen, I'll give him one more go at it then drop it.
Comment 36 David Cook 2021-06-02 03:39:53 UTC
(In reply to Tomás Cohen Arazi from comment #34)
> I agree with Fridolin: we should think of adding API routes for this. There
> should be a privileged route, and an unprivileged route.
> You shouldn't worry about sessions and all. Just add the required
> permissions to interact with the routes.

Yeah, I don't get why session data is being used here. 

I don't even know if there needs to be 2 routes. Maybe just 1 public/unprivileged API route.
Comment 37 David Cook 2021-06-02 03:40:39 UTC
Also, it's not clear from the test plan what the user is expected to see/do in terms of the autocomplete...
Comment 38 David Nind 2021-06-02 21:56:46 UTC
(In reply to Ivan Dziuba from comment #33)
..
> I checked it and it works :). Do you use Koha 21.05? Thanks.
..

Hi Ivan. I'm using master - I tried again and still get the same error for the CSS follow-up patch:

..
Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 27113: (follow-up) Move new CSS to main SCSS files
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/css/elasticsearch/autocomplete.css).
error: could not build fake ancestor
Patch failed at 0001 Bug 27113: (follow-up) Move new CSS to main SCSS files
..

I can get the patches to apply if I use interactive, comment out the CSS follow-up patch, then apply again using interactive and then just apply the CSS follow-up patch.

I'm assuming the error is because the follow-up CSS is being applied first, but what it is changing doesn't exist yet as it is one of the later patches.

I don't know enough about patches to fix this up, but will try and test with what I have.
Comment 39 David Nind 2021-06-03 00:50:43 UTC
Created attachment 121555 [details] [review]
Bug 27113: ElasticSearch: Autocomplete in input search

Usually the user knows only part of the title of the book or only the name of the author, etc. When he start search something Koha (ElasticSearch) predicts the rest of a word or expression which user is typing.  Autocomplete predicts that thanks to index of ElasticSearch.

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also).

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 David Nind 2021-06-03 00:50:50 UTC
Created attachment 121556 [details] [review]
Bug 27113: (follow-up) QA Tests

Bug 27113: (follow-up) QA Tests (2)

Bug 27113: (follow-up) Fixup warnings.

TEST PLAN
Important! In this patch we need to do reindex ElasticSearch. ElasticSearch must have all information in his index.
1. Go Intranet -> Preference -> SearchEngine -> ElasticSearch
!! APPLY PATCH !!
2. Mapping is good (Intranet -> Catalog -> Search engine configuration (Elasticsearch) ).
!Recommended 'Reset Mapping'  -> 'Yes'
3. In your koha-conf.xml file you must have good <index_name> for <elasticsearch> and version ES
4. Update Preference:
./installer/data/mysql/updatedatabase.pl
If that passe good you can look the lines:
 - DEV atomic update: bug_27113-elasticsearch_autocomplete_input_search.perl
 - Upgrade to XXX done : Bug 27113 - Autocomplete input on main page with elasticsearch
5. After that we can look two options in the preferences:
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
4. For add information in the index we must run script for reindexing:
./misc/search_tools/rebuild_elasticsearch.pl -v -d
5. Waiting for the end of indexing
6. Go on Preference and find :
-  IntranetAutocompleteElasticSearch;
-  OPACAutocompleteElasticSearch;
Value "Show" turn on autocomplete.
7. Now we have Autocomplete for Intranet/OPAC search input (advanced search also).

Signed-off-by: David Nind <david@davidnind.com>
Comment 41 David Nind 2021-06-03 00:50:57 UTC
Created attachment 121557 [details] [review]
Bug 27113: (follow-up) Correction with Session

I have changed CGI::Session->new() from CGI::Session->load() for use session. This condition - 'CGI::Session->load() or die CGI::Session->errstr();'

I am not sure that was good idea to use 'die' in this condition...What do you think?
Thanks!

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 David Nind 2021-06-03 00:51:04 UTC
Created attachment 121558 [details] [review]
Bug 27113: (follow-up) Move new CSS to main SCSS files

This patch removes the separate CSS files added for the autocomplete
feature and puts them into the "main" SCSS files. I think a separate
file isn't necessary because the amount of CSS it adds is so small.

I've also tweaked the style of the autocomplete menu shown when you use
the arrow keys to navigate through the autocomplete choices. I think the
previous white-on-light-green didn't have enough contrast.

To test you must rebuild the OPAC and staff client CSS:
https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client

Follow the previous test plan, being careful to observe how it
works when the autocomplete menu has been triggered and you use the
arrow keys to navigate through the results.

Signed-off-by: David Nind <david@davidnind.com>
Comment 43 David Nind 2021-06-03 00:55:36 UTC
I finally got this to work for me, and it is a really nice feature. Nice work!

I've signed off, but these are my suggestions for the wording for the new system preferences - the QA Team will probably comment on this is as well, so you may as well get ahead of them!:

1. IntranetAutocompleteElasticSearch: change system preference name to StaffInterfaceAutocompleteElasticsearch - We are using 'staff interface' now, instead of 'intranet' - not sure what the protocol is for system preference names, but for new system preferences we should probably start using StaffInterface; Elasticsearch doesn't have a capital S (see for example the SearchEngine system preference)

2. Wording for IntranetAutocompleteElasticSearch: 
   . Currently: Don't show/Show looking terms based on a provided text by using an ElasticSearch for Intranet. 
   . Suggested: Don't show/Show search suggestions in the staff interface when using Elasticsearch. 

3. Wording for OPACAutocompleteElasticSearch: 
   . Currently: Don't show/Show looking terms based on a provided text by using an ElasticSearch for OPAC. 
   . Change to: Don't show/Show search suggestions in the OPAC when using Elasticsearch.
   
Other testing notes (using koha-testing-docker):
. Package command to rebuild the index: koha-elasticsearch -d --rebuild -a -b kohadev
. Needed to change .env file as a result of recent commits to koha-testing-docker for Elasticsearch (compared default with custom .env file and added new variables)
. As indicated earlier, had to apply patch interactively: 1st excluding CSS patch, then again to include CSS patch
. Rebuild CSS as per CSS patch
Comment 44 David Cook 2021-06-03 01:42:45 UTC
I am tempted to mark this as Failed QA, since I'm pretty sure the session handling doesn't work as intended (I think it's saving the session data in a file rather than in the database), but I haven't actually tested it, so leaving it as is.
Comment 45 Katrin Fischer 2021-06-03 09:46:15 UTC
As QA: I really like David's suggestions for the rephrasing. I think it makes the use much more clear. But before spending time on these, we probably need to sort out the other requirements/comments.

Sessions: 
* David, can you be a bit more precise about what you think doesn't work?
* Tomas, can you help with what you imagined and maybe the sessions bit? Does this need to be fixed here or could it be a follow-up?

(In reply to Tomás Cohen Arazi from comment #34)
> I agree with Fridolin: we should think of adding API routes for this. There
> should be a privileged route, and an unprivileged route.
> You shouldn't worry about sessions and all. Just add the required
> permissions to interact with the routes.
Comment 46 Blou 2021-06-03 12:39:42 UTC
Ivan has already recoded it without sessions in our codebase.  But before pushing that change, we'll be waiting for Tomas' instructions regarding the svc/
Comment 47 Katrin Fischer 2021-06-03 12:41:38 UTC
(In reply to Blou from comment #46)
> Ivan has already recoded it without sessions in our codebase.  But before
> pushing that change, we'll be waiting for Tomas' instructions regarding the
> svc/

Hi Blou, thx for the update! So we can scratch that off the list. I'll ask Tomas to have another look.
Comment 48 David Cook 2021-06-03 23:57:52 UTC
(In reply to Blou from comment #46)
> Ivan has already recoded it without sessions in our codebase.  But before
> pushing that change, we'll be waiting for Tomas' instructions regarding the
> svc/

I'll mostly leave it to Tomas, but it might be worth looking over https://wiki.koha-community.org/wiki/Rest_Api_HowTo

Basically, the "svc/" API scripts are legacy, so we don't want to be adding new ones. We just want to build up the Mojolicious-based REST API.
Comment 49 Tomás Cohen Arazi 2021-06-08 19:27:32 UTC
(In reply to Blou from comment #46)
> Ivan has already recoded it without sessions in our codebase.  But before
> pushing that change, we'll be waiting for Tomas' instructions regarding the
> svc/

I think this is a subject worth discussing in the open, i.e. in koha-devel.

As I mentioned (not sure where) I believe we could try adding some filtering capabilities to GET /biblios to accomplish this. I proposed something similar on bug 25870. In that case, I proposed a 'q_ccl' query param that could take a CCL query so we could accept complex queries. The route would then extract the ids (biblionumbers) and return Koha::Biblios->to_api objects as required.

I don't think CCL is a cool feature, but it felt like good start: allowing people to query records through the API.

The ES syntax might be a high constraint for the feature (leaving Zebra users out), specially if you are querying things like title, author, etc. But that's another possibility: add a query parameter that only works with ES (q_es?).

Regarding routes, you need to think what the response would look like. And it will help you figure what you want. Is it ok/enough for GET /biblios?<filter> to return JSON representation of the biblio+biblioitems table columns? Unfortunately, the OpenAPI v2.0 schema doesn't provide a way to define different response schemas for different content-type responses, and thus we left GET /biblios/:biblio_id under-documented (it is supported on v3) but you can try

GET /biblios/1

and see the response to get an idea of what we already return [1].

It feels like we need to build a good query language, and reuse this routes.

[1] This is how we build the JSON representation of a biblio https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Biblio.pm#L867-L892
Comment 50 Ivan Dziuba 2021-06-14 19:39:02 UTC
(In reply to Tomás Cohen Arazi from comment #49)
> (In reply to Blou from comment #46)
> > Ivan has already recoded it without sessions in our codebase.  But before
> > pushing that change, we'll be waiting for Tomas' instructions regarding the
> > svc/
> 
> I think this is a subject worth discussing in the open, i.e. in koha-devel.
> 
> As I mentioned (not sure where) I believe we could try adding some filtering
> capabilities to GET /biblios to accomplish this. I proposed something
> similar on bug 25870. In that case, I proposed a 'q_ccl' query param that
> could take a CCL query so we could accept complex queries. The route would
> then extract the ids (biblionumbers) and return Koha::Biblios->to_api
> objects as required.
> 
> I don't think CCL is a cool feature, but it felt like good start: allowing
> people to query records through the API.
> 
> The ES syntax might be a high constraint for the feature (leaving Zebra
> users out), specially if you are querying things like title, author, etc.
> But that's another possibility: add a query parameter that only works with
> ES (q_es?).
> 
> Regarding routes, you need to think what the response would look like. And
> it will help you figure what you want. Is it ok/enough for GET
> /biblios?<filter> to return JSON representation of the biblio+biblioitems
> table columns? Unfortunately, the OpenAPI v2.0 schema doesn't provide a way
> to define different response schemas for different content-type responses,
> and thus we left GET /biblios/:biblio_id under-documented (it is supported
> on v3) but you can try
> 
> GET /biblios/1
> 
> and see the response to get an idea of what we already return [1].
> 
> It feels like we need to build a good query language, and reuse this routes.
> 
> [1] This is how we build the JSON representation of a biblio
> https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/
> Biblio.pm#L867-L892

I don't understand what is it means `It feels like we need to build a good query language, and reuse these routes.`? 
Here I use dynamic queries $q, $prefix and a kind of "$token" to understand for which query hints were returned and whether to show or skip them. (depends on the speed of writing).

Could you please clarify the structure of URL for that.

Thanks!