From da26e8654b20a4d831c5d82c0d27126c621c333e Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 1 Jun 2022 14:40:49 +0000 Subject: [PATCH] Bug 30327: Add options for sorting components This patch adds two new sysprefs: ComponentSortField ComponentSortOrder These allow the user to choose how components should be sorted when displaying on the details page of a record, and the corresponding search for all components This also updates our search from simple_search_compat to search_compat to allow for sorting options Note: Some sorting under ES is unclear - this is a separate issue to be invesitgated Our Zebra index does not offer 'record number' sorting, I will file a bug for that To test: 1 - Enable UseControlNumber (or not) 2 - Add some components to a record by control number or title depending on above 3 - Enable ShowComponentRecords syspref 4 - View the record that has components 5 - Note they are not sorted 6 - Apply patch, updatedatabase 7 - reload record 8 - Note components are sorted by title ascending 9 - Try different values for ComponentSortField and ComponentSortOrder 10 - Confirm sorting changes with system preferences 11 - Repeat test on staff and opac, with ES and Zebra search engines Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- Koha/Biblio.pm | 22 +++++++---- catalogue/detail.pl | 4 +- .../bug_30327_add_sortComponents_syspref.pl | 16 ++++++++ installer/data/mysql/mandatory/sysprefs.sql | 2 + .../admin/preferences/cataloguing.pref | 16 ++++++++ .../prog/en/modules/catalogue/detail.tt | 2 +- .../bootstrap/en/modules/opac-detail.tt | 2 +- opac/opac-detail.pl | 4 +- t/db_dependent/Koha/Biblio.t | 37 ++++++++++++------- 9 files changed, 80 insertions(+), 25 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index b7431f5c26..52cc36f2d6 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -516,8 +516,8 @@ sub suggestions { my $components = $self->get_marc_components(); -Returns an array of MARCXML data, which are component parts of -this object (MARC21 773$w points to this) +Returns an array of search results data, which are component parts of +this object (MARC21 773 points to this) =cut @@ -526,19 +526,19 @@ sub get_marc_components { return [] if (C4::Context->preference('marcflavour') ne 'MARC21'); - my $searchstr = $self->get_components_query; + my ( $searchstr, $sort ) = $self->get_components_query; my $components; if (defined($searchstr)) { my $searcher = Koha::SearchEngine::Search->new({index => $Koha::SearchEngine::BIBLIOS_INDEX}); - my ( $error, $results, $total_hits ); + my ( $error, $results, $facets ); eval { - ( $error, $results, $total_hits ) = $searcher->simple_search_compat( $searchstr, 0, $max_results ); + ( $error, $results, $facets ) = $searcher->search_compat( $searchstr, undef, [$sort], ['biblioserver'], $max_results, 0, undef, undef, 'ccl', 0 ); }; if( $error || $@ ) { $error //= q{}; $error .= $@ if $@; - warn "Warning from simple_search_compat: '$error'"; + warn "Warning from search_compat: '$error'"; $self->add_message( { type => 'error', @@ -547,7 +547,7 @@ sub get_marc_components { } ); } - $components = $results if defined($results) && @$results; + $components = $results->{biblioserver}->{RECORDS} if defined($results) && $results->{biblioserver}->{hits}; } return $components // []; @@ -565,6 +565,7 @@ sub get_components_query { my $builder = Koha::SearchEngine::QueryBuilder->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); my $marc = $self->metadata->record; + my $sort = C4::Context->preference('ComponentSortField') . "_" . C4::Context->preference('ComponentSortOrder'); my $searchstr; if ( C4::Context->preference('UseControlNumber') ) { @@ -597,8 +598,13 @@ sub get_components_query { $cleaned_title = $builder->clean_search_term($cleaned_title); $searchstr = "Host-item:($cleaned_title)"; } + my ($error, $query_str) = $builder->build_query_compat( undef, [$searchstr], undef, undef, [$sort], 0 ); + if( $error ){ + warn $error; + return; + } - return $searchstr; + return ($query_str, $sort); } =head3 subscriptions diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 990d108cf3..459fe6433f 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -219,7 +219,9 @@ if ( $showcomp eq 'both' || $showcomp eq 'staff' ) { ); } $template->param( ComponentParts => $parts ); - $template->param( ComponentPartsQuery => $biblio->get_components_query ); + my ( $comp_query, $comp_sort ) = $biblio->get_components_query; + my $cpq = $comp_query . "&sort_by=" . $comp_sort; + $template->param( ComponentPartsQuery => $cpq ); } } else { # check if we should show analytics anyway $show_analytics = 1 if $marc_record && @{$biblio->get_marc_components(1)}; # count matters here, results does not diff --git a/installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl b/installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl new file mode 100755 index 0000000000..0cb8f16dac --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_30327_add_sortComponents_syspref.pl @@ -0,0 +1,16 @@ +use Modern::Perl; + +return { + bug_number => "30327", + description => "Add ComponentsSortField and ComponentsSortOrder sysprefs", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + $dbh->do(q{ + INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('ComponentsSortField','title','call_number|pubdate|acqdate|title|author','Specify the default field used for sorting','Choice'), + ('ComponentsSortOrder','asc','asc|dsc|az|za','Specify the default sort order','Choice') + }); + say $out "Added ComponentsSortField and ComponentsSortOrder sysprefs"; + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index d2c7ee3bec..0ea470bcd3 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -144,6 +144,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('CoceProviders', '', 'aws,gb,ol', 'Coce providers', 'multiple'), ('COinSinOPACResults','1','','If ON, use COinS in OPAC search results page. NOTE: this can slow down search response time significantly','YesNo'), ('CollapseFieldsPatronAddForm','',NULL,'Collapse these fields by default when adding a new patron. These fields can still be expanded.','Multiple'), +('ComponentsSortField','title','call_number|pubdate|acqdate|title|author','Specify the default field used for sorting','Choice'), +('ComponentsSortOrder','asc','asc|dsc|az|za','Specify the default sort order','Choice'), ('ConfirmFutureHolds','0','','Number of days for confirming future holds','Integer'), ('ConsiderOnSiteCheckoutsAsNormalCheckouts','1',NULL,'Consider on-site checkouts as normal checkouts','YesNo'), ('CreateAVFromCataloguing', '1', '', 'Ability to create authorized values from the cataloguing module', 'YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref index 41399d7c97..6f66761c37 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref @@ -280,6 +280,22 @@ Cataloging: - pref: MaxComponentRecords - "records will be displayed." - "
UNIMARC is not supported." + - By default, sort component results in the staff interface by + - pref: ComponentSortField + default: title + choices: + call_number: call number + pubdate: date of publication + acqdate: date added + title: title + author: author + - ',' + - pref: ComponentSortOrder + choices: + asc: ascending. + dsc: descending. + az: from A to Z. + za: from Z to A. Importing: - - When matching on ISBN with the record import tool, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index c24c861c03..1fae17084f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -707,7 +707,7 @@ Note that permanent location is a code, and location may be an authval. [% END %] [% IF ComponentParts.size == Koha.Preference('MaxComponentRecords')%] -

Only [% ComponentParts.size | html %] results are shown: show all component parts

+

Only [% ComponentParts.size | html %] results are shown: show all component parts

[% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt index 7f49236e42..f6756d6a59 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt @@ -600,7 +600,7 @@ [% END %] [% IF ComponentParts.size == Koha.Preference('MaxComponentRecords')%] -

Only [% ComponentParts.size | html %] results are shown: show all component parts

+

Only [% ComponentParts.size | html %] results are shown: show all component parts

[% END %] diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 000eac3a51..7219eef18e 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -650,7 +650,9 @@ if ( $showcomp eq 'both' || $showcomp eq 'opac' ) { ); } $template->param( ComponentParts => $parts ); - $template->param( ComponentPartsQuery => $biblio->get_components_query ); + my ( $comp_query, $comp_sort ) = $biblio->get_components_query; + my $cpq = $comp_query . "&sort_by=" . $comp_sort; + $template->param( ComponentPartsQuery => $cpq ); } } else { # check if we should show analytics anyway $show_analytics = 1 if @{$biblio->get_marc_components(1)}; # count matters here, results does not diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index fc8445545d..890d4e9b90 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -518,7 +518,7 @@ subtest 'get_marc_components() tests' => sub { my $host_biblio = Koha::Biblios->find($host_bibnum); t::lib::Mocks::mock_preference( 'SearchEngine', 'Zebra' ); my $search_mod = Test::MockModule->new( 'Koha::SearchEngine::Zebra::Search' ); - $search_mod->mock( 'simple_search_compat', \&search_component_record2 ); + $search_mod->mock( 'search_compat', \&search_component_record2 ); my $components = $host_biblio->get_marc_components; is( ref($components), 'ARRAY', 'Return type is correct' ); @@ -529,8 +529,8 @@ subtest 'get_marc_components() tests' => sub { '->get_marc_components returns an empty ARRAY' ); - $search_mod->unmock( 'simple_search_compat'); - $search_mod->mock( 'simple_search_compat', \&search_component_record1 ); + $search_mod->unmock( 'search_compat'); + $search_mod->mock( 'search_compat', \&search_component_record1 ); my $component_record = component_record1()->as_xml(); is_deeply( @@ -538,13 +538,13 @@ subtest 'get_marc_components() tests' => sub { [$component_record], '->get_marc_components returns the related component part record' ); - $search_mod->unmock( 'simple_search_compat'); + $search_mod->unmock( 'search_compat'); - $search_mod->mock( 'simple_search_compat', + $search_mod->mock( 'search_compat', sub { Koha::Exception->throw("error searching analytics") } ); warning_like { $components = $host_biblio->get_marc_components } - qr{Warning from simple_search_compat: .* 'error searching analytics'}; + qr{Warning from search_compat: .* 'error searching analytics'}; is_deeply( $host_biblio->object_messages, @@ -556,35 +556,46 @@ subtest 'get_marc_components() tests' => sub { } ] ); - $search_mod->unmock( 'simple_search_compat'); + $search_mod->unmock( 'search_compat'); $schema->storage->txn_rollback; }; subtest 'get_components_query' => sub { - plan tests => 3; + plan tests => 6; my $biblio = $builder->build_sample_biblio(); my $biblionumber = $biblio->biblionumber; my $record = $biblio->metadata->record; t::lib::Mocks::mock_preference( 'UseControlNumber', '0' ); - is($biblio->get_components_query, "Host-item:(Some boring read)", "UseControlNumber disabled"); + t::lib::Mocks::mock_preference( 'ComponentSortField', 'author' ); + t::lib::Mocks::mock_preference( 'ComponentSortOrder', 'za' ); + my ( $comp_q, $comp_s ) = $biblio->get_components_query; + is($comp_q, "Host-item:(Some boring read)",, "UseControlNumber disabled"); + is($comp_s, "author_za", "UseControlNumber disabled sort is correct"); t::lib::Mocks::mock_preference( 'UseControlNumber', '1' ); + t::lib::Mocks::mock_preference( 'ComponentSortOrder', 'az' ); my $marc_001_field = MARC::Field->new('001', $biblionumber); $record->append_fields($marc_001_field); C4::Biblio::ModBiblio( $record, $biblio->biblionumber ); $biblio = Koha::Biblios->find( $biblio->biblionumber); - is($biblio->get_components_query, "(rcn:$biblionumber AND (bib-level:a OR bib-level:b))", "UseControlNumber enabled without MarcOrgCode"); + ( $comp_q, $comp_s ) = $biblio->get_components_query; + is($comp_q, "(rcn:$biblionumber AND (bib-level:a OR bib-level:b))", "UseControlNumber enabled without MarcOrgCode"); + is($comp_s, "author_az", "UseControlNumber enabled without MarcOrgCode sort is correct"); my $marc_003_field = MARC::Field->new('003', 'OSt'); $record->append_fields($marc_003_field); C4::Biblio::ModBiblio( $record, $biblio->biblionumber ); $biblio = Koha::Biblios->find( $biblio->biblionumber); - is($biblio->get_components_query, "(((rcn:$biblionumber AND cni:OSt) OR rcn:\"OSt $biblionumber\") AND (bib-level:a OR bib-level:b))", "UseControlNumber enabled with MarcOrgCode"); + t::lib::Mocks::mock_preference( 'ComponentSortField', 'title' ); + t::lib::Mocks::mock_preference( 'ComponentSortOrder', 'asc' ); + ( $comp_q, $comp_s ) = $biblio->get_components_query; + is($comp_q, "(((rcn:$biblionumber AND cni:OSt) OR rcn:\"OSt $biblionumber\") AND (bib-level:a OR bib-level:b))", "UseControlNumber enabled with MarcOrgCode"); + is($comp_s, "title_asc", "UseControlNumber enabled with MarcOrgCode sort if correct"); }; subtest 'orders() and active_orders() tests' => sub { @@ -1014,12 +1025,12 @@ sub component_record1 { } sub search_component_record1 { my @results = ( component_record1()->as_xml() ); - return ( undef, \@results, 1 ); + return ( undef, { biblioserver => { RECORDS => \@results, hits => 1 } }, 1 ); } sub search_component_record2 { my @results; - return ( undef, \@results, 0 ); + return ( undef, { biblioserver => { RECORDS => \@results, hits => 0 } }, 0 ); } sub host_record { -- 2.30.2