Bug 3387 - WITHDRAWN items shouldn't check in
Summary: WITHDRAWN items shouldn't check in
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-02 06:42 UTC by Mickey Coalwell
Modified: 2013-12-05 20:05 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:


Attachments
Bug 3387 - WITHDRAWN items shouldn't check in (5.42 KB, patch)
2012-07-17 13:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 3387 - WITHDRAWN items shouldn't check in - Followup (2.02 KB, patch)
2012-07-17 16:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 3387 - WITHDRAWN items shouldn't check in - Followup (2.13 KB, patch)
2012-07-17 17:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 3387 - WITHDRAWN items shouldn't check in (7.65 KB, patch)
2012-07-18 13:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 3387 - WITHDRAWN items shouldn't check in (7.75 KB, patch)
2012-08-21 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 3387 - WITHDRAWN items shouldn't check in - QA Followup (1.01 KB, patch)
2012-09-26 12:20 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:09:54 UTC


---- Reported by mcoalwell@nekls.org 2009-07-02 18:42:31 ----

"Withdrawn" status currently blocks checkout from occurring. It seems to me that "Withdrawn" status should also block check-in, prompting staff user with a message "ITEM IS WITHDRAWN." The reason this is important: For some libraries, LOST items older than x months cannot be returned, as a matter of policy. The replacement cost is billed to the patron. Items LOST longer than x months will be manually given a WITHDRAWN status by staff, in addition to LOST. This is intended to prevent check-in of LOST items older than x months. But if WITHDRAWN status allows check-in, then the Returned Item functionality in Fines will credit the account with the replacement cost. This should ONLY happen for items that are LOST, but NOT for items that are LOST/WITHDRAWN. I can't think of a situation in which Koha should legitimately allow a WITHDRAWN item to check in, so I'm reporting this as a bug rather than an enhancement; however, I'm open to discussion. :)



---- Additional Comments From oleonard@myacpl.org 2009-07-02 22:02:03 ----

I don't have an opinion either way, but I want to point out that at the moment there is no concrete definition of what "withdrawn" means in Koha. Some libraries may already be using it in a way that conflicts with your description. I hope folks will chime in, but it might be worth taking the issue to the Koha list for wider discussion.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:09 UTC  ---

This bug was previously known as _bug_ 3387 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3387

Unknown operating system Windows 2000. Setting to default OS "All".
Actual time not defined. Setting to 0.0

Comment 1 Kyle M Hall 2012-07-17 13:17:02 UTC
As of this date, Koha does not allow for returning of items marked as Withdrawn. One of our libraries reported this a problem to us, where they could not return an item marked as withdrawn, so I would assume they consider this behavior to be a bug. This is something that should probably be controlled by a system preference.
Comment 2 Kyle M Hall 2012-07-17 13:47:11 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2012-07-17 14:39:13 UTC
A few comments about this:

1. If I have the preference set to "Block" and I check in a withdrawn item I see a warning that "Item is withdrawn," but no indication that the item was not checked in. I think it should follow the pattern of the "wrongbranch" case in the template:

<div class="dialog alert"><h3>Cannot Check In</h3><p>This item must be checked in at its home library. <strong>NOT CHECKED IN</strong></p>
</div>

2. In this same case, the table of checked-in items which appears at the bottom of the page the "due date" and "patron" columns both say "not checked out." I'm guessing the patron information isn't available because it wasn't returned as part of the check-in process? I wonder if there is a better way to handle this while still preserving the display of the failed checkin in the table of scanned items?

3. If I have the preference set to "Don't block" I think I should see a "dialog message" style message that the item is marked as withdrawn so that the staff has the option of taking action.
Comment 4 Kyle M Hall 2012-07-17 16:17:09 UTC
> 1. If I have the preference set to "Block" and I check in a withdrawn item I
> see a warning that "Item is withdrawn," but no indication that the item was
> not checked in. I think it should follow the pattern of the "wrongbranch"
> case in the template:
> 
> <div class="dialog alert"><h3>Cannot Check In</h3><p>This item must be
> checked in at its home library. <strong>NOT CHECKED IN</strong></p>
> </div>

Done.
 
> 2. In this same case, the table of checked-in items which appears at the
> bottom of the page the "due date" and "patron" columns both say "not checked
> out." I'm guessing the patron information isn't available because it wasn't
> returned as part of the check-in process? I wonder if there is a better way
> to handle this while still preserving the display of the failed checkin in
> the table of scanned items?

This is an existing bug and is outside the scope of this patch. I will file a separate bug report for it.
 
> 3. If I have the preference set to "Don't block" I think I should see a
> "dialog message" style message that the item is marked as withdrawn so that
> the staff has the option of taking action.

A message is already displayed. If you would like to see it some other way, please file an enhancement bug for it.
Comment 5 Kyle M Hall 2012-07-17 16:18:31 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2012-07-17 16:47:23 UTC
I just tested the assertion that "Koha does not allow for returning of items marked as Withdrawn" on master.

This is untrue - you can check in a book that has been withdrawn, and furthermore, it will attempt to continue to fill holds even though it is withdrawn and "cannot be checked out."

Does this patch address the original bug, which was "block check out of books that are withdrawn?"
Comment 7 Kyle M Hall 2012-07-17 17:29:21 UTC
(In reply to comment #6)
> I just tested the assertion that "Koha does not allow for returning of items
> marked as Withdrawn" on master.
> 
> This is untrue - you can check in a book that has been withdrawn, and
> furthermore, it will attempt to continue to fill holds even though it is
> withdrawn and "cannot be checked out."

Either you are mistaken, or some part of your configuration is allowing withdrawn items to be returned. I'm using only the example data that comes with Koha.
Here is a screencast showing this behavior in action: http://screencast.com/t/6OYjRlju

> Does this patch address the original bug, which was "block check out of
> books that are withdrawn?"

The original bug was "block check *in* of books that are withdrawn" and I stated that my testing has shown that this is how Koha is currently working.
Comment 8 Owen Leonard 2012-07-17 17:30:45 UTC
(In reply to comment #4)
> This is an existing bug and is outside the scope of this patch. I will file
> a separate bug report for it.

The problem is mitigated greatly by the improved warning message.
  
> > 3. If I have the preference set to "Don't block" I think I should see a
> > "dialog message" style message that the item is marked as withdrawn so that
> > the staff has the option of taking action.
> 
> A message is already displayed.

I guess I assumed a message wasn't displayed at all because it's not displaying under this condition: When checking in a withdrawn item which must be transferred to another branch. And in that case I suppose the alert about the status can be deferred until it's at its destination.

One new problem, however, with the follow-up: the "<strong>NOT CHECKED IN</strong>" message is being displayed whether or not the item was checked in (i.e. for either setting of BlockReturnOfWithdrawnItems)
Comment 9 Kyle M Hall 2012-07-17 17:51:34 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2012-07-17 17:52:54 UTC
Good catch, it's just one of those days... : )=

> One new problem, however, with the follow-up: the "<strong>NOT CHECKED
> IN</strong>" message is being displayed whether or not the item was checked
> in (i.e. for either setting of BlockReturnOfWithdrawnItems)
Comment 11 Liz Rea 2012-07-17 18:11:17 UTC
I'm using the sample data too, so not sure what the difference is. It works this way both in master and in our 3.6.4 install.
Comment 12 Liz Rea 2012-07-17 18:23:14 UTC
I stand corrected, you'll be relieved to know that it does show a message "item is withdrawn" when there are no additional actions to be done on the item.

There is an additional problem, however, that has to do with holds - if the item has a hold on it it does not show the withdrawn message, and attempts to send the item to fill a hold. This scenario also probably needs to be tested with the new patch.
Comment 13 Owen Leonard 2012-07-17 18:32:06 UTC
Last thing, I hope: The database update runs with this description: "Add system preference IntranetSlipPrinterJS."
Comment 14 Kyle M Hall 2012-07-18 13:15:24 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2012-07-18 13:16:31 UTC
This new patch should solve the holds issue, and also fixes the dbrev description. I've squashed everything down to one patch for convenience, as none of them have been signed off yet.
Comment 16 Owen Leonard 2012-07-18 15:53:00 UTC
I just tested this scenario:

1. A biblio-level hold is on the title.
2. BlockReturnOfWithdrawnItems is set to "don't block"

Checking in the item prompted a regular transfer, not the holds prompt. Is this correct? It makes sense if the idea is that a withdrawn item should not be available to fulfill holds. I just wanted to double-check.
Comment 17 Kyle M Hall 2012-07-30 15:45:41 UTC
Yes, that is the intended behavior.

(In reply to comment #16)
> I just tested this scenario:
> 
> 1. A biblio-level hold is on the title.
> 2. BlockReturnOfWithdrawnItems is set to "don't block"
> 
> Checking in the item prompted a regular transfer, not the holds prompt. Is
> this correct? It makes sense if the idea is that a withdrawn item should not
> be available to fulfill holds. I just wanted to double-check.
Comment 18 Owen Leonard 2012-08-21 18:42:18 UTC
Created attachment 11725 [details] [review]
[SIGNED-OFF] Bug 3387 - WITHDRAWN items shouldn't check in

Adds a system preference BlockReturnOfWithdrawnItems to control
whether or not Koha should allow a withdrawn item to be returned
or not.

Also fixes the behavior where Koha will attempt to use a withdrawn
item to fill a hold.

Test Plan:
1) Set BlockReturnOfWithdrawnItems to "Block" ( this is the default )
2) Check an item out to a patron
3) Mark item as withdrawn
4) Attempt to return the item, you should still see the item on the
   borrower's record
5) Set BlockReturnOfWithdrawnItems to "Don't Block"
6) Attempt to return the item, you should see the item is no longer
   on the borrower's record.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 19 Jonathan Druart 2012-08-22 08:29:07 UTC
QA Comment:
This patch introduces a perlcritic's violation:
    Variable declared in conditional statement at line 1701, column 5.  Declare variables outside of the condition.  (Severity: 5)
caused by the following line:
    my ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->{'itemnumber'} ) unless ( $item->{'wthdrawn'} );

Marked as Failed QA
Comment 20 Kyle M Hall 2012-09-26 12:20:49 UTC
Created attachment 12524 [details] [review]
Bug 3387 - WITHDRAWN items shouldn't check in - QA Followup
Comment 21 Kyle M Hall 2012-09-26 12:21:41 UTC
Marking as Passed QA. If you feel this followup needs it's own sign-off, please reset the status.
Comment 22 Paul Poulain 2012-09-26 17:35:27 UTC
I find nothing wrong with this patch, but there's something I don't understand, functionnally speaking=
 * patron has a withdrawn item that is checked out
 * if no-one can't check-in the book, how will we "clean" the patron record ? the patron will have this book checked in forever, even once it has been paid.

isn't this a dead-end ?

Explanation welcomed before I push the patch ;-)

(+ I thought withdrawn status was for items withdrawn by the library -because they are broken for example-)
Comment 23 Kyle M Hall 2012-09-27 13:41:37 UTC
(In reply to comment #22)
> I find nothing wrong with this patch, but there's something I don't
> understand, functionnally speaking=
>  * patron has a withdrawn item that is checked out
>  * if no-one can't check-in the book, how will we "clean" the patron record
> ? the patron will have this book checked in forever, even once it has been
> paid.
> 
> isn't this a dead-end ?
> 
> Explanation welcomed before I push the patch ;-)
> 
> (+ I thought withdrawn status was for items withdrawn by the library
> -because they are broken for example-)

That is correct. An item should never be withdrawn if it is currently checked out to a patron, so that situation should never occur.
Comment 24 Nicole C. Engard 2012-09-27 13:50:34 UTC
Withdrawn means that you're removing it from your collection - also referred to as Weeding.  This should only happen with items that are not checked out (as Kyle said).  Worst case if you accidentally mark something withdrawn and it's still checked out you can always just edit the item and remove the withdrawn status before checking the item back in.

Nicole
Comment 25 Jared Camins-Esakov 2012-09-27 13:54:02 UTC
Paul,

> That is correct. An item should never be withdrawn if it is currently
> checked out to a patron, so that situation should never occur.

To add to what Kyle said, the only situation where it does occur is when a library wants to continue checking items out to a specific user when withdrawing them. The workflow is unnecessary in Koha, of course, but see quote 123. I think this is a good change.
Comment 26 Paul Poulain 2012-09-27 14:52:00 UTC
(In reply to comment #24)
> Withdrawn means that you're removing it from your collection - also referred
> to as Weeding.  This should only happen with items that are not checked out
> (as Kyle said).  Worst case if you accidentally mark something withdrawn and
> it's still checked out you can always just edit the item and remove the
> withdrawn status before checking the item back in.
> 
> Nicole

In this case, shouldn't we instead just update the behaviour and set back to enable check-in of a withdrawn book ?
Wasn't the initial report wrong when mcoalwell was saying we should not enable check-in of lost items ?
(IE: no need of a new syspref)
Comment 27 Mickey Coalwell 2012-09-27 14:53:28 UTC
Kyle, Nicole and Jared, I'm sorry, I disagree with you concerning the meaning/purpose of WITHDRAWN. It is not exclusively for weeded items, nor for items that are not checked out. 

If you will go back to the first post in this bug, you'll see I described the situation where one of our libraries uses WITHDRAWN quite legitimately as a means of preventing LOST materials over x months overdue to be checked in and credited once a replacement fee has been applied. This is a policy decision made by the library that items unreturned after x months will not be accepted. 

Paul's point is correct. A patron who ays for a WITHDRAWN item will indeed display it on their account "forever,"  -- and that is the library's intent. 

Does that make sense? I think we all agree that nothing needs to be done in terms of the fix's current functionality, however.
Comment 28 Kyle M Hall 2012-09-27 15:12:12 UTC
> In this case, shouldn't we instead just update the behaviour and set back to
> enable check-in of a withdrawn book ?

What everyone wants is to *not* allow checking in of withdrawn books.

> Wasn't the initial report wrong when mcoalwell was saying we should not
> enable check-in of lost items ?
> (IE: no need of a new syspref)

I cannot speak to what mcoalwell meant, but I can say that the consensus is that the correct behavior is to disallow checking in or withdrawn items. That includes me, Nicole and Jared. Mickey, while disagreeing on one point with us, is also agreeing that withdrawn items should not be returnable.
Comment 29 Paul Poulain 2012-09-28 10:01:03 UTC
(In reply to comment #27)
> Does that make sense? I think we all agree that nothing needs to be done in
> terms of the fix's current functionality, however.

(In reply to comment #28)
> What everyone wants is to *not* allow checking in of withdrawn books.

mmm... Sorry guys, but I don't understand: if the behaviour we want it NOT allowing checkin-g of withdrawn items is this patch needed ? ( because it adds a syspref to enable check-in of withdrawn items)

/me really confused...
Comment 30 Kyle M Hall 2012-09-28 12:23:44 UTC
(In reply to comment #29)
> (In reply to comment #27)
> > Does that make sense? I think we all agree that nothing needs to be done in
> > terms of the fix's current functionality, however.
> 
> (In reply to comment #28)
> > What everyone wants is to *not* allow checking in of withdrawn books.
> 
> mmm... Sorry guys, but I don't understand: if the behaviour we want it NOT
> allowing checkin-g of withdrawn items is this patch needed ? ( because it
> adds a syspref to enable check-in of withdrawn items)
> 
> /me really confused...

I added the system preference so we can leave the default behavior unchanged, as is common practice. I can add a followup to remove the choice if you wish.
Comment 31 Mickey Coalwell 2012-09-28 16:24:16 UTC
Kyle, you wrote, "I can add a followup to remove the choice if you wish."

Please don't do that! The whole point of this work was to allow an alternative to the default behavior, which was to allow checkin of WITHDRAWN items. My libraries don't want that to occur. Therefore, this syspref works for me. Removing the choice would invalidate the option. 

Paul, my understanding is that with this syspref Koha users now have the choice of either allowing or not allowing checkin for WITHDRAWN items. That is what I was hoping to get from this. 

I hope I'm following everyone's reasoning correctly here.
Comment 32 Paul Poulain 2012-10-01 15:32:01 UTC
Patch pushed to master

BUT Mickey, I've a functional problem with the "prevent" option (that is the current one already, so this patch introduces an alternative which sounds OK to me.
As stated in my comment 22, how to you deal with the situation of a checked-out item that is withdrawn ? once the book has been paid, you keep forever the information that the items had been checked out by patron X ?
Otherwise, how do you delete the information ?

Here, in France, we have a law for privacy. And a rule has been specified for a library: they can't keep track of this information for more than 3 months after the book has been returned (or refunded)
So the "prevent check-in of withdrawn items" is a non-sense (well, something illegal ;-) )
Comment 33 Paul Poulain 2012-10-01 15:36:46 UTC
(not patch pushed with a tiny follow-up for perlcritic:
		Variable declared in conditional statement at line 1836, column 5. Declare variables outside of the condition.
Comment 34 Chris Cormack 2013-04-20 23:03:53 UTC
Released in 3.10.0