From f81257f14ff0a57322e5ae6139ff4f7789d026e8 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 --- 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 75c9d7f19f..a06034a942 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -512,7 +512,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 = []; @@ -617,6 +619,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 ); @@ -627,7 +635,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 617d34b814..1c9321a4bb 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -640,6 +640,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 f6e818827c..216ba81410 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 @@ -62,3 +62,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 8104c6d8a1..f49a77ae07 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 @@ -237,6 +237,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.35.1