From 31b0f71f7f49d27bf2c01250cddf63b83139e7ba 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 | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 67a986b417..5ad917c9d0 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2064,14 +2064,14 @@ sub AddReturn { if ( $issue and $issue->is_overdue($return_date) ) { # fix fine days - my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date ); - if ($reminder){ - $messages->{'PrevDebarred'} = $debardate; - } else { - $messages->{'Debarred'} = $debardate if $debardate; + my $newdebardate = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date ); + + 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 { @@ -2361,7 +2361,7 @@ sub _calculate_new_debar_dt { else { $new_debar_dt = $return_date->clone()->add_duration($suspension_days); } - return $new_debar_dt; + return ($new_debar_dt, $has_been_extended); } return; } @@ -2371,7 +2371,7 @@ sub _debar_user_on_return { $return_date //= dt_from_string(); - my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date); + my ($new_debar_dt, $has_been_extended) = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date); return unless $new_debar_dt; @@ -2380,17 +2380,12 @@ 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, $is_a_reminder); - if ( $borrower->{debarred} && $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} && $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; } =head2 _FixOverduesOnReturn -- 2.17.1