🗺️ #2724 Remove Regions #2753
🗺️ #2724 Remove Regions #2753
Conversation
demariadaniel
requested review from
ciaranschutte,
justincorrigible and
joneubank
global/utils/common.tsx
Outdated
| ? new Date(parseInt(date) * 1000) | ||
| : new Date(date) | ||
| : date; | ||
|
|
There was a problem hiding this comment.
This seems unrelated to the removal of regions code throughout. An isolated PR might be good?
I don't quite understand why this change is needed. Is there a bug or ticket we're addressing with it?
GraphQL DateTime seems to be in use for our data ref: https://the-guild.dev/graphql/scalars/docs/scalars/date-time
DateTime is a UTC string eg. 2007-12-03T10:15:30Z
passing this through parseInt is going to terminate at the dash so we'll end up with 2007 and then create a Date object with 200700 which is Wed Dec 31 1969 19:33:27 GMT-0500 (Eastern Standard Time) as a Date
If keeping this functionality I think it's more clear as an if/else. Nested ternary with multiple parseInt and Date constructors is confusing to read for a straightforward displayDate util func
There was a problem hiding this comment.
This came about as a result of testing the Manage Program page.
Could certainly be moved to it's own PR.
Testing local Platform UI against local Gateway, pointing at Clinical service Dev, programs ALEXIS-CA, DATA-CA and several others (not all) crash when you open the Manage Program page
The error is RangeError: Invalid time value and the values passed in as the date argument are numeric string values such as 1696946508681 or 1705346233163
In fact many of the programs that work on locahost display an invitationAccepted date value of 0 which doesn't seem right either. So there are data issues.
To complicate matters more this bug appears on localhost, but Dev seems to be working fine... so maybe it's just some implementation detail I'm not familiar with that's causing things.
Suffice to say, another ticket is a good idea, given the ambiguity
I used nested ternary b/c we're declaring a variable. Using var declaration with if/else would require let so it seemed like, half a dozen of one issue, six of the other.
If/else would definitely be easier to read if each block returns the expected value, hadn't considered that.
There was a problem hiding this comment.
Reverted in 4c3f167a188d39eff09b30a2dc1148876bad07d8
This reverts commit a8844bd.
| @@ -667,7 +667,7 @@ export type Mutation = { | |||
| createJiraTicketWithReCaptcha: TicketCreationResponse; | |||
| /** | |||
| * Create new program | |||
| * For lists (Cancer Type, Primary Site, Institution, Regions, Countries) the entire new value must be provided, not just values being added. | |||
| * For lists (Cancer Type, Primary Site, Institution, Countries) the entire new value must be provided, not just values being added. | |||
There was a problem hiding this comment.
this file is generated gql from the api - I think the regions have been removed there already? If they haven't this will regenerate with region code
There was a problem hiding this comment.
Yeah sorry this was just find-and-replace carpet bombing, will be updated properly once API is updated
Separate API PR pending review: icgc-argo/platform-api#716
Description of changes
Removes all references to Region field for programs
Adds handling for Date formatting which is breaking on localhost
Link to Issue
#2724
Link to Gateway PR:
icgc-argo/platform-api#716
Type of Change
Checklist before requesting review