Bug 21196

Summary: C4::Overdues::CalcFine should consider default item type replacement cost
Product: Koha Reporter: Andrew Fuerste-Henry <andrew>
Component: Fines and feesAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: normal    
Priority: P5 - low CC: caroline.cyr-la-rose, fridolin.somers, jonathan.druart, katrin.fischer, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
Bug Depends on: 12768    
Bug Blocks:    
Attachments: Bug 21196: Add tests
Bug 21196: Use replacement cost from the item type if not set at item level
Bug 21196: Add tests
Bug 21196: Use replacement cost from the item type if not set at item level

Description Andrew Fuerste-Henry 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