From fa291f68d826dc540818b252b47e6a34fbeb7920 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 Signed-off-by: Michal Urban Signed-off-by: Martin Renvoize 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 Bug 29632: (QA follow-up) Add ENUM value to kohastructure.sql :-D Signed-off-by: Tomas Cohen Arazi Bug 29632: (QA follow-up) Fix COMMENT discrepancy on upgrade Signed-off-by: Tomas Cohen Arazi --- 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 %] [% END %] + [% IF search_field.type == "callnumber" %] + + [% ELSE %] + + [% END %] 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