Bug 28097

Summary:	t/db_dependent/Koha/Account/Line.t test fails with FinesMode set to calculate
Product:	Koha	Reporter:	Andrew Fuerste-Henry <andrew>
Component:	Fines and fees	Assignee:	Martin Renvoize (ashimema) <martin.renvoize>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	fridolin.somers, kyle, martin.renvoize, nick, victor
Version:	Main
Hardware:	All
OS:	All
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:	8338
Bug Blocks:
Attachments:	Bug 28097: Mock finesModes to disable recalculation Bug 28097: Mock finesModes to disable recalculation

Description Andrew Fuerste-Henry 2021-04-06 16:17:37 UTC

To recreate:
- Confirm finesmode is set to do not calculate
- prove t/db_dependent/Koha/Account/Line.t
- it passes!
- set FinesMode ot calculate and charge
- prove t/db_dependent/Koha/Account/Line.t
- it fails 

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.

#   Failed test 'checkout() tests'
#   at t/db_dependent/Koha/Account/Line.t line 615.
Can't call method "checkout" on an undefined value at t/db_dependent/Koha/Account/Line.t line 607.
# 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.32 cusr  0.65 csys =  4.00 CPU)
Result: FAIL

Comment 1 Martin Renvoize (ashimema) 2021-04-07 11:05:20 UTC

This is a side effect of bug 8338.

With finesMode set to 'production' and CalculateFinesOnReturn set to 'enabled' upon return we will first re-calculate the fine amount (in this case that would result in a '0' amount fine).. we then call '_FixOverduesOnReturn' which subsequently fines the '0' amount fine and deletes it on the assumption it's a backdated return which had resulted in an errant fine.

I wonder if, rather than delete the original fine, we should void it which would give a more thorough audit trail (and fix this bug).. However, I think such a VOID would still appear on the patrons account in the OPAC and that was what I believe people wanted to avoid with bug 8338.

The easy fix here is to adjust the test to mock the two preferences to ensure they are disabled for the test case.

Comment 2 Martin Renvoize (ashimema) 2021-04-07 11:13:43 UTC

Created attachment 119275 [details] [review]
Bug 28097: Mock finesModes to disable recalculation

Since bug 8338 the recalcuated zero amount fine is subsequently removed
in _FixOverduesOnReturn which causes this test to fail if finesMode is
'production' and 'CalculateFinesOnReturn' is enabled.

Perhaps we should be 'VOID'ing fines rather than completely deleting
them, but that's a question for another day.

Test plan
1/ Enable finesMode in the staff client
2/ Run the test and watch it fail
3/ Apply the patch
4/ Run the test and watch it pass

Comment 3 Nick Clemens (kidclamp) 2021-04-07 11:30:53 UTC

Created attachment 119276 [details] [review]
Bug 28097: Mock finesModes to disable recalculation

Since bug 8338 the recalcuated zero amount fine is subsequently removed
in _FixOverduesOnReturn which causes this test to fail if finesMode is
'production' and 'CalculateFinesOnReturn' is enabled.

Perhaps we should be 'VOID'ing fines rather than completely deleting
them, but that's a question for another day.

Test plan
1/ Enable finesMode in the staff client
2/ Run the test and watch it fail
3/ Apply the patch
4/ Run the test and watch it pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 4 Nick Clemens (kidclamp) 2021-04-07 11:31:18 UTC

Small fix, passing QA

Comment 5 Jonathan Druart 2021-04-08 13:42:25 UTC

Pushed to master for 21.05, thanks to everybody involved!

Comment 6 Fridolin Somers 2021-04-19 10:11:51 UTC

Pushed to 20.11.x for 20.11.05

Comment 7 Andrew Fuerste-Henry 2021-04-25 14:20:25 UTC

Pushed to 20.05.x for 20.05.11

Comment 8 Victor Grousset/tuxayo 2021-04-25 17:19:31 UTC

Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.