From a8f4aacb1db5820b530b2b1dc4072df624f9f1f5 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 25 Oct 2017 16:51:28 -0300 Subject: [PATCH] Bug 19444: Do not auto renew if patron is expired and BlockExpiredPatronOpacActions is set If the patron's account has expired and BlockExpiredPatronOpacActions is set, we expect auto renewal to be rejected. Test plan: Use the automatic_renewals.pl cronjob script to auto renew a checkout Before this patch, if the patron's account has expired the auto renew was done. With this patch, it will only be auto renewed if BlockExpiredPatronOpacActions is not set. Signed-off-by: Claire Gravely Signed-off-by: Julian Maurice Signed-off-by: Jonathan Druart --- C4/Circulation.pm | 5 ++ misc/cronjobs/automatic_renewals.pl | 1 + t/db_dependent/Circulation.t | 75 ++++++++++++++++++++++++++++- 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9397cbc1b5..a2a9e7bc53 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2699,6 +2699,11 @@ sub CanBookBeRenewed { } if ( $issue->auto_renew ) { + + if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { + return ( 0, 'auto_account_expired' ); + } + if ( defined $issuing_rule->no_auto_renewal_after and $issuing_rule->no_auto_renewal_after ne "" ) { # Get issue_date and add no_auto_renewal_after diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 190630149f..6a170ca20f 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -85,6 +85,7 @@ while ( my $auto_renew = $auto_renews->next ) { or $error eq 'on_reserve' or $error eq 'restriction' or $error eq 'overdue' + or $error eq 'auto_account_expired' or $error eq 'auto_too_late' or $error eq 'auto_too_much_oweing' or $error eq 'auto_too_soon' ) { diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 4da9557c81..1c6c0ffd20 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 112; +use Test::More tests => 113; use DateTime; @@ -63,7 +63,16 @@ my $itemtype = $builder->build( value => { notforloan => undef, rentalcharge => 0, defaultreplacecost => undef, processfee => undef } } )->{itemtype}; -my $patron_category = $builder->build({ source => 'Category', value => { category_type => 'P', enrolmentfee => 0 } }); +my $patron_category = $builder->build( + { + source => 'Category', + value => { + category_type => 'P', + enrolmentfee => 0, + BlockExpiredPatronOpacActions => -1, # Pick the pref value + } + } +); my $CircControl = C4::Context->preference('CircControl'); my $HomeOrHoldingBranch = C4::Context->preference('HomeOrHoldingBranch'); @@ -290,13 +299,23 @@ C4::Context->dbh->do("DELETE FROM accountlines"); branchcode => $branch, ); + my %expired_borrower_data = ( + firstname => 'Ça', + surname => 'Glisse', + categorycode => $patron_category->{categorycode}, + branchcode => $branch, + dateexpiry => dt_from_string->subtract( months => 1 ), + ); + my $renewing_borrowernumber = AddMember(%renewing_borrower_data); my $reserving_borrowernumber = AddMember(%reserving_borrower_data); my $hold_waiting_borrowernumber = AddMember(%hold_waiting_borrower_data); my $restricted_borrowernumber = AddMember(%restricted_borrower_data); + my $expired_borrowernumber = AddMember(%expired_borrower_data); my $renewing_borrower = Koha::Patrons->find( $renewing_borrowernumber )->unblessed; my $restricted_borrower = Koha::Patrons->find( $restricted_borrowernumber )->unblessed; + my $expired_borrower = Koha::Patrons->find( $expired_borrowernumber )->unblessed; my $bibitems = ''; my $priority = '1'; @@ -695,6 +714,58 @@ C4::Context->dbh->do("DELETE FROM accountlines"); $dbh->do('DELETE FROM accountlines WHERE borrowernumber=?', undef, $renewing_borrowernumber); }; + subtest "auto_account_expired | BlockExpiredPatronOpacActions" => sub { + plan tests => 6; + my $item_to_auto_renew = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblionumber, + homebranch => $branch, + holdingbranch => $branch, + } + }); + + $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 11'); + + my $ten_days_before = dt_from_string->add( days => -10 ); + my $ten_days_ahead = dt_from_string->add( days => 10 ); + + # Patron is expired and BlockExpiredPatronOpacActions=0 + # => auto renew is allowed + t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 0); + my $patron = $expired_borrower; + my $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + ( $renewokay, $error ) = + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' ); + Koha::Checkouts->find( $checkout->issue_id )->delete; + + + # Patron is expired and BlockExpiredPatronOpacActions=1 + # => auto renew is not allowed + t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); + $patron = $expired_borrower; + $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + ( $renewokay, $error ) = + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' ); + Koha::Checkouts->find( $checkout->issue_id )->delete; + + + # Patron is not expired and BlockExpiredPatronOpacActions=1 + # => auto renew is allowed + t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); + $patron = $renewing_borrower; + $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + ( $renewokay, $error ) = + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + is( $renewokay, 0, 'Do not renew, renewal is automatic' ); + is( $error, 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' ); + Koha::Checkouts->find( $checkout->issue_id )->delete; + }; + subtest "GetLatestAutoRenewDate" => sub { plan tests => 5; my $item_to_auto_renew = $builder->build( -- 2.39.5