Bug 19481 - Elasticsearch - Set default fields for sorting in mappings.yaml
Summary: Elasticsearch - Set default fields for sorting in mappings.yaml
Status: Pushed to Master
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: Julian Maurice
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 19482
  Show dependency treegraph
 
Reported: 2017-10-18 10:31 UTC by Nick Clemens
Modified: 2017-11-26 16:26 UTC (History)
6 users (show)

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


Attachments
Bug 19481 - Set default sort options as sortable in ES default mappings (2.76 KB, patch)
2017-10-18 10:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19481 - Set default sort options as sortable in ES default mappings (2.82 KB, patch)
2017-10-27 19:43 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19481 - Set default sort options as sortable in ES default mappings (2.89 KB, patch)
2017-11-20 08:08 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-10-18 10:31:23 UTC
Currently sorting options are hardcoded, some of the options are not defined as sortable in the default ES mappings leading to search errors
Comment 1 Nick Clemens 2017-10-18 10:33:56 UTC
Created attachment 68241 [details] [review]
Bug  19481 - Set default sort options as sortable in ES default mappings

To test:
1 - Setup Koha with ES
2 - perform a search
3 - try sorting by pubdate, acqdate, popularity, or title
4 - Search fails!
5 - Apply patch
6 - visit:
/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
7 - Re-index ES
8 - Perform search
9 - All sorting options should work
Comment 2 Fridolin SOMERS 2017-10-18 13:43:03 UTC
For UNIMARC, mapping for sorting by popularity is not really defined.
We could use 942$0 like in MARC21.
Comment 3 David Bourgault 2017-10-27 19:43:00 UTC
Created attachment 68788 [details] [review]
Bug 19481 - Set default sort options as sortable in ES default mappings

To test:
1 - Setup Koha with ES
2 - perform a search
3 - try sorting by pubdate, acqdate, popularity, or title
4 - Search fails!
5 - Apply patch
6 - visit:
/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
7 - Re-index ES
8 - Perform search
9 - All sorting options should work

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 4 David Bourgault 2017-10-27 19:44:25 UTC
Test worked as planned, however sorting by title and author seemed strange. I assume the issue wasn't introduced by this bug, but looking at the results list, it did not seem alphabetical to me.
Comment 5 Nick Clemens 2017-11-05 02:04:36 UTC
(In reply to David Bourgault from comment #4)
> Test worked as planned, however sorting by title and author seemed strange.
> I assume the issue wasn't introduced by this bug, but looking at the results
> list, it did not seem alphabetical to me.

See bug 19581 and bug 18827 for ordering issues
Comment 6 Julian Maurice 2017-11-10 10:34:11 UTC
Patch works as described (except for popularity when in UNIMARC, since the field is not defined).
But I wonder about 6th step in test plan. Shouldn't it be done in an updatedatabase script ?
Comment 7 Nick Clemens 2017-11-17 17:27:46 UTC
(In reply to Julian Maurice from comment #6)
> Patch works as described (except for popularity when in UNIMARC, since the
> field is not defined).
> But I wonder about 6th step in test plan. Shouldn't it be done in an
> updatedatabase script ?

I am not sure - if someone has custom mappings setup for some reasonn I don't think we want to alter them.

I think i would prefer to follow up on 19542 and add a warning if standard sort fields are not defined. Does that sound reasonable?
Comment 8 Julian Maurice 2017-11-20 08:07:48 UTC
(In reply to Nick Clemens from comment #7)
> I am not sure - if someone has custom mappings setup for some reasonn I
> don't think we want to alter them.
> 
> I think i would prefer to follow up on 19542 and add a warning if standard
> sort fields are not defined. Does that sound reasonable?
Ok
Comment 9 Julian Maurice 2017-11-20 08:08:01 UTC
Created attachment 69224 [details] [review]
Bug 19481 - Set default sort options as sortable in ES default mappings

To test:
1 - Setup Koha with ES
2 - perform a search
3 - try sorting by pubdate, acqdate, popularity, or title
4 - Search fails!
5 - Apply patch
6 - visit:
/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
7 - Re-index ES
8 - Perform search
9 - All sorting options should work

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 10 Jonathan Druart 2017-11-26 16:26:22 UTC
Pushed to master for 17.11, thanks to everybody involved!