From d0aea72710e5045083de555c79dcff2e95e917fa Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 Signed-off-by: George Harkins --- 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