Bug 7090 - Add "AllowItemsOnHoldCheckout" syspref to allow issue to the patron regardless of hold status
Summary: Add "AllowItemsOnHoldCheckout" syspref to allow issue to the patron regardles...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: 3.6
Hardware: All All
: P3 enhancement (vote)
Assignee: Srdjan Jankovic
QA Contact: Bugs List
URL:
Keywords:
Depends on: 7631
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-26 00:35 UTC by Srdjan Jankovic
Modified: 2012-10-25 23:10 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
patch (10.80 KB, patch)
2011-10-27 05:11 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (10.81 KB, patch)
2011-12-21 08:31 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Rebased Patch (2012-01-16) (10.89 KB, patch)
2012-01-16 20:10 UTC, Ian Walls
Details | Diff | Splinter Review
[Signed Off] bug_7090: AllowItemsOnHandCheckout syspref (11.31 KB, patch)
2012-02-01 17:05 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srdjan Jankovic 2011-10-26 00:35:58 UTC
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.
Comment 1 Srdjan Jankovic 2011-10-27 05:11:39 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2011-12-21 06:17:57 UTC
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".
Comment 3 Srdjan Jankovic 2011-12-21 08:31:52 UTC Comment hidden (obsolete)
Comment 4 Ian Walls 2012-01-16 20:10:26 UTC Comment hidden (obsolete)
Comment 5 Liz Rea 2012-02-01 16:18:34 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.
Comment 6 Liz Rea 2012-02-01 17:05:41 UTC
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.
Comment 7 Srdjan Jankovic 2012-02-01 22:57:39 UTC
Sorry, I'm not familiar with the procedure, do you need me to rebase it now?
Comment 8 Paul Poulain 2012-02-10 18:34:13 UTC
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
Comment 9 Paul Poulain 2012-02-10 18:48:00 UTC
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)
Comment 10 Liz Rea 2012-03-28 15:07:05 UTC
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.
Comment 11 Liz Rea 2012-03-28 15:10:02 UTC
actually, hold the phone on this - we might be very dumb.
Comment 12 Liz Rea 2012-03-28 15:39:21 UTC
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.
Comment 13 Liz Rea 2012-03-28 15:43:44 UTC
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.
Comment 14 Liz Rea 2012-03-28 19:34:11 UTC
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.