Bug 21196 - C4::Overdues::CalcFine should consider default item type replacement cost
Summary: C4::Overdues::CalcFine should consider default item type replacement cost
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on: 12768
Blocks:
  Show dependency treegraph
 
Reported: 2018-08-09 17:43 UTC by Andrew
Modified: 2018-11-12 09:20 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21196: Add tests (2.58 KB, patch)
2018-08-13 15:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21196: Use replacement cost from the item type if not set at item level (2.58 KB, patch)
2018-08-13 15:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21196: Add tests (2.62 KB, patch)
2018-09-24 19:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21196: Use replacement cost from the item type if not set at item level (2.63 KB, patch)
2018-09-24 19:43 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew 2018-08-09 17:43:13 UTC
When circ rules are set to cap at item replacement cost, calcfine only checks the price in the item record, not the default replacement cost by item type.

To test:
- Set useDefaultReplacementCost to use
- Set a default replacement cost for a given item type
- Create an item of that type with a replacementcost=0
- Create a circ rule for that item type that accrues fines but caps at replacement cost
- check the item out, forcing a due date in the past
- check that item in
- no fines are generated
Comment 1 Jonathan Druart 2018-08-13 15:43:00 UTC
Created attachment 77758 [details] [review]
Bug 21196: Add tests
Comment 2 Jonathan Druart 2018-08-13 15:43:04 UTC
Created attachment 77759 [details] [review]
Bug 21196: Use replacement cost from the item type if not set at item level

When circ rules are set to cap at item replacement cost, calcfine only
checks the price in the item record, not the default replacement cost by
item type.

Note: We do not take '0' into account, what if you do not want to set
a replacement cost for an item? It is how it is done in chargelostitem
so keeping the existing behaviour, but that may be considered as a bug (?)

Test plan:
- Set useDefaultReplacementCost to use
- Set a default replacement cost for a given item type
- Create an item of that type with a replacementcost=0
- Create a circ rule for that item type that accrues fines but caps at
replacement cost
- check the item out, forcing a due date in the past
- check that item in
=> Fines should have been generated
Comment 3 Caroline Cyr La Rose 2018-09-16 17:30:24 UTC
I can reproduce the original problem. I get fines, but way over the replacement cost, confirming the default item type replacement cost is not taken into account.

Once I apply the patch, the fines are capped at the default replacement cost. So it works as intended.
Comment 4 Katrin Fischer 2018-09-24 19:43:37 UTC
Created attachment 79340 [details] [review]
Bug 21196: Add tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2018-09-24 19:43:41 UTC
Created attachment 79341 [details] [review]
Bug 21196: Use replacement cost from the item type if not set at item level

When circ rules are set to cap at item replacement cost, calcfine only
checks the price in the item record, not the default replacement cost by
item type.

Note: We do not take '0' into account, what if you do not want to set
a replacement cost for an item? It is how it is done in chargelostitem
so keeping the existing behaviour, but that may be considered as a bug (?)

Test plan:
- Set useDefaultReplacementCost to use
- Set a default replacement cost for a given item type
- Create an item of that type with a replacementcost=0
- Create a circ rule for that item type that accrues fines but caps at
replacement cost
- check the item out, forcing a due date in the past
- check that item in
=> Fines should have been generated

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Nick Clemens 2018-09-26 19:34:50 UTC
Awesome work all!

Pushed to master for 18.11
Comment 7 Martin Renvoize 2018-09-27 12:27:30 UTC
Pushed to 18.05.x for 18.05.05
Comment 8 Fridolin SOMERS 2018-10-03 13:11:09 UTC
Pushed to 17.11.x for 17.11.11