Bug 22982 - Paying lost fee does not always remove lost item from checkouts
Summary: Paying lost fee does not always remove lost item from checkouts
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
: 21042 (view as bug list)
Depends on:
Blocks: 22563
  Show dependency treegraph
 
Reported: 2019-05-23 18:12 UTC by Kyle M Hall
Modified: 2019-06-11 13:45 UTC (History)
10 users (show)

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


Attachments
Bug 22982: Paying lost fee does not always remove lost item from checkouts (1.85 KB, patch)
2019-05-23 18:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22982: Add unit tests (5.20 KB, patch)
2019-05-23 18:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22982: Paying lost fee does not always remove lost item from checkouts (1.88 KB, patch)
2019-05-23 18:42 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22982: Add unit tests (5.23 KB, patch)
2019-05-23 18:42 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test (4.89 KB, patch)
2019-05-28 12:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22982: Paying lost fee does not always remove lost item from checkouts (1.93 KB, patch)
2019-05-31 18:30 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 22982: Add unit tests (5.28 KB, patch)
2019-05-31 18:30 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test (4.93 KB, patch)
2019-05-31 18:30 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 22982: Paying lost fee does not always remove lost item from checkouts (2.00 KB, patch)
2019-06-05 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22982: Add unit tests (5.34 KB, patch)
2019-06-05 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test (4.99 KB, patch)
2019-06-05 12:38 UTC, Martin Renvoize
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-23 18:12:43 UTC
Depending on how a lost fee is paid, it may or may not be removed from the patrons current checkouts. The current expected behavior is for the lost item to be removed from the patrons checkouts when the lost fee is paid in full.

This is due to the subroutine for handling of lost fees being included in the 'pay specific lines' code but not in the 'pay by amount' code.
Comment 1 Kyle M Hall 2019-05-23 18:16:00 UTC
Created attachment 90046 [details] [review]
Bug 22982: Paying lost fee does not always remove lost item from checkouts

Depending on how a lost fee is paid, it may or may not be removed from the patrons current checkouts. The current expected behavior is for the lost item to be removed from the patrons checkouts when the lost fee is paid in full.

This is due to the subroutine for handling of lost fees being included in the 'pay specific lines' code but not in the 'pay by amount' code.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account.t
Comment 2 Kyle M Hall 2019-05-23 18:16:20 UTC
Created attachment 90047 [details] [review]
Bug 22982: Add unit tests
Comment 3 Liz Rea 2019-05-23 18:42:17 UTC
Created attachment 90050 [details] [review]
Bug 22982: Paying lost fee does not always remove lost item from checkouts

Depending on how a lost fee is paid, it may or may not be removed from the patrons current checkouts. The current expected behavior is for the lost item to be removed from the patrons checkouts when the lost fee is paid in full.

This is due to the subroutine for handling of lost fees being included in the 'pay specific lines' code but not in the 'pay by amount' code.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account.t

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 4 Liz Rea 2019-05-23 18:42:27 UTC
Created attachment 90051 [details] [review]
Bug 22982: Add unit tests

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 5 Martin Renvoize 2019-05-24 14:15:14 UTC
This doesn't feel like it'll catch all cases.. especially as some things have already started to move to using the ->apply routines.

I should probably help to fold this into the rest of the accounts tree that's been growing again of late.
Comment 6 Andrew 2019-05-25 17:17:56 UTC
Is this a dupe of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21042?
Comment 7 Kyle M Hall 2019-05-28 11:36:12 UTC
*** Bug 21042 has been marked as a duplicate of this bug. ***
Comment 8 Kyle M Hall 2019-05-28 12:02:50 UTC
Created attachment 90152 [details] [review]
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test
Comment 9 Marcel de Rooy 2019-05-31 08:08:26 UTC
(In reply to Martin Renvoize from comment #5)
> This doesn't feel like it'll catch all cases.. especially as some things
> have already started to move to using the ->apply routines.
> 
> I should probably help to fold this into the rest of the accounts tree
> that's been growing again of late.

Martin: Please have another look too.
I am having some doubts too on cases where $fine->amountoutstanding > $balance_remaining in the first edit. $new_amountoutstanding may not be zero.
Comment 10 Kyle M Hall 2019-05-31 11:27:15 UTC
> I am having some doubts too on cases where $fine->amountoutstanding >
> $balance_remaining in the first edit. $new_amountoutstanding may not be zero.

In cases where $new_amountoutstanding is not zero, we should *not* remove the item from the patron's checkouts. We should only do so once the entire fee is paid off. If you feel otherwise please give specific details that I can act on.
Comment 11 Nadine Pierre 2019-05-31 18:30:31 UTC
Created attachment 90243 [details] [review]
Bug 22982: Paying lost fee does not always remove lost item from checkouts

Depending on how a lost fee is paid, it may or may not be removed from the patrons current checkouts. The current expected behavior is for the lost item to be removed from the patrons checkouts when the lost fee is paid in full.

This is due to the subroutine for handling of lost fees being included in the 'pay specific lines' code but not in the 'pay by amount' code.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account.t

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Comment 12 Nadine Pierre 2019-05-31 18:30:38 UTC
Created attachment 90244 [details] [review]
Bug 22982: Add unit tests

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Comment 13 Nadine Pierre 2019-05-31 18:30:43 UTC
Created attachment 90245 [details] [review]
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test

Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Comment 14 Martin Renvoize 2019-06-05 12:38:49 UTC
Created attachment 90327 [details] [review]
Bug 22982: Paying lost fee does not always remove lost item from checkouts

Depending on how a lost fee is paid, it may or may not be removed from the
patrons current checkouts. The current expected behavior is for the lost
item to be removed from the patrons checkouts when the lost fee is paid in
full.

This is due to the subroutine for handling of lost fees being included in
the 'pay specific lines' code but not in the 'pay by amount' code.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account.t

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2019-06-05 12:38:52 UTC
Created attachment 90328 [details] [review]
Bug 22982: Add unit tests

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2019-06-05 12:38:55 UTC
Created attachment 90329 [details] [review]
Bug 22982: (QA follow-up) Add behavior to Koha::Account::Line::apply and unit test

Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2019-06-05 12:39:43 UTC
I'm pretty happy with this one now, it all works for me, passes QA scripts and I can't see any regressions.

Thanks Kyle, Passing QA
Comment 18 Martin Renvoize 2019-06-05 12:41:38 UTC
Note to self.. this will conflict quietly with 22563 when it is pushed.
Comment 19 Martin Renvoize 2019-06-05 12:44:20 UTC
Nice work!

Pushed to master for 19.11.00
Comment 20 Fridolin SOMERS 2019-06-11 13:45:39 UTC
Pushed to 19.05.x for 19.05.01