Currently patrons are being blocked at self-check when they find an item on the shelf that they want and that item could also fill a hold. If the item is brought to the circ desk, the staff can override the 'hold' and check it out to the patron. The SIP2 devices do not allow for the override. We propose: 1) Add new SIP2 option or system preference ( or 2nd option to AllowItemsOnHoldCheckout ) to allow allow non-waiting items on hold to be checked out 2) Implement business logic in ILS::Transaction::Checkout or C4::Circulation 3) Add unit tests
I digged out the original bug report for Adding AllowItemsOnHoldCheckout and it looked like it was supposed to cover SIP2 as well: bug 7090 So maybe this is a bug/regression?
(In reply to Katrin Fischer from comment #1) > I digged out the original bug report for Adding AllowItemsOnHoldCheckout and > it looked like it was supposed to cover SIP2 as well: bug 7090 > > So maybe this is a bug/regression? Yes, I'd say it was a bug/regression. If it helps to narrow it down, we have a Koha installation on 16.11 with AllowItemsOnHoldCheckout set to Allow, and items on hold (not waiting) can be checked out via SIP2.
Created attachment 96234 [details] [review] Bug 23223: Add AllowItemsOnHoldCheckoutSIP preference This patch adds a new systempreference for behavior for AllowItemsOnHoldCheckout may difer between intranet, SCO and SIP To test: 1. apply this patch 2. perl installer/data/mysql/updatedatabase.pl SUCCESS => There is a new syspref called AllowItemsOnHoldCheckoutSIP 3. configure and start SIPServer 4. with misc/sip_cli_emulator.pl try to checkout an item with a hold CHECK => checkout is not allowed 5. set AllowItemsOnHoldCheckoutSIP to Allow 6. restart SIPServer 7. repeat step 4 SUCCESS => checkout is allowed 8. Sign off
Created attachment 96235 [details] [review] Bug 23223: Disable syspref cache in SIPServer When a syspref is modified, SIPServer doesn't update it's cached value until restarted. This patch disables syspref cache for SIPServer. To test: 1. set AllowItemsOnHoldCheckoutSIP preference to Don't allow 2. configure and start sipserver 3. with misc/sip_cli_emulator.pl try to checkout an item that has a hold CHECK => checkout is not allowed 4. set AllowItemsOnHoldCheckoutSIP preference to Allow 5. repeat step 3 CHECK => checkout is still not allowed 6. apply this patch and restart_all 7. repeat steps 1 to 5 SUCCESS => checkout is allowed / disallowed according to AllowItemsOnHoldCheckoutSIP preference value and no SIPServer restart is needed 8. Sign off
Agustin, we typically avoid controlling SIP behavior at the syspref level. Instead, it is always set at the SIP config account level. Some good examples of this are: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22016 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13411 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17826 Would you be able to rewrite your feature to be controlled by a new attribute in the SIP config, on a per-account basis?
Considering this patch is splitting the use of an existing system preference, I don't think my previous comment is a blocker ( though adding a config level override would definitely have merit ).
(In reply to Kyle M Hall from comment #6) > Considering this patch is splitting the use of an existing system > preference, I don't think my previous comment is a blocker ( though adding a > config level override would definitely have merit ). I am concerned about your followup disabling syspref caching for the SIP server. I'm starting to wonder if my first instinct was correct, and we should simply not have this behavior controlled by a syspref ( or have a conf setting take precedence over the syspref if defined in the config ).
Yikes, I agree.. I really don't like the sledgehammer approach of disabling cache entirely... we should discus a broad change like that on it's own bug. I presume you're doing it because your finding that caches are not properly flushed when sysprefs change.. I'd much rather see work carried out to try and improve that situation from that direction. As for Syspref vs Config file.. I generally lean towards sysprefs.. they're accessible to the librarian setting it up and as such clearer that there's a difference between SIP and other operations... I personally think only really the server level stuff belongs in the config files and functionality things like this belong in preferences.
(In reply to Martin Renvoize from comment #8) > Yikes, I agree.. I really don't like the sledgehammer approach of disabling > cache entirely... we should discus a broad change like that on it's own bug. > > I presume you're doing it because your finding that caches are not properly > flushed when sysprefs change.. I'd much rather see work carried out to try > and improve that situation from that direction. Take a look at bug 16758. > As for Syspref vs Config file.. I generally lean towards sysprefs.. they're > accessible to the librarian setting it up and as such clearer that there's a > difference between SIP and other operations... I personally think only > really the server level stuff belongs in the config files and functionality > things like this belong in preferences. I agree the idea behind putting configs in the SIPspecific file allows controlling the SIP server behaviour to match different vendors. That way we can have more than one server, with different behaviours depending on the vendor that is using it. BUT in this case is about circulation rules and it feels correct to keep it as a syspref. Unless there's a case for having different vendors different circulation rules for on hold items.
I think the best course of action is to move the followup to its' own bug report at this point. I have no problem with the main patch as it stands.
I filed bug 24761 to handle the caching part of this.
Comment on attachment 96235 [details] [review] Bug 23223: Disable syspref cache in SIPServer I'm obsoleting the sysprefs caching handling patch here, as I wasn't aware of bug 23640. Thanks Jonathan for pointing that out.
Created attachment 99957 [details] [review] Bug 23223: Add AllowItemsOnHoldCheckoutSIP preference This patch adds a new systempreference for behavior for AllowItemsOnHoldCheckout may difer between intranet, SCO and SIP To test: 1. apply this patch 2. perl installer/data/mysql/updatedatabase.pl SUCCESS => There is a new syspref called AllowItemsOnHoldCheckoutSIP 3. configure and start SIPServer 4. with misc/sip_cli_emulator.pl try to checkout an item with a hold CHECK => checkout is not allowed 5. set AllowItemsOnHoldCheckoutSIP to Allow 6. restart SIPServer 7. repeat step 4 SUCCESS => checkout is allowed 8. Sign off Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Hum? What are we doing here? What is suppose to do this patch? It adds a syspref that already exists...
(In reply to Jonathan Druart from comment #14) > Hum? What are we doing here? What is suppose to do this patch? We obsoleted the patch that dealt with caching as it overlapped with bug 23640, but forgot to extract from it the portion that makes SIP use the new syspref. > It adds a syspref that already exists... The new syspref does not yet exist, this patch adds the ability to have different behaviours for SIP than (say) SCO. It is just we missed the fact that the obsolete patch introduced the use of it.
I didn't know about bug 23233, which got pushed in December. So you are right, Jonathan. The original bug got solved by two different reports: - bug 23640 - bug 23233 Closing as duplicate *** This bug has been marked as a duplicate of bug 23233 ***