Bugzilla – Attachment 71737 Details for
Bug 15524
Set limit on maximum possible holds per patron by category
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15524 - Set limit on maximum possible holds per patron by category
Bug-15524---Set-limit-on-maximum-possible-holds-pe.patch (text/plain), 15.68 KB, created by
Marcel de Rooy
on 2018-02-16 11:19:59 UTC
(
hide
)
Description:
Bug 15524 - Set limit on maximum possible holds per patron by category
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2018-02-16 11:19:59 UTC
Size:
15.68 KB
patch
obsolete
>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
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 15524
:
64707
|
64708
|
67180
|
67181
|
67531
|
70389
|
70390
|
70391
|
70392
|
71704
|
71705
|
71706
|
71737
|
71738
|
71739
|
71740
|
71741
|
77648
|
77681
|
77682
|
78134
|
78135
|
78136
|
78137
|
78138
|
78139
|
78140