longoverdue.pl calls 'LostItem' C4::Circulation::LostItem calls _FixOverduesOnReturn C4::Circulation::_FixOverduesOnReturn create a Koha::Account::Line for the amount of the accruing fine and calls apply passing the existing fine Koha::Account::Line calls renew_item when the fee is paid The item is then renewed, a notice (potentially) sent, and then the item marked lost and charged The sending of a renewal notice is quite confusing to patrons
Created attachment 118766 [details] [review] Bug 27927: Prevent renewal on lost forgive The `RenewAccruingItemWhenPaid` preference should only renew upon payment (and writeoff?) and not upon automatic forgival triggered by `WhenLostForgiveFine`.
This will need some tests writing, but I wanted to get the patch up here and start proceeding with testing. I've currently only prevented the renewal for 'FORGIVEN' credit types, I wonder if we aught to exclude any other credit types...?
Created attachment 118781 [details] [review] Bug 27927: Prevent renewal on lost forgive The `RenewAccruingItemWhenPaid` preference should only renew upon payment (and writeoff?) and not upon automatic forgival triggered by `WhenLostForgiveFine`.
Created attachment 119193 [details] [review] Bug 27927: Add Unit Tests Add a unit test to check that 'FORGIVEN' credits do not prompt an auto-renewal with `RenewAccruingItemWhenPaid` enabled. Test plan 1/ Apply the test patch 2/ Run t/db_dependent/Koha/Account/Line.t and confirm it fails 3/ Apply subsequent patch 4/ Re-run the test and confirm it passes
Created attachment 119194 [details] [review] Bug 27927: Prevent renewal on lost forgive The `RenewAccruingItemWhenPaid` preference should only renew upon payment (and writeoff) and not upon automatic forgival triggered by `WhenLostForgiveFine`. Test plan 1/ Enable `WhenLostForgiveFine` and `RenewAccruingItemWhenPaid` 2/ Checkout an item and backdate it's due date 3/ Run longeroverdue.pl to mark the item as lost and create the fine 4/ Confirm that the item is not renewed as part of the process 5/ Signoff
The behavior is all good, but I get a test failure: root@kohadevbox:koha(master)$ prove t/db_dependent/Koha/Account/Line.t t/db_dependent/Koha/Account/Line.t .. 7/14 # Looks like you planned 6 tests but ran 3. t/db_dependent/Koha/Account/Line.t .. 9/14 # Failed test 'checkout() tests' # at t/db_dependent/Koha/Account/Line.t line 643. Can't call method "checkout" on an undefined value at t/db_dependent/Koha/Account/Line.t line 635. # Looks like your test exited with 255 just after 9. t/db_dependent/Koha/Account/Line.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 6/14 subtests Test Summary Report ------------------- t/db_dependent/Koha/Account/Line.t (Wstat: 65280 Tests: 9 Failed: 1) Failed test: 9 Non-zero exit status: 255 Parse errors: Bad plan. You planned 14 tests but ran 9. Files=1, Tests=9, 5 wallclock secs ( 0.02 usr 0.01 sys + 3.13 cusr 0.59 csys = 3.75 CPU) Result: FAIL
I'm really struggling to get this test to fail.. how are you running it?
Going back through this, the test passes if FinesMode is set to Don't Calculate, but fails if FinesMode is set to Calculate and Charge.
Created attachment 119235 [details] [review] Bug 27927: Add Unit Tests Add a unit test to check that 'FORGIVEN' credits do not prompt an auto-renewal with `RenewAccruingItemWhenPaid` enabled. Test plan 1/ Apply the test patch 2/ Run t/db_dependent/Koha/Account/Line.t and confirm it fails 3/ Apply subsequent patch 4/ Re-run the test and confirm it passes Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 119236 [details] [review] Bug 27927: Prevent renewal on lost forgive The `RenewAccruingItemWhenPaid` preference should only renew upon payment (and writeoff) and not upon automatic forgival triggered by `WhenLostForgiveFine`. Test plan 1/ Enable `WhenLostForgiveFine` and `RenewAccruingItemWhenPaid` 2/ Checkout an item and backdate it's due date 3/ Run longeroverdue.pl to mark the item as lost and create the fine 4/ Confirm that the item is not renewed as part of the process 5/ Signoff Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed off because I get that same test failure with FinesMode on without these patches. Filing a new bug for that.
Filed bug 28097 for that test
Aha, thanks Andrew.. I hadn't thought about changing FinesMode having been changed in the background... certainly a bug, the tests should lock the preference to ensure they pass regardless.
Created attachment 119676 [details] [review] Bug 27927: Add Unit Tests Add a unit test to check that 'FORGIVEN' credits do not prompt an auto-renewal with `RenewAccruingItemWhenPaid` enabled. Test plan 1/ Apply the test patch 2/ Run t/db_dependent/Koha/Account/Line.t and confirm it fails 3/ Apply subsequent patch 4/ Re-run the test and confirm it passes Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 119677 [details] [review] Bug 27927: Prevent renewal on lost forgive The `RenewAccruingItemWhenPaid` preference should only renew upon payment (and writeoff) and not upon automatic forgival triggered by `WhenLostForgiveFine`. Test plan 1/ Enable `WhenLostForgiveFine` and `RenewAccruingItemWhenPaid` 2/ Checkout an item and backdate it's due date 3/ Run longeroverdue.pl to mark the item as lost and create the fine 4/ Confirm that the item is not renewed as part of the process 5/ Signoff Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.05
Pushed to 20.05.x for 20.05.11
Missing dependencies for 19.11.x, it shouldn't be affected, no backport.