Currently if a librarian checkouts an item to a patron that is in the "Reserved" state the librarian will get a prompt saying "Item Whinney pooh / (1111111111 has been on hold for Joe (1111111111) at My Library since 15.10.2018". If there is little staff the holds might pile up and then patrons who actually went to the library cannot checkout those items. Some libraries prefer to give the first to the patrons who went actually to the physical library building instead of just reserved it online. Koha actually used to work like this with the SIP server before commit "Bug 15438 - Checking out an on-hold item sends holder's borrowernumber in AF (screen message) field."
Just to add: Koha also used to, and still does, and should continue to prevent checking out with SIP server if the item was in Waiting state, i.e. in the shelf/book pile where reserves waiting for pickup are.
I think this should probably be controlled by the AllowItemsOnHoldCheckout system preference?
(In reply to Katrin Fischer from comment #2) > I think this should probably be controlled by the AllowItemsOnHoldCheckout > system preference? I hope it could be because then Koha would not become so complex to configure for the user. Currently all the different ways of of checking out are a bit different (SCO, SIP, intranet) and it would be good to unify them. I think we should merge AllowItemsOnHoldCheckoutSCO syspref to AllowItemsOnHoldCheckout. Currently SCO allows to check out items with Waiting state if AllowItemsOnHoldCheckoutSCO is enabled but SIP doesn't with AllowItemsOnHoldCheckout enabled. I think how SCO works with Waiting state holds is bad, so here is my proposal: 1. We have only one syspref: AllowItemsOnHoldCheckout 2. AllowItemsOnHoldCheckout would only allow checking out holds in the Reserved state, i.e. holds that are *not* yet processed by the library's staff – this way we don't have items on the shelfs that cannot be actually checked out => confused patrons. Holds in the Waiting state (i.e. the ones that are processed by staff) could not be checked out except from the intranet if the librarian wants to override this policy. If just one syspref is not possible because some libraries want to allow reserves in waiting state to be checked out by other patrons (but why would anyone, because then "holds" lose their meaning entirely) then I suggest the following two sysprefs instead of AllowItemsOnHoldCheckout: AllowCheckoutIfReserveNotWaiting AllowCheckoutIfReserveWaiting
I'm bumping into this issue as well, which is why I'm scouring Bugzilla for more information. I do find it weird that the intranet, SCO, and SIP have different ways of handling checkouts/renewals for items with holds...
Actually, I just noticed that while you cannot renew an item that has a hold, you can check it in, ignore the hold, try to check it out again, and override the errors/warnings and check it out again to the original patron. So it looks like maybe you can already do what you want?
(In reply to David Cook from comment #5) > Actually, I just noticed that while you cannot renew an item that has a > hold, you can check it in, ignore the hold, try to check it out again, and > override the errors/warnings and check it out again to the original patron. One can renew item with unfilled hold by using AllowRenewalIfOtherItemsAvailable. At least that works when one clicks those checkboxes for renewal in the intranet's checkouts tab for patron. > So it looks like maybe you can already do what you want? No, the problem I personally have is that it is annoying the confirmation box comes for an item with unprocessed holds (this should be configurable with AllowItemsOnHoldCheckout) and also that SCO will let you check out items that have holds in the Waiting state (i.e. they have been processed and when patron shows up they expect them to still be there waiting for them – otherwise there is no point of holds). I'm looking to unify all these three different ways of checking out an item. I started to work on a patch yesterday, hopefully can share something working with you today!
(In reply to Joonas Kylmälä from comment #6) > (In reply to David Cook from comment #5) > > Actually, I just noticed that while you cannot renew an item that has a > > hold, you can check it in, ignore the hold, try to check it out again, and > > override the errors/warnings and check it out again to the original patron. > > One can renew item with unfilled hold by using > AllowRenewalIfOtherItemsAvailable. At least that works when one clicks those > checkboxes for renewal in the intranet's checkouts tab for patron. Ah sorry, that syspref is only when there are other items available that can fill the hold. So I guess we are both having the same issue in the end but it just appears now in many different forms. Btw, I think your problem doesn't exists with SIP server (you need to have AllowItemsOnHoldCheckout enabled).
(In reply to Joonas Kylmälä from comment #7) > (In reply to Joonas Kylmälä from comment #6) > > (In reply to David Cook from comment #5) > > > Actually, I just noticed that while you cannot renew an item that has a > > > hold, you can check it in, ignore the hold, try to check it out again, and > > > override the errors/warnings and check it out again to the original patron. > > > > One can renew item with unfilled hold by using > > AllowRenewalIfOtherItemsAvailable. At least that works when one clicks those > > checkboxes for renewal in the intranet's checkouts tab for patron. > > Ah sorry, that syspref is only when there are other items available that can > fill the hold. So I guess we are both having the same issue in the end but > it just appears now in many different forms. Btw, I think your problem > doesn't exists with SIP server (you need to have AllowItemsOnHoldCheckout > enabled). I have AllowItemsOnHoldCheckout activated but I'm trying to use the intranet for renewals while holds are waiting and that doesn't work :/.
(In reply to David Cook from comment #8) > I have AllowItemsOnHoldCheckout activated but I'm trying to use the intranet > for renewals while holds are waiting and that doesn't work :/. Is AllowRenewalIfOtherItemsAvailable enabled? And since you brought the topic up, I don't think AllowItemsOnHoldCheckout should be used for renewals because libraries at least in Finland, and I would guess elsewhere, want the item to be returned as quickly as possible if there are holds for it (that cannot be satisfied by other items), and not when the maximum amount of renewals has been reached. The librarian can override this restriction in intranet with the way you described earlier.
Created attachment 80714 [details] [review] Bug 21572: Unify checkout policy of reserved items in SIP, SCO, and intranet Instead of having two different system preferences, AllowItemsOnHoldCheckout and AllowItemsOnHoldCheckoutSCO, now we have only the former one. The preference is also moved now in the C4::Circulation::CanBookBeIssued subroutine so that we have only one single truth about whether the book can be checked out. This patch changes the current checkout policy a bit. Without this patch patron could check out item in Waiting state with the SCO module but after this patch it is not possible anymore. Only checking out items in Reserved state is possible. This is how the SIP server already currently works so now these two would be unified. Allowing to checkout items in the Waiting state doesn't make any sense because it would defeat the purpose of holds, therefore the SIP server approach on only allowing checking out Reserved state items was chosen. The unwanted message "Item XXXXX / (yyyy) has been on hold for Joe (yyyy) at My Library since 15.10.2018" on intranet will be gone now after this patch if AllowItemsOnHoldCheckout is enabled. To test: Testing in SCO, SIP, Intranet with AllowItemsOnHoldCheckout set 'Don't allow': 1. Apply patch 2. Have a biblio record with one item A 3. Make a hold on item A for patron Y (bib level or item, doesn't matter) 4. Try to checkout item A for patron X 5. It should fail (in case of intranet a warning/confirmation dialog should come) Testing in SCO, SIP, Intranet with AllowItemsOnHoldCheckout set 'Allow': 1. Apply patch 2. Have a biblio record with one item A 3. Make a hold on item A for patron Y (bib level or item, doesn't matter) 4. Try to checkout item A for patron X 5. It should succeed (in case of intranet there should be no confirmation dialog) Testing in SCO, SIP, Intranet with AllowItemsOnHoldCheckout set 'Allow' or 'Don't allow': 1. Apply patch 2. Have a biblio record with one item A 3. Make a hold on item A for patron Y (bib level or item, doesn't matter) 4. Check in item A in intranet so that it goes to Waiting status 5. Try to checkout item A for patron X 6. It should fail (in case of intranet a warning/confirmation dialog should come)
Created attachment 80715 [details] [review] Bug 21572: Remove unnecessary SIP checkout code regarding holds The loop that checks whether "Item is on hold shelf for another patron" is already done in C4::Circulation::CanBookBeIssued() so we don't have to do it again but just trust it to tell us the confirmation message "RESERVE_WAITING" when it is someone else's hold that the patron tries to checkout. This removes also the message "Item was reserved for you." since it could have have never appeared since RESERVED confirmation message comes only when the item is reserved to another patron. To test: 1. Apply patch 2. Have a biblio record with one item A 3. Make a hold on item A for patron Y (bib level or item, doesn't matter) 4. Check in item A in intranet so that it goes to Waiting status 5. Try to checkout item A for patron X with the SIP server 6. Checkout should fail with message "Item is on hold shelf for another patron." 1. Apply patch 2. Have a biblio record with two items A and B 3. Make a hold on item A for patron Y (bib level or item, doesn't matter) 4. Check in item A in intranet so that it goes to Waiting status 5. Try to checkout item B for patron X with the SIP server 6. Checkout should succeed 1. Apply patch 2. Make sure AllowItemsOnHoldCheckout is set "Don't allow" 3. Have a biblio record with one item A 4. Make a hold on item A for patron Y (bib level or item, doesn't matter) 5. Try to checkout item A for patron X with the SIP server 6. Checkout should fail with message "Item is reserved for another patron upon return."
Please review the patches I sent. I have only tested thoroughly the SIP code change, i.e. "Bug 21572: Remove unnecessary SIP checkout code regarding holds" and done its test plan. The first patch "Bug 21572: Unify checkout policy of reserved items in SIP, SCO, and intranet" I have not tested so much and was hoping if you could and I didn't have time to go through the provided test plan because there are 9 (or 12) different plans to go through and I have other things I need to focus on. Katrin or David, would you able to test? Maybe just for one of the three checkout ways (SIP, SCO, intra) if testing everything is too much.
Hi Joonas, please don't forget to assign the bug to yourself when providing a patch!
patch does not apply CONFLICT (content): Merge conflict in C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 21572: Unify checkout policy of reserved items in SIP, SCO, and intranet Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-21572-Unify-checkout-policy-of-reserved-items--bkchx6.patch
Ignoring the fact that AllowItemsOnHoldCheckout appears to be broken right now, this seems like it is just *removing* flexibility from Koha, right? Why would we want less flexibility instead of more?
(In reply to Kyle M Hall from comment #15) > Ignoring the fact that AllowItemsOnHoldCheckout appears to be broken right > now, this seems like it is just *removing* flexibility from Koha, right? Why > would we want less flexibility instead of more? Thanks for this comment, for now I think we should keep both of these sysprefs as discussed in bug #15131. I will make a new version of the patch where this setting is available again and also applies to SIP checkouts instead of just SCO module. Later on we could probably address #15131 in some better way like making a possibility to authorize another patron to pickup their items. I'm moving this bug back to ASSIGNED state.
Hi Joonas and Kyle, please also take a look at: bug 23233 bug 23223 and bug 7090 (which suggests that this was for SIP initially)
(In reply to Katrin Fischer from comment #17) > please also take a look at: > bug 23233 > bug 23223 > and bug 7090 (which suggests that this was for SIP initially) Mhm, okay so in #23233 Kyle says the AllowItemsOnHoldCheckout affects only SIP based checkouts which is almost true but not quite there because of one additional usage in the "has_pending_hold" function. If we get rid of the usage in "has_pending_hold" function then IMHO it would okay to merge AllowItemsOnHoldCheckout and AllowItemsOnHoldCheckoutSCO together since they are both self-checkouts. That lead me to the question why in #15131 this new syspref AllowItemsOnHoldCheckoutSCO was even introduced? Didn't the bug reporter just know because of the wrong documentation (as Jonathan points out in the report) that the AllowItemsOnHoldCheckout would have worked also in the situation described in the report?
(In reply to Joonas Kylmälä from comment #18) > (In reply to Katrin Fischer from comment #17) > > please also take a look at: > > bug 23233 > > bug 23223 > > and bug 7090 (which suggests that this was for SIP initially) > > Mhm, okay so in #23233 Kyle says the AllowItemsOnHoldCheckout affects only > SIP based checkouts which is almost true but not quite there because of one > additional usage in the "has_pending_hold" function. If we get rid of the > usage in "has_pending_hold" function then IMHO it would okay to merge > AllowItemsOnHoldCheckout and AllowItemsOnHoldCheckoutSCO together since they > are both self-checkouts. That lead me to the question why in #15131 this new > syspref AllowItemsOnHoldCheckoutSCO was even introduced? Didn't the bug > reporter just know because of the wrong documentation (as Jonathan points > out in the report) that the AllowItemsOnHoldCheckout would have worked also > in the situation described in the report? I think better to keep them separate - both are self checks, but... libraries. There might be a use case that we are not aware of right now.
(In reply to Katrin Fischer from comment #19) > I think better to keep them separate - both are self checks, but... > libraries. There might be a use case that we are not aware of right now. Okay, fine by me, but maybe we could also announce somewhere in the main page that we are deprecating it? Or better, in the syspref setttings page. This way we don't have to carry it with us just because theoretically somebody might want to use it. And so if somebody complains then we remove the deprecation. I'm gonna make this bug depend on #23233 (the renaming of AllowItemsOnHoldCheckout to AllowItemsOnHoldCheckoutSIP) since that sounds easy to fix now and would conflict with this patch.
> Okay, fine by me, but maybe we could also announce somewhere in the main > page that we are deprecating it? Or better, in the syspref setttings page. > This way we don't have to carry it with us just because theoretically > somebody might want to use it. And so if somebody complains then we remove > the deprecation. I'm not sure I understand what you are proposing we deprecate. Can you clarify that for me? Thanks!
(In reply to Kyle M Hall from comment #21) > > Okay, fine by me, but maybe we could also announce somewhere in the main > > page that we are deprecating it? Or better, in the syspref setttings page. > > This way we don't have to carry it with us just because theoretically > > somebody might want to use it. And so if somebody complains then we remove > > the deprecation. > > I'm not sure I understand what you are proposing we deprecate. Can you > clarify that for me? Thanks! To deprecate AllowItemsOnHoldCheckoutSCO and just have one syspref, e.g. "AllowItemsOnHoldCheckoutSCO".
(In reply to Joonas Kylmälä from comment #22) > > I'm not sure I understand what you are proposing we deprecate. Can you > > clarify that for me? Thanks! > > To deprecate AllowItemsOnHoldCheckoutSCO and just have one syspref, e.g. > "AllowItemsOnHoldCheckoutSCO". Sorry, I meant just to have for example "AllowItemsOnHoldCheckout"
(In reply to Joonas Kylmälä from comment #23) > (In reply to Joonas Kylmälä from comment #22) > > > I'm not sure I understand what you are proposing we deprecate. Can you > > > clarify that for me? Thanks! > > > > To deprecate AllowItemsOnHoldCheckoutSCO and just have one syspref, e.g. > > "AllowItemsOnHoldCheckoutSCO". > > Sorry, I meant just to have for example "AllowItemsOnHoldCheckout" Thanks! I tend to agree with Katrin. For instance, it may be that Koha SCO is being used on site and librarians want to prevent patrons from grabbing books from the hold shelf and SIP is being used off site and if the "patron has possession" of the item they need to be able to check it out. This scenario is completely contrived but I think it's illustrative ;)
(In reply to Kyle M Hall from comment #24) > Thanks! I tend to agree with Katrin. For instance, it may be that Koha SCO > is being used on site and librarians want to prevent patrons from grabbing > books from the hold shelf and SIP is being used off site and if the "patron > has possession" of the item they need to be able to check it out. This > scenario is completely contrived but I think it's illustrative ;) If this is the only scenario that could cause problem theoretically to someone then I suggest we still would unify the sysprefs and fix this by making an option in the branch level to allow checkouts of reserved items. In my opinion SIP and SCO should be both treated the same or libraries will start abusing these features like by using the SCO module in one branch because it can do the job even though SIP would be better because you don't have to maintain a different kinds of self checkout machine.
(In reply to Joonas Kylmälä from comment #25) > (In reply to Kyle M Hall from comment #24) > > Thanks! I tend to agree with Katrin. For instance, it may be that Koha SCO > > is being used on site and librarians want to prevent patrons from grabbing > > books from the hold shelf and SIP is being used off site and if the "patron > > has possession" of the item they need to be able to check it out. This > > scenario is completely contrived but I think it's illustrative ;) > > If this is the only scenario that could cause problem theoretically to > someone then I suggest we still would unify the sysprefs and fix this by > making an option in the branch level to allow checkouts of reserved items. > In my opinion SIP and SCO should be both treated the same or libraries will > start abusing these features like by using the SCO module in one branch > because it can do the job even though SIP would be better because you don't > have to maintain a different kinds of self checkout machine. To clarify, with reserved item I mean reserved and in waiting state (as the original patch is about removing the possibility of checking out waiting reserved items in SCO module)
I will start reworking the patches so that they show more clearly the logical differences between SIP and SCO checkout and that the code is the all in the CanBookBeIssued subroutine but now it can be just controlled with parameters. This way the deprecation discussion can be had separately.
Nudging this bug and wondering where it's at. Our library system would like to have the patron who pulled it off the shelf be able to check the item out via SCO, even if there is a pending hold for it somewhere in the system. This has been a major pain point during COVID as most checkouts have been via SCO.
I know this has been inactive for a while, but this is still a regular pain point for us. We are almost completely self checkout and our pending list is very active . For example, a patron can pull a book of the shelf to checkout, but a hold could be placed while they continue to browse. Now they require staff assistance to override this and they cannot fully use self checkout.