From 5bc322941bc437673624e27da9c3967a59d9c3b5 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Wed, 19 Apr 2023 20:58:48 -1000 Subject: [PATCH] Bug 33569: Fix sort by relevance in catalog search When ordering catalog search by relevance from advanced search, this is not visible in sort order combobox in staff interface. This is because sort by relevance is a special case not using sort order. This patch add a new method C4::Context->default_catalog_sort_by to use in both staff interface and OPAC. Test plan : 1.0) On staff interface 1.1) Define system preferences defaultSortField = 'relevance' and defaultSortOrder = 'descending' 1.2) Perform a catalog search from main page on any term, ie 'perl' => You see in sort combobox 'relevance' selected (with a check icon) 1.3) Perform an advanced search on any term, ie 'perl', with sort by relevance => Without patch : you see in sort combobox 'relevance' not selected (no check icon) => With patch : you see in sort combobox 'relevance' selected (with a check icon) 2.0) On OPAC 2.1) Define system preferences OPACdefaultSortField = 'relevance' and OPACdefaultSortOrder = 'descending' 2.2) Perform a catalog search from main page on any term, ie 'perl' => You see in sort combobox 'relevance' selected 3.0) Run prove t/Context.t Signed-off-by: David Nind Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 3bb7ea2dab03863e827fbb21060ac98a16dcbf4f) Signed-off-by: Matt Blenkinsop --- C4/Context.pm | 32 +++++++++++++++++++ catalogue/search.pl | 18 +++-------- .../prog/en/modules/catalogue/results.tt | 2 +- opac/opac-detail.pl | 2 +- opac/opac-search.pl | 16 ++-------- t/Context.t | 32 ++++++++++++++++++- 6 files changed, 72 insertions(+), 30 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 3819529dc2..35c5025745 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -499,6 +499,38 @@ sub csv_delimiter { return $delimiter; } +=head2 default_catalog_sort_by + + $delimiter = C4::Context->default_catalog_sort_by; + + Returns default sort by for catalog search. + For relevance no sort order is used. + + For staff interface, depends on system preferences 'defaultSortField' and 'defaultSortOrder'. + For OPAC interface, depends on system preferences 'OPACdefaultSortField' and 'OPACdefaultSortOrder'. + +=cut + +sub default_catalog_sort_by { + my $self = shift; + my ( $sort_by, $sort_field, $sort_order ); + if ( C4::Context->interface eq 'opac' ) { + $sort_field = C4::Context->preference('OPACdefaultSortField'); + $sort_order = C4::Context->preference('OPACdefaultSortOrder'); + } else { + $sort_field = C4::Context->preference('defaultSortField'); + $sort_order = C4::Context->preference('defaultSortOrder'); + } + if ( $sort_field && $sort_order ) { + if ( $sort_field eq 'relevance' ) { + $sort_by = $sort_field; + } else { + $sort_by = $sort_field . '_' . $sort_order; + } + } + return $sort_by; +} + =head2 Zconn $Zconn = C4::Context->Zconn diff --git a/catalogue/search.pl b/catalogue/search.pl index bf225911ef..f3b5d7bdbc 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -259,6 +259,8 @@ $template->param(advancedsearchesloop => $advancedsearchesloop); $template->param( searchid => scalar $cgi->param('searchid'), ); +my $default_sort_by = C4::Context->default_catalog_sort_by; + # The following should only be loaded if we're bringing up the advanced search template if ( $template_type eq 'advsearch' ) { @@ -322,12 +324,8 @@ if ( $template_type eq 'advsearch' ) { $template->param(outer_sup_servers_loop => $secondary_servers_loop,); # set the default sorting - if ( C4::Context->preference('defaultSortField') - && C4::Context->preference('defaultSortOrder') ) { - my $default_sort_by = - C4::Context->preference('defaultSortField') . '_' - . C4::Context->preference('defaultSortOrder'); - $template->param( sort_by => $default_sort_by ); + if ($default_sort_by) { + $template->param( sort_by => $default_sort_by ); } $template->param(uc(C4::Context->preference("marcflavour")) =>1 ); @@ -361,14 +359,6 @@ my $params = $cgi->Vars; # sort by is used to sort the query # in theory can have more than one but generally there's just one my @sort_by; -my $default_sort_by; -if ( C4::Context->preference('defaultSortField') - && C4::Context->preference('defaultSortOrder') ) { - $default_sort_by = - C4::Context->preference('defaultSortField') . '_' - . C4::Context->preference('defaultSortOrder'); -} - @sort_by = $cgi->multi_param('sort_by'); $sort_by[0] = $default_sort_by unless $sort_by[0]; foreach my $sort (@sort_by) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt index adf0168650..1d7fae8377 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt @@ -224,7 +224,7 @@ Sort
    - [% IF ( sort_by == "relevance_dsc" ) %] + [% IF ( sort_by == "relevance" ) %]
  • Relevance
    • [% ELSE %]
  • Relevance
    • diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 98c4f94a8c..3c2b8df6ec 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -238,7 +238,7 @@ if ($session->param('busc')) { @servers = ("biblioserver") unless (@servers); my ($default_sort_by, @sort_by); - $default_sort_by = C4::Context->preference('OPACdefaultSortField')."_".C4::Context->preference('OPACdefaultSortOrder') if (C4::Context->preference('OPACdefaultSortField') && C4::Context->preference('OPACdefaultSortOrder')); + $default_sort_by = C4::Context->default_catalog_sort_by; @sort_by = @{$arrParamsBusc->{'sort_by'}} if $arrParamsBusc->{'sort_by'}; $sort_by[0] = $default_sort_by if !$sort_by[0] && defined($default_sort_by); my ($error, $results_hashref, $facets); diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 65904676d1..958f4ca623 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -304,6 +304,8 @@ foreach my $advanced_srch_type (@advanced_search_types) { } $template->param(advancedsearchesloop => \@advancedsearchesloop); +my $default_sort_by = C4::Context->default_catalog_sort_by; + # The following should only be loaded if we're bringing up the advanced search template if ( $template_type && $template_type eq 'advsearch' ) { # load the servers (used for searching -- to do federated searching, etc.) @@ -314,11 +316,7 @@ if ( $template_type && $template_type eq 'advsearch' ) { $template->param(outer_sup_servers_loop => $secondary_servers_loop,); # set the default sorting - if ( C4::Context->preference('OPACdefaultSortField') - && C4::Context->preference('OPACdefaultSortOrder') ) { - my $default_sort_by = - C4::Context->preference('OPACdefaultSortField') . '_' - . C4::Context->preference('OPACdefaultSortOrder'); + if ($default_sort_by) { $template->param( sort_by => $default_sort_by ); } @@ -378,14 +376,6 @@ for (keys %$params) { # sort by is used to sort the query # in theory can have more than one but generally there's just one my @sort_by; -my $default_sort_by; -if ( C4::Context->preference('OPACdefaultSortField') - && C4::Context->preference('OPACdefaultSortOrder') ) { - $default_sort_by = - C4::Context->preference('OPACdefaultSortField') . '_' - . C4::Context->preference('OPACdefaultSortOrder'); -} - my @allowed_sortby = qw /acqdate_asc acqdate_dsc author_az author_za call_number_asc call_number_dsc popularity_asc popularity_dsc pubdate_asc pubdate_dsc relevance title_az title_za/; @sort_by = $cgi->multi_param('sort_by'); $sort_by[0] = $default_sort_by if !$sort_by[0] && defined($default_sort_by); diff --git a/t/Context.t b/t/Context.t index cf4a4b6036..b2091f0f96 100755 --- a/t/Context.t +++ b/t/Context.t @@ -18,7 +18,7 @@ use Modern::Perl; use DBI; -use Test::More tests => 33; +use Test::More tests => 34; use Test::MockModule; use Test::Warn; use YAML::XS; @@ -80,6 +80,36 @@ subtest 'csv_delimiter() tests' => sub { is( C4::Context->csv_delimiter, "\t", "csv_delimiter returns '\t' if system preference CSVDelimiter is tabulation" ); }; +subtest 'default_catalog_sort_by() tests' => sub { + + plan tests => 5; + + my $context = Test::MockModule->new( 'C4::Context' ); + + $context->mock( 'interface', 'intranet' ); + + t::lib::Mocks::mock_preference( 'defaultSortField', undef ); + is( C4::Context->default_catalog_sort_by, undef, "default_catalog_sort_by() returns undef if system preference defaultSortField is undefined" ); + + t::lib::Mocks::mock_preference( 'defaultSortField', 'title' ); + t::lib::Mocks::mock_preference( 'defaultSortOrder', 'az' ); + is( C4::Context->default_catalog_sort_by, 'title_az', "default_catalog_sort_by() returns concatenation of system preferences defaultSortField and defaultSortOrder" ); + + t::lib::Mocks::mock_preference( 'defaultSortField', 'relevance' ); + is( C4::Context->default_catalog_sort_by, 'relevance', "default_catalog_sort_by() returns only system preference defaultSortField if it is relevance" ); + + $context->mock( 'interface', 'opac' ); + + t::lib::Mocks::mock_preference( 'OPACdefaultSortField', 'pubdate' ); + t::lib::Mocks::mock_preference( 'OPACdefaultSortOrder', 'desc' ); + is( C4::Context->default_catalog_sort_by, 'pubdate_desc', "default_catalog_sort_by() returns concatenation of system preferences OPACdefaultSortField and OPACdefaultSortOrder" ); + + t::lib::Mocks::mock_preference( 'OPACdefaultSortField', 'relevance' ); + is( C4::Context->default_catalog_sort_by, 'relevance', "default_catalog_sort_by() returns only system preference OPACdefaultSortField if it is relevance" ); + + $context->unmock( 'interface' ); +}; + my $context = Test::MockModule->new('C4::Context'); my $userenv = {}; -- 2.39.5