Bug 24380 - Add option to recalculate fines upon a backdated return distinctly from `CalculateFinesOnReturn`
Summary: Add option to recalculate fines upon a backdated return distinctly from `Calc...
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 14591
Blocks:
  Show dependency treegraph
 
Reported: 2020-01-08 20:27 UTC by Kyle M Hall
Modified: 2020-05-22 13:39 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
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 `CalucalteFinesOnReturn` during upgrades.
Version(s) released in:
20.05.00


Attachments
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.76 KB, patch)
2020-01-08 20:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.81 KB, patch)
2020-01-17 16:06 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.30 KB, patch)
2020-01-22 19:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.30 KB, patch)
2020-01-22 19:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.82 KB, patch)
2020-01-22 19:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.26 KB, patch)
2020-02-24 17:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.26 KB, patch)
2020-02-24 17:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.82 KB, patch)
2020-02-24 17:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.26 KB, patch)
2020-04-08 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.26 KB, patch)
2020-04-08 14:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.82 KB, patch)
2020-04-08 14:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380: Add syspref CalculateFinesOnBackdate (5.22 KB, patch)
2020-04-08 14:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24380 - Unit Test (2.31 KB, patch)
2020-04-14 07:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24380 - Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.87 KB, patch)
2020-04-14 07:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24380: Add syspref CalculateFinesOnBackdate (5.27 KB, patch)
2020-04-14 07:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24380: Unit Test (2.38 KB, patch)
2020-04-15 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24380: Backdated returns do not update fines unless CalculateFinesOnReturn is enabled (1.94 KB, patch)
2020-04-15 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24380: Add syspref CalculateFinesOnBackdate (5.34 KB, patch)
2020-04-15 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24380: (QA follow-up) Do not use compare float with precision (1.16 KB, patch)
2020-04-15 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24380: Simplify condition (1.50 KB, patch)
2020-04-15 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.