Created attachment 105090 [details] [review]
Bug 25541 - Add ability to prevent checkin of items with holds

Some libraries would like patrons to be unable to return items with
holds via SIP. Instead, the screen message should indicate that the
patron should return that item at the circ desk so a librarian can use
it to fill the next hold right away and place it on the hold shelf.

Test Plan:
1) Apply this patch.
2) Place a hold for an item.
3) Enable the new SIP option no_holds_checkin for a SIP account.
4) Restart the SIP server.
5) Check in the item using the SIP CLI tool using the SIP account
   for which you set the new option.
6) Note the checkin fails with a screen message indicating you should
   return the item to the circulation desk.

Created attachment 105098 [details] [review]
Bug 25541: Add ability to prevent checkin via SIP of items with holds

Some libraries would like patrons to be unable to return items with
holds via SIP. Instead, the screen message should indicate that the
patron should return that item at the circ desk so a librarian can use
it to fill the next hold right away and place it on the hold shelf.

Test Plan:
1) Apply this patch.
2) Place a hold for an item.
3) Enable the new SIP option no_holds_checkin for a SIP account.
4) Restart the SIP server.
5) Check in the item using the SIP CLI tool using the SIP account
   for which you set the new option.
6) Note the checkin fails with a screen message indicating you should
   return the item to the circulation desk.

Is this similar to Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15729? I added similar functionality via a syspref.

(In reply to Barry Cannon from comment #3)
> Is this similar to Bug
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15729? I added
> similar functionality via a syspref.

Looks good! The problem with a syspref though is that the behavior cannot be changed on a per-account basis. It very well may be that such behavior is desired for a self check machine for example, but not for an AMH.

Created attachment 107791 [details] [review]
Bug 25541: Add ability to prevent checkin via SIP of items with holds

Some libraries would like patrons to be unable to return items with
holds via SIP. Instead, the screen message should indicate that the
patron should return that item at the circ desk so a librarian can use
it to fill the next hold right away and place it on the hold shelf.

Test Plan:
1) Apply this patch.
2) Place a hold for an item.
3) Enable the new SIP option no_holds_checkin for a SIP account.
4) Restart the SIP server.
5) Check in the item using the SIP CLI tool using the SIP account
   for which you set the new option.
6) Note the checkin fails with a screen message indicating you should
   return the item to the circulation desk.

Signed-off-by: Peter Lau <peter.lau@yccece.edu.hk>

Hi I found two issues with this:
- Does it need to be enabled by default? (we usually keep the previous behaviour)
- It adds the option to etc/SIPconfig.xml but it doesn't touch the one in debian/templates

I agree this fits better in the SIPconfig.xml, Barry.

Created attachment 108626 [details] [review]
Bug 25541: Add new param to debian sip config

> Hi I found two issues with this:
> - Does it need to be enabled by default? (we usually keep the previous
> behaviour)

It is not enabled by default, keeping previous behavior as the default.

> - It adds the option to etc/SIPconfig.xml but it doesn't touch the one in
> debian/templates

Added!

Minor niggle.. I'm not sure about the variable name 'no_holds_checkin'.. I think something like 'holds_block_checkin' might clarify it's meaning a bit?

Otherwise, this generally looks good.

(In reply to Martin Renvoize from comment #9)
> Minor niggle.. I'm not sure about the variable name 'no_holds_checkin'.. I
> think something like 'holds_block_checkin' might clarify it's meaning a bit?

Agreed, that's a much better name. Followup incoming...

Created attachment 109519 [details] [review]
Bug 25541: (QA follow-up) Rename no_holds_checkin to holds_block_checkin

Created attachment 109534 [details] [review]
Bug 25541: Add ability to prevent checkin via SIP of items with holds

Some libraries would like patrons to be unable to return items with
holds via SIP. Instead, the screen message should indicate that the
patron should return that item at the circ desk so a librarian can use
it to fill the next hold right away and place it on the hold shelf.

Test Plan:
1) Apply this patch.
2) Place a hold for an item.
3) Enable the new SIP option no_holds_checkin for a SIP account.
4) Restart the SIP server.
5) Check in the item using the SIP CLI tool using the SIP account
   for which you set the new option.
6) Note the checkin fails with a screen message indicating you should
   return the item to the circulation desk.

Signed-off-by: Peter Lau <peter.lau@yccece.edu.hk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 109535 [details] [review]
Bug 25541: Add new param to debian sip config

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 109536 [details] [review]
Bug 25541: (QA follow-up) Rename no_holds_checkin to holds_block_checkin

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 109537 [details] [review]
Bug 25541: (QA follow-up) Defualt to disabled + Correct message

This patch defaults the 'holds_block_checkin' configuration to disabled
(to maintain current behaviour on upgrades). It also updates a
copy/paste for siplog logging to make the message triggered by this
action unique as expected.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Works well. I took the liberty of adding a final QA followup to correct a copy/paste of the log message and, as Tomas highlighted, correct the default behaviour to allow check-ins as is currently the case.

Passing QA

Pushed to master for 20.11, thanks to everybody involved!

enhancement will not be backported to 20.05.x series

Hi,
I have a few questions about this BZ:
- which level is checked during the return: the biblio-level or the item-level? If an hold is on an item-level, the return of another item from the same biblio shouldn't be blocked.
- If an hold is "served" because an item is waiting or in transfer to another branch for a reserve, will the other items of the same biblio be blocked? They shouldn't be if an item is already here for the hold.

Thanks for your explanations :-) !

Sonia

*** Bug 15729 has been marked as a duplicate of this bug. ***