From de9d4d7d61b91a973b9dea25d92e53c60f4276eb Mon Sep 17 00:00:00 2001 From: David Bourgault Date: Wed, 22 Nov 2017 16:04:03 -0500 Subject: [PATCH] Bug 14784 - Fix checkin message for debarred patrons After thinking about it I changed the code's behavior to always display a reminder if the patron was previously debarred. This makes sense to me (it's only a reminder) and it correctly stacks with other messages. -- Test plan: Before 1) Select a user with active restrictions (manual restriction works) 2) Make sure finesday=0 for the user category. See [1] 3) Checkout and return an item (not overdue) A previous restriction reminder will appear 4) Checkout and return an overdue item (change the date at cehckout) No previous restriction reminder will appear After applying patch: Same steps, but a reminder should appear for step 4) I've also tested with finesday=1 with "no overdue", "grace period overdue" and "long overdue" test cases. The appropriate nessage appeared for each test. [1] The "finesday" setting is called "Suspension in days" in the web interface, if you're searching for it like I did... Signed-off-by: Katrin Fischer Signed-off-by: Victor Grousset --- C4/Circulation.pm | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9c8d5ba..4822d7d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1989,18 +1989,19 @@ sub AddReturn { my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox); defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!"; # zero is OK, check defined + my $newdebardate; if ( $issue and $issue->is_overdue ) { - # fix fine days + # fix fine days $today = dt_from_string($return_date) if $return_date; $today = $dropboxdate if $dropbox; - my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today ); - if ($reminder){ - $messages->{'PrevDebarred'} = $debardate; - } else { - $messages->{'Debarred'} = $debardate if $debardate; + $newdebardate = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today ); + + if ( $newdebardate ) { + $messages->{'Debarred'} = $newdebardate; } + } # there's no overdue on the item but borrower had been previously debarred - } elsif ( $issue->date_due and $patron->debarred ) { + if ( $patron->debarred ) { if ( $patron->debarred eq "9999-12-31") { $messages->{'ForeverDebarred'} = $patron->debarred; } else { @@ -2194,6 +2195,8 @@ Internal function, called only by AddReturn that calculates and updates Should only be called for overdue returns +Only returns a valid date if there is a new (or extended) debarment. + =cut sub _debar_user_on_return { @@ -2237,7 +2240,7 @@ sub _debar_user_on_return { if DateTime::Duration->compare( $max_sd, $suspension_days ) < 0; } - my ( $has_been_extended, $is_a_reminder ); + my ( $has_been_extended ); if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) { my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0]; if ( $debarment ) { @@ -2271,17 +2274,13 @@ sub _debar_user_on_return { expiration => $new_debar_dt->ymd(), type => 'SUSPENSION', }); - # if borrower was already debarred but does not get an extra debarment + + # Only return a valid date if there is a new (or extented) debarment my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); - my $new_debarment_str; - if ( $borrower->{debarred} eq $patron->is_debarred ) { - $is_a_reminder = 1; - $new_debarment_str = $borrower->{debarred}; - } else { - $new_debarment_str = $new_debar_dt->ymd(); + if ( $borrower->{debarred} ne $patron->is_debarred or $has_been_extended ) { + # FIXME Should return a DateTime object + return $new_debar_dt->ymd(); } - # FIXME Should return a DateTime object - return $new_debarment_str, $is_a_reminder; } } return; -- 2.7.4