Bug 27936 - AllowItemsOnHoldCheckoutSIP does not allow checkout of items currently waiting for a hold
Summary: AllowItemsOnHoldCheckoutSIP does not allow checkout of items currently waitin...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Joonas Kylmälä
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-11 17:04 UTC by Andrew Fuerste-Henry
Modified: 2021-12-13 21:10 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.05,20.05.11
Circulation function:


Attachments
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text (1.68 KB, patch)
2021-03-17 11:32 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text (1.67 KB, patch)
2021-03-17 11:45 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text (3.21 KB, patch)
2021-03-18 10:20 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text (3.28 KB, patch)
2021-03-18 12:22 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text (3.33 KB, patch)
2021-03-25 14:02 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2021-03-11 17:04:29 UTC
The syspref text says "[Allow/Don't allow] checkouts of items reserved to someone else via SIP checkout messages. If allowed do not generate RESERVE_WAITING and RESERVED warning. This allows self checkouts for those items. If using the holds queue items with pending holds will be marked as "unavailable" if this set to "Don't allow"."

However, setting the syspref to Allow *only* allows sets sip to ignore the RESERVED warning. Items that are currently waiting for hold pickup will still generate a RESERVE_WAITING message and block the checkout. Either the description or the behavior needs to change to make this accurate.

In contrast, AllowItemsOnHoldCheckoutSCO also says "If allowed do not generate RESERVE_WAITING and RESERVED warning" and, for the SCO, this is accurate. With AllowItemsOnHoldCheckoutSCO set to allow, items waiting for another patron's hold can be checked out without a warning at the SCO.
Comment 1 Andrew Fuerste-Henry 2021-03-11 17:14:55 UTC
To test:
1 - place a hold on Item1 for PatronA, check it in to set the hold to waiting
2 - place a hold on Item2 for PatronA, do not check it in so the hold stays at pending
3 - confirm AllowItemsOnHoldCheckoutSIP is set to Don't allow
4 - attempt to check Item1 out to PatronB via SIP, get an error that the item is on hold for another patron
5 - attempt to check Item2 out to PatronB via SIP, get an error that the item is on hold for another patron
6 - set AllowItemsOnHoldCheckoutSIP to Allow
7 - attempt to check Item1 out to PatronB via SIP again, still get an error that the item is on hold for another patron
8 - attempt to check Item2 out to PatronB via SIP, success!
Comment 2 Kyle M Hall (khall) 2021-03-11 17:20:14 UTC
What version of Koha did you test this on? If not master, can you make sure master exhibits the same behavior?

Thanks!
Comment 3 Andrew Fuerste-Henry 2021-03-11 17:27:27 UTC
I tested on master and then made Nick look at the code around it, in which he reports it only prevents RESERVE warnings.
Comment 4 Jeff Gaines 2021-03-11 17:47:11 UTC
The syspref is working as we were looking for, so if you do change this, please leave this functionality as an alternate option for it.

I cannot imagine the use case for Patron B to be able to walk up to the self-serve Hold Shelf, take an item held for Patron A, and check it out at a self checkout station without staff intervention/approval. Is that truly the intent of this?

If Patron B finds an item on the regular shelf that is on the Holds Queue report for Patron A but staff have not gone out and pulled it yet, then it's "finders keepers" and we want Patron B to be able to check it out. But once an item is trapped and on the Hold Shelf for patron A (and Patron A has been notified to come get it), we don't want anyone other than Patron A to be able to check it out without staff approval.
Comment 5 Jonathan Druart 2021-03-17 11:19:10 UTC
Looks like it has been caused by
  commit 338ba1d7692c1f75af7b0fb75eb5aac00a04a3df
  Bug 25690: Remove duplicated logic in SIP2 checkouts
Comment 6 Joonas Kylmälä 2021-03-17 11:23:18 UTC
Hi,

the behaviour has been always to prevent waiting holds being checked out via SIP2, the syspref text has been inaccurate. I can send patch.
Comment 7 Joonas Kylmälä 2021-03-17 11:32:37 UTC Comment hidden (obsolete)
Comment 8 Joonas Kylmälä 2021-03-17 11:38:19 UTC
Regarding the AllowItemsOnHoldCheckoutSCO behaviour being different see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21572#c3
Comment 9 Joonas Kylmälä 2021-03-17 11:45:01 UTC Comment hidden (obsolete)
Comment 10 Joonas Kylmälä 2021-03-18 10:20:35 UTC
Created attachment 118455 [details] [review]
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text

The waiting reserves have always been impossible to checkout via
SIP2. There was some confusion whether this is actually true but if
you check the commit "Bug 25690: Remove duplicated logic in SIP2
checkouts" (338ba1d769) and the removed code around line

  $self->screen_msg("Item is on hold for another patron.");

you will be sure that it has been impossible to checkout waiting holds.
Comment 11 Joonas Kylmälä 2021-03-18 10:21:57 UTC
The syspref description was also in sysprefs.sql which I missed in the earlier revision of the patch. Attached now a new version with the text there also corrected. Ready for Sign-Off.
Comment 12 Andrew Fuerste-Henry 2021-03-18 12:22:47 UTC
Created attachment 118458 [details] [review]
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text

The waiting reserves have always been impossible to checkout via
SIP2. There was some confusion whether this is actually true but if
you check the commit "Bug 25690: Remove duplicated logic in SIP2
checkouts" (338ba1d769) and the removed code around line

  $self->screen_msg("Item is on hold for another patron.");

you will be sure that it has been impossible to checkout waiting holds.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 13 Martin Renvoize (ashimema) 2021-03-25 14:02:17 UTC
Created attachment 118808 [details] [review]
Bug 27936: Clarify AllowItemsOnHoldCheckoutSIP syspref help text

The waiting reserves have always been impossible to checkout via
SIP2. There was some confusion whether this is actually true but if
you check the commit "Bug 25690: Remove duplicated logic in SIP2
checkouts" (338ba1d769) and the removed code around line

  $self->screen_msg("Item is on hold for another patron.");

you will be sure that it has been impossible to checkout waiting holds.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2021-03-25 14:02:33 UTC
Improved wording.. Passing QA
Comment 15 Jonathan Druart 2021-04-01 16:54:45 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 16 Fridolin Somers 2021-04-15 10:38:32 UTC
Pushed to 20.11.x for 20.11.05
Comment 17 Andrew Fuerste-Henry 2021-04-24 16:49:22 UTC
Pushed to 20.05.x for 20.05.11
Comment 18 Victor Grousset/tuxayo 2021-04-25 17:07:21 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.