h-REA / hREA

A ValueFlows / REA economic network coordination system implemented on Holochain and with supplied Javascript GraphQL libraries
https://docs.hrea.io
Other
143 stars 15 forks source link

Fix and test use of dates. fecha.parse usage is too strict and fails to parse dates without milliseconds given #200

Open Connoropolous opened 2 years ago

Connoropolous commented 2 years ago

It seems the issue doesn't occur if you pass a decimal after the seconds, for milliseconds

This input: hasPointInTime: "2022-02-20T01:30:15.01-08:00"

Became this: hasPointInTime: "2022-02-20T09:30:15.010Z"

without passing milliseconds you get back null. However, we know the values are saved to the db so its just an issue with client side decoding of fields.

an issue with fecha.parse in connection.ts and happening at the JS layer.

Screen Shot 2022-02-24 at 9 56 04 PM

@pospi also said: Add tests to ensure all necessary date formats are valid for DateTime fields in search fields (dates, date + time, date + time + seconds, date + time + seconds + millis).

May want to swap out the URI and DateTime GraphQL scalar resolvers; consider using https://github.com/Urigo/graphql-scalars.

pospi commented 2 years ago

I encountered this a while ago but hadn't logged. Seems to be an issue on the serialization on the way out- I had previously checked with Holochain Playground and found that the value is being correctly saved in the DB. May also be an issue with fecha.parse in connection.ts and happening at the JS layer.

Connoropolous commented 2 years ago

oh, well... It seems the issue doesn't occur if you pass a decimal after the seconds, for milliseconds or whatever

This input: hasPointInTime: "2022-02-20T01:30:15.01-08:00"

Became this: hasPointInTime: "2022-02-20T09:30:15.010Z"

which is fine. definitely smells like a bug, but a developer should I guess read about the system here and specify accordingly?

pospi commented 2 years ago

Interesting. Those fields are using chrono::datetime::DateTime<FixedOffset> in the backend, which I think accepts either format.

My tests are showing (checking the DHT through Playground)-

Not sure how you managed to get a value returned from the backend. There may also be other conversions happening in the fecha parsing behaviour because I just did 4e1255d4 and can now get 2022-01-10T01:30:15Z returned in the same format it was passed in as.

So, definitely some weird stuff with the chrono crate going on that needs to be investigated. I guess converting from a DateTime<FixedOffset> .into() another DateTime<FixedOffset> causes some timezone conversion to happen?

pospi commented 2 years ago

Related to #156