Bug 30346 - Editing circ rule with Overdue fines cap (amount) results in data loss and extra fines
Summary: Editing circ rule with Overdue fines cap (amount) results in data loss and ex...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
: 29610 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-03-23 19:20 UTC by Donna
Modified: 2022-06-02 19:51 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.06,21.05.15,20.11.19


Attachments
Bug 30346: Correctly transfer values when editing rules (1.94 KB, patch)
2022-04-11 19:39 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30346: Correctly transfer values when editing rules (2.01 KB, patch)
2022-04-11 20:06 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 30346: Correctly transfer values when editing rules (2.07 KB, patch)
2022-04-19 07:57 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Donna 2022-03-23 19:20:12 UTC
Similar to bug 29610, when editing a rule that has an Overdue fines cap (amount) the data in that field is lost.  Duplicating/copying a rule with an Overdue fines cap amount does not bring that value over either. 

To recreate:
--Create a circulation rule with a value in "Overdue fines cap (amount), or edit an existing rule to add it 
-Save the new (or edited) rule.
-Edit the same rule again to change any other field. Note that "Overdue fines cap (amount)" is now blank
-Save your edited rule - "Overdue fines cap (amount)" will save as blank.
Comment 1 Andrew Fuerste-Henry 2022-04-11 19:09:54 UTC
+1, unexpected changes to circ & fine rules are exceptionally unwelcome.
Comment 2 Lucas Gass 2022-04-11 19:34:51 UTC
This is because we are erroneously trying get the value from the 'Cap fine at replacement price' column which is a checked or not checked value. 

When we go through each of the columns we copy the current values over. There is a line like:

else if ( i == 15 ) 

And that is wrong.
Comment 3 Lucas Gass 2022-04-11 19:39:35 UTC
Created attachment 133198 [details] [review]
Bug 30346: Correctly transfer values when editing rules

To test:
--Create a circulation rule with a value in "Overdue fines cap (amount), or edit an existing rule to add it
-Save the new (or edited) rule.
-Edit the same rule again to change any other field. Note that "Overdue fines cap (amount)" is now blank
-Save your edited rule - "Overdue fines cap (amount)" will save as blank.

Apply patch

--Create a circ rule with a value in Overdue fines cap (amount) and check "Cap fine at replacement price" for that same rule.
-Save the rule
-Try ediding the rule and make sure both of those values now save correctly.
-Add several more rules and try playing with both "Cap fine at replacement price" and "Overdue fines cap (amount)". Make sure everything saves right.
Comment 4 Andrew Fuerste-Henry 2022-04-11 20:06:58 UTC
Created attachment 133199 [details] [review]
Bug 30346: Correctly transfer values when editing rules

To test:
--Create a circulation rule with a value in "Overdue fines cap (amount), or edit an existing rule to add it
-Save the new (or edited) rule.
-Edit the same rule again to change any other field. Note that "Overdue fines cap (amount)" is now blank
-Save your edited rule - "Overdue fines cap (amount)" will save as blank.

Apply patch

--Create a circ rule with a value in Overdue fines cap (amount) and check "Cap fine at replacement price" for that same rule.
-Save the rule
-Try ediding the rule and make sure both of those values now save correctly.
-Add several more rules and try playing with both "Cap fine at replacement price" and "Overdue fines cap (amount)". Make sure everything saves right.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 5 Martin Renvoize 2022-04-19 07:57:27 UTC
Created attachment 133388 [details] [review]
Bug 30346: Correctly transfer values when editing rules

To test:
--Create a circulation rule with a value in "Overdue fines cap (amount), or edit an existing rule to add it
-Save the new (or edited) rule.
-Edit the same rule again to change any other field. Note that "Overdue fines cap (amount)" is now blank
-Save your edited rule - "Overdue fines cap (amount)" will save as blank.

Apply patch

--Create a circ rule with a value in Overdue fines cap (amount) and check "Cap fine at replacement price" for that same rule.
-Save the rule
-Try ediding the rule and make sure both of those values now save correctly.
-Add several more rules and try playing with both "Cap fine at replacement price" and "Overdue fines cap (amount)". Make sure everything saves right.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2022-04-19 07:57:43 UTC
Good catch, PQA
Comment 7 Fridolin Somers 2022-04-21 06:44:22 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 8 Kyle M Hall 2022-04-29 17:18:06 UTC
Pushed to 21.11.x for 21.11.06
Comment 9 Andrew Fuerste-Henry 2022-05-03 20:02:14 UTC
Pushed to 21.05.x for 21.05.15
Comment 10 Victor Grousset/tuxayo 2022-05-10 01:39:31 UTC
*** Bug 29610 has been marked as a duplicate of this bug. ***
Comment 11 Victor Grousset/tuxayo 2022-05-10 01:41:18 UTC
Backported: Pushed to 20.11.x branch for 20.11.19
Comment 12 wainuiwitikapark 2022-05-18 22:50:07 UTC
Does this need to be backported to 19.11.x?