backdated checkin does not honor grace period

Bug #787542 reported by Jason Etheridge
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned

Bug Description

It reportedly did this in 1.4, though looking at the code it seems otherwise. So this is either a regression or a wishlist item.

Revision history for this message
Michael Peters (mrpeters) wrote :

I thought some "real data" might be helpful for this. Here is a case where the grace day was charged with backdating.

See the attached snippet from action.circulation and money.billable_xact_with_void_summary

Transaction checked in 6/29 at 8:41AM via "backdate" because it was left in the dropbox, which the library did not check until this morning. System forgave the fines for today, but fined the patron for the grace day (6/28).

-[ RECORD 1 ]-------+------------------------------
id | 17711858
usr | 1849479
xact_start | 2011-06-20 14:03:24-04
xact_finish | 2011-06-29 08:47:39.223718-04
unrecovered |
target_copy | 1059213
circ_lib | 25
circ_staff | 914329
checkin_staff | 914329
checkin_lib | 25
renewal_remaining | 1
due_date | 2011-06-27 23:59:00-04
stop_fines_time | 2011-06-28 23:59:00-04
checkin_time | 2011-06-28 23:59:00-04
duration | 7 days
fine_interval | 1 day
recurring_fine | 0.25
max_fine | 10.00
phone_renewal | f
desk_renewal | f
opac_renewal | f
duration_rule | 14_7_21_1
recurring_fine_rule | 25_cent_per_day
max_fine_rule | overdue_mid
stop_fines | CHECKIN
create_time | 2011-06-20 14:03:24-04
workstation | 11431
parent_circ |
checkin_workstation | 11430
checkin_scan_time | 2011-06-29 08:41:21.077073-04

evergreen=# SELECT * FROM money.billable_xact_with_void_summary WHERE id=17711858;
-[ RECORD 1 ]-----+------------------------------
id | 17711858
usr | 1849479
xact_start | 2011-06-20 14:03:24-04
xact_finish | 2011-06-29 08:47:39.223718-04
total_paid | 0.50
last_payment_ts | 2011-06-29 08:47:39.223718-04
last_payment_note | in drop box forgive late fee
last_payment_type | forgive_payment
total_owed | 0.50
last_billing_ts | 2011-06-29 23:59:00-04
last_billing_note | System Generated Overdue Fine
                  : System: VOIDED FOR BACKDATE
last_billing_type | Overdue materials
balance_owed | 0.00
xact_type | circulation

tags: added: backdate backdating checkin fine generator grace period
Revision history for this message
Mike Rylander (mrylander) wrote :

The behavioral change actually has nothing to do with grace period. It stems from a trade-off between voiding the correct amount of fines in the (presumably more common) case of backdating to after the end of a grace period and not voiding enough when backdating into a grace period. This trade-off was forced by the fact that the grace period is not stored within the system prior to 2.1.

Before http://svn.open-ils.org/trac/ILS/changeset/15694 we were voiding too much whenever backdating was used. When backdating into a grace period, this produced the intended effect, but when backdating to after the end of a grace period we voided one fine too many.

After that commit, where we align the time component of the requested backdate with that of the original due date, we reverse the situation and void just enough in the (presumed) common case of backdating to a time after the grace period -- but in the case where we would void into a grace period, where the user-expected behavior would be to void the grace period fines, we don't void the billing.

Unfortunately, because the grace period is not recorded as a parameter before 2.1 (as of http://git.evergreen-ils.org/?p=Evergreen.git;a=commit;h=a34306360faca0f505345ea82c114ddbfc13418f ), there is no way to know when we are backdating into a grace period and thus avoid the reported problem. This is essentially an unfixable problem in 2.0 and before.

I have not tested the pre-release 2.1 code base to see if the 1.6.1 and 2.0 behavior persists -- I expect it does -- but now it is possible to address.

Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Jason Stephenson (jstephenson) wrote :

It appears that backdated checkin may not be counting closed days, or closed days may also be eating up grace period days.

I've not confirmed this, yet, because none of our members have given us concrete examples, but I suspect something like the above is also going on in our case.

We've had a few tickets about checkin fine generation not honoring grace period, and it always seems to coincide with library closed days as well.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I should add that we are on code that has grace period in the database.

Revision history for this message
Thomas Berezansky (tsbere) wrote :

I covered this and auto-extending grace periods in the branch below. Won't help 2.0, and is probably too late for 2.1.

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

You need only add the remote once, regardless of how many branches you wish to look at.
To add this repo as working:
 Read-only (git protocol):
  git remote add working git://git.evergreen-ils.org/working/Evergreen
 Read-write (ssh protocol):
  git remote add working <email address hidden>:working/Evergreen

Once you have the remote added you can check out this branch:
git checkout -b grace_period_extend working/user/tsbere/grace_period_extend

tags: added: pullrequest
Dan Wells (dbw2)
Changed in evergreen:
assignee: nobody → Dan Wells (dbw2)
Revision history for this message
Dan Wells (dbw2) wrote :

I spent a good chunk of time testing this today and ran into just one issue. Whenever the last day of the grace period falls directly before a closed date, the grace period is not extending into the trailing closed dates. I created a quick diff to show the small change I believe to be necessary, with the idea being that we need to keep looking for closed dates until we have exceeded our grace (i.e. reached a fine-able date), not merely matched it. I am not 100% confident in this, but so far, it fixes my issue and hasn't caused any others.

Thanks,
Dan

Revision history for this message
Thomas Berezansky (tsbere) wrote :

I force-pushed the branch as a single commit for my sanity in looking at the changes as a whole when I went to work on it some more. This commit contains all the previous work, plus an OU setting:

circ.grace.extend.into_closed

When enabled if the grace period is followed by a closed date then the closed date (and any following it) will be included in the grace period. Otherwise the grace period can end on a day just before a closed date.

Due to my own thoughts on the matter (that grace periods shouldn't do that, basically), it defaults to off.

Revision history for this message
Mike Rylander (mrylander) wrote :

Pushed into master. Thanks, Thomas and Dan!

Changed in evergreen:
status: Confirmed → Fix Committed
Dan Wells (dbw2)
Changed in evergreen:
assignee: Dan Wells (dbw2) → nobody
milestone: none → 2.2.0
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.