@@ -, +, @@ --- C4/Circulation.pm | 17 +++-- Koha/Patron.pm | 39 ++++++----- ..._AutoRemoveOverduesRestrictions-syspref.pl | 13 ++-- .../Circulation/MarkIssueReturned.t | 67 ++++++++++++------- 4 files changed, 79 insertions(+), 57 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -2632,18 +2632,17 @@ sub MarkIssueReturned { } # Possibly remove any OVERDUES related debarment - my $overdue_restrictions = $patron->restrictions->search({ type => 'OVERDUES' }); - if (C4::Context->preference('AutoRemoveOverduesRestrictions') ne 'no' && $patron->is_debarred) { + my $overdue_restrictions = $patron->restrictions->search( { type => 'OVERDUES' } ); + if ( C4::Context->preference('AutoRemoveOverduesRestrictions') ne 'no' && $patron->is_debarred ) { my $remove_restrictions = - C4::Context->preference('AutoRemoveOverduesRestrictions') eq 'when_no_overdue_causing_debarment' ? - !$patron->has_restricting_overdues({ issue_branchcode => $issue_branchcode }) : - !$patron->has_overdues; - if ( - $remove_restrictions && $overdue_restrictions->count - ) { - DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' }); + C4::Context->preference('AutoRemoveOverduesRestrictions') eq 'when_no_overdue_causing_debarment' + ? !$patron->has_restricting_overdues( { issue_branchcode => $issue_branchcode } ) + : !$patron->has_overdues; + if ( $remove_restrictions && $overdue_restrictions->count ) { + DelUniqueDebarment( { borrowernumber => $borrowernumber, type => 'OVERDUES' } ); } } + }); return $issue_id; --- a/Koha/Patron.pm +++ a/Koha/Patron.pm @@ -1001,6 +1001,7 @@ sub has_overdues { } + =head3 has_restricting_overdues my $has_restricting_overdues = $patron->has_restricting_overdues({ issue_branchcode => $branchcode }); @@ -1010,7 +1011,7 @@ Returns true if patron has overdues that would result in debarment. =cut sub has_restricting_overdues { - my ($self, $params) = @_; + my ( $self, $params ) = @_; $params //= {}; my $date = dt_from_string()->truncate( to => 'day' ); @@ -1018,7 +1019,7 @@ sub has_restricting_overdues { # overdue messages is set with restrictions. Then only include overdue # issues older than that date when counting. #TODO: bail out/throw exception if $params->{issue_branchcode} not set? - my $debarred_delay = _get_overdue_debarred_delay($params->{issue_branchcode}, $self->categorycode()); + my $debarred_delay = _get_overdue_debarred_delay( $params->{issue_branchcode}, $self->categorycode() ); return 0 unless defined $debarred_delay; # Emulate the conditions in overdue_notices.pl. @@ -1032,22 +1033,23 @@ sub has_restricting_overdues { # using the condition that date_due must be less then the current date truncated to days (time set to 00:00:00) # offset by one day in the future. - $date->add(days => 1); + $date->add( days => 1 ); my $calendar; - if (C4::Context->preference('OverdueNoticeCalendar')) { + if ( C4::Context->preference('OverdueNoticeCalendar') ) { $calendar = Koha::Calendar->new( branchcode => $params->{issue_branchcode} ); } - my $dtf = Koha::Database->new->schema->storage->datetime_parser; - my $issues = $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime($date) } }); - my $now = dt_from_string(); - - while (my $issue = $issues->next) { - my $days_between = C4::Context->preference('OverdueNoticeCalendar') ? - $calendar->days_between(dt_from_string($issue->date_due), $now)->in_units('days') : - $now->delta_days(dt_from_string($issue->date_due))->in_units('days'); - if ($days_between >= $debarred_delay) { + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + my $issues = $self->_result->issues->search( { date_due => { '<' => $dtf->format_datetime($date) } } ); + my $now = dt_from_string(); + + while ( my $issue = $issues->next ) { + my $days_between = + C4::Context->preference('OverdueNoticeCalendar') + ? $calendar->days_between( dt_from_string( $issue->date_due ), $now )->in_units('days') + : $now->delta_days( dt_from_string( $issue->date_due ) )->in_units('days'); + if ( $days_between >= $debarred_delay ) { return 1; } } @@ -1056,7 +1058,7 @@ sub has_restricting_overdues { # Fetch first delayX value from overduerules where debarredX is set, or 0 for no delay sub _get_overdue_debarred_delay { - my ($branchcode, $categorycode) = @_; + my ( $branchcode, $categorycode ) = @_; my $dbh = C4::Context->dbh(); # We get default rules if there is no rule for this branch @@ -1065,21 +1067,22 @@ sub _get_overdue_debarred_delay { branchcode => $branchcode, categorycode => $categorycode } - ) - || Koha::OverdueRules->find( + ) + || Koha::OverdueRules->find( { branchcode => q{}, categorycode => $categorycode } - ); + ); - if ( $rule ) { + if ($rule) { return $rule->delay1 if $rule->debarred1; return $rule->delay2 if $rule->debarred2; return $rule->delay3 if $rule->debarred3; } } + =head3 track_login $patron->track_login; --- a/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl +++ a/installer/data/mysql/atomicupdate/bug_29145-modify_AutoRemoveOverduesRestrictions-syspref.pl @@ -1,13 +1,16 @@ use Modern::Perl; return { - bug_number => "29145", + bug_number => "29145", description => "Change type of AutoRemoveOverduesRestrictions system preference", - up => sub { + up => sub { my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; + 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'}); + $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 preference has been changed"; }, -} + } --- a/t/db_dependent/Circulation/MarkIssueReturned.t +++ a/t/db_dependent/Circulation/MarkIssueReturned.t @@ -177,12 +177,13 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { $schema->storage->txn_begin; - my $dbh = C4::Context->dbh; - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $dbh = C4::Context->dbh; + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); my $categorycode = $patron->categorycode; - my $branchcode = $patron->branchcode; + my $branchcode = $patron->branchcode; - $dbh->do(qq{ + $dbh->do( + qq{ INSERT INTO `overduerules` ( `categorycode`, `delay1`, @@ -193,30 +194,33 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { `debarred2` ) VALUES ('$categorycode', 6, 'ODUE', 0, 10, 'ODUE2', 1) - }); + } + ); - t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue'); + 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 $nine_days_ago = dt_from_string->subtract( days => 9 ); #->set_hour(23)->set_minute(59); - my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $nine_days_ago ); # overdue, but would not trigger debarment - my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $nine_days_ago ); # overdue, but would not trigger debarment - my $checkout_3 = AddIssue( $patron->unblessed, $item_3->barcode ); # not overdue + my $item_1 = $builder->build_sample_item; + my $item_2 = $builder->build_sample_item; + my $item_3 = $builder->build_sample_item; + my $nine_days_ago = dt_from_string->subtract( days => 9 ); #->set_hour(23)->set_minute(59); + my $checkout_1 = + AddIssue( $patron->unblessed, $item_1->barcode, $nine_days_ago ); # overdue, but would not trigger debarment + my $checkout_2 = + AddIssue( $patron->unblessed, $item_2->barcode, $nine_days_ago ); # overdue, but would not trigger debarment + my $checkout_3 = AddIssue( $patron->unblessed, $item_3->barcode ); # not overdue Koha::Patron::Debarments::AddUniqueDebarment( { borrowernumber => $patron->borrowernumber, type => 'OVERDUES', - comment => "OVERDUES_PROCESS simulation", + comment => "OVERDUES_PROCESS simulation", } ); C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_1->itemnumber ); - my $restrictions = $patron->restrictions; + my $restrictions = $patron->restrictions; my $THE_restriction = $restrictions->next; is( $THE_restriction->type->code, 'OVERDUES', 'OVERDUES debarment is not removed if patron still has overdues' ); @@ -225,25 +229,29 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { $restrictions = $patron->restrictions; is( $restrictions->count, 0, 'OVERDUES debarment is removed if patron does not have overdues' ); - t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue_causing_debarment'); + t::lib::Mocks::mock_preference( 'AutoRemoveOverduesRestrictions', 'when_no_overdue_causing_debarment' ); my $ten_days_ago = dt_from_string->subtract( days => 10 ); $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $ten_days_ago ); # overdue and would trigger debarment - $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $nine_days_ago ); # overdue, but would not trigger debarment + $checkout_2 = + AddIssue( $patron->unblessed, $item_2->barcode, $nine_days_ago ); # overdue, but would not trigger debarment Koha::Patron::Debarments::AddUniqueDebarment( { borrowernumber => $patron->borrowernumber, type => 'OVERDUES', - comment => "OVERDUES_PROCESS simulation", + comment => "OVERDUES_PROCESS simulation", } ); C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_1->itemnumber ); $restrictions = $patron->restrictions; - is($restrictions->count, 0, 'OVERDUES debarment is removed if remaining items would not result in patron debarment' ); + is( + $restrictions->count, 0, + 'OVERDUES debarment is removed if remaining items would not result in patron debarment' + ); $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $ten_days_ago ); # overdue and would trigger debarment @@ -251,14 +259,17 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { { borrowernumber => $patron->borrowernumber, type => 'OVERDUES', - comment => "OVERDUES_PROCESS simulation", + comment => "OVERDUES_PROCESS simulation", } ); C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); - $restrictions = $patron->restrictions->search({ type => 'OVERDUES' }); - is( $restrictions->count, 1, 'OVERDUES debarment is not removed if patron still has overdues that would trigger debarment' ); + $restrictions = $patron->restrictions->search( { type => 'OVERDUES' } ); + is( + $restrictions->count, 1, + 'OVERDUES debarment is not removed if patron still has overdues that would trigger debarment' + ); my $eleven_days_ago = dt_from_string->subtract( days => 11 ); @@ -266,7 +277,8 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $eleven_days_ago ); # $checkout_1 should now not trigger debarment with this new rule for specific branchcode - $dbh->do(qq{ + $dbh->do( + qq{ INSERT INTO `overduerules` ( `branchcode`, `categorycode`, @@ -278,12 +290,17 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { `debarred2` ) VALUES ('$branchcode', '$categorycode', 6, 'ODUE', 0, 11, 'ODUE2', 1) - }); + } + ); C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber ); $restrictions = $patron->restrictions; - is( $restrictions->count, 0, 'OVERDUES debarment is removed if remaining items would not result in patron debarment' ); + is( + $restrictions->count, 0, + 'OVERDUES debarment is removed if remaining items would not result in patron debarment' + ); $schema->storage->txn_rollback; }; + --