psp-6887 add date and time aliases to frontend. Add model typing to backend. #3516
Conversation
devinleighsmith
requested review from
asanchezr,
FuriousLlama,
stairaku,
PryancaJSharma,
eddherrera and
buddy326
as code owners
|
✅ No secrets were detected in the code. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## dev #3516 +/- ##
==========================================
- Coverage 69.57% 69.56% -0.02%
==========================================
Files 1371 1372 +1
Lines 33877 33895 +18
Branches 6299 6303 +4
==========================================
+ Hits 23570 23579 +9
- Misses 10053 10060 +7
- Partials 254 256 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
devinleighsmith
changed the title
|
converting to draft due to stability risk for a release sprint. |
devinleighsmith
force-pushed
the
psp-6887
branch
2 times, most recently
from
3ffacc2 to
e59956e
Compare
frontend/backend now use typings to better reflect desired use of field (id timestamp vs dateonly).
devinleighsmith
force-pushed
the
psp-6887
branch
from
e59956e to
e558d5c
Compare
|
✅ No secrets were detected in the code. |
| @@ -6,6 +6,7 @@ import { | |||
| import { Api_CompensationRequisition } from '@/models/api/CompensationRequisition'; | |||
| import { Api_Project } from '@/models/api/Project'; | |||
| import Api_TypeCode from '@/models/api/TypeCode'; | |||
| import { UtcIsoDateTime } from '@/models/api/UtcIsoDateTime'; | |||
There was a problem hiding this comment.
Note that the frontend nor the backend can guarantee that the date is in UTC. For that reason how about isoDateTime?
There was a problem hiding this comment.
and I mean guarantee as a type.
There was a problem hiding this comment.
by that logic there is no guarantee that the string is an iso formatted string either. I lean more towards leaving utc in as it describes the intended contents - in the same was as IsoDateTime does.
There was a problem hiding this comment.
@FuriousLlama any thoughts?
There was a problem hiding this comment.
We do control that the backend will format the DateTime type to an iso string, we can't enforce that the date time being encoded will be in utc. In a nutshell, there is no DateTimeUTC on the backend we can rely on so we cannot make that assumption on the frontend either. Although I apreciate the intent of the utc on the name, it might lead to bugs or mistakes where the developer presumes that using that type guarantees utc-ness
There was a problem hiding this comment.
I don't think I get it sorry, we have complete control over the format the backend returns dates in, which means we can either implicitly or explicitly format all date strings as utc date strings, ie:
20120915T155300–0000
There was a problem hiding this comment.
Thinking about this more, I feel like we also want to keep utc in the name since we want to explicitly only send utc strings to the backend. In my mind, there is never a time we would want to send or receive a date that is not storing a utc date.
There was a problem hiding this comment.
lets discuss in dev meeting tmrw.
time aliases for frontend/backend models.