Bug 9122

Summary: Writing off fees broken
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: PatronsAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: blocker    
Priority: P5 - low CC: chris, gmcharlt, jcamins, jonathan.druart, kyle.m.hall, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 9122: fix the problem when writing-off a fine
Bug 9122: fix the problem when writing-off a fine

Description Katrin Fischer 2012-11-20 22:21:48 UTC
Ok, I hope it's just me being tired:

Test plan 1:
- check out a few items with rental charges on them
- try to write off the whole fine, confirm that you really want to
- the fine stays on the patron account, you can repeat the write off
- the summary page notes the write off, but it's not taken into account for the total due

Test plan 2:
- add some fines manually, I did 'new card' and sundry
- try writing off all of them or one of them
- same: it seems to work, the summary shows it, but the fee is still there
Comment 1 Chris Cormack 2012-11-21 07:46:14 UTC
I confirm, writeoff works in 3.8.x and lower, broken in 3.10.x and master.

Working on a bisect to find the commit that broke it.
Comment 2 Chris Cormack 2012-11-21 08:01:56 UTC
[a8b168dc63df9d60e8e562f368f8bdbec520ffd8] Bug 7671 : add a real primary key accountlines_id in accountlines

This is what broke write offs.

This either needs to be reverted or fixed properly
Comment 3 Katrin Fischer 2012-11-21 14:05:04 UTC
Is someone working on a fix for this?
Comment 4 Paul Poulain 2012-11-21 15:20:08 UTC
Katrin, I can't reproduce your test plan 2 (note tested the 1 yet)
Test plan 2:
- add some fines manually, I did 'new card' and sundry
- try writing off all of them or one of them
- same: it seems to work, the summary shows it, but the fee is still there

I'll attach a screencast of what I did
Comment 5 Katrin Fischer 2012-11-21 15:22:46 UTC
Hi Paul,
before you do a screencast - test 1 is like test 2. The only difference is, that I wanted to make sure the problem was not only for rental charges and that it's not only writeoff all but also writing of individual fees.
Comment 6 Paul Poulain 2012-11-21 15:24:52 UTC
screencast too large
uploaded here = http://dl.free.fr/tPBjEJ0Au

password = Koha
Comment 7 Paul Poulain 2012-11-21 15:35:18 UTC
Katrin can't open my screencast, so explaining what it shows:

* reached http://localhost:5001/cgi-bin/koha/members/maninvoice.pl?borrowernumber=7852 page (yes, i'm running Plack)
* created 2 manual invoices, one of 111€ and one of 222€
* checked that http://localhost:5001/cgi-bin/koha/members/boraccount.pl?borrowernumber=7852 shows i'm owing 333€ in the Total due line
* reached http://localhost:5001/cgi-bin/koha/members/pay.pl?borrowernumber=7852 page
* clicked on "write off all button"
* a popup saying "Are you sure you want to write off 333.00 in outstanding fines? This cannot be undone!" appear, I say "OK"
* http://localhost:5001/cgi-bin/koha/members/boraccount.pl?borrowernumber=7852 now says "Total due = 0"

I tried also by clicking on individual "write off" line, and it worked too
Comment 8 Paul Poulain 2012-11-21 15:37:16 UTC
the boraccount.pl table show
21/11/2012 	N New Card  		111.00 	0.00 	Print
21/11/2012 	M Sundry  		222.00 	0.00 	Print
21/11/2012 	Writeoff  		222.00 	0.00 	Print
21/11/2012 	Writeoff  		111.00 	0.00 	Print 
Total due 	0.00
Comment 9 Paul Poulain 2012-11-21 15:53:24 UTC
katrin just told me on IRC that her tests were made on a fresh install, while mine where on an updated one.

I diff-ed show create table accountlines and could see no difference, mine is the same as a fresh one.

still a mystery... is there any SQL error thrown in your logs ?
Comment 10 Katrin Fischer 2012-11-21 16:28:56 UTC
My koha error logs were clean this morning, I didn't check for javascript errors.
Comment 11 Paul Poulain 2012-11-23 12:51:17 UTC
katrin, suggested I copy values in my accountlines table, in case it helps:
*************************** 17. row ***************************
  accountlines_id: 17
   borrowernumber: 7852
        accountno: 17
       itemnumber: NULL
             date: 2012-11-21
           amount: 111.000000
      description: N New Card
          dispute: NULL
      accounttype: N
amountoutstanding: 0.000000
    lastincrement: NULL
        timestamp: 2012-11-21 16:34:39
        notify_id: 1
     notify_level: 0
             note: 
       manager_id: 7852
*************************** 18. row ***************************
  accountlines_id: 18
   borrowernumber: 7852
        accountno: 18
       itemnumber: NULL
             date: 2012-11-21
           amount: 111.000000
      description: Writeoff
          dispute: NULL
      accounttype: W
amountoutstanding: NULL
    lastincrement: NULL
        timestamp: 2012-11-21 16:34:39
        notify_id: 0
     notify_level: 0
             note: NULL
       manager_id: 7852
*************************** 19. row ***************************
  accountlines_id: 19
   borrowernumber: 7852
        accountno: 19
       itemnumber: NULL
             date: 2012-11-21
           amount: 4.000000
      description: N New Card
          dispute: NULL
      accounttype: N
amountoutstanding: 0.000000
    lastincrement: NULL
        timestamp: 2012-11-21 16:42:23
        notify_id: 1
     notify_level: 0
             note: note
       manager_id: 7852
*************************** 20. row ***************************
  accountlines_id: 20
   borrowernumber: 7852
        accountno: 20
       itemnumber: NULL
             date: 2012-11-21
           amount: 4.000000
      description: Writeoff
          dispute: NULL
      accounttype: W
amountoutstanding: NULL
    lastincrement: NULL
        timestamp: 2012-11-21 16:42:23
        notify_id: 0
     notify_level: 0
             note: NULL
       manager_id: 7852
Comment 12 Paul Poulain 2012-11-23 13:04:36 UTC
Katrin++ !
She understood that the problem could be that I've only one patron with fines, and she was right. I reproduce the problem when having another patron with fines:
* patron A = 90€ fines
* patron B = 55€ fines
* writeoff 55€ from B => the total still appears as "B due 55€" !
Comment 13 Paul Poulain 2012-11-23 13:35:46 UTC Comment hidden (obsolete)
Comment 14 Jared Camins-Esakov 2012-11-23 14:07:30 UTC
Created attachment 13651 [details] [review]
Bug 9122: fix the problem when writing-off a fine

The problem was introduced by the addition of accountlines_id in bug 7671

The WriteOffFee sub was called with accountlines_id as second parameter
but still dealing with the accountno column.

As a result, the UPDATE was failing (updating nothing), and the fine
note written off even if the writeoff itself was saved. In other words:
the writeoff was not "attached" to the fine.

This patch fixes the problem.

Test plan: do what katrin says in initial description of the problem:
- add some fines manually, I did 'new card' and sundry
- try writing off all of them or one of them
- the fine is correctly written off

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 15 Jonathan Druart 2012-11-23 14:12:26 UTC
I did not test but the patch looks good.

Marked as Passed QA.
Comment 16 Jared Camins-Esakov 2012-11-23 14:18:36 UTC
This patch has been pushed to master.
Comment 17 Paul Poulain 2012-11-23 14:23:20 UTC
Patch pushed to branch 3.10.x
Comment 18 Chris Cormack 2013-04-24 07:22:54 UTC
Released in 3.10.0