Single Day Emergency Closings Fail to Update Due Dates Correctly

Bug #1818912 reported by John Amundson
194
This bug affects 41 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
High
Unassigned
3.10
Fix Released
High
Unassigned
3.11
Fix Released
High
Unassigned

Bug Description

Evergreen 3.2.4

When entering a single day emergency closing, item due dates are not always updating correctly. Single day closings are not taking into account already established closed days that fall after the closing when adjusting due dates on daily loans.

Multi-day closings do seem to be updating due dates correctly.

Here's an example of one of my tests:
Chose a Checkout Library with the following Always Closed Days: Tuesday, Friday, Sunday (Based on hours in Server Admin for the Org Unit)
Put in an Emergency closing for Saturday 12/01/18 with items due/set to shelf expire on that date.

Expected Behavior...
Due Dates and Shelf Expire Dates updated to Monday 12/03/18.

What actually happens..
Hourly Loans: Updated to the listed closing time on Monday 12/03/18 (expected)
Shelf Expire Time: Updated to the listed closing time on Monday 12/03/18 (expected)
Daily Loans: Updated To Sunday, 12/02/18 (NOT expected)

When entering only a single day close, daily loans are not taking into account always closed days or other days marked closed in the editor.

Revision history for this message
Jennifer Bruch (jbruch) wrote :

Hello All,
I can confirm that this bug is still affecting Evergreen 3.3.4. for the PAILS SPARK group.

On Friday, we used the Emergency Closing feature to make the next day, Saturday, a closed day. We checked the Process Immediately box, and it processed 200+ items.

Because our libraries are normally closed on Sunday, we expected the Due Date to update to Monday, 111/11/2019. However, this was not what happened. The items were updated to the Due Date of 11/10/2019 and promptly went overdue on Monday morning.

We are also concerned about another bug regarding this feature about adjusting the fines for Emergency Closing days.

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

Revision history for this message
Anna Barbounis (abarbouni) wrote :

Hello,

I used the Emergency Closing Feature on Friday to have materials that were due on Saturday to be due on our next working day which is Monday. Our library is normally closed on Sunday. To my surprise the due date for the materials were on Sunday. About 289 items were due on the wrong day. This was the first time I used this feature. Not only were the materials due on the wrong day, the system assessed an overdue fee. Needless to say I had a lot of voiding fines on Monday. It did not occur to me the system would pick Sunday as a due date since it's a closed day for our library.

Changed in evergreen:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Rogan Hamby (rogan-hamby) wrote :

The function evergreen.find_next_open_time is used to find the next time to set items. After an initial misconfiguration on my part that involving time zones I tested this and don't see an issue with the date returned. On a stock test data 3.4 beta system I set BR2 closed on Saturdays and set an emergency closing for a Yeti attack on today (Friday). It correctly gave me the end of the next open day.

The misconfiguration of the time zones though did lead me to observe how ugly that can get (on a test system fortunately). But it almost makes me wonder, given the bulk nature of these updates, if we shouldn't do something to mitigate against it so that for example, a next time can't be returned at 11pm or something like that. I'm also wondering if that's behind some of the issues seen. I'll set up a scenario to test this with actual checkouts as well.

evergreen=# select * from actor.org_unit_closed;
 id | org_unit | close_start | close_end | full_day | multi_day | reason | emergency_closing
----+----------+---------------------------+---------------------------+----------+-----------+-------------+-------------------
  7 | 5 | 2020-03-20 00:00:00.16-05 | 2020-03-20 23:59:59.16-05 | t | f | Yeti Attack | 6
(1 row)

evergreen=# select evergreen.find_next_open_time(5,now());
  find_next_open_time
------------------------
 2020-03-22 23:59:59-05
(1 row)

Revision history for this message
John Amundson (jamundson) wrote :

Since my initial report, I noticed that the undesired behavior doesn't just happen on single day closures, but also multiple day ones depending on surrounding closed dates.

Here is what we recommend to our libraries:

If adding a date surrounded by existing closed dates, you should add a closing for the entire period to ensure due dates are updated correctly.
For example, If you’re always closed Sunday, are closed Monday for the holiday, and the town approved closing Saturday, don’t enter just Saturday as an emergency closing, instead enter a multi-day closing of Saturday through Monday!

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

I believe I've identified a way some of this may be occurring. I've pushed a branch for folks to look at, but not yet marking it pull-request.

From the commit message:

When the emergency closing handler goes to grab the list of circulations and holds it should work on, it can miss some due to sub-second components of the closing start/end timestamps. This commit introduces a function for truncating timestamps to second-granularity, and then uses that function when looking for in-range objects.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-1818912-emergency-closing-due-date-push-fails

Revision history for this message
Rogan Hamby (rogan-hamby) wrote :

I've been trying to recreate the failed behavior on a stock test system and can't though I can see where on a production system something has definitely happened. One closed emergency event where all circs should have pushed out to a Monday had 74 do so but another 184 did not and fell on the Sunday where the library was closed.

I tested Mike's patch and I'm not sure if it fixed anything since I can't duplicate the problem but I can at least say it did no harm. RETURNS is misspelled on the line:

CREATE OR REPLACE FUNCTION evergreen.truncate_timestamptz_to_second (TIMESTAMPTZ) RERTURNS TIMESTAMPTZ

though. If anyone has any hints for duplicating this I'd be glad to test more.

Revision history for this message
John Amundson (jamundson) wrote :

I just performed the same test I presented in my initial report on a system running 3.4.2. Same behavior happened. I picked a day that only had 12 circulations due.

Chose a library that is always closed Sundays, (Hours set to Closed in the Organizational Unit).
Added a single-day emergency closing for a specific Saturday.
Noted that the items due on that Saturday were updated to Sunday at 11:59 PM instead of the next open day - Monday.

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

Thanks for testing, John. I think the "+ 1 second" part that I removed with the second truncation may still be necessary, so I'll work on an update to my branch that adds that back (unless you want to grab that). It'll be Monday before I can come back to it, though.

Thanks!

Revision history for this message
John Amundson (jamundson) wrote :

FYI, I didn't test your branch, Mike. (very sorry if that wasn't clear!). I was just providing a steps a tester might be able to take in response to Rogan's comment. I was waiting for a pull request to look at the branch, (and time to do it). Time is an odd concept at the moment.

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

Ah! Thanks for clarifying that, John! I'll clean up the branch (per Rogan's note) in any case and go back through the timestamp logic to see if that is, in fact, needed.

Will update when that is available.

Revision history for this message
Rogan Hamby (rogan-hamby) wrote :

Given the unclear cause reported here and it's presence in production but not test data I wonder if the actual cause (at least in part) might be: https://bugs.launchpad.net/evergreen/+bug/1870605

If anyone is following this bug I'd recommend we get this get tested and (assuming it pasts tests) pushed soon.

Revision history for this message
John Amundson (jamundson) wrote :

Rogan, we have installed the patch for bug 1870605 on our training server where I can reliably test this bug. It does not seem to have any impact.

Before installing the patch, I set up several circulations to be due Thursday 6/25/20. The library is Always closed on Friday. I entered a single day closing for 6/25/20. The circulations updated to Friday 6/26/20, a closed day.

After installing the patch, there was no difference. The library was always closed Sundays. I entered a closing for 6/27/20. The items incorrectly updated to Sunday 6/28/20.

This bug doesn't seem to have any relation to fines. The same behavior is observed whether or not a circulation has fines or doesn't'.

tags: added: admin-pages
removed: webstaffclient
Revision history for this message
Michele Morgan (mmorgan) wrote :

We are on 3.6.4 and have had a report of this behavior from one of out libraries:

Normal closings from 12/24 - 12/26 had been entered in advance
12/23 was entered and processed as an emergency closing
Items that had been due on 12/23 had their due dates updated to 12/24

I will take a look at Mike's branch in comment #5.

tags: added: circulation
Steven Mayo (stmayo)
Changed in evergreen:
assignee: nobody → Steven Mayo (stmayo)
Revision history for this message
Steven Mayo (stmayo) wrote :

I've taken my own crack at this and I think I may have gotten it. I didn't include Mike's truncation logic - so maaaybe there could still be some problem with the times being off by seconds - but I did find a logic error that ignored the hours of operation on the first day it tries to update timestamps to.

Eager to see how thoroughly this fixes the problem on bigger datasets.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/smayo/lp1818912-Emergency_Closing_Due_Date_Push_Fails

tags: added: pull
tags: added: pullrequest
removed: pull
Revision history for this message
Terran McCanna (tmccanna) wrote :
Steven Mayo (stmayo)
Changed in evergreen:
assignee: Steven Mayo (stmayo) → nobody
Revision history for this message
Andrea Neiman (aneiman) wrote :

Tested on terran-testbox.gapines.org and it does the thing - a one day closing pushes back all circs to the next open day, which in the case of my test pushed circs from a Friday to a Monday since Sat/Sun were typical closing days.

I consent to signing off on this with my name, Andrea Buntz Neiman, and my email, <email address hidden>.

Changed in evergreen:
milestone: none → 3.12-beta
tags: added: signedoff
Changed in evergreen:
assignee: nobody → Jane Sandberg (sandbergja)
Revision history for this message
Jane Sandberg (sandbergja) wrote :

Wow, nice! Nice to have this long-standing, high importance bug squashed! Thanks, Steven and Andrea. Merged to rel_3_10 and above.

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Jane Sandberg (sandbergja) → nobody
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.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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