Fix miss matched timezones in claim form

[mjamescompton]

Summary

Closes #2650

Changes

• Remove .toISOString() on date object and replace with function that returns formatted local time

Testing

Fill in form and make sure everything stays the same on multiple submissions of the same data.

Notes for Reviewers

The sdk assumes we are submitting local time for Amsterdam that is UTC+02:00 so that is converted in the sdk to UTC+00:00 before POST`in to the sever (simply it minuses 2 hours)

so 25/4/2020 00:00 is converted to 24/4/2020 10:00

When the time is return to the form from the server it receives 24/4/2020 10:00 which should be converted to local time but instead is displayed as is.

Using .toISOString() return the the date/time in UTC+00:00 and the time is removed.
Using the date object as is returns local time date/time as UTC+02:00.

Checklist

• Scope: The referenced issue is addressed, there are no unrelated changes.
• Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md. The target branch is set to master if the changes are fully compatible with existing API, database, configuration and CLI.
• Documentation: Relevant documentation is added or updated.
• Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
• Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

[coveralls]

Coverage increased (+0.05%) to 73.197% when pulling d3b9dab on issue/claim-form-timezones into 772db1a on master.

[bafonins]

The sdk assumes we are submitting local time for Amsterdam that is UTC+02:00 so that is converted in the sdk to UTC+00:00 before POST`in to the sever (simply it minuses 2 hours)

There is nothing in the js sdk that is related to date handling/transformation. The problem here is that yup's date casts dates according to local time. See https://codesandbox.io/s/competent-oskar-od6qn

I guess using .mixed() with custom validation should solve the issue.

[bafonins]

Suggested change

	// Copyright © 2019 The Things Network Foundation, The Things Industries B.V.
	// Copyright © 2020 The Things Network Foundation, The Things Industries B.V.

[bafonins]

jsdoc and some tests would be very helpful here

[mjamescompton]

Yup dates are outputted in local time yes but

if you log validationSchema.cast(values) you get

{claim_authentication_code: {…}}
claim_authentication_code:
valid_from: Sat Apr 25 2020 00:00:00 GMT+0200 (Central European Summer Time) {}
valid_to: Fri May 22 2020 00:00:00 GMT+0200 (Central European Summer Time) {}
value: "FHERHGKHDFKGHFD"

then the payload for the api call is

{"end_device":{
"ids":{"join_eui":"0000000000000001","dev_eui":"0000000000000001"},
"claim_authentication_code":{
"valid_from":"2020-04-24T22:00:00.000Z",
"valid_to":"2020-05-21T22:00:00.000Z",
"value":"FHERHGKHDFKGHFD"}},
"field_mask":{"paths":["claim_authentication_code"]}
}

So the sdk is setting the time to GMT+0000 purposely or not

[bafonins]

So the sdk is setting the time to GMT+0000 purposely or not

Once again, the js sdk has nothing to do with dates. There is no specific code to deal with dates, transform or anything else.

if you log validationSchema.cast(values) you get

Try logging JSON.stringify(validationSchema.cast(values), null, 2). It seems that codesandbox didnt save this part. See updated example https://codesandbox.io/s/epic-hooks-t2t7i

Sat Apr 25 2020 00:00:00 GMT+0200 - date formatted by the browser for you according to your local time.

[bafonins]

I get

claim_authentication_code: {
  value: "...",
  valid_from: "2020-06-23T21:00:00Z"
}

when selecting 24/06/2020 in the date picker. Notice -3 hrs since Im in GMT+3

[mjamescompton]

So if we are setting the time in the browser as the date at the beginning of the day ( 00:00 ) of your local time, that would be saved in the DB as GMT+0000 so that would 21:00 of the previous day.

On retrieving this the browser then converts this to local time and displays the correct date.

I think maybe the question here should be that, should the time be shown as local time in the browser and stored as GMT+0000

or

Should it always be GMT+0000

I would go with browser being local time @bafonins @kschiffer what do you think?

[bafonins]

Indeed, you are right.

I think maybe the question here should be that, should the time be shown as local time in the browser and stored as GMT+0000

From the user perspective, I would assume that date is set based on my timezone. I dont think we should force the users to set dates based on GMT+0, at least thats not very common for modern UIs.

[bafonins]

Suggested change

	- Timezones issue in claim auth code form, causing time to reverse on submission
	- Timezones issue in claim authentication code form, causing time to reverse on submission.

[kschiffer]

So for my understanding, the claim api expects a simple date string e.g. (`2020-06-10`) and uses the local timezone of the server?

If so, then that means that there can be discrepancies between the timezone of the client and the server, while the user will (rightfully) assume that his own timezone is going to be used. So ideally, we should display the underlying server timezone that is going to be used for this date.

That means that we need a way to retrieve the timezone of the server. @htdvisser do we already have something to achieve this?

It's edge casy enough that I'm ok with merging this as it is for now, but still this needs to be addressed eventually.

[htdvisser]

All our APIs always use UTC. The format is 2006-01-02T15:04:05Z.

[kschiffer]

So @mjamescompton, please add a field-description that informs the user that the date is UTC +0 based.

[bafonins]

So @mjamescompton, please add a field-description that informs the user that the date is UTC +0 based.

Why so? It is a common practice to store dates on servers/databases as UTC+0 timestamps for simplicity and for the apps that do not heavily depend on planning, e.g. event scheduling apps. I dont think we should force the users to think about this (at least in the console, for CLI is a different story), especially when our customers can be distributed around the globe. Here is an example:

1. User A is in LA (PDT, UTC-7) and his local time is 20-06-2020 15:00:00
2. User B is in Amsterdam (UTC+2) and his local time is 21-06-2020 00:00:00 (just 9hrs difference)
3. Server clock is 20-06-2020 22:00:00 as its timezone agnostic timestamp
4. User A sets claim authentication code to be valid for the next hour, hence setting the date time to 20-06-2020 16:00:00 (UTC-7) and sending 20-06-2020 23:00:00 in the request. User B sees this in the browser as 21-06-2020 01:00:00 (UTC+2).

For all, user A,B and the server date and time are set properly.

The only problem I can imagine happening is with DST, as different browsers might process this differently from one another. This can solved with date-fns which can also simplify dealing with dates in other parts of the app.

Moreover, if we decide to UTC+0 here, we introduce another inconsistency as we use users local time in many places across the Console, e.g. last_seen, created_at and others. These fields are set by the server in UTC+0, however we format them to the users local time.

As mentioned above, the CLI is a different story and I would expect the developers to check what is the format in the docs, we cannot really guide anyone there.

[kschiffer]

I agree with everything you said. I had a misconception there. I thought the server would only receive a simple date-string yyyy-mm-dd which would be ambiguous since there's not timezone information in that. Now I wonder, why don't we allow to set an exact point in time (date and time) if the server accepts such data?

[bafonins]

#1134 (comment) cc @johanstokking

[kschiffer]

I see.

@mjamescompton fine to merge this. We can still change it later if we find it more useful.

[bafonins]

@mjamescompton can you merge this PR? otherwise please add the blocked label