Bugzilla – Attachment 150114 Details for
Bug 33569
Order by relevance may not be visible
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33569: Fix sort by relevance in catalog search
Bug-33569-Fix-sort-by-relevance-in-catalog-search.patch (text/plain), 10.58 KB, created by
David Nind
on 2023-04-23 06:36:08 UTC
(
hide
)
Description:
Bug 33569: Fix sort by relevance in catalog search
Filename:
MIME Type:
Creator:
David Nind
Created:
2023-04-23 06:36:08 UTC
Size:
10.58 KB
patch
obsolete
>From 125f204fec8d45edb760b7180b7e63b8feac01aa Mon Sep 17 00:00:00 2001 >From: Fridolin Somers <fridolin.somers@biblibre.com> >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 <david@davidnind.com> >--- > 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 b6e4380640..458b16f9c9 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt >@@ -221,7 +221,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> >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.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 33569
:
149984
|
150114
|
150116