Bug 14380 - Overriding fine on renewal will cause duplicate fines
Summary: Overriding fine on renewal will cause duplicate fines
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.18
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-12 21:00 UTC by Barton Chittenden
Modified: 2016-06-21 21:37 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2015-06-12 21:00:59 UTC
Overriding fine on renewal will cause duplicate fines.

Because we use the due date in accountlines.description, and fines.pl uses accountlines.description to uniquely identify fines on update, changing date_due will create duplicate entries in accountlines.

select date, 
       amount, 
       description, 
       accounttype, 
       amountoutstanding, 
       lastincrement,
       timestamp 
from accountlines 
where itemnumber = 35540 
  and borrowernumber = 24990\G
*************************** 1. row ***************************
             date: 2015-05-05
           amount: 0.500000
      description: O'Connor's annotated criminal codes plus. 05/04/2015 11:59 PM
      accounttype: F
amountoutstanding: 0.500000
    lastincrement: 0.500000
        timestamp: 2015-06-08 12:42:27
*************************** 2. row ***************************
             date: 2015-06-08
           amount: 4.500000
      description: O'Connor's annotated criminal codes plus. 05/26/2015 11:59 PM
      accounttype: FU
amountoutstanding: 4.500000
    lastincrement: 0.500000
        timestamp: 2015-06-08 02:02:40
2 rows in set (0.00 sec)

Looking in old_issues, you can see that the item was renewed, and that date_due does match the second description... only the first accountlines entry is marked woth accounttype 'F' on return, however.

select 
    date_due, 
    returndate, 
    lastreneweddate, 
    renewals, 
    timestamp, 
    issuedate 
from old_issues 
where itemnumber = 35540 
  and borrowernumber = 24990\G
*************************** 1. row ***************************
       date_due: 2015-05-26 23:59:00
     returndate: 2015-06-08 12:42:27
lastreneweddate: 2015-05-05 00:00:00
       renewals: 1
      timestamp: 2015-06-08 12:42:27
      issuedate: 2015-04-13 09:23:20

To replicate:
1) check out an item
2) let the item go overdue and start accruing fines
3) renew the item
4) let the item go overdue again. Note that a second accountlines entry has been added
5) return the item. Note that only one accountlines entry is marked with accounttype 'F'.
Comment 1 Katrin Fischer 2015-06-14 10:39:02 UTC
Hm, I thimk the problem is not that there are 2 entries in accountlines - that seems correct to me, but both should be set to the same type on return?
Comment 2 Barton Chittenden 2015-06-15 12:58:11 UTC
(In reply to Katrin Fischer from comment #1)
> Hm, I thimk the problem is not that there are 2 entries in accountlines -
> that seems correct to me, but both should be set to the same type on return?

I feel that there should be no more than one entry in accountlines for a given row in the issues table... but the underlying problem is that using description as a lookup in the accountlines table is fundamentally broken.

Bug 13790 adds a unique id to issues/old_issues -- adding a corresponding foreign key to accountlines table would fix the bug going forward.
Comment 3 Katrin Fischer 2015-06-15 13:14:04 UTC
I think I disagree about the 'one line'. Once you renew the overdue process restarts and is not in any way related to the first one, I'd avoid mixing the data. It would make it really hard to see why the amount is as high as it is. With the 2 different due dates you get a clear clue.

I see a renewal more like a new checkout - only that Koha doesn't treat a renewal as a new checkout as some other systems do (which is fine!).

But I totally agree about the description being misused.

I think we need to think about this a bit more and should note it as a use case/test case we need to test and make sure works properly.
Comment 4 Barton Chittenden 2015-06-15 18:03:22 UTC
(In reply to Katrin Fischer from comment #3)
> I think I disagree about the 'one line'. Once you renew the overdue process
> restarts and is not in any way related to the first one, I'd avoid mixing
> the data. It would make it really hard to see why the amount is as high as
> it is. With the 2 different due dates you get a clear clue.
> 
> I see a renewal more like a new checkout - only that Koha doesn't treat a
> renewal as a new checkout as some other systems do (which is fine!).
> 
> But I totally agree about the description being misused.
> 
> I think we need to think about this a bit more and should note it as a use
> case/test case we need to test and make sure works properly.

Katrin,

After discussion with Kyle Hall, I agree with you assessment: Closing the first fine and creating a new fine on renewal is standard behavior for Koha. This still leaves the issue of the second Fine not being closed. I'm going to mark the ticket as "RESOLVED/INVALID", and open a new ticket which specifically addresses the un-closed fine. I'll carry your comments regarding a test case forward to the new ticket.