Attachment 172601 Details for Bug 10190 – Bug 10190: Update has_restricting_overdues to refer to circulation rules

[patch] Bug 10190: Update has_restricting_overdues to refer to circulation rules

Bug-10190-Update-hasrestrictingoverdues-to-refer-t.patch (text/plain), 7.22 KB, created by Martin Renvoize (ashimema) on 2024-10-10 14:39:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-10-10 14:39:58 UTC

Size: 7.22 KB

patch

obsolete

>From 72cd818a0504e59ccb06aa78a8d5f9631e9b612e Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Thu, 8 Aug 2024 14:52:31 +0100
>Subject: [PATCH] Bug 10190: Update has_restricting_overdues to refer to
> circulation rules
>
>This patch updates the Koha::Patron->has_restriction_overdues method to
>refer to the migrated overdue rules.
>
>We also correct an oversight in the method prior to this patch that
>meant that not all overdues were being correctly checked for a
>restriction.. we were incorrectly filtering to only overdues at the
>current checkin branch.
>
>Finally, we also take account of the itemtype level addition here too.
>
>NOTE: There weren't any unit tests for this Koha::Patron method.. we
>should add some here!
>
>Sponsored-by: Glasgow Colleges Library Group
>---
> C4/Circulation.pm |   2 +-
> Koha/Patron.pm    | 100 +++++++++++++++++++++++-----------------------
> 2 files changed, 50 insertions(+), 52 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index bd8ecfc58fa..ca375d102bf 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2673,7 +2673,7 @@ sub MarkIssueReturned {
>         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_restricting_overdues()
>                 : !$patron->has_overdues;
>             if ( $remove_restrictions && $overdue_restrictions->count ) {
>                 DelUniqueDebarment( { borrowernumber => $borrowernumber, type => 'OVERDUES' } );
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index fcbd6cd71ed..d2a104709bf 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -46,7 +46,6 @@ use Koha::Exceptions;
> use Koha::Exceptions::Password;
> use Koha::Holds;
> use Koha::Old::Checkouts;
>-use Koha::OverdueRules;
> use Koha::Patron::Attributes;
> use Koha::Patron::Categories;
> use Koha::Patron::Consents;
>@@ -1120,23 +1119,23 @@ sub has_overdues {
> 
> =head3 has_restricting_overdues
> 
>-my $has_restricting_overdues = $patron->has_restricting_overdues({ issue_branchcode => $branchcode });
>+  my $has_restricting_overdues = $patron->has_restricting_overdues();
> 
> Returns true if patron has overdues that would result in debarment.
> 
> =cut
> 
> sub has_restricting_overdues {
>-    my ( $self, $params ) = @_;
>-    $params //= {};
>-    my $date = dt_from_string()->truncate( to => 'day' );
>+    my ($self) = @_;
> 
>-    # 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.
>-    #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;
>+    # Fetch overdues
>+    my $overdues = $self->overdues;
>+
>+    # Short circuit if no overdues
>+    return 0 unless $overdues->count;
>+
>+    my $categorycode = $self->categorycode();
>+    my $calendar;
> 
>     # Emulate the conditions in overdue_notices.pl.
>     # The overdue_notices-script effectively truncates both issues.date_due and current date
>@@ -1148,54 +1147,53 @@ sub has_restricting_overdues {
>     # 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.
>+    my $date = dt_from_string()->truncate( to => 'day' )->add( days => 1 );
> 
>-    $date->add( days => 1 );
>+    # Work from oldest overdue to newest
>+    $overdues = $overdues->search( {}, { order_by => { '-desc' => 'me.date_due' } } );
>+    my $now = dt_from_string();
> 
>-    my $calendar;
>-    if ( C4::Context->preference('OverdueNoticeCalendar') ) {
>-        $calendar = Koha::Calendar->new( branchcode => $params->{issue_branchcode} );
>-    }
>+    my ( $itemtype, $branchcode ) = ( "", "" );
>+    while ( my $overdue = $overdues->next ) {
> 
>-    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();
>+        # Short circuit if we're looking at the same branch and itemtype combination as last time as we've
>+        # checked the oldest for this combination already
>+        next if ( ( $overdue->branchcode eq $branchcode ) && ( $overdue->item->itemtype eq $itemtype ) );
> 
>-    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;
>-}
>+        # Capture the current branchcode and itemtype
>+        $branchcode = $overdue->branchcode;
>+        $itemtype   = $overdue->item->itemtype;
> 
>-# Fetch first delayX value from overduerules where debarredX is set, or 0 for no delay
>-sub _get_overdue_debarred_delay {
>-    my ( $branchcode, $categorycode ) = @_;
>-    my $dbh = C4::Context->dbh();
>+        my $i = 0;
>+    DELAY: while (1) {
>+            $i++;
>+            my $overdue_rules = Koha::CirculationRules->get_effective_rules(
>+                {
>+                    rules        => [ "overdue_$i" . '_delay', "overdue_$i" . '_restrict' ],
>+                    categorycode => $categorycode,
>+                    branchcode   => $branchcode,
>+                    itemtype     => $itemtype,
>+                }
>+            );
> 
>-    # We get default rules if there is no rule for this branch
>-    my $rule = Koha::OverdueRules->find(
>-        {
>-            branchcode   => $branchcode,
>-            categorycode => $categorycode
>-        }
>-        )
>-        || Koha::OverdueRules->find(
>-        {
>-            branchcode   => q{},
>-            categorycode => $categorycode
>-        }
>-        );
>+            last DELAY if ( !defined( $overdue_rules->{ "overdue_$i" . '_delay' } ) );
> 
>-    if ($rule) {
>-        return $rule->delay1 if $rule->debarred1;
>-        return $rule->delay2 if $rule->debarred2;
>-        return $rule->delay3 if $rule->debarred3;
>+            next DELAY unless $overdue_rules->{ "overdue_$i" . '_restrict' };
>+
>+            if ( C4::Context->preference('OverdueNoticeCalendar') ) {
>+                $calendar = Koha::Calendar->new( branchcode => $branchcode );
>+            }
>+
>+            my $days_between =
>+                C4::Context->preference('OverdueNoticeCalendar')
>+                ? $calendar->days_between( dt_from_string( $overdue->date_due ), $now )->in_units('days')
>+                : $now->delta_days( dt_from_string( $overdue->date_due ) )->in_units('days');
>+            if ( $days_between >= $overdue_rules->{ "overdue_$i" . '_delay' } ) {
>+                return 1;
>+            }
>+        }
>     }
>+    return 0;
> }
> 
> =head3 update_lastseen
>-- 
>2.47.0

Actions: View | Diff | Splinter Review

Attachments on bug 10190: 170357 | 170358 | 170359 | 170360 | 170361 | 170362 | 170363 | 170364 | 170365 | 170366 | 170367 | 170368 | 170369 | 170370 | 170371 | 170372 | 170373 | 170374 | 170375 | 170376 | 170380 | 170449 | 171727 | 171728 | 171729 | 171730 | 171731 | 171732 | 171733 | 171734 | 171735 | 171736 | 171737 | 171738 | 171739 | 171740 | 171741 | 171742 | 171743 | 171744 | 171745 | 171746 | 171747 | 171748 | 171749 | 171750 | 171751 | 171752 | 171753 | 171754 | 171755 | 172588 | 172589 | 172590 | 172591 | 172592 | 172593 | 172594 | 172595 | 172596 | 172597 | 172598 | 172599 | 172600 | 172601 | 172602 | 172603 | 172604 | 172605 | 172606 | 172607 | 172608 | 172609 | 172612 | 172614 | 172617 | 172619 | 172621 | 172622 | 172623 | 172624 | 172625 | 172626 | 172630 | 172631 | 172632 | 172633 | 172634 | 172635 | 172636 | 172637 | 172638 | 172639 | 172640 | 172641 | 172642 | 172643 | 172644 | 172645 | 172646 | 172647 | 172648 | 172649 | 172650 | 172651 | 172652 | 172653 | 172654 | 172655 | 172656 | 172657 | 172658 | 172659 | 172660 | 172661 | 172662 | 172663