Sorry for the lag in adding feedback for this bug, but we spent a lot of time testing the new code at MassLNC and I then asked Jason to spend some time looking at the issues we discovered in the code. There are a few issues we found with the code that I'll outline below. Sorry for the forthcoming wordiness, but I want to make sure I provide all of the relevant details.

1. Previously-voided fines cause issues. 

Relevant settings: Void overdue fines when item is marked lost - true
Restore overdues on lost item return - true
Prohibit negative balance (default) - true

In this case we were working with a patron record with transactions that had occurred before the code was loaded. The patron had accrued $1.75 in overdue fines, but those fines were voided under the old method when the item was marked lost. A $22.00 lost fee was added to the record. There is a $10 payment towards the lost book fee. After the negative balance branch has been added to the system, the patron returns the book. We get $13.75 adjusted for the return of the lost item. The interface shows that 0.00 in overdue fees were reinstated. We also had a $.05 overdue fee added at checkin (more on that below), and end up with a negative balance of $1.70.

If a similar series of events happens with a transaction that starts after the negative balance branch is added, then we don't produce a negative balance. It appears as if the original voiding of the overdue fines is causing this negative balance to appear. When we update to an Evergreen release that has this code, I expect we will have many, many previous transactions like this one that will need to be handled properly so that negative balances aren't produced.

2. As I mentioned above, a stray overdue fine was added at checkin. Following a similar series of steps with transactions that start after the code was loaded also produces that stray overdue fine at checkin. In this testing, "Lost Checkin Generates New Overdues" is NOT set for the OU, so there was no reason new fines should have been added. Jason noted that there are some places where Dan moved the location of the code to handle the lost and long overdue statuses. It is now in front of some of the other code that may generate fines at checkin. Perhaps the changing of status before getting to the next steps is causing it. The change in question is in commit 08591c75a788f55875e1c3b7a16abe2aa853f3b5 in Circulate.pm around line 3591.

3. If you partially pay a lost book fee and then select "Void All Billings" in the staff client when the Prohibit Negative Balances (Default) is set to "True," a negative balance will be produced. Jason's code would first check the settings and then call a real_void_bills function when a negative balance shouldn't be produced, but it looks like this isn't happening anymore. Although the interface provides a warning that the entire bill will be voided, I think people will expect that the "no negative balance" setting means no negative balances will be produced.

4. I'm not a fan of the way the prohibit negative balance settings interact with the interval setting. I think most users will think "prohibit" means prohibit in all cases and that the interval settings would be used when the negative balances weren't prohibited. I think it would be more intuitive if the interval settings worked when the prohibit settings were either Unset or set to "false." In either case, we should probably provide some guidance on what needs to be done in the description for the interval settings.

5. I'm not quite sure if the following display happened in the original code or if it is a result of changes in the new code. The bottom row in the below screenshot is what displays in the patron's bill summary in a situation where 1) overdue fines accrued on a record and 2) the fines were "adjusted" after the item was marked lost. To most staff, it will appear as if an actual payment of $1.15 was made on this item even though the payment was really an adjustment. For this display, it almost seems better if 1) a  "system adjustment" column were added to show the adjustments separate from the payments or 2) the adjustments weren't included in the totals.

http://www.screencast.com/t/zEFyNpDnL

6. I mentioned this above, but I'll reiterate it again. We prefer not to use the language "adjustment payment" as it implies a payment was moved. We prefer simply calling it "adjustment" or "account adjustment."

Overall, the billing experts in my group said they would accept the changes brought about by the hybrid branch if the above issues were resolved (issues 1-3 being the most critical problems) because it is very important to use to have the negative balance issues resolved. Having said that, I want to go on record as saying that we still prefer the original approach offered by Jason. I think the hybrid approach will be the cause of a bit of confusion in mixed consortia where some libraries allow refunds and will therefore continue to use negative balances and other libraries that prohibit them.

To illustrate my concerns for those who are following along and haven't had a chance to review the functionality, I have included some screenshots below that show where I think the confusion will arise.

Let's look at an example of a lost item that was fully paid and then subsequently returned. For libraries that allow negative balances, you will see the following information in the full details section of the bill.

http://www.screencast.com/t/IcJmsDw0749

If the exact same scenario plays out in a library that doesn't allow negative balances, you will see the following:

http://www.screencast.com/t/PE3rpBbd

The two screens look exactly the same except for the fact that the top screenshot shows the original transaction as voided and the bottom transaction shows that it is not voided. It's going to be very difficult for a staff person to figure out what is going on with these records, particularly in a situation where the transaction happened in another library.

Although Jason has looked at the code, I don't think he feels entirely comfortable working with the hybrid code since it's sometimes difficult to tell what another developer's intentions are. Dan, would you be able to look at some of these issues?

Thanks all!
Kathy