Bug 35460

Summary: Unable to add or edit hold rules in circulation rules table
Product: Koha Reporter: Emmi Takkinen <emmi.takkinen>
Component: System AdministrationAssignee: Emmi Takkinen <emmi.takkinen>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: david, emily.lamancusa, fridolin.somers, gmcharlt, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33847
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.01
Bug Depends on: 8367    
Bug Blocks:    
Attachments: Bug 35460: Fix add and edit of hold rules in circulation rules table
Bug 35460: Fix add and edit of hold rules in circulation rules table

Description Emmi Takkinen 2023-12-01 12:53:03 UTC
Adding or editing following columns in circulation rules table doesn't seem to work:

- Holds allowed (total) 
- Holds allowed (daily)
- Holds per record (count)

To reproduce:
1. Create new rule or edit existing one.
2. Add or change values in following columns:
- Holds allowed (total) 
- Holds allowed (daily)
- Holds per record (count
3. Save rule.
=> Note that columns have value "Unlimited" on them. If you check database, columns haven't been saved.
Comment 1 Emmi Takkinen 2023-12-01 13:00:39 UTC
It seems that bug 8367 dropped those columns at some point from smart-rules.pl and now the saving isn't working.
Comment 2 Katrin Fischer 2023-12-01 13:04:59 UTC
Hi Emmi, do you have a link to the broken database update? it should have only added a column.
Comment 3 Emmi Takkinen 2023-12-01 13:08:22 UTC
(In reply to Katrin Fischer from comment #2)
> Hi Emmi, do you have a link to the broken database update? it should have
> only added a column.

It's not really a database update that is broken https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158023 If you look changes made to smart-rules.pl variable my $rules, these columns have been dropped from there. I'm currently making a patch for this.
Comment 4 Emmi Takkinen 2023-12-01 13:10:19 UTC
Created attachment 159449 [details] [review]
Bug 35460: Fix add and edit of hold rules in circulation rules table

Adding or editing following columns in circulation
rules table didn't work:
- Holds allowed (total)
- Holds allowed (daily)
- Holds per record (count)

At some point those columns were dropped from list of
saved columns in smart-rules.pl.

To test:
1. Create new rule or edit existing one.
2. Add or change values in following columns:
- Holds allowed (total)
- Holds allowed (daily)
- Holds per record (count
3. Save rule.
=> Note that columns have value "Unlimited" on them.
If you check database, columns haven't been saved.
4. Apply this patch.
5. Repeat steps 1. to 3.
=> Columns should now have values in them and they
are saved to database.

Sponsored-by: Koha-Suomi Oy
Comment 5 Katrin Fischer 2023-12-01 13:12:10 UTC
Thanks Emmi!

(just was got scared for a moment there. Still not nice, but not AS evil)
Comment 6 Emily Lamancusa 2023-12-01 14:17:43 UTC
Created attachment 159452 [details] [review]
Bug 35460: Fix add and edit of hold rules in circulation rules table

Adding or editing following columns in circulation
rules table didn't work:
- Holds allowed (total)
- Holds allowed (daily)
- Holds per record (count)

At some point those columns were dropped from list of
saved columns in smart-rules.pl.

To test:
1. Create new rule or edit existing one.
2. Add or change values in following columns:
- Holds allowed (total)
- Holds allowed (daily)
- Holds per record (count
3. Save rule.
=> Note that columns have value "Unlimited" on them.
If you check database, columns haven't been saved.
4. Apply this patch.
5. Repeat steps 1. to 3.
=> Columns should now have values in them and they
are saved to database.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 7 Emily Lamancusa 2023-12-01 14:19:19 UTC
Going straight to PQA for this one - it's a small, straightforward change and an important fix
Comment 8 Emmi Takkinen 2023-12-01 16:55:03 UTC
(In reply to Katrin Fischer from comment #5)
> Thanks Emmi!
> 
> (just was got scared for a moment there. Still not nice, but not AS evil)

Sorry for that :D but yeah,  it's not _that_ bad situation but quite nasty still. Those columns are easy to miss since they are at the end of table.
Comment 9 Katrin Fischer 2023-12-11 18:23:12 UTC
Always good to add the "Depends on:" to help our RMaints finding the right versions for backporting :)
Comment 10 Katrin Fischer 2023-12-11 18:47:20 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 11 Fridolin Somers 2023-12-12 07:44:34 UTC
Pushed to 23.11.x for 23.11.01
Comment 12 Lucas Gass 2023-12-14 21:21:29 UTC
Missing dependencies for 23.05.x, no backport.