Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🗺️ #2724 Remove Regions #2753

Merged
merged 4 commits into from
Feb 13, 2024
Merged

🗺️ #2724 Remove Regions #2753

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f5ddfd
Remove all references to regions
demariadaniel Feb 9, 2024
a8844bd
Fix Localhost Date Bug
demariadaniel Feb 9, 2024
9bdb9e2
Remove region header from Create Program
demariadaniel Feb 9, 2024
4c3f167
Revert "Fix Localhost Date Bug"
demariadaniel Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion components/pages/program-entity/ProgramCardsLayout/util.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const createProgramSummaryData = (programSummaryQuery: ProgramSummaryQuer
''
),
Institutions: programSummaryQuery?.institutions?.join(', ') || '',
'Processing Regions': programSummaryQuery?.regions?.join(', ') || '',
'Cancer Types': programSummaryQuery?.cancerTypes?.join(', ') || '',
};
};
Expand Down
1 change: 0 additions & 1 deletion components/pages/program-entity/gql/PROGRAM_SUMMARY_QUERY.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const PROGRAM_SUMMARY_QUERY = gql`
website
institutions
countries
regions
cancerTypes
primarySites
}
Expand Down
1 change: 0 additions & 1 deletion components/pages/program-entity/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export type ProgramSummaryQuery = {
website: string;
institutions: string[];
countries: string[];
regions: string[];
cancerTypes: string[];
primarySites: string[];
};
1 change: 0 additions & 1 deletion components/pages/submission-system/create-program/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const createProgramInput = (formData) => ({
website: formData.website,
institutions: formData.institutions,
countries: formData.countries,
regions: Array.from(formData.processingRegions),
membershipType: formData.membershipType,
admins: [
{
Expand Down
1 change: 0 additions & 1 deletion components/pages/submission-system/gql/PROGRAM_VALUES_QUERY.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const PROGRAM_VALUES_QUERY = gql`
cancerTypes
primarySites
institutions
regions
countries
}
}
Expand Down
33 changes: 0 additions & 33 deletions components/pages/submission-system/program-form/ProgramForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export default function CreateProgramForm({
membershipType?: string;
website?: string;
description?: string;
regions?: string[];
};
onSubmit: (data: any) => any;
}) {
Expand All @@ -96,7 +95,6 @@ export default function CreateProgramForm({
membershipType: string;
website: string;
description: string;
processingRegions: string[];
adminFirstName?: string;
adminLastName?: string;
adminEmail?: string;
Expand All @@ -111,7 +109,6 @@ export default function CreateProgramForm({
membershipType: program.membershipType || '',
website: program.website || '',
description: program.description || '',
processingRegions: program.regions || [],
adminFirstName: '',
adminLastName: '',
adminEmail: '',
Expand All @@ -136,8 +133,6 @@ export default function CreateProgramForm({

const { data: { programOptions = undefined } = {}, loading } = useQuery(PROGRAM_VALUES_QUERY);

const regionOptions = get(programOptions, 'regions', []);

/* ****************** *
* On Change Handlers
* ****************** */
Expand Down Expand Up @@ -403,34 +398,6 @@ export default function CreateProgramForm({
</Row>
</FormControl>

<Row>
<Col>
<SectionTitle>Processing Region</SectionTitle>
</Col>
</Row>

<FormControl error={validationErrors.processingRegions} required={true}>
<Row>
<Col>
<InputLabel htmlFor="Processing Region">
Please indicate the region where data can be processed.
</InputLabel>
<Select
aria-label="Processing Region"
id="checkbox-group-processing-region"
options={regionOptions.map((name) => ({ content: name, value: name }))}
onChange={(val) => setData({ key: 'processingRegions', val: [val] })}
onBlur={handleInputBlur('processingRegion')}
value={form.processingRegions[0] || ''}
size="lg"
/>
<ErrorText error={validationErrors.processingRegions} />
</Col>
</Row>
<Row>
<Col />
</Row>
</FormControl>
{!isEditing && (
<>
<Row>
Expand Down
1 change: 0 additions & 1 deletion components/pages/submission-system/program-form/validations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const baseValidations: yup.ObjectSchema<any> = yup.object({
.required(),
website: yup.string().label('Website').trim().url(),
description: yup.string().label('Description').trim(),
processingRegions: yup.array().of(yup.string()).label('Processing Regions').required(),
});

const adminValidations = yup.object().shape({
Expand Down
1 change: 0 additions & 1 deletion components/pages/submission-system/program-management/ManageProgramTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ const createUpdateProgramInput = (formData) => ({
website: formData.website,
institutions: formData.institutions,
countries: formData.countries,
regions: Array.from(formData.processingRegions),
membershipType: formData.membershipType,
cancerTypes: formData.cancerTypes,
primarySites: formData.primarySites,
Expand Down
55 changes: 0 additions & 55 deletions components/pages/submission-system/program-management/Profile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import join from 'lodash/join';
import replace from 'lodash/replace';
import { Col, Row } from 'react-grid-system';

const REGIONS = ['Africa', 'North America', 'Asia', 'Europe', 'Oceania', 'South America'];

const MISSING_ENTRY_TEXT = '--';

const arrayToText = (array) => (isEmpty(array) ? MISSING_ENTRY_TEXT : join(array, ', '));
Expand All @@ -40,7 +38,6 @@ type Program = {
membershipType?: string;
description?: string;
institutions?: string;
regions?: string;
};
function ProfileView({ program = {} as Program }) {
const theme = useTheme();
Expand Down Expand Up @@ -160,58 +157,6 @@ function ProfileView({ program = {} as Program }) {
</Left>
<Right>{arrayToText(program.institutions)}</Right>
</Row>

<SectionTitle>Processing Regions</SectionTitle>
<Row>
<Col
css={css`
padding: 7px 0;
`}
>
<InputLabel>
The data for this program CAN be processed in the following regions:
</InputLabel>
</Col>
</Row>
<Row>
<Col
css={css`
padding: 7px 0;
`}
>
<Icon width="15px" height="15px" name="checkmark" fill="success_dark" />
&nbsp;{replace(program.regions, ',(?! )', ', ')}
</Col>
</Row>

<Row>
<Col
css={css`
padding: 7px 0;
`}
>
<InputLabel>
The data for this program CANNOT be processed in the following regions:
</InputLabel>
</Col>
</Row>
<Row>
<Col
css={css`
padding: 7px 0;
`}
>
<Icon width="15px" height="15px" name="times" fill="error_dark" />
&nbsp;
{program.regions &&
join(
filter(REGIONS, (region) => {
return !program.regions.includes(region);
}),
', ',
)}
</Col>
</Row>
</div>
);
}
Expand Down
1 change: 0 additions & 1 deletion components/pages/submission-system/program-management/gql/PROGRAM_QUERY.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const PROGRAM_QUERY = gql`
website
institutions
countries
regions
membershipType
cancerTypes
primarySites
Expand Down
6 changes: 1 addition & 5 deletions generated/gql_types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* Returns Program object details of created program
*/
createProgram?: Maybe<Program>;
Expand Down Expand Up @@ -854,7 +854,6 @@ export type Program = {
membershipType?: Maybe<MembershipType>;
name?: Maybe<Scalars['String']>;
primarySites?: Maybe<Array<Maybe<Scalars['String']>>>;
regions?: Maybe<Array<Maybe<Scalars['String']>>>;
shortName: Scalars['String'];
submittedDonors?: Maybe<Scalars['Int']>;
users?: Maybe<Array<Maybe<ProgramUser>>>;
Expand Down Expand Up @@ -1061,7 +1060,6 @@ export type ProgramInput = {
membershipType: MembershipType;
name: Scalars['String'];
primarySites: Array<InputMaybe<Scalars['String']>>;
regions: Array<Scalars['String']>;
shortName: Scalars['String'];
website: Scalars['String'];
};
Expand All @@ -1072,7 +1070,6 @@ export type ProgramOptions = {
countries: Array<Maybe<Scalars['String']>>;
institutions: Array<Maybe<Scalars['String']>>;
primarySites: Array<Maybe<Scalars['String']>>;
regions: Array<Maybe<Scalars['String']>>;
};

export type ProgramUser = {
Expand Down Expand Up @@ -1320,7 +1317,6 @@ export type UpdateProgramInput = {
membershipType?: InputMaybe<MembershipType>;
name?: InputMaybe<Scalars['String']>;
primarySites?: InputMaybe<Array<InputMaybe<Scalars['String']>>>;
regions?: InputMaybe<Array<InputMaybe<Scalars['String']>>>;
website?: InputMaybe<Scalars['String']>;
};

Expand Down
11 changes: 9 additions & 2 deletions global/utils/common.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ export const asEnum = (obj, { name = 'enum' } = {}) =>

const dateFormat = 'yyyy-MM-dd';
export const displayDate = (date: string | Date) => {
const jsDate = typeof date === 'string' ? new Date(date) : date;
// Dates stored as UTC need to be converted to milliseconds; breaks on localhost
const jsDate =
typeof date === 'string'
? parseInt(date)
? new Date(parseInt(date) * 1000)
: new Date(date)
: date;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@demariadaniel demariadaniel Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in 4c3f167a188d39eff09b30a2dc1148876bad07d8

return formatDate(jsDate, dateFormat);
};

Expand Down Expand Up @@ -84,7 +91,7 @@ export const exportToTsv = <Data extends { [k: string]: string | number }>(
include: includeKeys = allKeys,
order = allKeys,
fileName = 'data.tsv',
headerDisplays = allKeys.reduce<typeof options['headerDisplays']>(
headerDisplays = allKeys.reduce<(typeof options)['headerDisplays']>(
(acc, key) => ({
...acc,
[key]: options.headerDisplays ? options.headerDisplays[key] : key,
Expand Down