Bug 22750 - Partial writeoff of lost fee causes item to be returned
Summary: Partial writeoff of lost fee causes item to be returned
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: 19.11
Hardware: All All
: P5 - low critical (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-22 17:11 UTC by Andrew Fuerste-Henry
Modified: 2021-06-14 21:31 UTC (History)
5 users (show)

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


Attachments
Bug 22750: (24592: Database Update) (3.14 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24592: Reword LOST_RETURN to LOST_FOUND) (15.85 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: Add tests) (2.76 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: Add `onpayment` option to `MarkLostItemsReturned`) (3.25 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: Filter out LOST_FOUND) (1.67 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: (follow-up) Add onpayment to sysprefs file) (1.35 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: (follow-up) Add LOST_FOUND logic to Koha::Account::pay) (4.13 KB, patch)
2020-10-06 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24474: (QA follow-up) Fix failing test) (1.42 KB, patch)
2020-10-06 09:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22750: (24592: Database Update) (3.21 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24592: Reword LOST_RETURN to LOST_FOUND) (15.94 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: Add tests) (2.82 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: Add `onpayment` option to `MarkLostItemsReturned`) (3.32 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: Filter out LOST_FOUND) (1.72 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: (follow-up) Add onpayment to sysprefs file) (1.42 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: (follow-up) Add LOST_FOUND logic to Koha::Account::pay) (4.19 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22750: (24474: (QA follow-up) Fix failing test) (1.47 KB, patch)
2020-10-15 13:51 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-04-22 17:11:00 UTC
To test:
- Set system preference MarkLostItemAsReturned to not mark items as returned
- Set system preference WhenLostChargeReplacementFee to charge
- Find an item with a non-zero replacement cost
- Check item out to patron
- Mark item as lost
- Confirm item is still in patron checkouts and patron has a lost fee for item
- Write off part of the lost fee
- Confirm the item no longer shows in patron checkouts
Comment 1 Andrew Fuerste-Henry 2019-04-22 17:11:32 UTC
Confirmed in 18.05 and 18.11
Comment 2 Andrew Fuerste-Henry 2019-04-22 17:15:51 UTC
In case it's relevant, I performed the partial writeoff using the "Write Off" button for the specific fee.
Comment 3 Kelly McElligott 2020-07-28 14:47:11 UTC
This is still the case in 19.11.  However I attempted to use Write Off selected and it still was removed from the patron's checkouts.
Comment 4 Jonathan Druart 2020-08-10 15:27:54 UTC
I think this has been fixed in 20.05 by bug 24474.
Comment 5 Martin Renvoize 2020-09-08 09:18:30 UTC
I'm going to take a look at providing a backport version for 24474 for the 19.11 branch. I've investigate if that will allow backports further at the same time, but I have a feeling too much of the foundations may have changed to make that possible.

Bear with me.
Comment 6 Martin Renvoize 2020-10-06 09:53:33 UTC
Created attachment 111271 [details] [review]
Bug 22750: (24592: Database Update)

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 7 Martin Renvoize 2020-10-06 09:53:38 UTC
Created attachment 111272 [details] [review]
Bug 22750: (24592: Reword LOST_RETURN to LOST_FOUND)

This patch updates the wording in the 'lost and found' process to more
closely reflect what the process is achieving by replacing 'RETURNED'
with 'FOUND'

Test plan:
1) Grep codebase for _FixAccountForLostAndReturned and note there are no
   longer any instanced of it.
2) Run t/db_dependent/Circulation.t and note it passes
3) Test returning/renewing an item that has been marked as lost and note
   the updated values in the accountlines now use LOST_FOUND as
   credit_type_code and 'FOUND' as the status for the 'LOST' fee
   (debit_type_code 'LOST')

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 8 Martin Renvoize 2020-10-06 09:53:42 UTC
Created attachment 111273 [details] [review]
Bug 22750: (24474: Add tests)

This does the following using the interface:
- Unselect all the options from MarkLostItemsAsReturned except for
  'onpayment'
- Check an item out
- Create a manual invoice LOST for the checked in item
- Renew the item
- Check that the item was not returned

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Martin Renvoize 2020-10-06 09:53:45 UTC
Created attachment 111274 [details] [review]
Bug 22750: (24474: Add `onpayment` option to `MarkLostItemsReturned`)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Martin Renvoize 2020-10-06 09:53:49 UTC
Created attachment 111275 [details] [review]
Bug 22750: (24474: Filter out LOST_FOUND)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Martin Renvoize 2020-10-06 09:53:52 UTC
Created attachment 111276 [details] [review]
Bug 22750: (24474: (follow-up) Add onpayment to sysprefs file)

This patch adds the required line to the circulation.pref file to
dispaly the onpayment option in the MarkLostItemReturned preference

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Martin Renvoize 2020-10-06 09:53:58 UTC
Created attachment 111277 [details] [review]
Bug 22750: (24474: (follow-up) Add LOST_FOUND logic to Koha::Account::pay)

This patch adds the same updated logic as found in
Koha::Account::Line->apply to handle LOST_FOUND credits being applied
against their associated LOST debit.

Currently a 'LOST_FOUND' credit is never created via the pay method and
so the additional test will always pass, but the addition here accounts
for possible future uses.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Martin Renvoize 2020-10-06 09:54:04 UTC
Created attachment 111278 [details] [review]
Bug 22750: (24474: (QA follow-up) Fix failing test)

The test was assuming default data and would fail if you ran through the
test plan (and disabled all but `onpayment` for the
'MarkLostItemsAsReturned' preference) on the data before running the
included unit tests.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Martin Renvoize 2020-10-06 09:55:16 UTC
Backport rebase of the patches in question.. I think these deserve a QA run for the 19.11 branch, though the backport was fairly trivial in the end.  Setting to SO to provoke such.
Comment 15 Nick Clemens 2020-10-15 13:51:00 UTC
Created attachment 111738 [details] [review]
Bug 22750: (24592: Database Update)

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Nick Clemens 2020-10-15 13:51:10 UTC
Created attachment 111739 [details] [review]
Bug 22750: (24592: Reword LOST_RETURN to LOST_FOUND)

This patch updates the wording in the 'lost and found' process to more
closely reflect what the process is achieving by replacing 'RETURNED'
with 'FOUND'

Test plan:
1) Grep codebase for _FixAccountForLostAndReturned and note there are no
   longer any instanced of it.
2) Run t/db_dependent/Circulation.t and note it passes
3) Test returning/renewing an item that has been marked as lost and note
   the updated values in the accountlines now use LOST_FOUND as
   credit_type_code and 'FOUND' as the status for the 'LOST' fee
   (debit_type_code 'LOST')

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2020-10-15 13:51:15 UTC
Created attachment 111740 [details] [review]
Bug 22750: (24474: Add tests)

This does the following using the interface:
- Unselect all the options from MarkLostItemsAsReturned except for
  'onpayment'
- Check an item out
- Create a manual invoice LOST for the checked in item
- Renew the item
- Check that the item was not returned

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2020-10-15 13:51:19 UTC
Created attachment 111741 [details] [review]
Bug 22750: (24474: Add `onpayment` option to `MarkLostItemsReturned`)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens 2020-10-15 13:51:24 UTC
Created attachment 111742 [details] [review]
Bug 22750: (24474: Filter out LOST_FOUND)

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens 2020-10-15 13:51:29 UTC
Created attachment 111743 [details] [review]
Bug 22750: (24474: (follow-up) Add onpayment to sysprefs file)

This patch adds the required line to the circulation.pref file to
dispaly the onpayment option in the MarkLostItemReturned preference

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens 2020-10-15 13:51:32 UTC
Created attachment 111744 [details] [review]
Bug 22750: (24474: (follow-up) Add LOST_FOUND logic to Koha::Account::pay)

This patch adds the same updated logic as found in
Koha::Account::Line->apply to handle LOST_FOUND credits being applied
against their associated LOST debit.

Currently a 'LOST_FOUND' credit is never created via the pay method and
so the additional test will always pass, but the addition here accounts
for possible future uses.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2020-10-15 13:51:36 UTC
Created attachment 111745 [details] [review]
Bug 22750: (24474: (QA follow-up) Fix failing test)

The test was assuming default data and would fail if you ran through the
test plan (and disabled all but `onpayment` for the
'MarkLostItemsAsReturned' preference) on the data before running the
included unit tests.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Jonathan Druart 2020-10-22 07:46:06 UTC
Sending to 19.11 RMaints queue.