From c91b0d49a6375a2a7a3576d118700f4752ba03fb Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 13 May 2015 14:04:50 +0200 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- 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 1e722c7a2d..50b0f3eda4 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 662051ea6a..e1b89333cc 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() { [% END %] -[% IF ( TOO_MANY ) %] +[% IF TOO_MANY and TOO_MANY == 'TOO_MANY_CHECKOUTS' %]
  • Too many checked out. [% current_loan_count %] checked out, only [% max_loans_allowed %] are allowed.
  • [% END %] +[% IF TOO_MANY and TOO_MANY == 'TOO_MANY_ONSITE_CHECKOUTS' %] +
  • Too many on-site checked out. [% current_loan_count %] on-site checked out, only [% max_loans_allowed %] are allowed.
  • +[% END %] + [% IF ( BORRNOTSAMEBRANCH ) %]
  • This patrons is from a different library ([% BORRNOTSAMEBRANCH %])
  • [% END %] diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t index bf158e5571..9b32ea9989 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' ); -- 2.39.5