Bug 30694 - Impossible to delete line in circulation and fine rules
Summary: Impossible to delete line in circulation and fine rules
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Emmi Takkinen
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-05-05 14:31 UTC by Klas Blomberg
Modified: 2023-12-28 20:44 UTC (History)
6 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:
23.05.00,22.11.02,22.05.09, 21.11.16
Circulation function:

Attachments
Bug 30694: Set decreaseloanholds undef when deleting circulation rule (1.64 KB, patch)
2022-11-15 07:33 UTC, Emmi Takkinen 		Details | Diff | Splinter Review
Bug 30694: Set decreaseloanholds undef when deleting circulation rule (1.70 KB, patch)
2022-11-15 16:29 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 30694: Set decreaseloanholds undef when deleting circulation rule (1.75 KB, patch)
2022-12-16 14:37 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Klas Blomberg 2022-05-05 14:31:37 UTC 
If you have entered a value in the column "Decreased loan period for high holds" for a circulation rule - you can't delete that line.

The only thing that will happend is that all numerical values are wiped out - giving today as duedate
Comment 1 Katrin Fischer 2022-06-06 21:12:57 UTC 
It does appear to behave correctly in current master. Could you maybe verify if it's still an issue in newer versions and add a step by step on how to test?
Comment 2 Emmi Takkinen 2022-11-15 07:05:58 UTC 
We are facing similar problem on version 21.11, but with "Holds pickup period (day)" field. However in current master the problem field is "Decreased loan period for high holds" as Klas mentioned.

To test:
1. Create new circulation rule and fill all fields.
2. Try to delete the rule.
=> Rule isn't deleted, all fields except "Decreased loan period for high holds" are now empty or have value "Unlimited".
3. Modify rule and set field "Decreased loan period for high holds" empty.
4. Delete the rule.
=> Rule is deleted successfully.
5. Create new rule, fill all fields expect field "Decreased loan period for high holds".
6. Delete the rule.
=> Rule is deleted successfully.
Comment 3 Emmi Takkinen 2022-11-15 07:13:05 UTC 
Also to add to this, logs show no error and browsers console shows no Javascript etc. errors.
Comment 4 Emmi Takkinen 2022-11-15 07:33:08 UTC 
Created attachment 143891 [details] [review]
Bug 30694: Set decreaseloanholds undef when deleting circulation rule

When one tries to delete rule with field "Decreased loan period
for high holds" filled, delete fails without errors. This happens
because rule isn't set in code section handling the delete in
smart-rules.pl.

To test:
1. Create a new circulation rule and fill all fields.
2. Try to delete the rule.
=> Rule isn't deleted, all fields except "Decreased loan period
for high holds" are now empty or have value "Unlimited".
3. Modify the rule and set field "Decreased loan period for high
holds" empty.
4. Delete the rule.
=> Rule is deleted successfully.
5. Create a new rule, fill all fields expect field "Decreased loan
period for high holds".
6. Delete the rule.
=> Rule is deleted successfully.
7. Apply this patch.
8. Again create a new rule with field "Decreased loan period for high holds" filled.
9. Try to delete the rule.
=> Rule is deleted successfully.

Sponsored-by: Koha-Suomi Oy
Comment 5 Emmi Takkinen 2022-11-15 09:19:45 UTC 
Another side note, we are using bug 8367 so our problem with field "Holds pickup period (day)" comes from there and has nothing to with this bug.
Comment 6 Katrin Fischer 2022-11-15 16:14:02 UTC 
(In reply to Klas Blomberg from comment #0)
> If you have entered a value in the column "Decreased loan period for high
> holds" for a circulation rule - you can't delete that line.
> 
> The only thing that will happend is that all numerical values are wiped out
> - giving today as duedate

Confirmed on master.
Comment 7 Katrin Fischer 2022-11-15 16:29:50 UTC 
Created attachment 143918 [details] [review]
Bug 30694: Set decreaseloanholds undef when deleting circulation rule

When one tries to delete rule with field "Decreased loan period
for high holds" filled, delete fails without errors. This happens
because rule isn't set in code section handling the delete in
smart-rules.pl.

To test:
1. Create a new circulation rule and fill all fields.
2. Try to delete the rule.
=> Rule isn't deleted, all fields except "Decreased loan period
for high holds" are now empty or have value "Unlimited".
3. Modify the rule and set field "Decreased loan period for high
holds" empty.
4. Delete the rule.
=> Rule is deleted successfully.
5. Create a new rule, fill all fields expect field "Decreased loan
period for high holds".
6. Delete the rule.
=> Rule is deleted successfully.
7. Apply this patch.
8. Again create a new rule with field "Decreased loan period for high holds" filled.
9. Try to delete the rule.
=> Rule is deleted successfully.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Kyle M Hall (khall) 2022-12-16 14:37:54 UTC 
Created attachment 144685 [details] [review]
Bug 30694: Set decreaseloanholds undef when deleting circulation rule

When one tries to delete rule with field "Decreased loan period
for high holds" filled, delete fails without errors. This happens
because rule isn't set in code section handling the delete in
smart-rules.pl.

To test:
1. Create a new circulation rule and fill all fields.
2. Try to delete the rule.
=> Rule isn't deleted, all fields except "Decreased loan period
for high holds" are now empty or have value "Unlimited".
3. Modify the rule and set field "Decreased loan period for high
holds" empty.
4. Delete the rule.
=> Rule is deleted successfully.
5. Create a new rule, fill all fields expect field "Decreased loan
period for high holds".
6. Delete the rule.
=> Rule is deleted successfully.
7. Apply this patch.
8. Again create a new rule with field "Decreased loan period for high holds" filled.
9. Try to delete the rule.
=> Rule is deleted successfully.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Tomás Cohen Arazi (tcohen) 2022-12-23 17:03:44 UTC 
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 Jacob O'Mara 2023-01-13 11:55:10 UTC 
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 11 Lucas Gass (lukeg) 2023-01-19 23:48:31 UTC 
Backported to 22.05.x for upcoming 22.05.09
Comment 12 Arthur Suzuki 2023-01-26 10:57:13 UTC 
applied to 21.11.x for 21.11.16
Comment 13 Wainui Witika-Park 2023-03-15 01:32:54 UTC 
Not backported to 21.05.x