Bug 29145: Fix conditions for patron debarring overdues

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
David Gustafsson 2022-10-13 13:20:12 +02:00 committed by Tomas Cohen Arazi
parent ee8ef4b174
commit 4678505e1e
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
3 changed files with 66 additions and 34 deletions

View file

@ -2624,23 +2624,19 @@ sub MarkIssueReturned {
$item->last_returned_by( $patron->borrowernumber )->store;
}
# 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 = C4::Context->preference('AutoRemoveOverduesRestrictions') eq 'when_no_overdue_causing_debarment';
# Possibly remove any OVERDUES related debarment
my $overdue_restrictions = $patron->restrictions->search({ type => 'OVERDUES' });
if ( C4::Context->preference('AutoRemoveOverduesRestrictions' ne 'no')
&& $patron->debarred
&& $overdue_restrictions->count
&& !$patron->has_overdues({
ignore_unrestricted => $has_overdue_ignore_unrestricted,
issue_branch => $issue->{'branchcode'} })
) {
DelUniqueDebarment({ borrowernumber => $borrowernumber, 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_debarring_overdues({ issue_branchcode => $issue_branchcode }) :
!$patron->has_overdues;
if (
$remove_restrictions && $overdue_restrictions->count
) {
DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
}
}
});
return $issue_id;

View file

@ -1008,24 +1008,59 @@ Returns the number of patron's overdues
=cut
sub has_overdues {
my ($self, $params) = @_;
my ($self) = @_;
my $date = dt_from_string();
my $dtf = Koha::Database->new->schema->storage->datetime_parser;
return $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime($date) } })->count;
}
sub has_debarring_overdues {
my ($self, $params) = @_;
$params //= {};
my $date = dt_from_string()->truncate( to => 'day' );
# If ignoring unrestricted overdues, calculate which delay value for
# overdue messages is set with restrictions. Then only include overdue
# issues older than that date when counting.
if($params->{ignore_unrestricted}) {
my $branchcode = $params->{issue_branchcode};
my $date_offset = _get_overdue_restrict_delay($params->{issue_branchcode}, $self->categorycode());
$date->subtract(days => $date_offset);
#TODO: bail out/throw exception if $params->{issue_branchcode} not set?
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.
# The overdue_notices-script effectively truncates both issues.date_due and current date
# to days when selecting overdue issues.
# Hours and minutes for issues.date_due is usually set to 23 and 59 respectively, though can theoretically
# be set to any other value (truncated to minutes, except if CalcDateDue gets a $startdate)
#
# No matter what time of day date_due is set to, overdue_notices.pl will select all issues that are due
# the current date or later. We can emulate this query by instead of truncating both to days in the SQL-query,
# 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);
my $calendar;
if (C4::Context->preference('OverdueNoticeCalendar')) {
$calendar = Koha::Calendar->new( branchcode => $params->{issue_branchcode} );
}
my $dtf = Koha::Database->new->schema->storage->datetime_parser;
return $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime( $date )} })->count;
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;
}
}
return 0;
}
# Fetch first delayX value from overduerules where debarredX is set, or 0 for no delay
sub _get_overdue_restrict_delay {
sub _get_overdue_debarred_delay {
my ($branchcode, $categorycode) = @_;
my $dbh = C4::Context->dbh();
@ -1047,8 +1082,7 @@ sub _get_overdue_restrict_delay {
return $overdue_rules->{"delay2"} if($overdue_rules->{"debarred2"});
return $overdue_rules->{"delay3"} if($overdue_rules->{"debarred3"});
}
return 0;
return undef;
}
=head3 track_login

View file

@ -201,9 +201,9 @@ subtest 'AutoRemoveOverduesRestrictions' => sub {
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, $item_1->barcode, $five_days_ago ); # overdue, but would not trigger debarment
my $checkout_2 = AddIssue( $patron, $item_2->barcode, $five_days_ago ); # overdue, but would not trigger debarment
my $nine_days_ago = dt_from_string->subtract( days => 9 ); #->set_hour(23)->set_minute(59);
my $checkout_1 = AddIssue( $patron, $item_1->barcode, $nine_days_ago ); # overdue, but would not trigger debarment
my $checkout_2 = AddIssue( $patron, $item_2->barcode, $nine_days_ago ); # overdue, but would not trigger debarment
my $checkout_3 = AddIssue( $patron, $item_3->barcode ); # not overdue
Koha::Patron::Debarments::AddUniqueDebarment(
@ -227,10 +227,12 @@ subtest 'AutoRemoveOverduesRestrictions' => sub {
t::lib::Mocks::mock_preference('AutoRemoveOverduesRestrictions', 'when_no_overdue_causing_debarment');
my $eleven_days_ago = dt_from_string->subtract( days => 11 );
my $ten_days_ago = dt_from_string->subtract( days => 10 );
$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
$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
print STDERR "DUEDATE: " . $nine_days_ago->stringify . "\n";
Koha::Patron::Debarments::AddUniqueDebarment(
{
@ -245,7 +247,7 @@ subtest 'AutoRemoveOverduesRestrictions' => sub {
$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
$checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $ten_days_ago ); # overdue and would trigger debarment
Koha::Patron::Debarments::AddUniqueDebarment(
{
@ -260,12 +262,12 @@ subtest 'AutoRemoveOverduesRestrictions' => sub {
$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 );
my $eleven_days_ago = dt_from_string->subtract( days => 11 );
# overdue and would trigger debarment
$checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $thirteen_days_ago );
$checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $eleven_days_ago );
# $chechout_1 should now not trigger debarment with this new rule for specific branchcode
# $checkout_1 should now not trigger debarment with this new rule for specific branchcode
$dbh->do(qq{
INSERT INTO `overduerules` (
`branchcode`,
@ -277,7 +279,7 @@ subtest 'AutoRemoveOverduesRestrictions' => sub {
`letter2`,
`debarred2`
)
VALUES ('$branchcode', '$categorycode', 6, 'ODUE', 0, 12, 'ODUE2', 1)
VALUES ('$branchcode', '$categorycode', 6, 'ODUE', 0, 11, 'ODUE2', 1)
});
C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item_2->itemnumber );