an item scanned at a self check station connected to Koha over SIP2 should issue regardless of whether there is a hold on the item. If the patron has the book in hand, they should be allowed to check it out; otherwise, what's to stop them from just taking it? This also prevents patrons from self-checking out holds that are waiting for THEM. Work To Be Done: - Add new system preference that asks whether self-check blocks checkout if the material in hand is on hold for someone else - Controls checkouts using Koha’s SCO - Controls checkouts over SIP2 How to test: - Set preference to allow checkouts of items in hand - Checkout a book with a hold on it for another via SIP. Checkout should proceed, hold should still exist - Checkout a book with a hold on it for another via SCO. Checkout should proceed, hold should still exist - Check out a book with a hold on it for yourself via SIP. Checkout should proceed, hold should be marked filled. - Check out a book with a hold on it for yourself via SCO. Checkout should proceed, hold should be marked filled. - Set preference to not allow checkouts of items in hand if hold is on them - Checkout a book with a hold on it for another via SIP. Checkout should not proceed, hold should still exist - Checkout a book with a hold on it for another via SCO. Checkout should not proceed, hold should still exist - Check out a book with a hold on it for yourself via SIP. Checkout should proceed, hold should be marked filled. - Check out a book with a hold on it for yourself via SCO. Checkout should proceed, hold should be marked filled.
Created attachment 6069 [details] [review] patch
Patch doesn't apply on current master: Observe AllowItemsOnHandCheckout syspref when using self checkout -------------------------- Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: bug_7090: AllowItemsOnHoldCheckout syspref Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging C4/Circulation.pm CONFLICT (content): Merge conflict in C4/Circulation.pm Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging opac/sco/sco-main.pl Failed to merge in the changes. Patch failed at 0001 bug_7090: AllowItemsOnHoldCheckout syspref When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort".
Created attachment 6900 [details] [review] patch
Created attachment 7177 [details] [review] Rebased Patch (2012-01-16) Rebased on current master from 2012-01-16, 20:00 UTC
I plan to sign off on this today - it will need to be rebased due to syspref/updatedatabase changes - but otherwise works a treat.
Created attachment 7413 [details] [review] [Signed Off] bug_7090: AllowItemsOnHandCheckout syspref Observe AllowItemsOnHandCheckout syspref when using SIP self checkout Signed-off-by: Liz Rea <wizzyrea@gmail.com> To test: * place an item on hold for patron A * attempt to circulate that item to patron B (via SIP/selfcheck) syspref off: item should not circulate to patron B Syspref On: item should circulate to patron B Both conditions passed in our testing. Also verified that normal staff client behavior regarding this situation was preserved. It was.
Sorry, I'm not familiar with the procedure, do you need me to rebase it now?
QA comment * this patch seems quite large, but in fact, it's mostly reindenting consequence of a new if () { } * add consistently a new syspref passed QA
Well, found a QA comment to do = there was a tab instead of 4 spaces in the updatedatabase. I've fixed it before pushing, as there's the DBrev number to push. Srdjan, about your comment: > Sorry, I'm not familiar with the procedure, do you need me to rebase it now? When a patch does not apply, the author of the patch is supposed to deal with the rebase. There is an exception to this rule: a conflict on updatedatabase can usually be solved easily by the sign-offer, the QAer or the RM, as it's a very common problem. that's why i've fixed it (and it was trivial : your syspref was added at the end of the syspref file, and another patch already had added another line. Just tell git that both lines must be added (vi sysprefs.sql, remove <<<, ==== and >>> lines, git add sysprefs.sql;git am --resolved, and you're done)
There's a fringe case here that wasn't addressed: an item with a hold that has been confirmed waiting for patron A was placed on the 3M pad by patron B. 3M allowed and confirmed the checkout. This should not happen. In addition, even though the 3M machine said "checked out", Koha did not recognize the checkout (the item did not register as issued to patron B). The item, according to Koha, was still on hold for patron A, and not checked out. When this situation is recreated through the web interface, Koha produced an error message within patron B's account: Item Silver borne / (0003012017669) has been waiting for Patron A (cardnumber) at the Library since 03/28/2012. A staff member was required to confirm the transaction on a waiting item. In normal workflow, staff would not confirm to allow patron B to checkout an item already confirmed on hold for patron A. Ideally, Koha should trigger an error message similar to the error message produced in patron B's account.
actually, hold the phone on this - we might be very dumb.
Right, ok. Not dumb. :) An item that is in status "waiting" should never be checked out by the self check. With this patch, currently it does *appear* to issue the item, but the issue isn't registered in Koha. Don't worry about the missing issue - please just deny issue to 3rd party patrons (i.e. not the patron who has the reserve) on items that are waiting.
Allowing waiting items to be checked out to a 3rd party patron is a matter of human judgement that we cannot expect software to compensate for.
Never mind, dumb after all. The testing of this feature was taking place on the non-upgraded production server instead of the upgraded test server. This issue is moot.