From 9a12c7fa18abe89857aacbded9b72d1520c86f52 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@koha-community.org>
Date: Wed, 13 May 2015 14:04:50 +0200
Subject: [PATCH] [PASSED QA] Bug 14045: Change prototype of TooMany to raise a
 better warning

With this patch, the user will know why the checkout is refused.

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
---
 C4/Circulation.pm                                  |  65 +++++++----
 .../prog/en/modules/circ/circulation.tt            |   6 +-
 t/db_dependent/Circulation/TooMany.t               | 128 +++++++++++++++------
 3 files changed, 145 insertions(+), 54 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 1e722c7..50b0f3e 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -462,16 +462,28 @@ sub TooMany {
 
         if ( $onsite_checkout ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
-                return ($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') ) {
             if ( $checkout_count >= $max_checkouts_allowed ) {
-                return ($checkout_count, $max_checkouts_allowed);
+                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 ($checkout_count - $onsite_checkout_count, $max_checkouts_allowed);
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
             }
         }
     }
@@ -503,16 +515,28 @@ sub TooMany {
 
         if ( $onsite_checkout ) {
             if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
-                return ($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') ) {
             if ( $checkout_count >= $max_checkouts_allowed ) {
-                return ($checkout_count, $max_checkouts_allowed);
+                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 ($checkout_count - $onsite_checkout_count, $max_checkouts_allowed);
+                return {
+                    reason => 'TOO_MANY_CHECKOUTS',
+                    count => $checkout_count - $onsite_checkout_count,
+                    max_allowed => $max_checkouts_allowed,
+                };
             }
         }
     }
@@ -861,21 +885,20 @@ sub CanBookBeIssued {
 #
     # JB34 CHECKS IF BORROWERS DON'T HAVE ISSUE TOO MANY BOOKS
     #
-    my ($current_loan_count, $max_loans_allowed) = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout } );
-    # if TooMany max_loans_allowed returns 0 the user doesn't have permission to check out this book
-    if (defined $max_loans_allowed && $max_loans_allowed == 0) {
-        $needsconfirmation{PATRON_CANT} = 1;
-    } else {
-        if($max_loans_allowed){
-            if ( C4::Context->preference("AllowTooManyOverride") ) {
-                $needsconfirmation{TOO_MANY} = 1;
-                $needsconfirmation{current_loan_count} = $current_loan_count;
-                $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
-            } else {
-                $issuingimpossible{TOO_MANY} = 1;
-                $issuingimpossible{current_loan_count} = $current_loan_count;
-                $issuingimpossible{max_loans_allowed} = $max_loans_allowed;
-            }
+    my $toomany = TooMany( $borrower, $item->{biblionumber}, $item, { onsite_checkout => $onsite_checkout } );
+    # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book
+    if ( $toomany ) {
+        if ( $toomany->{max_allowed} == 0 ) {
+            $needsconfirmation{PATRON_CANT} = 1;
+        }
+        if ( C4::Context->preference("AllowTooManyOverride") ) {
+            $needsconfirmation{TOO_MANY} = $toomany->{reason};
+            $needsconfirmation{current_loan_count} = $toomany->{count};
+            $needsconfirmation{max_loans_allowed} = $toomany->{max_allowed};
+        } else {
+            $needsconfirmation{TOO_MANY} = $toomany->{reason};
+            $issuingimpossible{current_loan_count} = $toomany->{count};
+            $issuingimpossible{max_loans_allowed} = $toomany->{max_allowed};
         }
     }
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index 662051e..e1b8933 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -252,10 +252,14 @@ $(document).ready(function() {
     </li>
 [% END %]
 
-[% IF ( TOO_MANY ) %]
+[% IF TOO_MANY and TOO_MANY == 'TOO_MANY_CHECKOUTS' %]
     <li>Too many checked out. [% current_loan_count %] checked out, only [% max_loans_allowed %] are allowed.</li>
 [% END %]
 
+[% IF TOO_MANY and TOO_MANY == 'TOO_MANY_ONSITE_CHECKOUTS' %]
+    <li>Too many on-site checked out. [% current_loan_count %] on-site checked out, only [% max_loans_allowed %] are allowed.</li>
+[% END %]
+
 [% IF ( BORRNOTSAMEBRANCH ) %]
     <li>This patrons is from a different library ([% BORRNOTSAMEBRANCH %])</li>
 [% END %]
diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t
index bf158e5..9b32ea9 100644
--- a/t/db_dependent/Circulation/TooMany.t
+++ b/t/db_dependent/Circulation/TooMany.t
@@ -101,25 +101,41 @@ subtest '1 Issuingrule exist 0 0: no issue allowed' => sub {
     });
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 0);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 0, 0 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 0,
+            max_allowed => 0,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 0, 0 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 0,
+            max_allowed => 0,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 0, 0 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 0,
+            max_allowed => 0,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 0, 0 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 0,
+            max_allowed => 0,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
 
@@ -183,8 +199,12 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub {
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 0);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
     is(
@@ -195,13 +215,21 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub {
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
 
@@ -231,20 +259,32 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed, Do a OSCO' => sub {
         'CO should be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
 
@@ -271,8 +311,12 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 0);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
     is(
@@ -283,13 +327,21 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
 
@@ -305,20 +357,32 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
         'CO should be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 0'
     );
 
     t::lib::Mocks::mock_preference('ConsiderOnSiteCheckoutsAsNormalCheckouts', 1);
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item ),
+        {
+            reason => 'TOO_MANY_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'CO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
     is_deeply(
-        [ C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ) ],
-        [ 1, 1 ],
+        C4::Circulation::TooMany( $patron, $biblio->{biblionumber}, $item, { onsite_checkout => 1 } ),
+        {
+            reason => 'TOO_MANY_ONSITE_CHECKOUTS',
+            count => 1,
+            max_allowed => 1,
+        },
         'OSCO should not be allowed if ConsiderOnSiteCheckoutsAsNormalCheckouts == 1'
     );
 
-- 
1.9.1