Bugzilla – Attachment 88928 Details for
Bug 21946
Group circulation by item type
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21946: Update C4::Circulation->TooMany to check parent itemtypes
Bug-21946-Update-C4Circulation-TooMany-to-check-pa.patch (text/plain), 21.36 KB, created by
Liz Rea
on 2019-04-26 14:53:49 UTC
(
hide
)
Description:
Bug 21946: Update C4::Circulation->TooMany to check parent itemtypes
Filename:
MIME Type:
Creator:
Liz Rea
Created:
2019-04-26 14:53:49 UTC
Size:
21.36 KB
patch
obsolete
>From ef4e2b48f2cac1370d6f165a3b9de114282dcbc9 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 2 Apr 2019 14:46:00 +0000 >Subject: [PATCH] Bug 21946: Update C4::Circulation->TooMany to check parent > itemtypes > >To test: >1 - prove -v t/db_dependent/Circulation/TooMany.t > >Signed-off-by: Liz Rea <wizzyrea@gmail.com> >--- > C4/Circulation.pm | 214 ++++++++++++++++++++++---------- > t/db_dependent/Circulation/TooMany.t | 233 ++++++++++++++++++++++++++++++++++- > 2 files changed, 380 insertions(+), 67 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 94e9dfa708..66ffcd4c27 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -374,9 +374,9 @@ sub transferbook { > > > sub TooMany { >- my $borrower = shift; >+ my $borrower = shift; > my $biblionumber = shift; >- my $item = shift; >+ my $item = shift; > my $params = shift; > my $onsite_checkout = $params->{onsite_checkout} || 0; > my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0; >@@ -389,8 +389,26 @@ sub TooMany { > ? $item->{'itype'} # item-level > : $item->{'itemtype'}; # biblio-level > >+ my ($type_object, $parent_type, $parent_maxissueqty_rule); >+ $type_object = Koha::ItemTypes->find( $type ); >+ $parent_type = $type_object->parent_type if $type_object; >+ my $child_types = Koha::ItemTypes->search( parent_type => $type ); >+ # Find any children if we are a parent_type; >+ > # given branch, patron category, and item type, determine > # applicable issuing rule >+ >+ $parent_maxissueqty_rule = Koha::CirculationRules->get_effective_rule( >+ { >+ categorycode => $cat_borrower, >+ itemtype => $parent_type, >+ branchcode => $branch, >+ rule_name => 'maxissueqty', >+ } >+ ) if $parent_type; >+ # If the parent rule is for default type we discount it >+ $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && $parent_maxissueqty_rule->itemtype eq '*'; >+ > my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule( > { > categorycode => $cat_borrower, >@@ -399,6 +417,8 @@ sub TooMany { > rule_name => 'maxissueqty', > } > ); >+ >+ > my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule( > { > categorycode => $cat_borrower, >@@ -413,9 +433,17 @@ sub TooMany { > # how many loans the patron already has that meet that > # rule > if (defined($maxissueqty_rule) and defined($maxissueqty_rule->rule_value)) { >+ > my @bind_params; >- my $count_query = q| >- SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts >+ my $count_query = ""; >+ >+ if (C4::Context->preference('item-level_itypes')) { >+ $count_query .= q|SELECT COALESCE( SUM( IF(items.itype = '| .$type . q|',1,0) ), 0) as type_total, COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts|; >+ } else{ >+ $count_query .= q|SELECT COALESCE(SUM( IF(biblioitems.itype = '| .$type . q|',1,0) ), 0) as type_total, COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts|; >+ } >+ >+ $count_query .= q| > FROM issues > JOIN items USING (itemnumber) > |; >@@ -445,15 +473,31 @@ sub TooMany { > push @bind_params, $maxissueqty_rule->categorycode; > push @bind_params, $cat_borrower; > } else { >- # rule has specific item type, so count loans of that >- # specific item type >+ my @types; >+ if ( $parent_maxissueqty_rule ) { >+ # if we have a parent item type then we count loans of the >+ # specific item type or its siblings or parent >+ push @types, $parent_type; >+ my $children = Koha::ItemTypes->search({ parent_type => $parent_type }); >+ while ( my $child = $children->next ){ >+ push @types, $child->itemtype; >+ } >+ } elsif ( $child_types ) { >+ # If we are a parent type, we need to count all child types and our own type >+ while ( my $child = $child_types->next ){ >+ push @types, $child->itemtype; >+ } >+ push @types, $type; # And don't forget to count our own types >+ } else { push @types, $type; } # Otherwise only count the specific itemtype >+ my $types_param = ( '?,' ) x @types; >+ $types_param =~ s/,$//; > if (C4::Context->preference('item-level_itypes')) { >- $count_query .= " WHERE items.itype = ? "; >+ $count_query .= " WHERE items.itype IN (" . $types_param . ")"; > } else { > $count_query .= " JOIN biblioitems USING (biblionumber) >- WHERE biblioitems.itemtype= ? "; >+ WHERE biblioitems.itemtype IN (" . $types_param . ")"; > } >- push @bind_params, $type; >+ push @bind_params, @types; > } > > $count_query .= " AND borrowernumber = ? "; >@@ -471,38 +515,53 @@ sub TooMany { > } > } > >- my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params ); >+ my ( $checkout_count_type, $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params ); > >- my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef; > my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef; > >- if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) { >- if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) { >- return { >- reason => 'TOO_MANY_ONSITE_CHECKOUTS', >- count => $onsite_checkout_count, >- max_allowed => $max_onsite_checkouts_allowed, >- } >- } >- } >- if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) { >- my $delta = $switch_onsite_checkout ? 1 : 0; >- if ( $checkout_count >= $max_checkouts_allowed + $delta ) { >- return { >- reason => 'TOO_MANY_CHECKOUTS', >- count => $checkout_count, >- max_allowed => $max_checkouts_allowed, >- }; >- } >- } elsif ( not $onsite_checkout ) { >- if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed ) { >- return { >- reason => 'TOO_MANY_CHECKOUTS', >- count => $checkout_count - $onsite_checkout_count, >- max_allowed => $max_checkouts_allowed, >- }; >- } >+ # If parent rules exists >+ if ( defined($parent_maxissueqty_rule) and defined($parent_maxissueqty_rule->rule_value) ){ >+ my $max_checkouts_allowed = $parent_maxissueqty_rule->rule_value; >+ >+ my $qty_over = _check_max_qty({ >+ checkout_count => $checkout_count, >+ onsite_checkout_count => $onsite_checkout_count, >+ onsite_checkout => $onsite_checkout, >+ max_checkouts_allowed => $max_checkouts_allowed, >+ max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, >+ switch_onsite_checkout => $switch_onsite_checkout >+ }); >+ return $qty_over if defined $qty_over; >+ >+ >+ # If the parent rule is less than or equal to the child, we only need check the parent >+ if( $maxissueqty_rule->rule_value < $parent_maxissueqty_rule->rule_value && defined($maxissueqty_rule->itemtype) ) { >+ my $max_checkouts_allowed = $maxissueqty_rule->rule_value; >+ my $qty_over = _check_max_qty({ >+ checkout_count => $checkout_count_type, >+ onsite_checkout_count => $onsite_checkout_count, >+ onsite_checkout => $onsite_checkout, >+ max_checkouts_allowed => $max_checkouts_allowed, >+ max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, >+ switch_onsite_checkout => $switch_onsite_checkout >+ }); >+ return $qty_over if defined $qty_over; >+ } >+ >+ } else { >+ my $max_checkouts_allowed = $maxissueqty_rule->rule_value; >+ my $qty_over = _check_max_qty({ >+ checkout_count => $checkout_count, >+ onsite_checkout_count => $onsite_checkout_count, >+ onsite_checkout => $onsite_checkout, >+ max_checkouts_allowed => $max_checkouts_allowed, >+ max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, >+ switch_onsite_checkout => $switch_onsite_checkout >+ }); >+ return $qty_over if defined $qty_over; > } >+ >+ > } > > # Now count total loans against the limit for the branch >@@ -528,35 +587,18 @@ sub TooMany { > } > my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params ); > my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty}; >- my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty}; >- >- if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) { >- if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) { >- return { >- reason => 'TOO_MANY_ONSITE_CHECKOUTS', >- count => $onsite_checkout_count, >- max_allowed => $max_onsite_checkouts_allowed, >- } >- } >- } >- if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) { >- my $delta = $switch_onsite_checkout ? 1 : 0; >- if ( $checkout_count >= $max_checkouts_allowed + $delta ) { >- return { >- reason => 'TOO_MANY_CHECKOUTS', >- count => $checkout_count, >- max_allowed => $max_checkouts_allowed, >- }; >- } >- } elsif ( not $onsite_checkout ) { >- if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed ) { >- return { >- reason => 'TOO_MANY_CHECKOUTS', >- count => $checkout_count - $onsite_checkout_count, >- max_allowed => $max_checkouts_allowed, >- }; >- } >- } >+ my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty} || undef; >+ >+ my $qty_over = _check_max_qty({ >+ checkout_count => $checkout_count, >+ onsite_checkout_count => $onsite_checkout_count, >+ onsite_checkout => $onsite_checkout, >+ max_checkouts_allowed => $max_checkouts_allowed, >+ max_onsite_checkouts_allowed => $max_onsite_checkouts_allowed, >+ switch_onsite_checkout => $switch_onsite_checkout >+ }); >+ return $qty_over if defined $qty_over; >+ > } > > if ( not defined( $maxissueqty_rule ) and not defined($branch_borrower_circ_rule->{patron_maxissueqty}) ) { >@@ -567,6 +609,46 @@ sub TooMany { > return; > } > >+sub _check_max_qty { >+ my $params = shift; >+ my $checkout_count = $params->{checkout_count}; >+ my $onsite_checkout_count = $params->{onsite_checkout_count}; >+ my $onsite_checkout = $params->{onsite_checkout}; >+ my $max_checkouts_allowed = $params->{max_checkouts_allowed}; >+ my $max_onsite_checkouts_allowed = $params->{max_onsite_checkouts_allowed}; >+ my $switch_onsite_checkout = $params->{switch_onsite_checkout}; >+ >+ if ( $onsite_checkout and defined $max_onsite_checkouts_allowed ) { >+ if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) { >+ return { >+ reason => 'TOO_MANY_ONSITE_CHECKOUTS', >+ count => $onsite_checkout_count, >+ max_allowed => $max_onsite_checkouts_allowed, >+ } >+ } >+ } >+ if ( C4::Context->preference('ConsiderOnSiteCheckoutsAsNormalCheckouts') ) { >+ my $delta = $switch_onsite_checkout ? 1 : 0; >+ if ( $checkout_count >= $max_checkouts_allowed + $delta ) { >+ return { >+ reason => 'TOO_MANY_CHECKOUTS', >+ count => $checkout_count, >+ max_allowed => $max_checkouts_allowed, >+ }; >+ } >+ } elsif ( not $onsite_checkout ) { >+ if ( $checkout_count - $onsite_checkout_count >= $max_checkouts_allowed ) { >+ return { >+ reason => 'TOO_MANY_CHECKOUTS', >+ count => $checkout_count - $onsite_checkout_count, >+ max_allowed => $max_checkouts_allowed, >+ }; >+ } >+ } >+ >+ return; >+} >+ > =head2 CanBookBeIssued > > ( $issuingimpossible, $needsconfirmation, [ $alerts ] ) = CanBookBeIssued( $patron, >diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t >index 1f1c6a7ee0..6c1c2a29f0 100644 >--- a/t/db_dependent/Circulation/TooMany.t >+++ b/t/db_dependent/Circulation/TooMany.t >@@ -15,7 +15,7 @@ > # with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; >-use Test::More tests => 8; >+use Test::More tests => 9; > use C4::Context; > > use C4::Members; >@@ -659,7 +659,238 @@ subtest 'General vs specific rules limit quantity correctly' => sub { > 'We are allowed one from the branch specifically now' > ); > >+}; >+ >+subtest 'itemtype group tests' => sub { >+ plan tests => 13; >+ >+ t::lib::Mocks::mock_preference('CircControl', 'ItemHomeLibrary'); >+ my $rule = $builder->build({ >+ source => 'Issuingrule', >+ value => { >+ categorycode => '*', >+ itemtype => '*', >+ branchcode => '*', >+ issuelength => 1, >+ firstremind => 1, # 1 day of grace >+ finedays => 2, # 2 days of fine per day of overdue >+ lengthunit => 'days', >+ } >+ }); >+ >+ my $parent_itype = $builder->build({ >+ source=>'Itemtype', >+ value => { >+ parent_type => undef, >+ rentalcharge => undef, >+ rentalcharge_daily => undef, >+ rentalcharge_hourly => undef, >+ notforloan => 0, >+ } >+ }); >+ my $child_itype_1 = $builder->build({ >+ source=>'Itemtype', >+ value => { >+ parent_type => $parent_itype->{itemtype}, >+ rentalcharge => 0, >+ rentalcharge_daily => 0, >+ rentalcharge_hourly => 0, >+ notforloan => 0, >+ } >+ }); >+ my $child_itype_2 = $builder->build({ >+ source=>'Itemtype', >+ value => { >+ parent_type => $parent_itype->{itemtype}, >+ rentalcharge => 0, >+ rentalcharge_daily => 0, >+ rentalcharge_hourly => 0, >+ notforloan => 0, >+ } >+ }); >+ >+ my $branch = $builder->build({source => 'Branch',}); >+ my $category = $builder->build({source => 'Category',}); >+ my $patron = $builder->build({ >+ source => 'Borrower', >+ value => { >+ categorycode => $category->{categorycode}, >+ branchcode => $branch->{branchcode}, >+ }, >+ }); >+ my $item = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$child_itype_1->{itemtype} >+ }); >+ >+ my $all_iq_rule = $builder->build({ >+ source=>'CirculationRule', >+ value => { >+ branchcode => $branch->{branchcode}, >+ categorycode => $category->{categorycode}, >+ itemtype => undef, >+ rule_name => 'maxissueqty', >+ rule_value => 1 >+ } >+ }); >+ is( >+ C4::Circulation::TooMany( $patron, $item->biblionumber, $item->unblessed ), >+ undef, >+ 'Checkout allowed, using all rule of 1' >+ ); >+ >+ #Checkout an item >+ my $issue = C4::Circulation::AddIssue( $patron, $item->barcode, dt_from_string() ); >+ like( $issue->issue_id, qr|^\d+$|, 'The issue should have been inserted' ); >+ #Patron has 1 checkout of child itype1 >+ >+ my $parent_iq_rule = $builder->build({ >+ source=>'CirculationRule', >+ value => { >+ branchcode => $branch->{branchcode}, >+ categorycode => $category->{categorycode}, >+ itemtype => $parent_itype->{itemtype}, >+ rule_name => 'maxissueqty', >+ rule_value => 2 >+ } >+ }); >+ >+ is( >+ C4::Circulation::TooMany( $patron, $item->biblionumber, $item->unblessed ), >+ undef, >+ 'Checkout allowed, using parent type rule of 2' >+ ); >+ >+ my $child1_iq_rule = $builder->build_object({ >+ class=>'Koha::CirculationRules', >+ value => { >+ branchcode => $branch->{branchcode}, >+ categorycode => $category->{categorycode}, >+ itemtype => $child_itype_1->{itemtype}, >+ rule_name => 'maxissueqty', >+ rule_value => 1 >+ } >+ }); >+ >+ is_deeply( >+ C4::Circulation::TooMany( $patron, $item->biblionumber, $item->unblessed ), >+ { >+ reason => 'TOO_MANY_CHECKOUTS', >+ count => 1, >+ max_allowed => 1, >+ }, >+ 'Checkout not allowed, using specific type rule of 1' >+ ); >+ >+ my $item_1 = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$child_itype_2->{itemtype} >+ }); >+ >+ my $child2_iq_rule = $builder->build({ >+ source=>'CirculationRule', >+ value => { >+ branchcode => $branch->{branchcode}, >+ categorycode => $category->{categorycode}, >+ itemtype => $child_itype_2->{itemtype}, >+ rule_name => 'maxissueqty', >+ rule_value => 3 >+ } >+ }); >+ >+ is( >+ C4::Circulation::TooMany( $patron, $item_1->biblionumber, $item_1->unblessed ), >+ undef, >+ 'Checkout allowed' >+ ); >+ >+ #checkout an item >+ $issue = C4::Circulation::AddIssue( $patron, $item_1->barcode, dt_from_string()); >+ like( $issue->issue_id, qr|^\d+$|, 'the issue should have been inserted' ); >+ #patron has 1 checkout of childitype1 and 1 checkout of childitype2 >+ >+ is_deeply( >+ C4::Circulation::TooMany( $patron, $item->biblionumber, $item->unblessed ), >+ { >+ reason => 'TOO_MANY_CHECKOUTS', >+ count => 2, >+ max_allowed => 2, >+ }, >+ 'Checkout not allowed, using parent type rule of 2, checkout of sibling itemtype counted' >+ ); >+ >+ my $parent_item = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$parent_itype->{itemtype} >+ }); >+ >+ is_deeply( >+ C4::Circulation::TooMany( $patron, $parent_item->biblionumber, $parent_item->unblessed ), >+ { >+ reason => 'TOO_MANY_CHECKOUTS', >+ count => 2, >+ max_allowed => 2, >+ }, >+ 'Checkout not allowed, using parent type rule of 2, checkout of child itemtypes counted' >+ ); >+ >+ >+ #increase parent type to greater than specific >+ my $circ_rule_object = Koha::CirculationRules->find( $parent_iq_rule->{id} ); >+ $circ_rule_object->rule_value(4)->store(); >+ >+ is( >+ C4::Circulation::TooMany( $patron, $item->biblionumber, $item_1->unblessed ), >+ undef, >+ 'Checkout allowed, using specific type rule of 3' >+ ); >+ >+ my $item_2 = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$child_itype_2->{itemtype} >+ }); >+ #checkout an item >+ $issue = C4::Circulation::AddIssue( $patron, $item_2->barcode, dt_from_string(), undef, undef, undef ); >+ like( $issue->issue_id, qr|^\d+$|, 'the issue should have been inserted' ); >+ #patron has 1 checkoout of childitype1 and 2 of childitype2 > >+ is( >+ C4::Circulation::TooMany( $patron, $item_2->biblionumber, $item_2->unblessed ), >+ undef, >+ 'Checkout allowed, using specific type rule of 3, checkout of sibling itemtype not counted' >+ ); >+ >+ $child1_iq_rule->rule_value(2)->store(); #Allow 2 checkouts for child type 1 >+ >+ my $item_3 = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$child_itype_1->{itemtype} >+ }); >+ my $item_4 = $builder->build_sample_item({ >+ homebranch=>$branch->{branchcode}, >+ holdingbranch=>$branch->{branchcode}, >+ itype=>$child_itype_2->{itemtype} >+ }); >+ >+ #checkout an item >+ $issue = C4::Circulation::AddIssue( $patron, $item_4->barcode, dt_from_string(), undef, undef, undef ); >+ like( $issue->issue_id, qr|^\d+$|, 'the issue should have been inserted' ); >+ #patron has 1 checkout of childitype 1 and 3 of childitype2 >+ >+ is_deeply( >+ C4::Circulation::TooMany( $patron, $item_3->biblionumber, $item_3->unblessed ), >+ { >+ reason => 'TOO_MANY_CHECKOUTS', >+ max_allowed => 4, >+ count => 4, >+ }, >+ 'Checkout not allowed, using specific type rule of 2, checkout of sibling itemtype not counted, but parent rule (4) prevents another' >+ ); > > }; > >-- >2.11.0
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 21946
:
87713
|
87714
|
87715
|
87716
|
87717
|
87718
|
87719
|
88392
|
88574
|
88575
|
88576
|
88577
|
88578
|
88579
|
88580
|
88581
|
88926
|
88927
|
88928
|
88929
|
88930
|
88931
|
88932
|
88933
|
88934
|
89167
|
89168
|
89169
|
89170
|
89171
|
89172
|
89173
|
89174
|
89175
|
89176
|
93075
|
93076
|
93077
|
93078
|
93079
|
93080
|
93081
|
93082
|
93083
|
93084
|
93086
|
93087
|
93088
|
93089
|
93090
|
93091
|
93092
|
93093
|
93094
|
93095
|
93096
|
93097
|
93098
|
93099
|
93100
|
93101
|
93102
|
93103
|
93104
|
93105
|
96977
|
96978
|
96979
|
96980
|
96981
|
96982
|
96983
|
96984
|
96985
|
96986
|
96987
|
99419
|
99420
|
99421
|
99422
|
99423
|
99424
|
107035
|
107036
|
107037
|
107038
|
107039
|
107040
|
107041
|
107278
|
107279
|
107280
|
107281
|
107282
|
107283
|
107284
|
107467
|
107489
|
107686
|
107687
|
107688
|
107689
|
107690
|
107691
|
107692
|
107693
|
107694
|
107701
|
107733