Bug 21512

Summary: CalculateFinesOnReturn doesn't work without the syspref finesMode
Product: Koha Reporter: Olli-Antti Kivilahti <olli-antti.kivilahti>
Component: CirculationAssignee: Olli-Antti Kivilahti <olli-antti.kivilahti>
Status: RESOLVED WONTFIX QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: aleisha, gmcharlt, hien.pham, josef.moravec, katrin.fischer, kyle.m.hall
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: 0001-Bug-21512-CalculateFinesOnReturn-doesn-t-work-withou.patch

Description Olli-Antti Kivilahti 2018-10-07 11:57:31 UTC
Fines are not created for checking in an Item, if the syspref 'CalculateFinesOnReturn' is enabled without the syspref finesMode being in the 'production' -mode (Calculate and charge).


CalculateFinesOnReturn must work without the fines.pl -feature.


The problem is with the subroutine C4::Circulation::_CalculateAndUpdateFine()

Initially it looks like it must be invoked from the fines.pl as well. But that subroutine is only used within the C4::Circulation module for AddRenew and AddReturn.
Hence the _CalculateAndUpdateFine() has nothing to do with fines.pl|finesMode.


This patch replaces the finesMode-check with CalculateFinesOnReturn-check.


I am not sure why there needs to be a specific syspref for the fines.pl, or why CalculateFinesOnReturn is needed, as the fine amounts can be tweaked using the circulation rules matrix and the fines.pl-cronjob can either be ran, or not, with an endless possibility of parameters that shouldn't be in the sysprefs.
Comment 1 Olli-Antti Kivilahti 2018-10-07 12:01:02 UTC
Created attachment 80174 [details] [review]
0001-Bug-21512-CalculateFinesOnReturn-doesn-t-work-withou.patch
Comment 2 Olli-Antti Kivilahti 2018-10-07 12:14:39 UTC
Bug found and fix sponsored by Minna Kivinen @ https://www.hamk.fi/
Comment 3 Katrin Fischer 2018-10-28 17:50:29 UTC
I am not sure I agree with the intent. To me it feels like the right behaviour that FinesMode does also keep CalculateFinesOnReturn from calculating. A change like this could result in some surprises for libraries.

Could we imagine a way to achieve your goal without a possibly unwanted surprise?