Bug 23018 - Refunding a lost item fee may trigger error if any fee has been written off related to that item
Summary: Refunding a lost item fee may trigger error if any fee has been written off r...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-30 18:15 UTC by Kyle M Hall
Modified: 2019-08-06 15:57 UTC (History)
6 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:
19.11.00,19.05.03


Attachments
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item (1.87 KB, patch)
2019-05-30 18:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item (1.89 KB, patch)
2019-05-30 19:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item (1.94 KB, patch)
2019-06-17 07:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item (2.04 KB, patch)
2019-07-05 09:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2019-05-30 18:15:38 UTC
If a lost item fee is written off, it creates a writeoff with the same itemnumber but the amount column is a negative number.

Now, when the lost item is returned, it calls _FixAccountForLostAndReturned which searchs for all accountlines with an account type of 'L' or 'W'.

Then, if the *first* of those lines has an outstanding balance, a credit is created and *all* the lines found are passed to $credit->apply. The apply method then calls is_debit on each line and when it hits the writeoff, it throws an exception ( Koha::Exceptions::Account::IsNotDebit ).

When this happens, the credit is creating but not applied the the outstanding lost item fee, and the librarian is presented with an error page.

It seems like the solution is to only fetch the first result since it seems like that is the only one we should be operating on.
Comment 1 Kyle M Hall 2019-05-30 18:18:11 UTC
Created attachment 90205 [details] [review]
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item

If a lost item fee is written off, it creates a writeoff with the same itemnumber but the amount column is a negative number.

Now, when the lost item is returned, it calls _FixAccountForLostAndReturned which searchs for all accountlines with an account type of 'L' or 'W'.

Then, if the *first* of those lines has an outstanding balance, a credit is created and *all* the lines found are passed to $credit->apply. The apply method then calls is_debit on each line and when it hits the writeoff, it throws an exception ( Koha::Exceptions::Account::IsNotDebit ).

When this happens, the credit is creating but not applied the the outstanding lost item fee, and the librarian is presented with an error page.

It seems like the solution is to only fetch the first result since it seems like that is the only one we should be operating on.

Test Plan:
1) Create a lost item with a fee
2) Create a manual debit using the barcode of the item
3) Write off that manual debit
4) Check in the item, note the error
5) Apply this patch
6) Repeat steps 1-3
7) Check in the item, no error!
Comment 2 Kyle M Hall 2019-05-30 19:09:13 UTC
Created attachment 90206 [details] [review]
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item

If a lost item fee is written off, it creates a writeoff with the same itemnumber but the amount column is a negative number.

Now, when the lost item is returned, it calls _FixAccountForLostAndReturned which searchs for all accountlines with an account type of 'L' or 'W'.

Then, if the *first* of those lines has an outstanding balance, a credit is created and *all* the lines found are passed to $credit->apply. The apply method then calls is_debit on each line and when it hits the writeoff, it throws an exception ( Koha::Exceptions::Account::IsNotDebit ).

When this happens, the credit is creating but not applied the the outstanding lost item fee, and the librarian is presented with an error page.

It seems like the solution is to only fetch the first result since it seems like that is the only one we should be operating on.

Test Plan:
1) Create a lost item with a fee
2) Create a manual debit using the barcode of the item
3) Write off that manual debit
4) Check in the item, note the error
5) Apply this patch
6) Repeat steps 1-3
7) Check in the item, no error!
Comment 3 Kyle M Hall 2019-05-30 19:10:30 UTC
This bug will be fixed by bug 22563, however that bug will probably not be backported to previous releases of Koha. This bug patch would be a good candidate for Koha 19.05 and 18.11
Comment 4 Martin Renvoize 2019-06-17 07:58:53 UTC
Created attachment 90640 [details] [review]
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item

If a lost item fee is written off, it creates a writeoff with the same itemnumber but the amount column is a negative number.

Now, when the lost item is returned, it calls _FixAccountForLostAndReturned which searchs for all accountlines with an account type of 'L' or 'W'.

Then, if the *first* of those lines has an outstanding balance, a credit is created and *all* the lines found are passed to $credit->apply. The apply method then calls is_debit on each line and when it hits the writeoff, it throws an exception ( Koha::Exceptions::Account::IsNotDebit ).

When this happens, the credit is creating but not applied the the outstanding lost item fee, and the librarian is presented with an error page.

It seems like the solution is to only fetch the first result since it seems like that is the only one we should be operating on.

Test Plan:
1) Create a lost item with a fee
2) Create a manual debit using the barcode of the item
3) Write off that manual debit
4) Check in the item, note the error
5) Apply this patch
6) Repeat steps 1-3
7) Check in the item, no error!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2019-06-17 07:59:47 UTC
Signing off.. it's basically the same change I made in bug 22563 only more backport friendly.

Works as expected.
Comment 6 Marcel de Rooy 2019-07-05 09:29:44 UTC
Created attachment 91330 [details] [review]
Bug 23018: Refunding a lost item fee may trigger error if any fee has been written off related to that item

If a lost item fee is written off, it creates a writeoff with the same itemnumber but the amount column is a negative number.

Now, when the lost item is returned, it calls _FixAccountForLostAndReturned which searchs for all accountlines with an account type of 'L' or 'W'.

Then, if the *first* of those lines has an outstanding balance, a credit is created and *all* the lines found are passed to $credit->apply. The apply method then calls is_debit on each line and when it hits the writeoff, it throws an exception ( Koha::Exceptions::Account::IsNotDebit ).

When this happens, the credit is creating but not applied the the outstanding lost item fee, and the librarian is presented with an error page.

It seems like the solution is to only fetch the first result since it seems like that is the only one we should be operating on.

Test Plan:
1) Create a lost item with a fee
2) Create a manual debit using the barcode of the item
3) Write off that manual debit
4) Check in the item, note the error
5) Apply this patch
6) Repeat steps 1-3
7) Check in the item, no error!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Martin Renvoize 2019-07-15 11:21:57 UTC
Nice work!

Pushed to master for 19.11.00
Comment 8 Fridolin SOMERS 2019-07-30 05:35:10 UTC
Pushed to 19.05.x for 19.05.03
Comment 9 Lucas Gass 2019-08-06 15:57:38 UTC
tried many times to recreate this bug in 18.11.x and was unable to. not backporting to 18.11.x