Bug 10748 - Add option to block return of lost items
Summary: Add option to block return of lost items
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-18 13:23 UTC by Katrin Fischer
Modified: 2017-11-08 16:50 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 10748: Add the ability to block return of lost items (9.27 KB, patch)
2017-10-12 16:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10748: Add the ability to block return of lost items (9.28 KB, patch)
2017-10-13 20:48 UTC, Dominic Pichette
Details | Diff | Splinter Review
Bug 10748: Add tests (2.08 KB, patch)
2017-11-03 16:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10748: Add the ability to block return of lost items (9.37 KB, patch)
2017-11-06 08:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10748: Add tests (2.16 KB, patch)
2017-11-06 08:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10748: (QA follow-up) Add comment in SIP's do_checkin (1012 bytes, patch)
2017-11-06 08:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!