If holds for a patron category are set to Unlimited in 'Default checkout, hold policy by patron category' but a holds limit exists in 'Default checkout, hold and return policy', the latter will apply. To recreate: -Set max holds at 5 for Patron Category A in 'Default checkout, hold policy by patron category' -Set max holds at 2 in 'Default checkout, hold and return policy' -Attempt to place multiple holds for a patron in category A - you will be stopped on the 6th hold. -Change max holds to Unlimited for Patron Category A in 'Default checkout, hold policy by patron category' -Keep 'Default checkout, hold and return policy' at 2 max holds -Attempt to place multiple holds for that patron - you'll be stopped after the 2nd hold (In the test above, the circulation matrix has an Patron Category A/All item types max of 10 holds.) This appears to apply in 21.05 and 21.11.
This is valid in master as well: Default checkout, hold and return policy Max holds: 1 Default checkout, hold policy by patron category: Staff: Max holds: Unlimited You can't place a second hold. Only the first rule is applied, although the GUI says that the patron category rule will be used. This shows that the "Unlimited" rule is actually not seen/ignored. In circulation rules we have: | 34 | NULL | NULL | NULL | patron_maxissueqty | | | 35 | NULL | NULL | NULL | patron_maxonsiteissueqty | | | 36 | NULL | NULL | NULL | max_holds | 1 | | 31 | NULL | S | NULL | patron_maxonsiteissueqty | 80 | | 33 | NULL | S | NULL | patron_maxissueqty | 80 | The line for max_holds and Staff (S) is missing.
*** Bug 29061 has been marked as a duplicate of this bug. ***
I'm not sure if this is a distinct bug or a wrinkle within this one. Sara and Katrin's comments show that when the staff interface says a category has Unlimited holds that rule doesn't really get saved in the database. We have a slightly different scenario. Our Default checkout, hold and return policy sets unlimited checkouts and 15 holds for all patrons. I want to make our staff category have unlimited checkouts and unlimited holds. That should mean saving an entry in Default checkout, hold policy by patron category setting blank values for both rules. Koha will not save that at all, it just doesn't create a line in that table. If I cheat and save a line with no value in total checkouts, a (meaningless for us) value in onsite checkouts, and no value in holds, then it saves and tells me that both checkouts and holds will be unlimited (but then I run into what Sara and Katrin have documented, the rule doesn't *really* exist).
I am tempted to update severity on this one - circulation rules are complex as is and they should do what they say in the GUI.
(In reply to Katrin Fischer from comment #4) > I am tempted to update severity on this one - circulation rules are complex > as is and they should do what they say in the GUI. Yes, let's. Making this Major.
Related to bug 24669, why didn't we use strip_non_numeric for one occurrence?
Created attachment 143984 [details] [review] Bug 30794: Insert blank rule (unlimited) for max_holds Related to bug 24669, why didn't we use strip_non_numeric for one occurrence? Note that there is still a problem, if none of the 3 inputs is filled, no rule will be created (ie. action ignored)
Could you test this patch?
Test plan: 1: have an All/All rule in the circ matrix that allows unlimited holds 2: set Default checkout, hold, and return policy to maximum 1 total hold allowed 3: set Default checkout, hold policy by patron category to allow Board patrons to have unlimited holds (remember you can't just save a blank set of rules here, so you'll want to put a number into Total Checkouts, nothing in Total Holds, and save that -- you'll end up with "Unlimited" showing in Total Holds). 4: Place a hold for a Board patron. It works! 5: Try to place a second hold for the same Board patron. It tells you you're over the limit, "[patron] can only place a maximum of 1 total holds." 6: In reports, select * from circulation_rules where rule_name='max_holds', see that your Board patron rule didn't get created APPLY PATCH 7: Back in circ rules, delete and re-create your Default checkout, hold policy by patron category rule for Board patrons 8: Try your second hold again. It works! 9: Run your report again, see you now have a rule where categorycode=B, rule_name=max_holds, rule_value is null
Created attachment 144011 [details] [review] Bug 30794: Insert blank rule (unlimited) for max_holds Related to bug 24669, why didn't we use strip_non_numeric for one occurrence? Note that there is still a problem, if none of the 3 inputs is filled, no rule will be created (ie. action ignored) Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
QA: Looking here
> Note that there is still a problem, if none of the 3 inputs is filled, > no rule will be created (ie. action ignored) | 99 | NULL | NULL | NULL | patron_maxissueqty | | | 100 | NULL | NULL | NULL | patron_maxonsiteissueqty | | | 101 | NULL | NULL | NULL | max_holds | | | 102 | NULL | B | NULL | patron_maxonsiteissueqty | | | 103 | NULL | B | NULL | patron_maxissueqty | | | 104 | NULL | B | NULL | max_holds | | Looks like the rule is created though?
(In reply to Marcel de Rooy from comment #12) > > Note that there is still a problem, if none of the 3 inputs is filled, > > no rule will be created (ie. action ignored) > > Looks like the rule is created though? Ah. The rule is there but it is not shown on the interface !
I had something similar the other day with open_article_requests_limit. See bug 27945
Should we open a new bug report for that? Or is it already somewhere?
Created attachment 144050 [details] [review] Bug 30794: Insert blank rule (unlimited) for max_holds Related to bug 24669, why didn't we use strip_non_numeric for one occurrence? Note that there is still a problem, if none of the 3 inputs is filled, no rule will be created (ie. action ignored) Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #15) > Should we open a new bug report for that? Or is it already somewhere? I haven't created it.
Pushed to master for 22.11. Nice work everyone, thanks!