Bugzilla – Attachment 139827 Details for
Bug 29632
Callnumber sorting is incorrect in Elasticsearch
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29632: [21.11.X] Don't sort cn-sort numerically
Bug-29632-2111X-Dont-sort-cn-sort-numerically.patch (text/plain), 8.95 KB, created by
Nick Clemens (kidclamp)
on 2022-08-26 13:02:36 UTC
(
hide
)
Description:
Bug 29632: [21.11.X] Don't sort cn-sort numerically
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-08-26 13:02:36 UTC
Size:
8.95 KB
patch
obsolete
>From fa291f68d826dc540818b252b47e6a34fbeb7920 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 28 Jun 2022 13:47:45 +0000 >Subject: [PATCH] Bug 29632: [21.11.X] Don't sort cn-sort numerically > >When defining our sort fields in we defined all as 'numeric' > >For other string containing numbers this is likely correct, however, >for callnumbers it is not. e.g. E45 should sort before E7 > >This patch adds a new 'callnumber' type and deifnes this for cn-sort and >adds to the field maping a sort without numeric set > >To test: >0 - Be using ES with Koha >1 - On records with single item, add callnumbers: > VA65 E7 R63 1984 > VA65 E7 T35 1990 > VA65 E45 R67 1985 >2 - Add public note 'shrimp' or something to make them easily searchable as a group >3 - Search for 'shrimp', sort by callnumber >4 - Note E45 comes last, it should come first >5 - Apply patch >6 - Reset ES mappings >7 - Reindex ES >8 - Repeat search >9 - Sorting should be correct when set to callnumber > >Signed-off-by: David Nind <david@davidnind.com> >Signed-off-by: Michal Urban <michalurban177@gmail.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Bug 29632: Unit tests > >This patch adds unit tests, as well as changing existing test to use a mock >and read the data as passed in tests, rather than relying on what exists in >the DB > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Bug 29632: (QA follow-up) Add ENUM value to kohastructure.sql :-D > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > >Bug 29632: (QA follow-up) Fix COMMENT discrepancy on upgrade > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/SearchEngine/Elasticsearch.pm | 2 + > .../elasticsearch/field_config.yaml | 4 ++ > .../searchengine/elasticsearch/mappings.yaml | 2 +- > .../data/mysql/atomicupdate/bug_29632.pl | 16 ++++++ > installer/data/mysql/kohastructure.sql | 2 +- > .../searchengine/elasticsearch/mappings.tt | 5 ++ > .../Koha/SearchEngine/Elasticsearch.t | 51 +++++++++++++++++-- > 7 files changed, 76 insertions(+), 6 deletions(-) > create mode 100755 installer/data/mysql/atomicupdate/bug_29632.pl > >diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm >index 1cf8c0121b..d2bfbd5b79 100644 >--- a/Koha/SearchEngine/Elasticsearch.pm >+++ b/Koha/SearchEngine/Elasticsearch.pm >@@ -209,6 +209,8 @@ sub get_elasticsearch_mappings { > $es_type = 'stdno'; > } elsif ($type eq 'year') { > $es_type = 'year'; >+ } elsif ($type eq 'callnumber') { >+ $es_type = 'cn_sort'; > } > > if ($search) { >diff --git a/admin/searchengine/elasticsearch/field_config.yaml b/admin/searchengine/elasticsearch/field_config.yaml >index d78bdf06dd..cf90482aa2 100644 >--- a/admin/searchengine/elasticsearch/field_config.yaml >+++ b/admin/searchengine/elasticsearch/field_config.yaml >@@ -71,3 +71,7 @@ sort: > type: icu_collation_keyword > index: false > numeric: true >+ cn_sort: >+ type: icu_collation_keyword >+ index: false >+ numeric: false >diff --git a/admin/searchengine/elasticsearch/mappings.yaml b/admin/searchengine/elasticsearch/mappings.yaml >index 28429878f9..4f40e9b0c8 100644 >--- a/admin/searchengine/elasticsearch/mappings.yaml >+++ b/admin/searchengine/elasticsearch/mappings.yaml >@@ -1167,7 +1167,7 @@ biblios: > marc_type: marc21 > sort: 1 > suggestible: '' >- type: '' >+ type: 'callnumber' > cn-suffix: > label: cn-suffix > mappings: >diff --git a/installer/data/mysql/atomicupdate/bug_29632.pl b/installer/data/mysql/atomicupdate/bug_29632.pl >new file mode 100755 >index 0000000000..d7a671ae9d >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_29632.pl >@@ -0,0 +1,16 @@ >+use Modern::Perl; >+ >+return { >+ bug_number => "29632", >+ description => "Add callnumber type to allow sorting", >+ up => sub { >+ my ($args) = @_; >+ my ($dbh, $out) = @$args{qw(dbh out)}; >+ $dbh->do(q{ >+ ALTER TABLE `search_field` MODIFY COLUMN `type` >+ enum('','string','date','number','boolean','sum','isbn','stdno','year','callnumber') NOT NULL >+ COMMENT 'what type of data this holds, relevant when storing it in the search engine' >+ }); >+ say $out "Add callnumber to search_field type enum"; >+ }, >+}; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index a02bd3b724..bb995dec28 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -4483,7 +4483,7 @@ CREATE TABLE `search_field` ( > `id` int(11) NOT NULL AUTO_INCREMENT, > `name` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'the name of the field as it will be stored in the search engine', > `label` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'the human readable name of the field, for display', >- `type` enum('','string','date','number','boolean','sum','isbn','stdno','year') COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine', >+ `type` enum('','string','date','number','boolean','sum','isbn','stdno','year','callnumber') COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'what type of data this holds, relevant when storing it in the search engine', > `weight` decimal(5,2) DEFAULT NULL, > `facet_order` tinyint(4) DEFAULT NULL COMMENT 'the order place of the field in facet list if faceted', > `staff_client` tinyint(1) NOT NULL DEFAULT 1, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >index 417b533b6a..a0d5ded9d7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt >@@ -210,6 +210,11 @@ a.add, a.delete { > [% ELSE %] > <option value="stdno">Std. Number</option> > [% END %] >+ [% IF search_field.type == "callnumber" %] >+ <option value="callnumber" selected="selected">Call Number</option> >+ [% ELSE %] >+ <option value="callnumber">Call Number</option> >+ [% END %] > </select> > </td> > <td data-order="[% search_field.weight | html %]"> >diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t >index 90e99ace39..1e309f77ac 100755 >--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t >+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch.t >@@ -127,14 +127,57 @@ subtest 'get_elasticsearch_settings() tests' => sub { > > subtest 'get_elasticsearch_mappings() tests' => sub { > >- plan tests => 1; >+ plan tests => 3; > > my $mappings; > >- # test reading mappings >- my $es = Koha::SearchEngine::Elasticsearch->new( {index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX} ); >- $mappings = $es->get_elasticsearch_mappings(); >+ my @mappings = ( >+ { >+ name => 'cn-sort', >+ type => 'callnumber', >+ facet => 0, >+ suggestible => 0, >+ searchable => 1, >+ sort => 1, >+ marc_type => 'marc21', >+ marc_field => '001', >+ }, >+ { >+ name => 'isbn', >+ type => 'string', >+ facet => 0, >+ suggestible => 0, >+ searchable => 1, >+ sort => 1, >+ marc_type => 'marc21', >+ marc_field => '020a', >+ }, >+ ); >+ my $search_engine_module = Test::MockModule->new('Koha::SearchEngine::Elasticsearch'); >+ $search_engine_module->mock('_foreach_mapping', sub { >+ my ($self, $sub) = @_; >+ >+ foreach my $map (@mappings) { >+ $sub->( >+ $map->{name}, >+ $map->{type}, >+ $map->{facet}, >+ $map->{suggestible}, >+ $map->{sort}, >+ $map->{searchable}, >+ $map->{marc_type}, >+ $map->{marc_field} >+ ); >+ } >+ }); >+ >+ my $search_engine_elasticsearch = Koha::SearchEngine::Elasticsearch::Search->new({ index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX }); >+ $mappings = $search_engine_elasticsearch->get_elasticsearch_mappings(); >+ >+ is( $mappings->{properties}{"cn-sort__sort"}{index}, 'false', 'Field mappings parsed correctly for sort for callnumber type' ); >+ is( $mappings->{properties}{"cn-sort__sort"}{numeric}, 'false', 'Field mappings parsed correctly for sort for callnumber type' ); > is( $mappings->{properties}{isbn__sort}{index}, 'false', 'Field mappings parsed correctly' ); >+ > }; > > subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' => sub { >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29632
:
136682
|
136754
|
137182
|
137539
|
137605
|
137606
|
137823
|
138611
| 139827