From a2fb7ef8f109a6334a2d2a8e511da0a600b8a46f 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
---
 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 53b74bb..b323b71 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -373,9 +373,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;
@@ -388,8 +388,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,
@@ -398,6 +416,8 @@ sub TooMany {
             rule_name    => 'maxissueqty',
         }
     );
+
+
     my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
         {
             categorycode => $cat_borrower,
@@ -412,9 +432,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)
         |;
@@ -444,15 +472,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 = ? ";
@@ -470,38 +514,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
@@ -527,35 +586,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}) ) {
@@ -566,6 +608,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 1f1c6a7..6c1c2a2 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.1.4