| Summary: | longoverdue cronjob renews items before marking lost when both RenewAccruingItemWhenPaid and WhenLostForgiveFine are enabled | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Fines and fees | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
| Status: | CLOSED FIXED | QA Contact: | Nick Clemens (kidclamp) <nick> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | andrew, fridolin.somers, george, martin.renvoize, victor |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9805 | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
21.05.00,20.11.05,20.05.11
|
|
| Circulation function: | |||
| Bug Depends on: | 25508 | ||
| Bug Blocks: | 24300 | ||
| Attachments: |
Bug 27927: Prevent renewal on lost forgive
Bug 27927: Prevent renewal on lost forgive Bug 27927: Add Unit Tests Bug 27927: Prevent renewal on lost forgive Bug 27927: Add Unit Tests Bug 27927: Prevent renewal on lost forgive Bug 27927: Add Unit Tests Bug 27927: Prevent renewal on lost forgive |
||
|
Description
Nick Clemens (kidclamp)
2021-03-11 12:51:55 UTC
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. |