Bug 20385 - ElasticSearch authority search raises Software error
Summary: ElasticSearch authority search raises Software error
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Alex Arnaud
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17373 20509
  Show dependency treegraph
 
Reported: 2018-03-13 10:46 UTC by Nicolas Legrand
Modified: 2018-05-22 09:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20385 - Don't try to get info for bad authtypecodes (2.09 KB, patch)
2018-03-13 15:49 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20385 - Don't try to get info for bad authtypecodes (2.15 KB, patch)
2018-03-13 16:35 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 20385: (follow-up) Assign blank instead of inappropraite value (1.12 KB, patch)
2018-03-23 17:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20385 - Don't try to get info for bad authtypecodes (2.20 KB, patch)
2018-03-30 13:25 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 20385: (follow-up) Assign blank instead of inappropraite value (1.18 KB, patch)
2018-03-30 13:26 UTC, Alex Arnaud
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Legrand 2018-03-13 10:46:54 UTC
With UNIMARC authorities and ES 5.8, from authority home.

Default contains שלו (search entire record) None Submit ->

http://localhost:9999/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&authtypecode=&marclist=all&and_or=and&excluding=&operator=contains&value=%D7%A9%D7%9C%D7%95&orderby=

Software error:

Can't call method "auth_tag_to_report" on an undefined value at /home/koha/kohaclone/Koha/SearchEngine/Elasticsearch/Search.pm line 221.
Comment 1 Nick Clemens 2018-03-13 15:49:32 UTC
Created attachment 72802 [details] [review]
Bug 20385 - Don't try to get info for bad authtypecodes

TO test:
0 - Have a koha with ES enabled and some auth records
1 - backup your db
2 - sudo koha-mysql <<instance>>
3 - UPDATE auth_types SET authtypecode="PERSO_NAMB" WHERE
authtypecode="PERSO_NAME" (or adjust as necessary)
4 - Search for some authorities where a PERSO_NAME record will be
returned
5 - KA_BOOM!
6 - Apply patch
7 - Results are returned, they do not display a code, viewing record you
won't see code
Comment 2 Nicolas Legrand 2018-03-13 16:35:21 UTC
Created attachment 72810 [details] [review]
Bug 20385 - Don't try to get info for bad authtypecodes

TO test:
0 - Have a koha with ES enabled and some auth records
1 - backup your db
2 - sudo koha-mysql <<instance>>
3 - UPDATE auth_types SET authtypecode="PERSO_NAMB" WHERE
authtypecode="PERSO_NAME" (or adjust as necessary)
4 - Search for some authorities where a PERSO_NAME record will be
returned
5 - KA_BOOM!
6 - Apply patch
7 - Results are returned, they do not display a code, viewing record you
won't see code

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 3 Nicolas Legrand 2018-03-13 16:36:09 UTC
works great, thanks!
Comment 4 Alex Arnaud 2018-03-14 10:39:40 UTC
Comment on attachment 72810 [details] [review]
Bug 20385 - Don't try to get info for bad authtypecodes

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

::: Koha/SearchEngine/Elasticsearch/Search.pm
@@ +217,4 @@
>              # with the record. It's not documented why this is the case, so
>              # it's not reproduced here yet.
>              my $authtype           = $rs->single;
> +            my $auth_tag_to_report = $authtype ? $authtype->auth_tag_to_report : $authtypecode;

Works fine but i feel not confortable with the fact of putting an auth type code inside a variable supposed to contain a marc field tag. Why not just ''?
Comment 5 Marcel de Rooy 2018-03-23 06:38:36 UTC
(In reply to Alex Arnaud from comment #4)
> Comment on attachment 72810 [details] [review] [review]
> Bug 20385 - Don't try to get info for bad authtypecodes
> 
> Review of attachment 72810 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/SearchEngine/Elasticsearch/Search.pm
> @@ +217,4 @@
> >              # with the record. It's not documented why this is the case, so
> >              # it's not reproduced here yet.
> >              my $authtype           = $rs->single;
> > +            my $auth_tag_to_report = $authtype ? $authtype->auth_tag_to_report : $authtypecode;
> 
> Works fine but i feel not confortable with the fact of putting an auth type
> code inside a variable supposed to contain a marc field tag. Why not just ''?

Agreed. Failed QA
Comment 6 Nick Clemens 2018-03-23 17:13:03 UTC
Created attachment 73212 [details] [review]
Bug 20385: (follow-up) Assign blank instead of inappropraite value
Comment 7 Nick Clemens 2018-03-23 17:13:43 UTC
I think this was bad copy paste, removed the assignment
Comment 8 Jonathan Druart 2018-03-27 17:30:26 UTC
Lowering severity as it looks more like a configuration issue.
Comment 9 Alex Arnaud 2018-03-30 13:25:48 UTC
Created attachment 73479 [details] [review]
Bug 20385 - Don't try to get info for bad authtypecodes

TO test:
0 - Have a koha with ES enabled and some auth records
1 - backup your db
2 - sudo koha-mysql <<instance>>
3 - UPDATE auth_types SET authtypecode="PERSO_NAMB" WHERE
authtypecode="PERSO_NAME" (or adjust as necessary)
4 - Search for some authorities where a PERSO_NAME record will be
returned
5 - KA_BOOM!
6 - Apply patch
7 - Results are returned, they do not display a code, viewing record you
won't see code

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 10 Alex Arnaud 2018-03-30 13:26:30 UTC
Created attachment 73480 [details] [review]
Bug 20385: (follow-up) Assign blank instead of inappropraite value

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 11 Jonathan Druart 2018-04-02 19:46:31 UTC
We really need tests for these ES methods!
Comment 12 Jonathan Druart 2018-04-02 21:15:30 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 13 Nick Clemens 2018-05-14 21:23:54 UTC
Pushed to stable for 17.11.06

Awesome work all!
Comment 14 Fridolin SOMERS 2018-05-22 09:44:02 UTC
Not pushed to 17.05.x