Bug 10748

Summary: Add option to block return of lost items
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: dominic.pichette, gmcharlt, jonathan.druart, kyle.m.hall, m.de.rooy, nicolas.legrand, severine.queune
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12363
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 10748: Add the ability to block return of lost items
Bug 10748: Add the ability to block return of lost items
Bug 10748: Add tests
Bug 10748: Add the ability to block return of lost items
Bug 10748: Add tests
Bug 10748: (QA follow-up) Add comment in SIP's do_checkin

Description Katrin Fischer 2013-08-18 13:23:40 UTC
It seems there is currently no way to block users from returning an item that has been marked as lost.

We already have BlockReturnOfWithdrawnItems to block the return of withdrawn items, so something similar for lost items would make sense.
Comment 1 Séverine Queune 2017-09-11 15:08:36 UTC
The BULAC (http://www.bulac.fr/) would like to sponsore this development.

Katrin, do you know if someone  worked further on a patch ?

As long as “Change sponsored?” is set on “Seeking developer”, anyone wanting to quote us for the development of such a feature or just ask for more information is welcome to write to us: <sigb@bulac.fr>.
Comment 2 Katrin Fischer 2017-10-08 15:09:46 UTC
Hi Séverine, as far as I know noone has worked on a patch for that yet.
Comment 3 Jonathan Druart 2017-10-12 16:41:07 UTC
Created attachment 68034 [details] [review]
Bug 10748: Add the ability to block return of lost items

Mimicking what does BlockReturnOfWithdrawnItems we can easily add a new
syspref to block return of lost items.
This patch adds BlockReturnOfLostItems, if set to 'Block' a item marked
as lost cannot be checked in.

Test plan:
1/ Set BlockReturnOfLostItems to 'Do not block'
2/ Check an item out to a patron
3/ Edit the item and mark it as lost (*)
4/ Check the item in
=> The item is checked in
5/ Edit the item and remove the lost status
6/ Check the item out again
7/ Edit the item and mark it as lost (*)
8/ Check the item in
=> The item is not checked in

(*) There are 2 ways to mark an item lost:
- From the item list view (/catalogue/moredetail.pl?biblionumber=42)
If you set the lost status from this form, the issue will be returned
Maybe this should be optional (?)

- From the edit items form (/cataloguing/additem.pl?biblionumber=42)
It is the form you must use to not mark the issue returned.
Comment 4 Dominic Pichette 2017-10-13 20:48:10 UTC
Created attachment 68112 [details] [review]
Bug 10748: Add the ability to block return of lost items

Mimicking what does BlockReturnOfWithdrawnItems we can easily add a new
syspref to block return of lost items.
This patch adds BlockReturnOfLostItems, if set to 'Block' a item marked
as lost cannot be checked in.

Test plan:
1/ Set BlockReturnOfLostItems to 'Do not block'
2/ Check an item out to a patron
3/ Edit the item and mark it as lost (*)
4/ Check the item in
=> The item is checked in
5/ Edit the item and remove the lost status
6/ Check the item out again
7/ Edit the item and mark it as lost (*)
8/ Check the item in
=> The item is not checked in

(*) There are 2 ways to mark an item lost:
- From the item list view (/catalogue/moredetail.pl?biblionumber=42)
If you set the lost status from this form, the issue will be returned
Maybe this should be optional (?)

- From the edit items form (/cataloguing/additem.pl?biblionumber=42)
It is the form you must use to not mark the issue returned.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 5 Séverine Queune 2017-10-17 12:07:38 UTC
Hi Jonathan,

No problem using the patch.

About plan test :
  - point 5 : the lost status is automatically reset at check-in, so this action is not necessary.
  - I think a point is missing between 6 and 7, to set the BlockReturnOfLostItems to 'Block'.

We discuss about the return of item marked as lost using the "Items" tab : we agreed it should be optional (we don't want to return books that way, but some library may do).
For information, behaviour is the same with 'Batch item modification', we think it could be good if marking lost items using this tool could not return items on loan.
Comment 6 Jonathan Druart 2017-10-17 13:31:27 UTC
(In reply to Séverine Queune from comment #5)
> Hi Jonathan,
> 
> No problem using the patch.
> 
> About plan test :
>   - point 5 : the lost status is automatically reset at check-in, so this
> action is not necessary.
>   - I think a point is missing between 6 and 7, to set the
> BlockReturnOfLostItems to 'Block'.

Yes of course, the main step was missing! :)

> We discuss about the return of item marked as lost using the "Items" tab :
> we agreed it should be optional (we don't want to return books that way, but
> some library may do).
> For information, behaviour is the same with 'Batch item modification', we
> think it could be good if marking lost items using this tool could not
> return items on loan.

It is what I wanted to tell with "Maybe this should be optional (?)", and actually there is already a bug report, see bug 12363. I will take a look at it soon.
Comment 7 Marcel de Rooy 2017-11-03 09:44:50 UTC
Please add a test case for AddReturn.
Comment 8 Jonathan Druart 2017-11-03 16:27:42 UTC
Created attachment 68919 [details] [review]
Bug 10748: Add tests
Comment 9 Marcel de Rooy 2017-11-06 08:48:42 UTC
There is a difference with the Withdrawn case.
If the item is Lost, checking it in will remove the lost status. So the second checkin will not be blocked anymore although you set the pref.
If the item is withdrawn, the status remains the same. So a second try will be blocked again.
No 'blocker' for me.

Tested it with SIP too.
Got a failed checkin. So that is OK.
The message "Checkin failed" is not very informative though.
You need to inspect syslog (staff member ;) to see this warning:
Checkin failed: probably for Wrongbranch or withdrawn
This warn comes from SIP/ILS.pm line 237.
It could be 'enriched' with the blocked lost-case, but it is more a comment for developers than something useful. Adding a comment in a follow-up.
Comment 10 Marcel de Rooy 2017-11-06 08:49:08 UTC
Created attachment 68950 [details] [review]
Bug 10748: Add the ability to block return of lost items

Mimicking what does BlockReturnOfWithdrawnItems we can easily add a new
syspref to block return of lost items.
This patch adds BlockReturnOfLostItems, if set to 'Block' a item marked
as lost cannot be checked in.

Test plan:
1/ Set BlockReturnOfLostItems to 'Do not block'
2/ Check an item out to a patron
3/ Edit the item and mark it as lost (*)
4/ Check the item in
=> The item is checked in
5/ Edit the item and remove the lost status
6/ Check the item out again
7/ Edit the item and mark it as lost (*)
8/ Check the item in
=> The item is not checked in

(*) There are 2 ways to mark an item lost:
- From the item list view (/catalogue/moredetail.pl?biblionumber=42)
If you set the lost status from this form, the issue will be returned
Maybe this should be optional (?)

- From the edit items form (/cataloguing/additem.pl?biblionumber=42)
It is the form you must use to not mark the issue returned.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-11-06 08:49:12 UTC
Created attachment 68951 [details] [review]
Bug 10748: Add tests

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-11-06 08:49:16 UTC
Created attachment 68952 [details] [review]
Bug 10748: (QA follow-up) Add comment in SIP's do_checkin

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Jonathan Druart 2017-11-06 13:55:59 UTC
(In reply to Marcel de Rooy from comment #9)
> Adding a comment in a follow-up.

Thanks a lot Marcel!
Comment 14 Jonathan Druart 2017-11-08 16:50:15 UTC
Pushed to master for 17.11, thanks to everybody involved!