Right now when you return an item that was lost the patron's card is credited with the lost fee, but not all libraries refund lost fees and sometimes the fee is refunded after the patron has paid for it - causing all kinds of financial issues.

Created attachment 9283[details][review]Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.
Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.

Created attachment 10614[details][review]Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.
Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.

Created attachment 10892[details][review]Bug 7189 - preference to control if returning lost items gives refund
The previous patch contained a conflict string ("<<<<<<<").
Rebased patch.
Comment:
I think the modification (conditionnal test with the syspref RefundLostItemFeeOnReturn's value) should be in the AddIssue routine and not in AddReturn.
Otherwise, I don't understand your patch and comment :)

> I think the modification (conditionnal test with the syspref
> RefundLostItemFeeOnReturn's value) should be in the AddIssue routine and not
> in AddReturn.
I'm not sure why you would think that. This is about refunding the lost item fee when the item is returned, it has nothing to do with issuing an item.
Test Plan:
1) Set RefundLostItemFeeOnReturn to "Refund"
2) Check an item out to a patron ( the item must have a replacement price )
3) Mark the item as lost
4) Verify a lost item fee has been added to the patron's record
5) Return the item
6) Verify the lost item fee has been removed from the patron's record
7) Set RefundLostItemFeeOnReturn to "Don't refund"
8) Repeat steps 2 through 6 with another item
9) Verify the lost item fee still remains on the patron's record

(In reply to comment #8)
> 9) Verify the lost item fee still remains on the patron's record
In my test this is not the case. Either way a refund is applied to the account. I have verified that the database update occurred and that the setting is correct.
I also think it would be good to include additional information on the check-in screen. Could it say "Item was lost, now found. A refund was applied to [Patron]'s account" or "Item was lost, now found. The fine remains on [Patron]'s account" ?

Created attachment 10951[details][review]Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.
Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.

Here is an updated patch. I now understand Jonathan's comment, he swapped the subroutines in his sentence. The old patch worked, but only when trying to issue a lost item. New patch works for explicit returning of a lost item as well. I've also added the messages suggested by Owen.

Created attachment 10971[details][review]Bug 8455 - Check ins processed through "Check Out" tab of the Patron Record ignore Circulation System Preferences
If you check-in an item through the "Check Out" tab of the Patron Record, the return/check-in ignores the "InProcessingToShelvingCart" and "ReturnToShelvingCart" circulation system preferences. Or rather, there just isn't any logic to handle them in the renewscript.pl file.
I'm pretty much just bringing the code straight across from returns.pl, although some minor changes need to be made due to context.
Both system preferences are important to include, since you can check-out an in processing item to a patron. While a regular check-in will clear the in processing status, a check-in through the Patron module will cause the item to have its status returned to "In Processing" rather than going to "Shelving Cart", if the preference is set, of course.
To test the scenario, turn on one of (or both of) these settings and try checking books in through the Circulation module and the Patron module.
Before the patch, you will see that only the Circulation module will change the shelving location.
After the patch, you should see identical behaviour.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

This seems to be working, but with both settings for RefundLostItemFeeOnReturn I see the same message on check-in, "Any lost item fees for this item will remain on the patron's account." It looks like errmsgloo.LostItemFeeRefunded isn't evaluating as true when it should.
Is it possible to link that error message to the patron's account? Not a blocking issue, but would be very nice.

Created attachment 11005[details][review]Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.
Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.

(In reply to comment #14)
> This seems to be working, but with both settings for
> RefundLostItemFeeOnReturn I see the same message on check-in, "Any lost item
> fees for this item will remain on the patron's account." It looks like
> errmsgloo.LostItemFeeRefunded isn't evaluating as true when it should.
Fixed.
> Is it possible to link that error message to the patron's account? Not a
> blocking issue, but would be very nice.
I agree, that would be nice, but to implement such a feature. However, when an item is marked lost, the item is removed from the borrower's record. I had thought there was a system preference to control the behavior, but I cannot find it.
We could also use _FixAccountForLostAndReturned to get the borrowernumber, but only in the event that the fee is refunded. That would still leave the generic message for when the fee is not refunded.

Created attachment 11054[details][review]
[SIGNED-OFF] Bug 7189 - preference to control if returning lost items gives refund
Right now when you return an item that was lost the patron's card is
credited with the lost fee, but not all libraries refund lost fees
and sometimes the fee is refunded after the patron has paid for it,
causing all kinds of financial issues.
Adds the syspref RefundLostItemFeeOnReturn to control whether
returning a lost item refunds the fee charged for losing that
item. Enabled by default to maintain Koha's current functionality.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

QA Comment:
* FFUSP (Fundation For Usefull System Preferences) agrees that FFAUSP leader must not complain for this one ;-)
* passes test-qa.pl
* Not a lot of code. What is written passes QA,
But I can't test this patch. I have an item with items.price and items.replacementprice set to 10
I've LOST authorised values defined, and they are attached to items.lost
When I open catalogue/moredetail.pl?biblionumber=21668&itemnumber=23170#item23170
I see the "LOST list". If I choose one of them, the book is automatically checked-in, that's OK, but the price is not charged to the patron.
What did I miss ?
(marking passed QA, it's probably me, but I won't push until I could test)

bumping my question:
> QA Comment:
> * FFUSP (Fundation For Usefull System Preferences) agrees that FFAUSP
> leader must not complain for this one ;-)
> * passes test-qa.pl
> * Not a lot of code. What is written passes QA,
>
> But I can't test this patch. I have an item with items.price and
> items.replacementprice set to 10
> I've LOST authorised values defined, and they are attached to items.lost
>
> When I open
> catalogue/moredetail.pl?biblionumber=21668&itemnumber=23170#item23170
> I see the "LOST list". If I choose one of them, the book is automatically
> checked-in, that's OK, but the price is not charged to the patron.
> What did I miss ?
>
> (marking passed QA, it's probably me, but I won't push until I could test)

> > When I open
> > catalogue/moredetail.pl?biblionumber=21668&itemnumber=23170#item23170
> > I see the "LOST list". If I choose one of them, the book is automatically
> > checked-in, that's OK, but the price is not charged to the patron.
> > What did I miss ?
> >
> > (marking passed QA, it's probably me, but I won't push until I could test)
I just retested the patch, and everything appears to be working correctly for me. I'm not sure why you are not seeing the replacement price getting charged to the patron when you mark it lost. This patch does not touch that part of the codebase, and there does not appear to be any way to configure Koha to not charge a lost fee when an item is marked as lost from moredetail.pl.

(In reply to comment #20)
> > > When I open
> > > catalogue/moredetail.pl?biblionumber=21668&itemnumber=23170#item23170
> > > I see the "LOST list". If I choose one of them, the book is automatically
> > > checked-in, that's OK, but the price is not charged to the patron.
> > > What did I miss ?
> > >
> > > (marking passed QA, it's probably me, but I won't push until I could test)
If you mark a checked out item as lost when on master, does it charge a fee to the borrower?