Bug 7189 - preference to control if returning lost items gives refund
Summary: preference to control if returning lost items gives refund
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.12
Hardware: All All
: P5 - low new feature (vote)
Assignee: Kyle M Hall
QA Contact: Paul Poulain
URL:
Keywords:
: 7070 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-07 16:33 UTC by Nicole C. Engard
Modified: 2014-05-26 21:04 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7189 - preference to control if returning lost items gives refund (3.42 KB, patch)
2012-04-24 13:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7189 - preference to control if returning lost items gives refund (4.80 KB, patch)
2012-07-02 16:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7189 - preference to control if returning lost items gives refund (4.56 KB, patch)
2012-07-17 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7189 - preference to control if returning lost items gives refund (6.74 KB, patch)
2012-07-18 13:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8455 - Check ins processed through "Check Out" tab of the Patron Record ignore Circulation System Preferences (2.79 KB, patch)
2012-07-18 14:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7189 - Followup - perltidy renewscript.pl (4.94 KB, patch)
2012-07-18 14:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7189 - preference to control if returning lost items gives refund (6.74 KB, patch)
2012-07-19 12:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7189 - preference to control if returning lost items gives refund (6.83 KB, patch)
2012-07-20 17:53 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-11-07 16:33:27 UTC
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.
Comment 1 Christopher Brannon 2012-04-10 15:28:01 UTC
Along the same lines of bug 7070.  We agree.  This should not be an automatic process.

Christopher Brannon
Comment 2 Kyle M Hall 2012-04-24 12:48:52 UTC

*** This bug has been marked as a duplicate of bug 7639 ***
Comment 3 Kyle M Hall 2012-04-24 12:49:59 UTC
*** Bug 7070 has been marked as a duplicate of this bug. ***
Comment 4 Kyle M Hall 2012-04-24 13:08:27 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2012-06-25 09:01:30 UTC
I tested the path, but no preference 'RefundLostItemFeeOnReturn' shows up if I do a search on the preferences. 

I do not find any modified .pref file in the patch.

Marc
Comment 6 Kyle M Hall 2012-07-02 16:41:35 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2012-07-17 13:47:35 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2012-07-17 14:12:02 UTC
> 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
Comment 9 Owen Leonard 2012-07-17 15:10:48 UTC
(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" ?
Comment 10 Kyle M Hall 2012-07-18 13:54:30 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2012-07-18 13:56:47 UTC
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.
Comment 12 Kyle M Hall 2012-07-18 14:10:25 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-07-18 14:11:14 UTC Comment hidden (obsolete)
Comment 14 Owen Leonard 2012-07-18 15:32:59 UTC
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.
Comment 15 Kyle M Hall 2012-07-19 12:15:18 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2012-07-19 12:34:11 UTC
(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.
Comment 17 Owen Leonard 2012-07-20 17:53:33 UTC
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>
Comment 18 Paul Poulain 2012-08-03 12:57:19 UTC
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)
Comment 19 Paul Poulain 2012-08-31 15:12:10 UTC
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)
Comment 20 Kyle M Hall 2012-09-12 15:51:02 UTC
> > 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.
Comment 21 Kyle M Hall 2012-09-12 15:52:35 UTC
(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?
Comment 22 Jared Camins-Esakov 2012-11-01 19:35:16 UTC
String changes. Will hold for post-3.10.0.
Comment 23 Paul Poulain 2012-11-29 16:32:15 UTC
As the QA is OK, there is a signoff on the patch, I think it must be my test database that is wrong, so setting passed QA
Comment 24 Jared Camins-Esakov 2012-11-30 13:02:29 UTC
This patch has been pushed to master.