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.

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.

> 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.

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?"

(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.

(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)

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)

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.

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.

Last thing, I hope: The database update runs with this description: "Add system preference IntranetSlipPrinterJS."

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.

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.

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.

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>

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

Created attachment 12524 [details] [review]
Bug 3387 - WITHDRAWN items shouldn't check in - QA Followup

Marking as Passed QA. If you feel this followup needs it's own sign-off, please reset the status.

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-)

(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.

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

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.

(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)

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.

> 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.

(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...

(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.

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.

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 ;-) )

(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.

Released in 3.10.0