backdating overdue checkins over spring time change retains fine

[Dmagick]

Evergreen 2.1

When a circulation is due immediately before the spring time change and checked in immediately afterwards, if the item overdue is then backdated in order to eliminate accrued fines, at least one day's worth of fines remain because of the change in timezone, which throws off the internal calculation by one hour. Where the fine should be voided, it is not.

[Thomas Berezansky (tsbere)]

Do you have a grace period involved here? Because this may be the real issue in that case:

https://bugs.launchpad.net/evergreen/+bug/956456

We were finding that one day grace periods were resulting in one day of fines (where a min of two should be there) when backdating.

[Robert J Jackson (rjackson-deactivatedaccount)]

Seeing similar issues for backdating in 2.2.2 in that site sets a previous day's backdate, then proceeds to check in multiple items. Some of the items end up with current date on the due date.

[Robert J Jackson (rjackson-deactivatedaccount)]

Sorry - that should read "some of the items end up with current date on the check in date"!

[James Fournie (jfournie)]

I can't confirm this issue but we've definitely noticed problems with fines around daylight savings changes.

Also, I've posted a small patch for a somewhat related issue, just with general circ and time zones in #1074195

[Robert J Jackson (rjackson-deactivatedaccount)]

More documentation from the backdating session that showed some entries being backdated while others were not. Here is an example action.circulation entry for one that worked followed by one that did not:

evergreen=# select * from action.circulation where id = 29515118;
-[ RECORD 1 ]-------+------------------------------
id | 29515118
usr | 1310603
xact_start | 2012-10-26 13:51:56-04
xact_finish | 2012-11-01 10:16:44.522933-04
unrecovered |
target_copy | 29424547
circ_lib | 115
circ_staff | 1305905
checkin_staff | 1373245
checkin_lib | 115
renewal_remaining | 1
due_date | 2012-11-02 23:59:00-04
stop_fines_time | 2012-10-31 23:59:00-04
checkin_time | 2012-10-31 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 | 2012-10-26 13:51:56-04
workstation | 10167
parent_circ |
checkin_workstation | 9700
checkin_scan_time | 2012-11-01 10:16:44.522933-04
grace_period | 1 day
copy_location | 16801

evergreen=# select * from action.circulation where id = 29274112;
-[ RECORD 1 ]-------+-----------------------------
id | 29274112
usr | 1310358
xact_start | 2012-10-16 17:29:33-04
xact_finish | 2012-11-01 10:13:22.41029-04
unrecovered |
target_copy | 28013870
circ_lib | 115
circ_staff | 1305905
checkin_staff | 1373245
checkin_lib | 115
renewal_remaining | 1
due_date | 2012-11-06 23:59:00-05
stop_fines_time | 2012-11-01 00:59:00-04
checkin_time | 2012-11-01 00:59:00-04
duration | 21 days
fine_interval | 1 day
recurring_fine | 0.25
max_fine | 10.00
phone_renewal | f
desk_renewal | f
opac_renewal | f
duration_rule | 21_21_21_1
recurring_fine_rule | 25_cent_per_day
max_fine_rule | overdue_mid
stop_fines | CHECKIN
create_time | 2012-10-16 17:29:33-04
workstation | 9483
parent_circ |
checkin_workstation | 9700
checkin_scan_time | 2012-11-01 10:13:22.41029-04
grace_period | 1 day
copy_location | 16830

[Thomas Berezansky (tsbere)]

Note that the due date for the second one is the 6th, which is *after* daylight savings time changes on the 4th. The checkin time was thus likely bumped to the -05 time zone, then moved back to -04 when stored.

As a quick show:

First one:
due_date | 2012-11-02 23:59:00-04

Second one:
due_date | 2012-11-06 23:59:00-05

I think the solution is to change the logic in the perl to stop manually building a date/time from the backdate and the due date, but instead use the DateTime methods to do this for us.

Thus, in the checkin_handle_backdate function do something like:

$new_date->set_hour($original_date->hour()); $new_date->set_minute($original_date->minute());

And then format from $new_date fully, including time zone.

Anyone have thoughts on that?

[Jeff Godin (jgodin)]

Confirmed in 2.2 under slightly different circumstances, but I believe it's the same issue:

Currently in US/Eastern the offset from UTC is -0400. Circulations with a due_date having a -0500 offset (roughly, anything due after the DST "fall back" date of Nov 4) are affected when backdated to an -0400 timestamp.

Example (no problem):

Given a circulation with due_time 2012-11-02 23:59:59-04 checked in today with a backdate value of today, we'll end up with a checkin_time of 2012-11-01 23:59:59-04 and a checkin_scan_time of 2012-11-01 16:22:57.559085-04

All is well here, and all timestamps have an -0400 offset.

Here's a problem example:

Circ with a due_time after Nov 4 DST "fall back" date: 2012-11-14 23:59:59-05 checked in today with a backdate value of today, we have a checkin_time of 2012-11-02 00:59:59-04 and a checkin_scan_time of 2012-11-01 16:26:20.8181-04

The circulation shows a checkin_time with a date one day after the backdate date supplied. Since these are being backdated to "today" (unusual, yes), this means that the circulations were returned "tomorrow". Since they're not due yet, there's no extra day of fines, but the future checkin date does stick out.

The issue is how checkin_handle_backdate in OpenILS::Application::Circ::Circulate is "taking the due-time from the original due-date".

Since we're pushing all due times to 23:59:59 when the circulation is day-granular, and we're already not providing a means for a backdate of (for example) an hourly checkout, the simplest fix for this issue while not losing any functionality might be to always backdate to 23:59:59. This could then be re-addressed if support for non-day-granular backdating is added.

[Thomas Berezansky (tsbere)]

Here is a patch to change how the original time is taken from the original due date:

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

[Robert J Jackson (rjackson-deactivatedaccount)]

Thanks Thomas.

I can confirm that under 2.2 your patch fixes backdating when the due date is past the time change as was one of the example circs pasted previously.

[Jason Stephenson (jstephenson)]

Robert or Jeff,

Could one of you sign off on the branch if it does indeed fix your issues?

Or anyone else who can confirm the branch resolves the problem.

Thanks

[Robert J Jackson (rjackson-deactivatedaccount)]

Jason,

Sorry - I don't think I have sign off capabilities.

[Jason Stephenson (jstephenson)]

Do you have the ability to push to the working repository? If so, then you have sign off capability. You'd basically make a new branch in your local repo, and git cherry-pick -s the commits, then push to the working repo. One of the committers could then take that branch and test it before committing to master in the main repository.

[Lebbeous Fogle-Weekley (lebbeous)]

Looks good. Signed-off and pushed to master, rel_2_3 and rel_2_2.