Bug 25541 - Add ability to prevent checkin via SIP of items with holds
Summary: Add ability to prevent checkin via SIP of items with holds
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 26896
  Show dependency treegraph
 
Reported: 2020-05-19 12:30 UTC by Kyle M Hall
Modified: 2020-11-20 13:49 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
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. This feature is enabled by adding the flag holds_block_checkin to an account in the SIP configuration file, and setting the value of it to "1".
Version(s) released in:
20.11.00


Attachments
Bug 25541 - Add ability to prevent checkin of items with holds (8.13 KB, patch)
2020-05-19 12:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25541: Add ability to prevent checkin via SIP of items with holds (8.14 KB, patch)
2020-05-19 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25541: Add ability to prevent checkin via SIP of items with holds (8.18 KB, patch)
2020-08-04 16:59 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25541: Add new param to debian sip config (1.05 KB, patch)
2020-08-19 13:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25541: (QA follow-up) Rename no_holds_checkin to holds_block_checkin (4.96 KB, patch)
2020-09-02 12:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25541: Add ability to prevent checkin via SIP of items with holds (8.24 KB, patch)
2020-09-02 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25541: Add new param to debian sip config (1.10 KB, patch)
2020-09-02 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25541: (QA follow-up) Rename no_holds_checkin to holds_block_checkin (5.01 KB, patch)
2020-09-02 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25541: (QA follow-up) Defualt to disabled + Correct message (2.99 KB, patch)
2020-09-02 13:42 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2020-05-19 12:30:50 UTC
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.
Comment 1 Kyle M Hall 2020-05-19 12:39:49 UTC
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.
Comment 2 Kyle M Hall 2020-05-19 14:01:52 UTC
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.
Comment 3 Barry Cannon 2020-07-21 10:48:37 UTC
Is this similar to Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15729? I added similar functionality via a syspref.
Comment 4 Kyle M Hall 2020-07-21 13:18:48 UTC
(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.
Comment 5 Andrew Fuerste-Henry 2020-08-04 16:59:59 UTC
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>
Comment 6 Tomás Cohen Arazi 2020-08-11 15:51:48 UTC
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.
Comment 7 Kyle M Hall 2020-08-19 13:11:06 UTC
Created attachment 108626 [details] [review]
Bug 25541: Add new param to debian sip config
Comment 8 Kyle M Hall 2020-08-19 13:12:19 UTC
> 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!
Comment 9 Martin Renvoize 2020-09-02 11:47:16 UTC
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.
Comment 10 Kyle M Hall 2020-09-02 12:50:55 UTC
(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...
Comment 11 Kyle M Hall 2020-09-02 12:54:23 UTC
Created attachment 109519 [details] [review]
Bug 25541: (QA follow-up) Rename no_holds_checkin to holds_block_checkin
Comment 12 Martin Renvoize 2020-09-02 13:42:19 UTC
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>
Comment 13 Martin Renvoize 2020-09-02 13:42:22 UTC
Created attachment 109535 [details] [review]
Bug 25541: Add new param to debian sip config

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2020-09-02 13:42:26 UTC
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>
Comment 15 Martin Renvoize 2020-09-02 13:42:29 UTC
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>
Comment 16 Martin Renvoize 2020-09-02 13:43:51 UTC
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
Comment 17 Jonathan Druart 2020-09-03 13:04:36 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 18 Lucas Gass 2020-09-14 21:53:53 UTC
enhancement will not be backported to 20.05.x series
Comment 19 Koha Team University Lyon 3 2020-11-12 18:16:49 UTC
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