https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha@univ-lyon3.fr -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 69382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69382&action=edit Bug 19690: Smart rules: Term "If any unavailable" is confusing and just means No This patch replaces the term by No. Also "If all unavailable" is refined to "Only if other items unavailable". The online help text is adjusted. Two comments in DisallowHoldIfItemsAvailable.t are adjusted too. Test plan: Go to smart rules, look at on shelf holds. Read the help text for this page. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think the patch is actually wrong: [% ELSIF rule.onshelfholds == 2 %] - If all unavailable + Only if other items unavailable [% ELSE %] - If any unavailable + No OnShelfHolds Allowed If all unavailable = No (no onshelf holds are allowed) If any anavailable = If at least one is checked out Yes = It doesn't matter if anything is checked out or not (also yes is good here) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Please have a look at the code in IsAvailableForItemLevelRequest: } elsif ( $on_shelf_holds == 2 ) { my @items = ... my $any_available = 0; foreach my $i (@items) { ... $any_available = 1 unless $i->itemlost ... } return $any_available ? 0 : 1; } The variable name could put you on the wrong track, but 2 == all_unavailable. What does the above say? If you choose for all_unavailable and there is at lease one item available ("On shelf"), you cannot place the hold. If all are unavailable, you can place the item level hold. Finally, we have the remaining case (on_shelf_holds == 0 /"If any unavailable"): return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting" The item level hold is only allowed now if the current item is on loan or waiting. (Using the term "any" here is making no sense to me..) === What makes it more complicated, is the use of the sub OnShelfHoldsAllowed in Koha (opac-detail, opac-shelves). In opac-detail-sidebar.inc the following test is used before displaying a place hold link: [% IF ( AllowOnShelfHolds OR ItemsIssued ) %] opac-shelves does more or less the same by first testing allow_onshelf_holds and testing if issued in the else branch. Note that in these tests the values 1 and 2 are just interpreted as one case (shelf holds are allowed). === Conclusion: The name is confusing/misleading. This is partially hidden by the obscure use of onshelfholds in other places. We cannot solve this by a rename only. So I am leaving it as-is. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #69382|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |--- -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #4)

Sorry, you lost me :(

Well, this confirms that the term confusing is quite correct! :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #6 from Magnus Enger <magnus@libriotech.no> --- Would "If on loan" make more sense? That seems to be what is actually checked by the code. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #8 from Magnus Enger <magnus@libriotech.no> --- (In reply to Katrin Fischer from comment #7)

Not quite, because it doesn't transport that it will take effect if at least one is on loan.

Ah, but that is not how the code in C4::Reserves::IsAvailableForItemLevelRequest() works, I think: 1163 sub IsAvailableForItemLevelRequest { 1164 my $item = shift; 1165 my $borrower = shift; 1166 my $pickup_branchcode = shift; ... 1196 if ( $on_shelf_holds == 1 ) { # Yes 1197 return 1; 1198 } elsif ( $on_shelf_holds == 2 ) { # If all unavailable 1199 my @items = 1200 Koha::Items->search( { biblionumber => $item->{biblionumber} } ); 1201 1202 my $any_available = 0; 1203 1204 foreach my $i (@items) { 1205 ... 1221 return $any_available ? 0 : 1; 1222 } else { # on_shelf_holds == 0 "If any unavailable" (the description is rather cryptic and could still be improved) 1223 return $item->{onloan} || IsItemOnHoldAndFound( $item->{itemnumber} ); 1224 } 1225 } If on_shelf_holds == 1 it always returns 1, the item that was passed in can always be be put a hold on. If on_shelf_holds == 2 it loops over all the items attached to the biblio of the item that was passed in, so it can check if they all are unavailable or not, and if any are available it returns 0. (There is a bug in how this is done when holdallowed = 0 that I will report and submit a patch for soon.) If on_shelf_holds == 0 it only checks the item that was passed in, there is no looping over all the items to tell if at least one is onloan. (There is a small bug in how this is done too...) So what the code does is look at the item that was passed in to IsAvailableForItemLevelRequest and if it is onloan (or OnHoldAndFound) it says it is OK to place a hold on that item. That is why I think "If on loan" might make sense. Now this might not be what was intended for the "If any unavailable" setting, but then the code is wrong, IMHO. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This is still confusing in 2025. Note what the manual says: If any unavailable: This will allow to place a hold as soon as one or more items of the record are checked out. It doesn’t matter if there are still one or more items available on the shelf. This is completely wrong. The code checks if the item ITSELF is not available. And does not care about any other item. I wished that the code would do what the manual said actually. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #11)

Saving a mattermost remark from cait here:

I think there is a difference in record level and item level hold that probably needs to be explained. I believe it only allows item level holds on checked out, but record level if any item is gone

Yeah, I guess that should be it. We should add this to the manual or even try to get it into smart rules somehow ? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Manual -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |minor -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.03 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00,25.11.03 |26.05.00,25.11.03,25.05.09 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Pushed to oldoldstable Version(s)|26.05.00,25.11.03,25.05.09 |26.05.00,25.11.03,25.05.09, released in| |24.11.15 CC| |baptiste.wojtkowski@biblibr | |e.com --- Comment #21 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Pushed into 24.11.x for 24.11.15 nice work everyone -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19690 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldoldstable |Needs documenting --- Comment #23 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 -- You are receiving this mail because: You are watching all bug changes.