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 <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

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) {

koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt

@@ -224,7 +224,7 @@
                                     Sort <span class="caret"></span>
                                 </button>
                                 <ul class="dropdown-menu">
-                                    [% IF ( sort_by == "relevance_dsc" ) %]
+                                    [% IF ( sort_by == "relevance" ) %]
                                         <li><a href="/cgi-bin/koha/catalogue/search.pl?count=[% results_per_page | uri %]&sort_by=relevance[% PROCESS sort_search_query %]"><i class="fa fa-check"></i> Relevance</a></li>
                                     [% ELSE %]
                                         <li><a href="/cgi-bin/koha/catalogue/search.pl?count=[% results_per_page | uri %]&sort_by=relevance[% PROCESS sort_search_query %]">Relevance</a></li>

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);

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);

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 = {};