jsgoupil/quickbooks-sync

QuickBooks does not handle UTC time properly, so we should have a way to handle timezone for the developer

Closed this issue · 13 comments

A request as such:
2016-05-17T20:00:00
Actually means that it checks for 20:00 LOCAL time on QuickBooks machine.

If we are in PST. For 20:00 local time, it's actually 3:00am (next day)

Now, if we set
request.FromModifiedDate = new DateTime(long.Parse(lastSync), DateTimeKind.Utc); // 3:00am UTC

This will actually end up requesting a real UTC time. Then QuickBooks will actually think you are requesting a 3:00am local time.

Then I have to end up doing this:
request.FromModifiedDate = new DateTime(long.Parse(lastSync), DateTimeKind.Utc).ToLocalTime(); // Which would be the client timezone

or

request.FromModifiedDate = Instant.FromDateTimeUtc(new DateTime(long.Parse(lastSync), DateTimeKind.Utc)).InZone(localTimeZone).ToDateTimeUnspecified();

I need some help on this. We store our dates as UTC in MS SQL Server and are trying to use FromModifiedDate. Our Quick Books instance is running is PST and we are currently calling this:

FromModifiedDate = new DATETIMETYPE(SyncLog.LastRun, TimeZoneInfo.FindSystemTimeZoneById("Pacific Standard Time"))

SyncLog.LastRun is a DateTime object. How should we correctly use DATETIMETYPE to get our desired results?

Hey @valmont
I mentioned in the bug how to handle it. You need to convert to local time. You can use DateTime.
The current passing of the TimeZone is only to handle the Daylight Time Saving bug. Which you can leave the SyncManager handle as well if you set the TimeZoneBugFix

@jsgoupil A suggestion was to send the date to QuickBooks as "2016-05-17T20:00:00+0:00", have you tried this?

Issue I'm having is that I don't know which timezone our clients are on, so converting them to the server localtime is still potentially off by a few hours.

He @farlock85 , this bug is a little old, so you can try to use the TimeZoneBugFix? I don't remember if trying to send that specific date works to QuickBooks but I don't think it does work. Let me know if it does?

Maybe you could get the timezone information from the CompanyQueryRs.
That way you get where the company is located.

@jsgoupil I updated the DATETIMETYPE.cs ToString() to

public override string ToString()
        {
            var k = value.ToString(" K", CultureInfo.InvariantCulture).Trim();

            // QuickBooks doesn't support Z format.
            if (k == "Z")
            {
                k = "+00:00";
            }

            return value.ToString("yyyy-MM-ddTHH:mm:ss", CultureInfo.InvariantCulture) + k;
        }

And that seems to work fine, I haven't tested in different timezones yet though.

I can give it a shot, but it all depends what QuickBooks return to you when you make a request. If I remember well from a few years ago, the timezone was simply ignored. So I was missing some customers when querying.

@farlock85 You seem to be right, with that fix, I think we could get rid of the entire TimezoneBugFix

OK. After further investigation, the TimezoneBugFix is necessary...

As an example, I set my time to somewhere in April (when DST is active)
I modify 1 customer in QuickBooks.

image

Its modified date is off by 1h with the current time.

However, your fix might still be good.

Thanks @jsgoupil, appreciate all the effort, it helped us alot with our integration platform!

@farlock85 You're welcome, I just added a way to add multiple requests in one step in this new version.
I also offer consulting if you guys are interested! You can find my info on my personal website / github profile.

tofer commented

I have been testing a completely different fix for this, and thought I would chime in here and see what your thoughts are. What got me going on this was the point you made above

If I remember well from a few years ago, the timezone was simply ignored

For all I can tell this seems to be correct. If we were to ignore the offset value at the end of the timestamp returned from QuickBooks, the time would be accurate with respect to the Local time of the machine QuickBooks is installed on. The fact that that is parsed with the timestamp is what's causing it to be adjust by an hour during DST when that offset is wrong.

While yes this is Intuit's fault, I think the simpler answer to solving it is to simply not parse the offset portion of the returned timestamp, and always considering it a local time with no zone or offset information. Consumers of the library, if needing an offset, could convert the value to a DateTimeOffset using the zone information they configure themselves.

I'm still working on testing all the various operations when leaving off the Offset for requests, but from all the documentation I've read from Intuit it doesn't seem like QuickBooks needs an offset provided on requests... it will just consider it local time.

To completely remove ambiguity, I think what this ideally calls for is to use Jon Skeet's excellent NodaTime library. I've been using it for a few years now, and it's really perfect for situations like this where DateTime is simply too ambiguous. The LocalDateTime type would be helpful, but of course the biggest problem with that is the massive breaking change that would impose.

The breaking change doesn't matter for me though, so I've been testing a branch were I use NodaTime.LocalDateTime as the wrapped value on DATETIMETYPE and NodaTime.LocalDate for DATETYPE. If it works out, I love using the NodaTime library so much I may just maintain a separate branch going forward for my use... we'll see.

@tofer Do you think the recent changes break the correct "implementation" to talk with QuickBooks? In regards to getting the latest TimeUpdated and making another request afterwards.
My client is currently using an old version of this library and I haven't had time to update it.

tofer commented

Do you think the recent changes break the correct "implementation" to talk with QuickBooks? In regards to getting the latest TimeUpdated and making another request afterwards.

Just to be clear, you're referring to this change correct?

if (k == "Z")
{
    k = string.empty;
}

to

if (k == "Z")
{
    k = "+00:00";
}

"Z" should only happen for DateTimeKind.Utc dates, so if they are using the value of TimeUpdated as returned from QuickBooks, my guess is that this should be fine (as in this nets them no change).