Closed Dates Editor Displaying Incorrect Closed Duration

[Jennifer Pringle]

Evergreen 2.10

(Admin -> Local Administration -> Closed Dates Editor)

Closures are displaying with an extra closed day in the staff client in 2.10. For example, when I enter July 1st as a closed date it displays as "From 2016-07-01 at 00:00 through 2016-07-02 at 23:59". It should display as "From 2016-07-01 at 00:00 through 2016-07-01 at 23:59"

The closed date is correct in the database.

This issue is present for all types of closures - single day, multiple date, and detailed.

I have tested this on our 2.10 servers as well as Equinox's 2.10 community server.

I have also tested and confirmed that this issue is not present in 2.8 or 2.9.

[Jason Stephenson (jstephenson)]

I just confirmed it on a system running master. It does not affect our production system running 2.9.

[Michele Morgan (mmorgan)]

I can confirm that this problem exists on a 2.10.5 system. but not a 2.9.1 system.

This is a display issue in the days closed editor, the closed date ranges are saved correctly in the database. Still, this will be very confusing for staff entering holiday closures.

The database entry is:

close_start - close_end
2016-10-01 00:00:00.288-04 - 2016-10-01 23:59:59.288-04

The days closed editor displays this as:

From 2016-10-01 at 00:00 through 2016-10-02 at 23:59

Here's a link to a relevant IRC discussion:

http://irc.evergreen-ils.org/evergreen/2016-09-08#i_265869

[Dan Wells (dbw2)]

The issue here is our use of toISOSting() as a formatting step. Unfortunately, to quote the Mozilla docs, with toISOString():

'The timezone is always zero UTC offset, as denoted by the suffix "Z".'

For all of us in the western hemisphere, 11:59:59pm is already tomorrow UTC, (e.g. 3:59:59am the next day for -04 timezone offset), so we end up "gaining" a day when trying to format using the current cdDateToDate().

Two possible solutions:

First, we can pre-adjust the time based on our timezone before running it through toISOString(). So, in EDT, we first subtract four hours to account for the 4 hours which will be "added". Here is one recipe for doing that, borrowed from StackOverflow:

var tzoffset = (new Date()).getTimezoneOffset() * 60000; //offset in milliseconds
var localISOTime = (new Date(Date.now() - tzoffset)).toISOString().slice(0,-1);
// => '2015-01-26T06:40:36.181' (dropping the now erroneous 'Z' offset)

In our case, however, I am more inclined to just drop the parsing and toISOString()-ing altogether. I am not seeing a path to this function where we aren't reasonably sure what format the data is already in, so we could simply truncate off the time and be done with it.

These solutions are both pretty simple fixes, so feel free to throw together a branch or weigh in on direction if you are so inclined. I will push something along one of these lines (depending on how each works out in practice) if nobody jumps on it.

[Jason Stephenson (jstephenson)]

I'm inclined to go with just lopping off the time from the date string and being done with it. As Dan says, we should be dealing with strings from Postgres here and we know what format they'll be in already.

All the timezone math seems like it would be error prone, as would manually building an ISO format string from a Date object.

[Mike Rylander (mrylander)]

I'm with Jason and Dan.

If/when the timezone branch gets in, we'll need to revisit this. Likely we'll need a YAOUS to provide the org unit's timezone, and use that in contexts where we should deal with a timestamp in an explicit timezone, such as this, instead of the client's. Those cases will be rare compared to the rest of the places we deal with timestamps, but both identifiable (once the TZ stuff is in effect) and important.

[Jason Stephenson (jstephenson)]

I want to add that Mike is correct that we'll possibly need a new solution when the timezone branch gets in. I tested this with that branch applied, and the closed dates till appear to be 1 day off. This is what I expected, but I wanted to verify that.

[Dan Scott (denials)]

This stems from commit ede7e78925a07d where the XUL/JSAN requirements were removed, and util.date.formatted_date() was no longer being called, instead returning ISO-formatted dates.

Would this also mean that any locale-based display of date and time formats has been lost and everybody now gets ISO? The formatted_date() function would format dates and times using Dojo's functionality (also pulling in the AOUS format.date and format.time)?

[Dan Scott (denials)]

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString looks like a standard cross-browser means of doing locale-sensitive date/time work; the "locales" and "options" arguments aren't available in IE until IE 11, but Firefox/Chrome/Opera/Safari all look good.

[Dan Scott (denials)]

Yeah; the following seems to work reasonably well right now:

function cdDateToDate(date) {
    var date_obj = new Date(Date.parse(date));
    return date_obj.toLocaleDateString();
}

That displays closed dates like:

    Every Day From 20/08/2016 through 21/08/2016
and
    All Day 20/02/2017

... so we lose the sane YYYY-mm-dd display, but we get actual date comparisons.

Passing a different locale as the first argument to toLocateDateString() doesn't seem to change the results in the staff client.

Of course, toLocaleDateString() only got support for the locales and options arguments in Firefox 29, and I completely forgot about XUL being hella-old, heh :) But I think toLocaleDateString() and friends is a good way forward for the web staff client, and perhaps we can live with this minor regression in localized date formatting in the XUL staff client until such time as we get away from it entirely?

[Dan Scott (denials)]

I've pushed user/dbs/lp1594937_closed_dates_off_by_one_display to http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbs/lp1594937_closed_dates_off_by_one_display accordingly.

[Chris Sharp (chrissharp123)]

I can confirm that Dan's fix does the trick. Signoff branch here:

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

[Dan Scott (denials)]

Thanks for testing and signing off, Chris! Pushed to master, rel_2_11, and rel_2_10.