When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Confirmed as a problem in master by kidclamp and khall.
Created attachment 95625 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue.
Created attachment 95626 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Test Plan: 1) Back date a checkout so it is overdue 2) Run fines.pl to generate the fine 3) Return the item, backdating to the same date/time is was due 4) Note the fine was not removed 5) Apply this patch 6) Repeat steps 1-3 7) Fine should be zeroed out now!
Created attachment 95654 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Test Plan: 1) Back date a checkout so it is overdue 2) Run fines.pl to generate the fine 3) Return the item, backdating to the same date/time is was due 4) Note the fine was not removed 5) Apply this patch 6) Repeat steps 1-3 7) Fine should be zeroed out now!
Created attachment 95655 [details] [review] Bug 24075: Unit tests
Created attachment 95656 [details] [review] Bug 24075: Unit tests
Comment on attachment 95654 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded Review of attachment 95654 [details] [review]: ----------------------------------------------------------------- ::: C4/Circulation.pm @@ +1966,4 @@ > MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy ); > }; > unless ( $@ ) { > + if ( $return_date_is_forced || ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue && !$item->itemlost ) ) { This feels somewhat wrong.. I'm sure Katrin would say "But we don't ever want to recalculate fines if we have 'CalculateFinesOnReturn' turned off" So.. could this perhaps be: `if ( C4::Context->preference('CalculateFinesOnReturn') && ($return_date_is_forced || $is_overdue) && !$item->itemlost ) {` Instead?
I like it! Updated patch incoming! (In reply to Martin Renvoize from comment #6) > Comment on attachment 95654 [details] [review] [review] > Bug 24075: Backdating a return to the exact due date and time results in the > fine not being refunded > > Review of attachment 95654 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Circulation.pm > @@ +1966,4 @@ > > MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy ); > > }; > > unless ( $@ ) { > > + if ( $return_date_is_forced || ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue && !$item->itemlost ) ) { > > This feels somewhat wrong.. I'm sure Katrin would say "But we don't ever > want to recalculate fines if we have 'CalculateFinesOnReturn' turned off" > > So.. could this perhaps be: > > `if ( C4::Context->preference('CalculateFinesOnReturn') && > ($return_date_is_forced || $is_overdue) && !$item->itemlost ) {` > > Instead?
Created attachment 95706 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Test Plan: 1) Back date a checkout so it is overdue 2) Run fines.pl to generate the fine 3) Return the item, backdating to the same date/time is was due 4) Note the fine was not removed 5) Apply this patch 6) Repeat steps 1-3 7) Fine should be zeroed out now!
Created attachment 95707 [details] [review] Bug 24075: Unit tests
I would say it has been caused by commit 467c24cb9da02deb00bf49333351a0843cdeda10 Bug 14591: book drop / drop box mode incorrectly decrements accrued overdue fines
Created attachment 95719 [details] [review] Bug 24075: Alternative 1
Created attachment 95720 [details] [review] Bug 24075: Alternative 2
Lets golf a little.. I've uploaded a couple of alternatives.. let me know which of the three options seems clearest
Created attachment 95723 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Test Plan: 1) Back date a checkout so it is overdue 2) Run fines.pl to generate the fine 3) Return the item, backdating to the same date/time is was due 4) Note the fine was not removed 5) Apply this patch 6) Repeat steps 1-3 7) Fine should be zeroed out now! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 95724 [details] [review] Bug 24075: Unit tests
Tests pass: prove t/db_dependent/Circulation/CalcFine.t t/db_dependent/Fines.t t/db_dependent/Circulation.t Behavior is the described one. Passing!
Created attachment 95727 [details] [review] Bug 24075: Backdating a return to the exact due date and time results in the fine not being refunded When you use the date picker or book drop mode and check in an item that is due on that date, a fine is assessed and not refunded. For example: item 12345 is due on 11/19/2109 23:59. On 11/20/2019 I check the item in using date picker/book drop setting the check in date to 11/19/2019 23:59, the patron is charged a fine, and the fine is not cleared as would be expected, since the item is being checked in before it is overdue. Test Plan: 1) Back date a checkout so it is overdue 2) Run fines.pl to generate the fine 3) Return the item, backdating to the same date/time is was due 4) Note the fine was not removed 5) Apply this patch 6) Repeat steps 1-3 7) Fine should be zeroed out now! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 95728 [details] [review] Bug 24075: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Nice work! Pushed to master for 19.11.00
Will this be backdated to 19.05? Thanks! -Marti Fuerst
Pushed to 19.05.x for 19.05.05