Handle date and timestamp objects more gracefully

[chrismclarke]

By default whenever firestore saves a date object it converts to a timestamp to support nanoseconds for use in queries (recent behaviour change). Either want to update interfaces to acknowledge this is the data format, find a way to disable this behaviour, or add a conversion for required fields on data return.

[thisismattia]

@vatsan28 wanted to take this on.. @chrismclarke are you already working on it?

[chrismclarke]

I think this warrants discussion, so if @vatsan28 wants to take a look and make a proposal we can determine how best to implement.

[vatsan28]

Talking about a possible approach with Chris on slack.

[chrismclarke]

(my comment from discussion)
Whenever data is sent to firebase in date format it is stored in date format, but when retrieved date objects are replaced by firebase's own custom timestamp object. So we need to decide whether happy to use that (and if so find a way to ensure all typescript interfaces use the correct format and people are aware in the future), or find a way to force firebase to pass data back as date format (or an efficient way to convert timestamps to date)

to be honest it's a bit of a pain issue and I'm waiting to see if anyone in the firebase community comes up with a solution, rather than trying to hack something together. it's why I intentionally didn't add a help wanted label to the issue, you are still welcome to dive deeper into community forums to see if other people are having this problem and what they have done, but equally I think there might also be better uses of your time.

[vatsan28]

I looked at the code to identify where we use date time attributes other than the defaults _created and _modified.

1. Events list component makes use of the event date.
2. In the discussions page, the PostList component has a row that indicates the number of days since the post was created. This is a default _created attribute.

This is the current state. Can you guys confirm if i missed any other attribute in any other model? I don't think i have access to the firebase console to check.

For a future state, i tried replicating this on my small learning project. I used a simple epoch to readable date conversion. So we could add a similar method to the utils/helpers.ts. When i think about adding a conversion at the time we fetch the records, we could add an instance method to the ModuleStore class. This can be used going forward for fields/attributes that come back with an epoch time other than the defaults - _created and _modified.

Thoughts? @chrismclarke

[chrismclarke]

I think for this type of thing it might be better to put in a hacks folder just to make it clear that this is simply a response to otherwise undesired behaviour patterns (which still hoping a more elegant solution can be found for in the future, e.g. moving database provider). I would suggest anything we include would make sense on the Database class as this specifically links to firebase

[BenGamma]

Any updates on this ?

[BenGamma]

I recently added two methods to handle dates in thesrc/utils/helpers file https://github.com/OneArmyWorld/onearmy/blob/d72ee3a0810617063b3c661726f71ed0284394c1/src/utils/helpers.ts#L29
Does that help in any ways ? Feel like this issue is blocked, maybe need to be break down into smaller ?

[chrismclarke]

I think for now we just keep using these methods and make clear in typings what data formats are in use are sufficient. I expect this to be a non-issue in future versioning where we provide alternate database options. Firestore look set to not budge on the issue (they will keep converting any dates sent to timestamps and return the data in timestamps), so closing

[chrismclarke]

I think it's time to re-open this as we're now starting to deal with other databases (not just) firestore which could easily add confusion.

I suggest we plan to change all dates to ISOString format, so that it saves as a string and avoids confusing conversion.

ISOStrings represent date as YYYY-MM-DDTHH:mm:ss.sssZ , and so not only are standardised across timezones and provide ms precision to help avoid duplicate data, but should also support querying (e.g. get data where _modified > 2019-08-03T10:24:05.585Z)

This should be fairly straightforward to do, using find/replace to change Timestamp to string format (or possibly better to create a type type ISOString = string, which still just declares the data as expected to be a string, but also as a reminder devs that it is an ISOString. Then any use of previous timestamp variables should be checked (although hopefully will get automatically flagged by typings)

[chrismclarke]

OK, this is a pain point on something else I'm working on so plan to create a PR to migrate all the backend functionality and existing data