@@ -, +, @@ --- C4/Circulation.pm | 22 ++--- Koha/Patron.pm | 4 +- ..._AutoRemoveOverduesRestrictions-syspref.pl | 13 +++ ...ue-debarment-removal-allow-unrestricted.pl | 14 --- installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../admin/preferences/circulation.pref | 15 ++-- .../Circulation/MarkIssueReturned.t | 89 +++++++++++++++++-- 7 files changed, 116 insertions(+), 43 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl delete mode 100644 installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -2484,6 +2484,7 @@ sub MarkIssueReturned { # Retrieve the issue my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return; + my $issue_branchcode = $issue->branchcode; return unless $issue->borrowernumber == $borrowernumber; # If the item is checked out to another patron we do not return it @@ -2530,18 +2531,17 @@ sub MarkIssueReturned { # The reason this is here, and not in Koha::Patron->has_overdues() is # to make sure it will not cause any side effects elsewhere, since this # is only relevant for removal of debarments. - my $has_overdue_ignore_unrestricted = 0; - if(C4::Context->preference('ODueDebarmentRemovalAllowUnrestricted')) { - $has_overdue_ignore_unrestricted = 1; - } + my $has_overdue_ignore_unrestricted = C4::Context->preference('AutoRemoveOverduesRestrictions') eq 'when_no_overdue_causing_debarment'; - # Remove any OVERDUES related debarment if the borrower has no overdues - if ( C4::Context->preference('AutoRemoveOverduesRestrictions') - && $patron->debarred - && !$patron->has_overdues({ - ignore_unrestricted => $has_overdue_ignore_unrestricted, - issue_branch => $issue->{'branchcode'} }) - && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } + # Possibly remove any OVERDUES related debarment + if ( + C4::Context->preference('AutoRemoveOverduesRestrictions') ne 'no' + && $patron->debarred + && !$patron->has_overdues({ + ignore_unrestricted => $has_overdue_ignore_unrestricted, + issue_branchcode => $issue_branchcode + }) + && @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) } ) { DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); } --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -968,11 +968,11 @@ sub _get_overdue_restrict_delay { my $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = ? AND categorycode = ?"; - my $rqoverduerules = $dbh->prepare($query); + my $rqoverduerules = $dbh->prepare($query); $rqoverduerules->execute($branchcode, $categorycode); # We get default rules if there is no rule for this branch - if($rqoverduerules->rows == 0){ + if($rqoverduerules->rows == 0) { $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = '' AND categorycode = ?"; $rqoverduerules = $dbh->prepare($query); --- a/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl +++ a/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl @@ -0,0 +1,13 @@ +use Modern::Perl; + +return { + bug_number => "", + description => "Change type of AutoRemoveOverduesRestrictions system preference", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + # Do you stuffs here + $dbh->do(q{UPDATE `systempreferences` SET `type` = 'Choice', `options` = 'no|when_no_overdue|when_no_overdue_causing_debarment', `explanation` = 'Defines if and on what conditions OVERDUES debarments should automatically be lifted when overdue items are returned by the patron.', `value` = CASE `value` WHEN '1' THEN 'when_no_overdue' WHEN '0' THEN 'no' ELSE `value` END WHERE variable = 'AutoRemoveOverduesRestrictions'}); + say $out "Type of AutoRemoveOverduesRestrictions system prefernce has ben changed"; + }, +} --- a/installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl +++ a/installer/data/mysql/atomicupdate/odue-debarment-removal-allow-unrestricted.pl @@ -1,14 +0,0 @@ -use Modern::Perl; - -return { - bug_number => "", - description => "Add system preference", - up => sub { - my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; - # Do you stuffs here - $dbh->do(q{INSERT IGNORE INTO systempreferences (`variable`, `value`, `options`, `explanation`, `type`) VALUES ('ODueDebarmentRemovalAllowUnrestricted', '0', null, 'Allow removal of OVERDUES debarment when overdues still exist, but has not reached restricting delay', 'YesNo')}); - # Print useful stuff here - say $out "System preference added"; - }, -} --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/installer/data/mysql/mandatory/sysprefs.sql @@ -83,7 +83,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AutomaticItemReturn','1',NULL,'If ON, Koha will automatically set up a transfer of this item to its homebranch','YesNo'), ('AutoLinkBiblios','0',NULL,'If enabled, link biblio to authorities on creation and edit','YesNo'), ('autoMemberNum','0','','If ON, patron number is auto-calculated','YesNo'), -('AutoRemoveOverduesRestrictions','0',NULL,'Defines whether an OVERDUES debarment should be lifted automatically if all overdue items are returned by the patron.','YesNo'), +('AutoRemoveOverduesRestrictions','no','no|when_no_overdue|when_no_overdue_causing_debarment', 'Defines if and on what conditions OVERDUES debarments should automatically be lifted when overdue items are returned by the patron.','Choice'), ('AutoRenewalNotices','cron','cron|preferences|never','How should Koha determine whether to end autorenewal notices','Choice'), ('AutoResumeSuspendedHolds','1',NULL,'Allow suspended holds to be automatically resumed by a set date.','YesNo'), ('AutoReturnCheckedOutItems', '0', '', 'If disabled, librarian must confirm return of checked out item when checking out to another.', 'YesNo'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -654,12 +654,6 @@ Circulation: 1: Store 0: "Don't store" - 'the last patron to return an item. This setting is independent of the opacreadinghistory and AnonymousPatron system preferences.' - - - - pref: ODueDebarmentRemovalAllowUnrestricted - choices: - yes: Allow - no: Do not allow - - removal of Overdue debarments when patron has overdue items but none are old enough to have reached restricting delay. Used in combination with AutoRemoveOverduesRestrictions. Holds policy: - - pref: EnableItemGroups @@ -978,11 +972,14 @@ Circulation: 0: "Don't cumulate" - the restriction periods. - + - When returning items - pref: AutoRemoveOverduesRestrictions + type: choice choices: - 1: "Do" - 0: "Don't" - - allow OVERDUES restrictions triggered by sent notices to be cleared automatically when all overdue items are returned by a patron. + when_no_overdue: "if patron has no remaining overdue items" + when_no_overdue_causing_debarment: "if patron has no remaining items that is cause for debarment" + no: "don't" + - remove OVERDUES restriction triggered by sent notices. The difference between removing restriction when no remaining overdue items exists and doing so only when any of the items would result in debarment is that the latter option will respect possible grace periods of overdue rules also on returns and not only when generating overdue notices. - - If a patron is restricted, - pref: RestrictionBlockRenewing --- a/t/db_dependent/Circulation/MarkIssueReturned.t +++ a/t/db_dependent/Circulation/MarkIssueReturned.t @@ -173,20 +173,37 @@ subtest 'Manually pass a return date' => sub { }; subtest 'AutoRemoveOverduesRestrictions' => sub { - plan tests => 2; + plan tests => 5; $schema->storage->txn_begin; - t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 1); - + my $dbh = C4::Context->dbh; my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - t::lib::Mocks::mock_userenv( { branchcode => $patron->branchcode } ); + my $categorycode = $patron->categorycode; + my $branchcode = $patron->branchcode; + + $dbh->do(qq{ + INSERT INTO `overduerules` ( + `categorycode`, + `delay1`, + `letter1`, + `debarred1`, + `delay2`, + `letter2`, + `debarred2` + ) + VALUES ('$categorycode', 6, 'ODUE', 0, 10, 'ODUE2', 1) + }); + + t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue'); + + t::lib::Mocks::mock_userenv( { branchcode => $branchcode } ); my $item_1 = $builder->build_sample_item; my $item_2 = $builder->build_sample_item; my $item_3 = $builder->build_sample_item; my $five_days_ago = dt_from_string->subtract( days => 5 ); - my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # overdue - my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue + my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # overdue, but would not trigger debarment + my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue, but would not trigger debarment my $checkout_3 = AddIssue( $patron->unblessed, $item_3->barcode ); # not overdue Koha::Patron::Debarments::AddUniqueDebarment( @@ -207,5 +224,65 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); is( scalar @$debarments, 0, 'OVERDUES debarment is removed if patron does not have overdues' ); + t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue_causing_debarment'); + + my $eleven_days_ago = dt_from_string->subtract( days => 11 ); + + $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $eleven_days_ago ); # overdue and would trigger debarment + $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue, but would not trigger debarment + + Koha::Patron::Debarments::AddUniqueDebarment( + { + borrowernumber => $patron->borrowernumber, + type => 'OVERDUES', + comment => "OVERDUES_PROCESS simulation", + } + ); + + C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_1->itemnumber ); + + $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); + is( scalar @$debarments, 0, 'OVERDUES debarment is removed if remaning items would not result in patron debarment' ); + + $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $eleven_days_ago ); # overdue and would trigger debarment + + Koha::Patron::Debarments::AddUniqueDebarment( + { + borrowernumber => $patron->borrowernumber, + type => 'OVERDUES', + comment => "OVERDUES_PROCESS simulation", + } + ); + + C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); + + $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); + is( $debarments->[0]->{type}, 'OVERDUES', 'OVERDUES debarment is not removed if patron still has overdues that would trigger debarment' ); + + my $thirteen_days_ago = dt_from_string->subtract( days => 13 ); + + # overdue and would trigger debarment + $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $thirteen_days_ago ); + + # $chechout_1 should now not trigger debarment with this new rule for specific branchcode + $dbh->do(qq{ + INSERT INTO `overduerules` ( + `branchcode`, + `categorycode`, + `delay1`, + `letter1`, + `debarred1`, + `delay2`, + `letter2`, + `debarred2` + ) + VALUES ('$branchcode', '$categorycode', 6, 'ODUE', 0, 12, 'ODUE2', 1) + }); + + C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); + + $debarments = Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber }); + is( scalar @$debarments, 0, 'OVERDUES debarment is removed if remaning items would not result in patron debarment' ); + $schema->storage->txn_rollback; }; --