Add option to protect against most-reported negative-balance-causing situation

[Mike Rylander]

The situation that has been reported as the worst offender for creating negative balances for folks that do not want that to happen goes like this:

1) Item is lost or goes long-overdue
2) Fees/fines charged
3) Said fees/fines are paid in full
4) Item is found and returned
4a) "Void fines and fees on lost item return" settings are enabled
4b) "Reinstate overdue fines on lost item return" setting is enabled
4c) System dutifully voids the lost fines/fees
4d) System dutifully reinstates the overdue fines

So we add an YAOUS to ensure that when a lost item is returned and the balance of the transaction is exactly $0, the lost fines and fees are not voided EVEN IF "void fines and fees on lost/long-overdue return" settings are enabled, and voided overdue fees are likewise not reinstated EVEN IF "restore overdue on lost/long-overdue return" settings are enabled.

The reason for the second condition is that the point of this new setting is to have Evergreen consider zero-balance lost/long-overdue transactions as "handled", and that they should not automatically change.

Note: This is mostly orthogonal to the "Avoid negative balances" work going on in parallel as it addresses a specific situation, but it will be a stop-gap toward the same end if that should happen to slip past 2.8.

[Mike Rylander (mrylander)]

Here's the branch that implements this option:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-1413592-avoid-lost-void-on-zero-balance

[Mike Rylander (mrylander)]

I've force-pushed a change to that branch, in case anyone was already looking at it.

[Mike Rylander (mrylander)]

After some discussion with the instigator of this project (thanks, MassLNC!), I offer a replacement branch for this bug. In this new branch, we avoid both voiding of lost/long-overdue fines/fees, AND reinstatement of overdue fines, if they had been previously voided. The semantic change is to treat 0-balance lost and long-overdue transactions as "handled".

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-1413592-avoid-changing-lost-LO-on-zero-balance

[Dan Wells (dbw2)]

I think the idea and implementation here look fine. In regards to the Cond. Negative Balances branch, the original report described this as "a stop-gap toward the same end if that should happen to slip past 2.8." They will definitely overlap in function, and I understand hope might be fading (or gone), but I am posting to ask that any would-be reviewers not push this in for a few more weeks. Feel free to test and sign-off, of course, but this branch is simple and targeted enough that I don't think an extended review period will be necessary, so I hope others don't see any harm in waiting a bit.

On the flip side, I see definite merit in an approach (like this) which emphasizes avoiding negative balances rather than correcting them. It would be painful given the time already invested in the other branch, but is this a path which can eventually meet all the various needs? Has anyone thought it through that far?

[Kathy Lussier (klussier)]

Off the top of my head, we have at least two other situations (if not more) that would continue to generate negative balances if this code were added to Evergreen:

* When a partially-paid lost item is returned
* When a patron pays overdue fines for an item before it is checked in and then a "forgive fines" checkin is used.

I think what makes this branch so straightforward is that it's a simple yes or no as to whether a bill is voided or not.

The other two cases are more complex in that the system can't just void or not void an entire bill, but needs to somehow "adjust" the remaining balance of the bill.

[Mike Rylander (mrylander)]

Dan,

I think the two approaches are complementary, and I would prefer that both this and the existing negative balance branch get in.

I was, in the past, hesitant to go down the road of YAOUS' modifying YAOUS' in this area, because that way lies madness, but the semantics of the proposed change seem pretty well confined. What I would not want is a large list of modifiers that change the behavior of all the various settings individually (and might then be modified in other ways). Here, we're just saying, "if it's completely reconciled, just accept that and move on" without regard for any more complex state inspection.

Like Kathy says, your branch is much more sophisticated. Looking at the early commits on your branch, it looks like mine happens earlier than yours, and will just avoid running your logic. Since yours is meant to deal with situations where the transaction isn't considered "done", and mine is meant (in essence) to strengthen the definition of "done" to cover fully-paid lost/LO transactions, I think having both is the best way to go.

And, last but definitely not least, thanks for reviewing the code!

[Mike Rylander (mrylander)]

Oh, and I'm 100% cool with waiting to merge this. There may be some mild overlap with yours, and however you want to handle merging is fine.

[Kathy Lussier (klussier)]

Hi Mike,

Overall, this looks good, but I still find one situation where fines are being applied upon the return ofa lost and fully paid item. If the item is returned a few days after being marked lost and the library has the "Lost Checkin Generates New Overdues" setting enabled, the checkin will create new overdue fines that have accrued since the item was marked lost.

[Mike Rylander (mrylander)]

Kathy,

I've pushed a commit to that same branch that stops the "generate new overdues" behavior from being triggered when "don't change zero-balance lost transactions" is in effect.

[Kathy Lussier (klussier)]

Looks good. Signed off and added a release notes entry.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp-1413592-avoid-changing-lost-LO-on-zero-balance

[Mike Rylander (mrylander)]

Thanks, Kathy. Your release notes signed off and the lot pushed to master.