From eae494fb33fdd0bd980f6cf4a13f9feba464c8a8 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Mon, 17 Oct 2022 18:31:38 +0200 Subject: [PATCH] Bug 31846: Add syspref for serials search results limit To test: 1) Make sure SerialsSearchResultsLimit syspref is unset or set to 0. 2) Perform an advanced search on serials without any conditions and confirm all serials are listed as expected. 3) Set SerialsSearchResultsLimit to a value less the the number of total subscriptions, perform the search again, and confirm that the number of serials has been limited to the set value. 4) Ensure all tests pass in t/db_dependent/Serials.t Sponsored-by: Gothenburg University Library Signed-off-by: Kelly McElligott Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- C4/Serials.pm | 12 ++- .../bug-31846-serials-search-results-limit.pl | 17 +++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../en/modules/admin/preferences/serials.pref | 5 + .../prog/en/modules/serials/serials-search.tt | 5 + serials/serials-search.pl | 40 ++++--- t/db_dependent/Serials.t | 102 ++++++++++++++++-- 7 files changed, 155 insertions(+), 27 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug-31846-serials-search-results-limit.pl diff --git a/C4/Serials.pm b/C4/Serials.pm index 1eae56bc8d..dfa93ce1ca 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -517,7 +517,9 @@ subscription expiration date. =cut sub SearchSubscriptions { - my ( $args ) = @_; + my ( $args, $params ) = @_; + + $params //= {}; my $additional_fields = $args->{additional_fields} // []; my $matching_record_ids_for_additional_fields = []; @@ -624,6 +626,12 @@ sub SearchSubscriptions { $sth->execute(@where_args); my $results = $sth->fetchall_arrayref( {} ); + my $total_results = @{$results}; + + if ($params->{results_limit} && $total_results > $params->{results_limit}) { + $results = [splice(@{$results}, 0, $params->{results_limit})]; + } + for my $subscription ( @$results ) { $subscription->{cannotedit} = not can_edit_subscription( $subscription ); $subscription->{cannotdisplay} = not can_show_subscription( $subscription ); @@ -634,7 +642,7 @@ sub SearchSubscriptions { } - return @$results; + return wantarray ? @{$results} : { results => $results, total => $total_results }; } diff --git a/installer/data/mysql/atomicupdate/bug-31846-serials-search-results-limit.pl b/installer/data/mysql/atomicupdate/bug-31846-serials-search-results-limit.pl new file mode 100755 index 0000000000..d7309af900 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-31846-serials-search-results-limit.pl @@ -0,0 +1,17 @@ +use Modern::Perl; + +return { + bug_number => "31846", + description => "Add SerialsSearchResultsLimit syspref", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + # Do you stuffs here + $dbh->do(q{ + INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) + VALUES ('SerialsSearchResultsLimit', NULL, NULL, 'Serials search results limit', 'integer') + }); + # Print useful stuff here + say $out "SerialsSearchResultsLimit syspref added"; + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 6af4e7b210..078b27eb11 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -672,6 +672,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('SendAllEmailsTo','',NULL,'All emails will be redirected to this email if it is not empty','free'), ('SeparateHoldings','0',NULL,'Separate current branch holdings from other holdings','YesNo'), ('SeparateHoldingsBranch','homebranch','homebranch|holdingbranch','Branch used to separate holdings','Choice'), +('SerialsSearchResultsLimit', NULL, NULL, 'Serials search results limit', 'integer'), ('SessionRestrictionByIP','1','Check for change in remote IP address for session security. Disable only when remote IP address changes frequently.','','YesNo'), ('SessionStorage','mysql','mysql|Pg|tmp','Use database or a temporary file for storing session data','Choice'), ('ShelfBrowserUsesCcode','1','0','Use the item collection code when finding items for the shelf browser.','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref index e712b6419e..e24ea209a6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref @@ -63,3 +63,8 @@ Serials: 1: Do 0: Don't - prefill the notes from the last 'Arrived' serial when generating the next 'Expected' issue. + - + - Show only the + - pref: SerialsSearchResultsLimit + class: integer + - first serials when performing an advanced serials search. diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt index 39f712b930..7684b8bb55 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt @@ -231,6 +231,11 @@ [% END %] [% ELSE %] [% IF ( done_searched ) %] + [% IF orig_total > total %] +
+

The search originally produced [% orig_total | html %] results and was limited to [% total | html %].

+
+ [% END %]

Serials subscriptions ([% total | html %] found)

[% ELSE %]

Serials subscriptions search

diff --git a/serials/serials-search.pl b/serials/serials-search.pl index 45b85817d1..80b2003814 100755 --- a/serials/serials-search.pl +++ b/serials/serials-search.pl @@ -55,6 +55,7 @@ my $searched = $query->param('searched') || 0; my $mana = $query->param('mana') || 0; my @subscriptionids = $query->multi_param('subscriptionid'); my $op = $query->param('op'); +my $orig_total; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -90,7 +91,7 @@ for my $field ( @additional_fields ) { my @subscriptions; my $mana_statuscode; -if ($searched){ +if ($searched) { if ($mana) { my $result = Koha::SharedContent::search_entities("subscription",{ title => $title, @@ -102,20 +103,25 @@ if ($searched){ @subscriptions = @{ $result->{data} }; } else { - @subscriptions = SearchSubscriptions( - { - biblionumber => $biblionumber, - title => $title, - issn => $ISSN, - ean => $EAN, - callnumber => $callnumber, - publisher => $publisher, - bookseller => $bookseller, - branch => $branch, - additional_fields => \@additional_field_filters, - location => $location, - expiration_date => $expiration_date, - }); + my $subscriptions = SearchSubscriptions( + { + biblionumber => $biblionumber, + title => $title, + issn => $ISSN, + ean => $EAN, + callnumber => $callnumber, + publisher => $publisher, + bookseller => $bookseller, + branch => $branch, + additional_fields => \@additional_field_filters, + location => $location, + expiration_date => $expiration_date, + }, + { results_limit => C4::Context->preference('SerialsSearchResultsLimit') } + ); + @subscriptions = @{$subscriptions->{results}}; + $orig_total = $subscriptions->{total}; + } } @@ -142,8 +148,7 @@ if ($mana) { search_only => 1 ); } -else -{ +else { # to toggle between create or edit routing list options if ($routing) { for my $subscription ( @subscriptions) { @@ -178,6 +183,7 @@ else openedsubscriptions => \@openedsubscriptions, closedsubscriptions => \@closedsubscriptions, total => @openedsubscriptions + @closedsubscriptions, + orig_total => $orig_total, title_filter => $title, ISSN_filter => $ISSN, EAN_filter => $EAN, diff --git a/t/db_dependent/Serials.t b/t/db_dependent/Serials.t index eb432dc914..65c563d164 100755 --- a/t/db_dependent/Serials.t +++ b/t/db_dependent/Serials.t @@ -16,7 +16,7 @@ use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Acquisition::Booksellers; use t::lib::Mocks; use t::lib::TestBuilder; -use Test::More tests => 52; +use Test::More tests => 54; BEGIN { use_ok('C4::Serials', qw( updateClaim NewSubscription GetSubscription GetSubscriptionHistoryFromSubscriptionId SearchSubscriptions ModSubscription GetExpirationDate GetSerials GetSerialInformation NewIssue AddItem2Serial DelSubscription GetFullSubscription PrepareSerialsData GetSubscriptionsFromBiblionumber ModSubscriptionHistory GetSerials2 GetLatestSerials GetNextSeq GetSeq CountSubscriptionFromBiblionumber ModSerialStatus findSerialsByStatus HasSubscriptionStrictlyExpired HasSubscriptionExpired GetLateOrMissingIssues check_routing addroutingmember GetNextDate )); @@ -26,6 +26,8 @@ my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; my $dbh = C4::Context->dbh; +$dbh->do('DELETE FROM subscription'); + my $builder = t::lib::TestBuilder->new(); # This could/should be used for all untested methods @@ -76,14 +78,84 @@ my $notes = "a\nnote\non\nseveral\nlines"; my $internalnotes = 'intnotes'; my $ccode = 'FIC'; my $subscriptionid = NewSubscription( - undef, "", undef, undef, $budget_id, $biblionumber, - '2013-01-01', $frequency_id, undef, undef, undef, - undef, undef, undef, undef, undef, undef, - 1, $notes, ,undef, '2013-01-01', undef, $pattern_id, - undef, undef, 0, $internalnotes, 0, - undef, undef, 0, undef, '2013-12-31', 0, - undef, undef, undef, $ccode + undef, + "", + undef, + undef, + $budget_id, + $biblionumber, + '2013-01-01', + $frequency_id, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + 1, + $notes, + undef, + '2013-01-01', + undef, + $pattern_id, + undef, + undef, + 0, + $internalnotes, + 0, + undef, + undef, + 0, + undef, + '2013-12-31', 0, + undef, + undef, + undef, + $ccode +); +NewSubscription( + undef, + "", + undef, + undef, + $budget_id, + $biblionumber, + '2013-01-02', + $frequency_id, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + undef, + 1, + $notes, + undef, + '2013-01-02', + undef, + $pattern_id, + undef, + undef, + 0, + $internalnotes, + 0, + undef, + undef, + 0, + undef, + '2013-12-31', + 0, + undef, + undef, + undef, + $ccode ); my $subscriptioninformation = GetSubscription( $subscriptionid ); @@ -108,6 +180,20 @@ isa_ok( \@subscriptions, 'ARRAY' ); @subscriptions = SearchSubscriptions({ biblionumber => $subscriptioninformation->{bibnum}, orderby => 'title' }); isa_ok( \@subscriptions, 'ARRAY' ); +@subscriptions = SearchSubscriptions({}); +is( + @subscriptions, + 2, + 'SearchSubscriptions returned the expected number of subscriptions when results_limit is not set' +); + +@subscriptions = SearchSubscriptions({}, { results_limit => 1 }); +is( + @subscriptions, + 1, + 'SearchSubscriptions returned only one subscription when results_limit is set to "1"' +); + my $frequency = GetSubscriptionFrequency($subscriptioninformation->{periodicity}); my $old_frequency; if (not $frequency->{unit}) { -- 2.39.5