Fine generation does not factor adjustments into max fines calculation

[Dan Wells]

As part of the negative balance avoidance features for Evergreen 2.9, "adjustments" were added to the existing bill/payment setup. This was done to allow for the possibility of removing a portion of a billing, usually because the bill had been partially paid, but occasionally for other use cases, such as crediting overdues when marking an item as lost.

In cases where overdues have been adjusted down for any reason, however, the system no longer correctly calculates a max fine. The adjusted bill still counts toward the total, so the fine is capped prematurely.

Fine generation needs to become aware of adjustments and handle them appropriately.

Branch to follow...

[Dan Wells (dbw2)]

Branch with test:

working/user/dbwells/lp1686194_missing_adjustments_max_fines

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbwells/lp1686194_missing_adjustments_max_fines

[Jeff Davis (jdavis-sitka)]

Confirmed on 2.12. The fix worked for me in testing, signed off here:

user/jeffdavis/lp1686194_missing_adjustments_max_fines_signoff

[Blake GH (bmagic)]

I would like to supplement this bug report with this:

The logic doesn't seem to take into account that the total fines could be less* than the max_fines value.

extreme scenario:

Item is overdue for 2 days (20 cents per day). The library has a Lost action trigger for 3 days. The item receives overdue fines for 2 days, and on the third day, it's marked lost. The library wants the overdue fines to be removed and the lost fine and lost processing fee to be applied. On day 6, the item is returned which, as per the library settings, should void the lost fee and processing fee. Then reinstate the overdues plus any overdues that would have generated had it never been marked lost. The logic seems to ignore the date on which it was returned, and generate the overdue fines equal max_fines.

[Dan Wells (dbw2)]

Blake, are you actually seeing this behavior, or just reading through the code? By far the best thing you could do here is write a new case for the test suite which demonstrates the bug. Do that, and you will be my hero, and I will fix your bug.

In any case, unless you think the patch causes this new issue, I think it sounds like probably a different bug altogether. The fine generator as designed always generates fines through "now", then potentially voids/adjusts for backdate. The number of fines necessary to reach "now" is calculated at line 554, and I am not sure how max_fines acting as an early abort might actually extend the number of fines generated.

[Blake GH (bmagic)]

Dan, we are wrestling with this on production. This is behavior we are seeing. The patch on this bug sounds like it's working. Perhaps this needs it's own bug. I added it to this bug because this bug was reported due to our findings in a situation. An ongoing situation that has revealed this potential issue as well.

Below is a query that helps me find these (marked lost by action trigger in the last 30 days) You might want to tweak the date range depending on the size of your database.

select au.usrname,
acirc.id,acirc.xact_start,acirc.target_copy,acirc.due_date,acirc.stop_fines_time,acirc.checkin_time,acirc.duration,acirc.fine_interval,acirc.recurring_fine,acirc.max_fine,acirc.stop_fines,
mmbxs.total_paid,mmbxs.last_payment_ts,mmbxs.last_payment_type,mmbxs.total_owed,mmbxs.balance_owed,
ac.barcode,ac.price,ac.circ_modifier,ac.status_changed_time,
mb.sum,
maa.sum,
mp.sum,
mp.sum - maa.sum,
acirc.checkin_time - acirc.due_date,
(extract(day from (acirc.checkin_time - acirc.due_date))),
(extract(day from (acirc.checkin_time - acirc.due_date))) * recurring_fine,
(case when (((extract(day from (acirc.checkin_time - acirc.due_date))) * recurring_fine) > acirc.max_fine) then max_fine else ((extract(day from (acirc.checkin_time - acirc.due_date))) * recurring_fine) end),
(case when (((extract(day from (acirc.checkin_time - acirc.due_date))) * recurring_fine) > acirc.max_fine) then max_fine else ((extract(day from (acirc.checkin_time - acirc.due_date))) * recurring_fine) end) - mmbxs.balance_owed

from
action.circulation acirc,
money.materialized_billable_xact_summary mmbxs,
asset.copy ac,
actor.usr au,
(select sum(amount) sum,xact from money.billing where not voided group by 2) mb,
(select sum(amount) sum,xact from money.account_adjustment where not voided group by 2) maa,
(select sum(amount) sum,xact from money.payment where not voided group by 2) mp
where
mp.xact=acirc.id and
mb.xact=acirc.id and
maa.xact=acirc.id and
au.id=acirc.usr and
acirc.xact_finish is null and
acirc.id in(
select target from action_trigger.event where
event_def in(select id from action_trigger.event_definition where reactor='MarkItemLost')
and run_time > now() - '30 days'::interval
)
and
acirc.id=mmbxs.id and
ac.id=acirc.target_copy and
ac.status!=3 and
mmbxs.balance_owed > 0 and
acirc.stop_fines = 'MAXFINES'

[Galen Charlton (gmc)]

Pushed to master, rel_2_12, and rel_2_11. Thanks, Dan and Jeff!

Blake: please go ahead and open a separate bug.