Attachment 144775 Details for Bug 29145 – Bug 29145: Fix conditions for patron debarring overdues

[patch] Bug 29145: Fix conditions for patron debarring overdues

Bug-29145-Fix-conditions-for-patron-debarring-over.patch (text/plain), 9.99 KB, created by David Gustafsson on 2022-12-21 16:42:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Gustafsson

Created: 2022-12-21 16:42:37 UTC

Size: 9.99 KB

patch

obsolete

>From 562e433cd23518840284bd61858b3eb245c1f993 Mon Sep 17 00:00:00 2001
>From: David Gustafsson <david.gustafsson@ub.gu.se>
>Date: Thu, 13 Oct 2022 13:20:12 +0200
>Subject: [PATCH] Bug 29145: Fix conditions for patron debarring overdues
>
>Signed-off-by: Michaela <michaela.sieber@kit.edu>
>---
> C4/Circulation.pm                             | 27 ++++------
> Koha/Patron.pm                                | 52 +++++++++++++++----
> .../Circulation/MarkIssueReturned.t           | 24 +++++----
> 3 files changed, 67 insertions(+), 36 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index ce33dde3e6..a719df3a78 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2540,24 +2540,19 @@ sub MarkIssueReturned {
>             $item->last_returned_by( $patron );
>         }
> 
>-        # 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
>-        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' });
>+        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 &&
>+                @{ GetDebarments({ borrowernumber => $borrowernumber, type => 'OVERDUES' }) }
>+            ) {
>+                DelUniqueDebarment({ borrowernumber => $borrowernumber, type => 'OVERDUES' });
>+            }
>         }
>-
>     });
> 
>     return $issue_id;
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index ffa771104a..b3edbe0cbc 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -979,24 +979,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();
> 
>@@ -1018,8 +1053,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
>diff --git a/t/db_dependent/Circulation/MarkIssueReturned.t b/t/db_dependent/Circulation/MarkIssueReturned.t
>index 21a4227d46..3396e7d812 100755
>--- a/t/db_dependent/Circulation/MarkIssueReturned.t
>+++ b/t/db_dependent/Circulation/MarkIssueReturned.t
>@@ -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->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 $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(
>@@ -226,10 +226,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, $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_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
>+    print STDERR "DUEDATE: " . $nine_days_ago->stringify . "\n";
> 
>     Koha::Patron::Debarments::AddUniqueDebarment(
>         {
>@@ -244,7 +246,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(
>         {
>@@ -259,12 +261,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`,
>@@ -276,7 +278,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 );
>-- 
>2.35.1

Actions: View | Diff | Splinter Review

Attachments on bug 29145: 125495 | 140819 | 140820 | 141822 | 141823 | 142394 | 142395 | 142396 | 142814 | 142815 | 142816 | 142817 | 143213 | 144773 | 144774 | 144775 | 144776 | 144777 | 144778 | 144779 | 144780 | 144781 | 144782 | 145720 | 145749 | 145750 | 145751 | 153804 | 153805 | 153806 | 153807 | 153808 | 153809 | 153810 | 153811 | 153812 | 153813 | 153814 | 156392 | 156393 | 156394 | 156395 | 156396 | 156397 | 156398 | 156399 | 156400 | 156401 | 156402 | 156403 | 156404