Bug 24380

Summary: Add option to recalculate fines upon a backdated return distinctly from `CalculateFinesOnReturn`
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Fines and feesAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: 1joynelson, jonathan.druart, katrin.fischer, kelly, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement allows libraries to set the option to calculate fines upon a backdated return distinctly from the broader option to always recalculate fines on return option. **New system preference**: `CalculateFinesOnBackdate` defaults to enabled on new installations or the value of `CalculateFinesOnReturn` during upgrades.
Version(s) released in:
20.05.00
Bug Depends on: 14591    
Bug Blocks:    
Attachments: Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380 - Unit Test
Bug 24380 - Unit Test
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380 - Unit Test
Bug 24380 - Unit Test
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380 - Unit Test
Bug 24380 - Unit Test
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380: Add syspref CalculateFinesOnBackdate
Bug 24380 - Unit Test
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380: Add syspref CalculateFinesOnBackdate
Bug 24380: Unit Test
Bug 24380: Backdated returns do not update fines unless CalculateFinesOnReturn is enabled
Bug 24380: Add syspref CalculateFinesOnBackdate
Bug 24380: (QA follow-up) Do not use compare float with precision
Bug 24380: Simplify condition

Description Kyle M Hall 2020-01-08 20:27:25 UTC
In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.
Comment 1 Kyle M Hall 2020-01-08 20:28:48 UTC
Created attachment 97055 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!
Comment 2 ByWater Sandboxes 2020-01-17 16:06:59 UTC
Created attachment 97544 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 3 Jonathan Druart 2020-01-21 15:13:49 UTC
Kyle, can you provide tests please?
Comment 4 Kyle M Hall 2020-01-22 19:10:22 UTC
(In reply to Jonathan Druart from comment #3)
> Kyle, can you provide tests please?

Absolutely!
Comment 5 Kyle M Hall 2020-01-22 19:10:49 UTC
Created attachment 97750 [details] [review]
Bug 24380 - Unit Test
Comment 6 Kyle M Hall 2020-01-22 19:11:31 UTC
Created attachment 97751 [details] [review]
Bug 24380 - Unit Test
Comment 7 Kyle M Hall 2020-01-22 19:11:38 UTC
Created attachment 97752 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 8 Jonathan Druart 2020-02-05 12:22:05 UTC
CONFLICT (content): Merge conflict in t/db_dependent/Circulation/Returns.t
Comment 9 Kyle M Hall 2020-02-24 17:14:48 UTC
Created attachment 99536 [details] [review]
Bug 24380 - Unit Test
Comment 10 Kyle M Hall 2020-02-24 17:15:12 UTC
Created attachment 99537 [details] [review]
Bug 24380 - Unit Test
Comment 11 Kyle M Hall 2020-02-24 17:15:21 UTC
Created attachment 99538 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 12 Martin Renvoize 2020-04-08 08:18:29 UTC
I have a vague recollection of Katrin being keen to NOT do this.. perhaps it should be behind YASP (Yet Another System Preference)
Comment 13 Katrin Fischer 2020-04-08 11:45:51 UTC
(In reply to Martin Renvoize from comment #12)
> I have a vague recollection of Katrin being keen to NOT do this.. perhaps it
> should be behind YASP (Yet Another System Preference)

Yes, we have an alternate fine calculation that is used with some of our libraries - recalculating would come up with the wrong values. So we have turned off the pref there. I think while we coudl argue that's our problem - there is also the staticfines.pl job and possible others with similar needs. I'd really like to put this under the same or a separate pref.
Comment 14 Kyle M Hall 2020-04-08 14:39:47 UTC
Created attachment 102574 [details] [review]
Bug 24380 - Unit Test
Comment 15 Kyle M Hall 2020-04-08 14:40:09 UTC
Created attachment 102575 [details] [review]
Bug 24380 - Unit Test
Comment 16 Kyle M Hall 2020-04-08 14:40:19 UTC
Created attachment 102576 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 17 Kyle M Hall 2020-04-08 14:40:22 UTC
Created attachment 102577 [details] [review]
Bug 24380: Add syspref CalculateFinesOnBackdate
Comment 18 Katrin Fischer 2020-04-08 20:52:50 UTC
(In reply to Kyle M Hall from comment #17)
> Created attachment 102577 [details] [review] [review]
> Bug 24380: Add syspref CalculateFinesOnBackdate

Thank you Kyle!
Comment 19 Martin Renvoize 2020-04-14 07:49:57 UTC
Created attachment 102894 [details] [review]
Bug 24380 - Unit Test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2020-04-14 07:50:00 UTC
Created attachment 102895 [details] [review]
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2020-04-14 07:50:04 UTC
Created attachment 102896 [details] [review]
Bug 24380: Add syspref CalculateFinesOnBackdate

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2020-04-14 07:50:30 UTC
Thanks Kyle, all works as expected.. Signing off
Comment 23 Jonathan Druart 2020-04-15 10:50:16 UTC
Created attachment 102981 [details] [review]
Bug 24380: Unit Test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2020-04-15 10:50:23 UTC
Created attachment 102982 [details] [review]
Bug 24380: Backdated returns do not update fines unless CalculateFinesOnReturn is enabled

In Koha 18.11 backdating a return triggered a recalculation of the fine. This was removed in bug 14591, and I believe it was in error. The bug report itself has no justification for the change in behavior.

Test Plan:
1) Disable CalculateFinesOnReturn
2) Backdate an overdue with fines, note the fine does not change
3) Apply this patch
4) Repeat step 2
5) The fine should be updated!

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Jonathan Druart 2020-04-15 10:50:30 UTC
Created attachment 102983 [details] [review]
Bug 24380: Add syspref CalculateFinesOnBackdate

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Jonathan Druart 2020-04-15 10:50:36 UTC
Created attachment 102984 [details] [review]
Bug 24380: (QA follow-up) Do not use compare float with precision

forbidden pattern: Do not use compare float with precision (bug 24408).

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 27 Jonathan Druart 2020-04-15 10:50:41 UTC
Created attachment 102985 [details] [review]
Bug 24380: Simplify condition

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Jonathan Druart 2020-04-15 10:51:15 UTC
(In reply to Jonathan Druart from comment #27)
> Created attachment 102985 [details] [review] [review]
> Bug 24380: Simplify condition
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

IMO it eases the read of the condition, feel free to disagree and obsolete.
Comment 29 Martin Renvoize 2020-04-15 11:31:20 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 30 Joy Nelson 2020-05-05 22:38:09 UTC
does not apply cleanly to 19.11.x branch.  please rebase if needed.