From 4acfbe283a4855a5dac246ad7d96548ed89bb12c Mon Sep 17 00:00:00 2001 From: Kyle M Hall <kyle@bywatersolutions.com> Date: Wed, 28 Jun 2017 08:54:03 -0400 Subject: [PATCH] Bug 15524 - Set limit on maximum possible holds per patron by category Content-Type: text/plain; charset=utf-8 It's possible to set a limit on the maximum number of holds for a particular branch/category/itemtype, but not on the total number of holds for a given patron (by branch/category). This new rule works in conjunction with the existing branch/borrower/item rules in that Koha will use the lower of the two limits. This new rule counts all holds of all types, which prevents bib-level holds from not being counted for the purpose of these limits. This makes the most sense and was also requested by the sponsor. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Go to the circ rules editor, note the new max holds rules by patron category in the "Checkout limit by patron category". ( Should we rename this section? ) 4) Create find a patron that is allowed to place a hold, count the number of holds that patron has. Lets make that number 'X'. 5) Set the new max holds rule to X for "All libraries" 6) Note the patron can no longer place another hold 7) Set the new max holds rule to X + 1 for the patron's home library 8) Note the patron can again place another hold 9) Set the new max holds rule to X for the patron's home library 10) Note the patron can no longer place another hold Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- C4/Reserves.pm | 23 ++++++++ admin/smart-rules.pl | 35 ++++++----- installer/data/mysql/atomicupdate/bug_15524.perl | 18 ++++++ installer/data/mysql/kohastructure.sql | 2 + .../prog/en/modules/admin/smart-rules.tt | 8 +++ t/db_dependent/Holds.t | 69 +++++++++++++++++++++- 6 files changed, 140 insertions(+), 15 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_15524.perl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 73b86b6..bf8e499 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -398,6 +398,29 @@ sub CanItemBeReserved { return 'tooManyReserves'; } + # Now we need to check hold limits by patron category + my $schema = Koha::Database->new()->schema(); + my $rule = $schema->resultset('BranchBorrowerCircRule')->find( + { + branchcode => $borrower->{branchcode}, + categorycode => $borrower->{categorycode}, + } + ); + $rule ||= $schema->resultset('DefaultBorrowerCircRule')->find( + { + categorycode => $borrower->{categorycode} + } + ); + if ( $rule && defined $rule->max_holds ) { + my $total_holds_count = Koha::Holds->search( + { + borrowernumber => $borrower->{borrowernumber} + } + )->count(); + + return 'tooManyReserves' if $total_holds_count >= $rule->max_holds; + } + my $circ_control_branch = C4::Circulation::_GetCircControlBranch( $item->unblessed(), $borrower ); my $branchitemrule = diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 718b682..3429778 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -254,10 +254,13 @@ elsif ($op eq "add-branch-cat") { my $categorycode = $input->param('categorycode'); my $maxissueqty = $input->param('maxissueqty'); my $maxonsiteissueqty = $input->param('maxonsiteissueqty'); + my $max_holds = $input->param('max_holds'); $maxissueqty =~ s/\s//g; $maxissueqty = undef if $maxissueqty !~ /^\d+/; $maxonsiteissueqty =~ s/\s//g; $maxonsiteissueqty = undef if $maxonsiteissueqty !~ /^\d+/; + $max_holds =~ s/\s//g; + $max_holds = undef if $max_holds !~ /^\d+/; if ($branch eq "*") { if ($categorycode eq "*") { @@ -265,21 +268,22 @@ elsif ($op eq "add-branch-cat") { FROM default_circ_rules"); my $sth_insert = $dbh->prepare(q| INSERT INTO default_circ_rules - (maxissueqty, maxonsiteissueqty) - VALUES (?, ?) + (maxissueqty, maxonsiteissueqty, max_holds) + VALUES (?, ?, ?) |); my $sth_update = $dbh->prepare(q| UPDATE default_circ_rules SET maxissueqty = ?, - maxonsiteissueqty = ? + maxonsiteissueqty = ?, + max_holds = ? |); $sth_search->execute(); my $res = $sth_search->fetchrow_hashref(); if ($res->{total}) { - $sth_update->execute($maxissueqty, $maxonsiteissueqty); + $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds ); } else { - $sth_insert->execute($maxissueqty, $maxonsiteissueqty); + $sth_insert->execute( $maxissueqty, $maxonsiteissueqty, $max_holds ); } } else { my $sth_search = $dbh->prepare("SELECT count(*) AS total @@ -287,21 +291,22 @@ elsif ($op eq "add-branch-cat") { WHERE categorycode = ?"); my $sth_insert = $dbh->prepare(q| INSERT INTO default_borrower_circ_rules - (categorycode, maxissueqty, maxonsiteissueqty) - VALUES (?, ?, ?) + (categorycode, maxissueqty, maxonsiteissueqty, max_holds) + VALUES (?, ?, ?, ?) |); my $sth_update = $dbh->prepare(q| UPDATE default_borrower_circ_rules SET maxissueqty = ?, - maxonsiteissueqty = ? + maxonsiteissueqty = ?, + max_holds = ? WHERE categorycode = ? |); $sth_search->execute($categorycode); my $res = $sth_search->fetchrow_hashref(); if ($res->{total}) { - $sth_update->execute($maxissueqty, $maxonsiteissueqty, $categorycode); + $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $categorycode, $max_holds ); } else { - $sth_insert->execute($categorycode, $maxissueqty, $maxonsiteissueqty); + $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds ); } } } elsif ($categorycode eq "*") { @@ -333,13 +338,14 @@ elsif ($op eq "add-branch-cat") { AND categorycode = ?"); my $sth_insert = $dbh->prepare(q| INSERT INTO branch_borrower_circ_rules - (branchcode, categorycode, maxissueqty, maxonsiteissueqty) - VALUES (?, ?, ?, ?) + (branchcode, categorycode, maxissueqty, maxonsiteissueqty, max_holds) + VALUES (?, ?, ?, ?, ?) |); my $sth_update = $dbh->prepare(q| UPDATE branch_borrower_circ_rules SET maxissueqty = ?, maxonsiteissueqty = ? + max_holds = ? WHERE branchcode = ? AND categorycode = ? |); @@ -347,9 +353,9 @@ elsif ($op eq "add-branch-cat") { $sth_search->execute($branch, $categorycode); my $res = $sth_search->fetchrow_hashref(); if ($res->{total}) { - $sth_update->execute($maxissueqty, $maxonsiteissueqty, $branch, $categorycode); + $sth_update->execute($maxissueqty, $maxonsiteissueqty, $max_holds, $branch, $categorycode); } else { - $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty); + $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds); } } } @@ -545,6 +551,7 @@ my @sorted_branch_cat_rules = sort { $a->{'humancategorycode'} cmp $b->{'humanca foreach my $entry (@sorted_branch_cat_rules, @sorted_row_loop) { $entry->{unlimited_maxissueqty} = 1 unless defined($entry->{maxissueqty}); $entry->{unlimited_maxonsiteissueqty} = 1 unless defined($entry->{maxonsiteissueqty}); + $entry->{unlimited_max_holds} = 1 unless defined($entry->{max_holds}); } @sorted_row_loop = sort by_category_and_itemtype @row_loop; diff --git a/installer/data/mysql/atomicupdate/bug_15524.perl b/installer/data/mysql/atomicupdate/bug_15524.perl new file mode 100644 index 0000000..4e23d91 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_15524.perl @@ -0,0 +1,18 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + if( !column_exists( 'branch_borrower_circ_rules', 'max_holds' ) ) { + $dbh->do(q{ + ALTER TABLE branch_borrower_circ_rules ADD COLUMN max_holds INT(4) NULL DEFAULT NULL AFTER maxonsiteissueqty + }); + } + + if( !column_exists( 'default_borrower_circ_rules', 'max_holds' ) ) { + $dbh->do(q{ + ALTER TABLE default_borrower_circ_rules ADD COLUMN max_holds INT(4) NULL DEFAULT NULL AFTER maxonsiteissueqty + }); + } + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 15524 - Set limit on maximum possible holds per patron by category)\n"; +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 44901ab..3cddb8b 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -349,6 +349,7 @@ CREATE TABLE `branch_borrower_circ_rules` ( -- includes default circulation rule `categorycode` VARCHAR(10) NOT NULL, -- the patron category this rule applies to (categories.categorycode) `maxissueqty` int(4) default NULL, -- the maximum number of checkouts this patron category can have at this branch `maxonsiteissueqty` int(4) default NULL, -- the maximum number of on-site checkouts this patron category can have at this branch + max_holds INT(4) NULL DEFAULT NULL, -- the maximum number of holds a patron may have at a time PRIMARY KEY (`categorycode`, `branchcode`), CONSTRAINT `branch_borrower_circ_rules_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE, @@ -365,6 +366,7 @@ CREATE TABLE `default_borrower_circ_rules` ( -- default checkout rules found und `categorycode` VARCHAR(10) NOT NULL, -- patron category this rul `maxissueqty` int(4) default NULL, `maxonsiteissueqty` int(4) default NULL, + max_holds INT(4) NULL DEFAULT NULL, -- the maximum number of holds a patron may have at a time PRIMARY KEY (`categorycode`), CONSTRAINT `borrower_borrower_circ_rules_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index e1a5839..19b7b9a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -464,6 +464,7 @@ <th>Patron category</th> <th>Total current checkouts allowed</th> <th>Total current on-site checkouts allowed</th> + <th>Maximum total holds allowed (count)</th> <th> </th> </tr> [% FOREACH branch_cat_rule_loo IN branch_cat_rule_loop %] @@ -490,6 +491,12 @@ [% branch_cat_rule_loo.maxonsiteissueqty %] [% END %] </td> + <td>[% IF ( branch_cat_rule_loo.unlimited_max_holds ) %] + Unlimited + [% ELSE %] + [% branch_cat_rule_loo.max_holds %] + [% END %] + </td> <td class="actions"> <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete-branch-cat&categorycode=[% branch_cat_rule_loo.categorycode %]&branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a> @@ -506,6 +513,7 @@ </td> <td><input name="maxissueqty" size="3" /></td> <td><input name="maxonsiteissueqty" size="3" /></td> + <td><input name="max_holds" size="3" /></td> <td class="actions"><button type="submit" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> Add</td> </tr> </table> diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 1e6c234..29ce0c5 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 55; +use Test::More tests => 56; use MARC::Record; use C4::Biblio; use C4::Items; @@ -411,6 +411,73 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); +subtest 'Test max_holds per library/patron category' => sub { + plan tests => 6; + + $dbh->do('DELETE FROM reserves'); + $dbh->do('DELETE FROM issuingrules'); + + ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST'); + ( $item_bibnum, $item_bibitemnum, $itemnumber ) = + AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, + $bibnum ); + $dbh->do( + q{ + INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) + VALUES (?, ?, ?, ?, ?) + }, + {}, + '*', '*', 'TEST', 99, 99 + ); + AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); + AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); + AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); + + my $count = + Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count(); + is( $count, 3, 'Patron now has 3 holds' ); + + my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + is( $ret, 'OK', 'Patron can place hold with no borrower circ rules' ); + + my $rule_all = $schema->resultset('DefaultBorrowerCircRule')->new( + { + categorycode => $category->{categorycode}, + max_holds => 3, + } + )->insert(); + + my $rule_branch = $schema->resultset('BranchBorrowerCircRule')->new( + { + branchcode => $branch_1, + categorycode => $category->{categorycode}, + max_holds => 5, + } + )->insert(); + + $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' ); + + $rule_branch->delete(); + + $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a category rule of 3' ); + + $rule_all->delete(); + $rule_branch->max_holds(3); + $rule_branch->insert(); + + $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a branch/category rule of 3' ); + + $rule_branch->max_holds(5); + $rule_branch->update(); + $rule_branch->max_holds(5); + $rule_branch->insert(); + + $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' ); +}; # Helper method to set up a Biblio. sub create_helper_biblio { -- 2.1.4