From f3bda225aa237c92b9be997dcb343fde163e2c94 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 22 Feb 2024 10:50:24 -0500 Subject: [PATCH 01/12] fix: Remove parentheses if studyAbbr is empty --- src/components/Organizations/StudyTooltip.tsx | 51 +++++++++++++++++++ src/content/organizations/ListView.tsx | 41 ++------------- src/utils/formUtils.test.ts | 34 +++++++++++++ src/utils/formUtils.ts | 18 +++++++ 4 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 src/components/Organizations/StudyTooltip.tsx diff --git a/src/components/Organizations/StudyTooltip.tsx b/src/components/Organizations/StudyTooltip.tsx new file mode 100644 index 00000000..90bdd434 --- /dev/null +++ b/src/components/Organizations/StudyTooltip.tsx @@ -0,0 +1,51 @@ +import React, { ElementType, FC } from 'react'; +import { Typography, styled } from '@mui/material'; +import Tooltip from '../Tooltip'; +import { formatFullStudyName } from '../../utils'; + +type Props = { + _id: Organization["_id"]; + studies: Organization["studies"]; +}; + +const StyledStudyCount = styled(Typography)<{ component: ElementType }>(({ theme }) => ({ + textDecoration: "underline", + cursor: "pointer", + color: theme.palette.primary.main, +})); + +const TooltipBody: FC = ({ _id, studies }) => ( + + {studies?.map(({ studyName, studyAbbreviation }) => ( + + {formatFullStudyName(studyName, studyAbbreviation)} +
+ + ))} + +); + +/** + * Organization list view tooltip for studies + * + * @param Props + * @returns {React.FC} + */ +const StudyTooltip: FC = ({ _id, studies }) => ( + } + placement="top" + open={undefined} + onBlur={undefined} + disableHoverListener={false} + arrow + > + + other + {" "} + {studies.length - 1} + + +); + +export default StudyTooltip; diff --git a/src/content/organizations/ListView.tsx b/src/content/organizations/ListView.tsx index 8ef71230..2075a47a 100644 --- a/src/content/organizations/ListView.tsx +++ b/src/content/organizations/ListView.tsx @@ -9,10 +9,10 @@ import { import { Link, LinkProps, useLocation } from "react-router-dom"; import { Controller, useForm } from 'react-hook-form'; import PageBanner from "../../components/PageBanner"; -import Tooltip from '../../components/Tooltip'; import { useOrganizationListContext, Status } from '../../components/Contexts/OrganizationListContext'; import SuspenseLoader from '../../components/SuspenseLoader'; import usePageTitle from '../../hooks/usePageTitle'; +import StudyTooltip from '../../components/Organizations/StudyTooltip'; type T = Partial; @@ -162,12 +162,6 @@ const StyledTablePagination = styled(TablePagination)<{ component: React.Element background: "#F5F7F8", }); -const StyledStudyCount = styled(Typography)<{ component: ElementType }>(({ theme }) => ({ - textDecoration: "underline", - cursor: "pointer", - color: theme.palette.primary.main, -})); - const columns: Column[] = [ { label: "Name", @@ -188,24 +182,9 @@ const columns: Column[] = [ return ( <> - {studies[0].studyAbbreviation} + {studies[0].studyAbbreviation || studies[0].studyName} {studies.length > 1 && " and "} - {studies.length > 1 && ( - } - placement="top" - open={undefined} - onBlur={undefined} - disableHoverListener={false} - arrow - > - - other - {" "} - {studies.length - 1} - - - )} + {studies.length > 1 && ()} ); }, @@ -227,20 +206,6 @@ const columns: Column[] = [ }, ]; -const StudyContent: FC<{ _id: Organization["_id"], studies: Organization["studies"] }> = ({ _id, studies }) => ( - - {studies?.map(({ studyName, studyAbbreviation }) => ( - - {studyName} - {" ("} - {studyAbbreviation} - {") "} -
- - ))} - -); - /** * View for List of Organizations * diff --git a/src/utils/formUtils.test.ts b/src/utils/formUtils.test.ts index f453f255..3f47adfb 100644 --- a/src/utils/formUtils.test.ts +++ b/src/utils/formUtils.test.ts @@ -189,3 +189,37 @@ describe("programToSelectOption cases", () => { expect(selectOption.value).toEqual(""); }); }); + +describe('formatFullStudyName cases', () => { + it('should return the study name with abbreviation if abbreviation is provided', () => { + const studyName = 'Study Name'; + const studyAbbreviation = 'SN'; + const result = utils.formatFullStudyName(studyName, studyAbbreviation); + expect(result).toBe('Study Name (SN)'); + }); + + it('should return the study name without abbreviation if abbreviation is not provided', () => { + const studyName = 'Study Name'; + const result = utils.formatFullStudyName(studyName, ''); + expect(result).toBe('Study Name'); + }); + + it('should return the study name without abbreviation if abbreviation is undefined', () => { + const studyName = 'Study Name'; + const result = utils.formatFullStudyName(studyName, undefined); + expect(result).toBe('Study Name'); + }); + + it('should remove extra spaces from the study name', () => { + const studyName = ' Study Name '; + const result = utils.formatFullStudyName(studyName, ''); + expect(result).toBe('Study Name'); + }); + + it('should remove extra spaces from the study abbreviation', () => { + const studyName = 'Study Name'; + const studyAbbreviation = ' SN '; + const result = utils.formatFullStudyName(studyName, studyAbbreviation); + expect(result).toBe('Study Name (SN)'); + }); +}); diff --git a/src/utils/formUtils.ts b/src/utils/formUtils.ts index ccc3e814..394b313c 100644 --- a/src/utils/formUtils.ts +++ b/src/utils/formUtils.ts @@ -110,3 +110,21 @@ export const programToSelectOption = (program: ProgramOption): SelectOption => ( label: `${program.name || ""}${program.abbreviation ? ` (${program.abbreviation})` : ""}`?.trim(), value: program.name || "" }); + +/** + * Formats an Approved Study Name and Abbreviation into a single string. + * If the abbreviation is provided, it will be enclosed in parentheses. + * + * @example Alphabetic Study (AS) + * @example Alphabetic Study + * @param studyName The full name of the study + * @param studyAbbreviation The abbreviation of the study + * @returns The formatted study name + */ +export const formatFullStudyName = (studyName: string, studyAbbreviation: string): string => { + if (studyAbbreviation && studyAbbreviation.length > 0) { + return `${studyName.trim()} (${studyAbbreviation.trim()})`; + } + + return studyName.trim(); +}; From 0fe3ddbf8f7b5454e66e807015f0c04e46457fdf Mon Sep 17 00:00:00 2001 From: Alec M Date: Wed, 6 Mar 2024 10:09:49 -0500 Subject: [PATCH 02/12] fix: Fallback to `study`.`name` if abbr. is empty --- src/components/Contexts/FormContext.tsx | 2 +- src/content/questionnaire/sections/B.tsx | 1 - src/types/ApprovedStudies.d.ts | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/Contexts/FormContext.tsx b/src/components/Contexts/FormContext.tsx index 5f8378e9..4b64395e 100644 --- a/src/components/Contexts/FormContext.tsx +++ b/src/components/Contexts/FormContext.tsx @@ -172,7 +172,7 @@ export const FormProvider: FC = ({ children, id } : ProviderProps application: { _id: newState?.data?.["_id"] === "new" ? undefined : newState?.data?.["_id"], programName: data?.program?.name, - studyAbbreviation: data?.study?.abbreviation, + studyAbbreviation: data?.study?.abbreviation || data?.study?.name, questionnaireData: JSON.stringify(data), } } diff --git a/src/content/questionnaire/sections/B.tsx b/src/content/questionnaire/sections/B.tsx index 4b4ee1f8..2d41baa1 100644 --- a/src/content/questionnaire/sections/B.tsx +++ b/src/content/questionnaire/sections/B.tsx @@ -379,7 +379,6 @@ const FormSectionB: FC = ({ SectionOption, refs }: FormSection readOnly={readOnlyInputs} hideValidation={readOnlyInputs} tooltipText="Provide a short abbreviation or acronym (e.g., NCI-MATCH) for the study." - required /> Date: Wed, 6 Mar 2024 10:31:55 -0500 Subject: [PATCH 03/12] fix: Identical abbr to name should not appear twice --- src/utils/formUtils.test.ts | 7 +++++++ src/utils/formUtils.ts | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utils/formUtils.test.ts b/src/utils/formUtils.test.ts index 3f47adfb..8c3d1adb 100644 --- a/src/utils/formUtils.test.ts +++ b/src/utils/formUtils.test.ts @@ -222,4 +222,11 @@ describe('formatFullStudyName cases', () => { const result = utils.formatFullStudyName(studyName, studyAbbreviation); expect(result).toBe('Study Name (SN)'); }); + + it("should ignore the abbreviation if its equal to the study name", () => { + const studyName = 'Study Name'; + const studyAbbreviation = 'Study Name'; + const result = utils.formatFullStudyName(studyName, studyAbbreviation); + expect(result).toBe('Study Name'); + }); }); diff --git a/src/utils/formUtils.ts b/src/utils/formUtils.ts index 394b313c..d68e6512 100644 --- a/src/utils/formUtils.ts +++ b/src/utils/formUtils.ts @@ -113,7 +113,7 @@ export const programToSelectOption = (program: ProgramOption): SelectOption => ( /** * Formats an Approved Study Name and Abbreviation into a single string. - * If the abbreviation is provided, it will be enclosed in parentheses. + * If the abbreviation is provided and not equal to the name, it will be included in parentheses. * * @example Alphabetic Study (AS) * @example Alphabetic Study @@ -122,6 +122,9 @@ export const programToSelectOption = (program: ProgramOption): SelectOption => ( * @returns The formatted study name */ export const formatFullStudyName = (studyName: string, studyAbbreviation: string): string => { + if (studyAbbreviation === studyName) { + return studyName.trim(); + } if (studyAbbreviation && studyAbbreviation.length > 0) { return `${studyName.trim()} (${studyAbbreviation.trim()})`; } From 247b3e50ca7e53bdea6711e89c8524bda4d5254b Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 11 Mar 2024 09:27:34 -0400 Subject: [PATCH 04/12] fix: Duplicate studyAbbr appearing --- src/content/organizations/OrganizationView.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/content/organizations/OrganizationView.tsx b/src/content/organizations/OrganizationView.tsx index 38bf9421..04bab2ec 100644 --- a/src/content/organizations/OrganizationView.tsx +++ b/src/content/organizations/OrganizationView.tsx @@ -22,6 +22,7 @@ import { } from '../../graphql'; import ConfirmDialog from '../../components/Organizations/ConfirmDialog'; import usePageTitle from '../../hooks/usePageTitle'; +import { formatFullStudyName } from '../../utils'; type Props = { _id: Organization["_id"] | "new"; @@ -415,10 +416,7 @@ const OrganizationView: FC = ({ _id }: Props) => { > {approvedStudies?.listApprovedStudies?.map(({ studyName, studyAbbreviation }) => ( - {studyName} - {" ("} - {studyAbbreviation} - {") "} + {formatFullStudyName(studyName, studyAbbreviation)} ))} From a87ec35d4f0be05314c4bc8d71dc95aa328b2a20 Mon Sep 17 00:00:00 2001 From: Alec M Date: Tue, 12 Mar 2024 10:59:31 -0400 Subject: [PATCH 05/12] refactor: Prefer study `_id` over study abbreviation --- .../organizations/OrganizationView.tsx | 34 +++++---- src/utils/formUtils.test.ts | 75 +++++++++++++++++++ src/utils/formUtils.ts | 19 +++++ 3 files changed, 114 insertions(+), 14 deletions(-) diff --git a/src/content/organizations/OrganizationView.tsx b/src/content/organizations/OrganizationView.tsx index 04bab2ec..2843f045 100644 --- a/src/content/organizations/OrganizationView.tsx +++ b/src/content/organizations/OrganizationView.tsx @@ -22,17 +22,14 @@ import { } from '../../graphql'; import ConfirmDialog from '../../components/Organizations/ConfirmDialog'; import usePageTitle from '../../hooks/usePageTitle'; -import { formatFullStudyName } from '../../utils'; +import { formatFullStudyName, mapOrganizationStudyToId } from '../../utils'; type Props = { _id: Organization["_id"] | "new"; }; type FormInput = Omit & { - /** - * Select boxes cannot contain objects, using `studyAbbreviation` instead - */ - studies: ApprovedStudy["studyAbbreviation"][]; + studies: ApprovedStudy["_id"][]; }; const StyledContainer = styled(Container)({ @@ -173,6 +170,7 @@ const OrganizationView: FC = ({ _id }: Props) => { const activeSubs = dataSubmissions?.filter((ds) => !inactiveSubmissionStatus.includes(ds?.status)); organization?.studies?.forEach((s) => { + // NOTE: The `Submission` type only has `studyAbbreviation`, we cannot compare IDs if (activeSubs?.some((ds) => ds?.studyAbbreviation === s?.studyAbbreviation)) { activeStudies[s?.studyAbbreviation] = true; } @@ -194,7 +192,7 @@ const OrganizationView: FC = ({ _id }: Props) => { fetchPolicy: "cache-and-network", }); - const { data: approvedStudies } = useQuery(LIST_APPROVED_STUDIES, { + const { data: approvedStudies, refetch: refetchStudies } = useQuery(LIST_APPROVED_STUDIES, { context: { clientName: 'backend' }, fetchPolicy: "cache-and-network", }); @@ -240,14 +238,14 @@ const OrganizationView: FC = ({ _id }: Props) => { const onSubmit = async (data: FormInput) => { setSaving(true); - const studyAbbrToName: { [studyAbbreviation: string]: Pick } = {}; - approvedStudies?.listApprovedStudies?.forEach(({ studyName, studyAbbreviation }) => { - studyAbbrToName[studyAbbreviation] = { studyName, studyAbbreviation }; + const studyMap: { [_id: string]: Pick } = {}; + approvedStudies?.listApprovedStudies?.forEach(({ _id, studyName, studyAbbreviation }) => { + studyMap[_id] = { studyName, studyAbbreviation }; }); const variables = { ...data, - studies: data.studies.map((abbr) => (studyAbbrToName[abbr])).filter((s) => !!s?.studyName && !!s?.studyAbbreviation), + studies: data.studies.map((_id) => (studyMap[_id]))?.filter((s) => !!s?.studyName) || [], }; if (_id === "new" && !organization?._id) { @@ -314,17 +312,25 @@ const OrganizationView: FC = ({ _id }: Props) => { (async () => { const { data, error } = await getOrganization({ variables: { orgID: _id, organization: _id } }); - if (error || !data?.getOrganization) { navigate("/organizations", { state: { error: "Unable to fetch organization" } }); return; } + // No studies or original request did not complete. Refetch + let studyList: ApprovedStudy[] = approvedStudies?.listApprovedStudies; + if (!studyList?.length) { + const { data } = await refetchStudies(); + studyList = data?.listApprovedStudies; + } + setOrganization(data?.getOrganization); setDataSubmissions(data?.listSubmissions?.submissions); setFormValues({ ...data?.getOrganization, - studies: data?.getOrganization?.studies?.filter((s) => !!s?.studyName && !!s?.studyAbbreviation).map(({ studyAbbreviation }) => studyAbbreviation) || [], + studies: data?.getOrganization?.studies + ?.map((s) => mapOrganizationStudyToId(s, studyList || [])) + ?.filter((_id) => !!_id) || [], }); })(); }, [_id]); @@ -414,8 +420,8 @@ const OrganizationView: FC = ({ _id }: Props) => { inputProps={{ "aria-labelledby": "studiesLabel" }} multiple > - {approvedStudies?.listApprovedStudies?.map(({ studyName, studyAbbreviation }) => ( - + {approvedStudies?.listApprovedStudies?.map(({ _id, studyName, studyAbbreviation }) => ( + {formatFullStudyName(studyName, studyAbbreviation)} ))} diff --git a/src/utils/formUtils.test.ts b/src/utils/formUtils.test.ts index 8c3d1adb..88856fa0 100644 --- a/src/utils/formUtils.test.ts +++ b/src/utils/formUtils.test.ts @@ -230,3 +230,78 @@ describe('formatFullStudyName cases', () => { expect(result).toBe('Study Name'); }); }); + +describe('mapOrganizationStudyToId cases', () => { + it('should return the id of the matching study', () => { + const studies = [ + { _id: '1', studyName: 'Study 1', studyAbbreviation: 'S1' }, + { _id: '2', studyName: 'Study 2', studyAbbreviation: 'S2' }, + ] as ApprovedStudy[]; + + const study = { studyName: 'Study 1', studyAbbreviation: 'S1' }; + const result = utils.mapOrganizationStudyToId(study, studies); + + expect(result).toBe('1'); + }); + + it("should return the first matching study's id", () => { + const studies = [ + { _id: '1', studyName: 'MATCH', studyAbbreviation: 'MA' }, + { _id: '2', studyName: 'Study 2', studyAbbreviation: 'S2' }, + { _id: '3', studyName: 'MATCH', studyAbbreviation: 'MA' }, + ] as ApprovedStudy[]; + + const study = { studyName: 'MATCH', studyAbbreviation: 'MA' }; + const result = utils.mapOrganizationStudyToId(study, studies); + + expect(result).toBe('1'); + }); + + it('should return an empty string if no matching study is found', () => { + const studies = [ + { _id: '1', studyName: 'Study 1', studyAbbreviation: 'S1' }, + { _id: '2', studyName: 'Study 2', studyAbbreviation: 'S2' }, + ] as ApprovedStudy[]; + + const study = { studyName: 'Study 3', studyAbbreviation: 'S3' }; + const result = utils.mapOrganizationStudyToId(study, studies); + + expect(result).toBe(''); + }); + + it("should not throw an exception if the study is undefined", () => { + const studies = [ + { _id: '1', studyName: 'Study 1', studyAbbreviation: 'S1' }, + { _id: '2', studyName: 'Study 2', studyAbbreviation: 'S2' }, + ] as ApprovedStudy[]; + + expect(() => utils.mapOrganizationStudyToId(undefined, studies)).not.toThrow(); + }); + + it("should not throw an exception if the study is null", () => { + const studies = [ + { _id: '1', studyName: 'Study 1', studyAbbreviation: 'S1' }, + { _id: '2', studyName: 'Study 2', studyAbbreviation: 'S2' }, + ] as ApprovedStudy[]; + + expect(() => utils.mapOrganizationStudyToId(null, studies)).not.toThrow(); + }); + + it("should not throw an exception if the approved studies are corrupt", () => { + const studies = [ + null, + { invalidObject: "true" }, + { AAAA: undefined }, + ] as unknown as ApprovedStudy[]; + + const study = { studyName: 'Study 1', studyAbbreviation: 'S1' }; + + expect(() => utils.mapOrganizationStudyToId(study, studies)).not.toThrow(); + }); + + it("should not throw an exception if the approved studies are undefined", () => { + const study = { studyName: 'Study 1', studyAbbreviation: 'S1' }; + + expect(() => utils.mapOrganizationStudyToId(study, undefined)).not.toThrow(); + }); +}); diff --git a/src/utils/formUtils.ts b/src/utils/formUtils.ts index d68e6512..a311743f 100644 --- a/src/utils/formUtils.ts +++ b/src/utils/formUtils.ts @@ -131,3 +131,22 @@ export const formatFullStudyName = (studyName: string, studyAbbreviation: string return studyName.trim(); }; + +/** + * Attempts to map a study name + abbreviation combination to an approved study ID. + * + * - Will return the first match found + * - If no match is found, an empty string is returned + * + * @param Study information from Organization object + * @param studies List of approved studies + * @returns The ID of the study if found, otherwise an empty string + */ +export const mapOrganizationStudyToId = ( + orgStudy: Pick, + studies: Pick[] +): ApprovedStudy["_id"] => { + const { studyName, studyAbbreviation } = orgStudy || {}; + + return studies?.find((study) => study?.studyName === studyName && study?.studyAbbreviation === studyAbbreviation)?._id || ""; +}; From a87db9834366ba35ce7e7afacc05423c7e28869d Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 14 Mar 2024 10:35:35 -0400 Subject: [PATCH 06/12] CRDCDH-956 Sort charts by Node Total Also add missing test cases for stats --- src/utils/statisticUtils.test.ts | 94 ++++++++++++++++++++++++++++++++ src/utils/statisticUtils.ts | 23 ++++++-- 2 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 src/utils/statisticUtils.test.ts diff --git a/src/utils/statisticUtils.test.ts b/src/utils/statisticUtils.test.ts new file mode 100644 index 00000000..64406ead --- /dev/null +++ b/src/utils/statisticUtils.test.ts @@ -0,0 +1,94 @@ +import * as utils from "./statisticUtils"; + +describe("compareNodeStats cases", () => { + const node1A = { total: 1, nodeName: "NodeA" } as SubmissionStatistic; + const node1B = { total: 1, nodeName: "NodeB" } as SubmissionStatistic; + const node3A = { total: 3, nodeName: "AAA Node" } as SubmissionStatistic; + const node3B = { total: 3, nodeName: "Node3" } as SubmissionStatistic; + const node5 = { total: 5, nodeName: "Node5" } as SubmissionStatistic; + + it("should correctly sort in ascending order by node.total", () => { + const sortedStats = [node3B, node1A, node5].sort(utils.compareNodeStats); + + expect(sortedStats).toEqual([node1A, node3B, node5]); + }); + + it("should correctly sort in ascending order by node.nodeName when node.total is equal", () => { + const sortedStats = [node3B, node1A, node3A, node1B].sort(utils.compareNodeStats); + + expect(sortedStats).toEqual([node1A, node1B, node3A, node3B]); + }); + it("should return 1 when a.total is greater than b.total", () => { + const a = { total: 2, nodeName: "Node 1" } as SubmissionStatistic; + const b = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(1); + }); + + it("should return 1 when a.nodeName comes after b.nodeName", () => { + const a = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; + const b = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(1); + }); + + it("should return 0 when both nodes are equal 1/2", () => { + const a = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; + const b = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(0); + }); + + it("should return 0 when both nodes are equal 2/2", () => { + const a = { total: 0, nodeName: "" } as SubmissionStatistic; + const b = { total: 0, nodeName: "" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(0); + }); + + it("should return -1 when a.total is less than b.total", () => { + const a = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; + const b = { total: 2, nodeName: "Node 2" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(-1); + }); + + it("should return -1 when a.nodeName comes before b.nodeName", () => { + const a = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; + const b = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; + + expect(utils.compareNodeStats(a, b)).toEqual(-1); + }); +}); + +describe('calculateMaxDomain cases', () => { + it.each([-1, NaN, undefined, 0, Infinity, -Infinity])('should default to 1 when dataMax is invalid (%s)', (dataMax) => { + expect(utils.calculateMaxDomain(dataMax)).toBe(1); + }); + + it('should round up to the nearest 1,000 when dataMax above 1,000', () => { + expect(utils.calculateMaxDomain(1001)).toBe(2000); + expect(utils.calculateMaxDomain(2500)).toBe(3000); + expect(utils.calculateMaxDomain(10000)).toBe(10000); + expect(utils.calculateMaxDomain(23949)).toBe(24000); + }); + + it('should round up to the nearest 100 when dataMax is between 100 and 1,000', () => { + expect(utils.calculateMaxDomain(101)).toBe(200); + expect(utils.calculateMaxDomain(550)).toBe(600); + expect(utils.calculateMaxDomain(1000)).toBe(1000); + }); + + it('should round up to the nearest 10 when dataMax is between 10 and 100', () => { + expect(utils.calculateMaxDomain(11)).toBe(20); + expect(utils.calculateMaxDomain(55)).toBe(60); + expect(utils.calculateMaxDomain(99)).toBe(100); + expect(utils.calculateMaxDomain(100)).toBe(100); + }); + + it("should round up to the nearest 10 when dataMax is between 1 and 10", () => { + expect(utils.calculateMaxDomain(1)).toBe(10); + expect(utils.calculateMaxDomain(5)).toBe(10); + expect(utils.calculateMaxDomain(10)).toBe(10); + }); +}); diff --git a/src/utils/statisticUtils.ts b/src/utils/statisticUtils.ts index fb67f86c..32380ded 100644 --- a/src/utils/statisticUtils.ts +++ b/src/utils/statisticUtils.ts @@ -31,14 +31,25 @@ export const buildPrimaryChartSeries = (stats: SubmissionStatistic[], omitSeries })); /** - * A utility function to sort the node statistics by the node name + * A utility function to sort the node statistics in ascending order by: + * + * - SubmissionStatistic.total (primary) + * - SubmissionStatistic.nodeName (secondary) + * * This utility is required because the API does not guarantee the order of the nodes, * and changing the order of the nodes causes re-animation of the charts. * - * @param stats Data Submissions statistics - * @returns The sorted statistics + * @param a The first SubmissionStatistic + * @param b The second SubmissionStatistic + * @returns The comparison result */ -export const compareNodeStats = (a: SubmissionStatistic, b: SubmissionStatistic) => a.nodeName.localeCompare(b.nodeName); +export const compareNodeStats = (a: SubmissionStatistic, b: SubmissionStatistic): number => { + if (a.total === b.total) { + return a.nodeName.localeCompare(b.nodeName); + } + + return a.total - b.total; +}; /** * Format a Y-Axis tick label @@ -62,8 +73,8 @@ export const formatTick = (tick: number, normalized = false) => (normalized * @param dataMax The maximum value in the dataset * @returns The calculated maximum domain */ -export const calculateMaxDomain = (dataMax: number) => { - if (dataMax <= 0 || Number.isNaN(dataMax)) { +export const calculateMaxDomain = (dataMax: number): number => { + if (dataMax <= 0 || Number.isNaN(dataMax) || !Number.isFinite(dataMax)) { return 1; } if (dataMax > 1000) { From 0367de4214b3ddffab69aa9d291a3e82d5d7d334 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 14 Mar 2024 10:44:00 -0400 Subject: [PATCH 07/12] fix: Test cases using wrong comparison The test cases would break if the difference in node totals is greater than 1 --- src/utils/statisticUtils.test.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/utils/statisticUtils.test.ts b/src/utils/statisticUtils.test.ts index 64406ead..9f85f634 100644 --- a/src/utils/statisticUtils.test.ts +++ b/src/utils/statisticUtils.test.ts @@ -18,18 +18,18 @@ describe("compareNodeStats cases", () => { expect(sortedStats).toEqual([node1A, node1B, node3A, node3B]); }); - it("should return 1 when a.total is greater than b.total", () => { - const a = { total: 2, nodeName: "Node 1" } as SubmissionStatistic; + it("should return >1 when a.total is greater than b.total", () => { + const a = { total: 5, nodeName: "Node 1" } as SubmissionStatistic; const b = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; - expect(utils.compareNodeStats(a, b)).toEqual(1); + expect(utils.compareNodeStats(a, b)).toBeGreaterThan(0); }); - it("should return 1 when a.nodeName comes after b.nodeName", () => { + it("should return >0 when a.nodeName comes after b.nodeName", () => { const a = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; const b = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; - expect(utils.compareNodeStats(a, b)).toEqual(1); + expect(utils.compareNodeStats(a, b)).toBeGreaterThan(0); }); it("should return 0 when both nodes are equal 1/2", () => { @@ -46,18 +46,18 @@ describe("compareNodeStats cases", () => { expect(utils.compareNodeStats(a, b)).toEqual(0); }); - it("should return -1 when a.total is less than b.total", () => { + it("should return 0< when a.total is less than b.total", () => { const a = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; - const b = { total: 2, nodeName: "Node 2" } as SubmissionStatistic; + const b = { total: 4, nodeName: "Node 2" } as SubmissionStatistic; - expect(utils.compareNodeStats(a, b)).toEqual(-1); + expect(utils.compareNodeStats(a, b)).toBeLessThan(0); }); - it("should return -1 when a.nodeName comes before b.nodeName", () => { + it("should return 0< when a.nodeName comes before b.nodeName", () => { const a = { total: 1, nodeName: "Node 1" } as SubmissionStatistic; const b = { total: 1, nodeName: "Node 2" } as SubmissionStatistic; - expect(utils.compareNodeStats(a, b)).toEqual(-1); + expect(utils.compareNodeStats(a, b)).toBeLessThan(0); }); }); From ec48f8bad90f94906a7eeaaf3a65f1a98c309a5c Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 14 Mar 2024 16:36:48 -0400 Subject: [PATCH 08/12] fix: Random test failures in CI --- src/components/Contexts/AuthContext.test.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/Contexts/AuthContext.test.tsx b/src/components/Contexts/AuthContext.test.tsx index 52930d06..17781a13 100644 --- a/src/components/Contexts/AuthContext.test.tsx +++ b/src/components/Contexts/AuthContext.test.tsx @@ -145,10 +145,11 @@ describe("AuthContext > AuthProvider Tests", () => { const screen = render(); - await waitFor(() => expect(screen.getByTestId("first-name").textContent).toEqual("The API updated my first name")); - - const cachedUser = JSON.parse(localStorage.getItem("userDetails")); - expect(cachedUser.firstName).toEqual("The API updated my first name"); + await waitFor(() => expect(screen.getByTestId("first-name").textContent).toEqual(mocks[0].result.data.getMyUser.firstName)); + await waitFor(() => { + const cachedUser = JSON.parse(localStorage.getItem("userDetails")); + expect(cachedUser.firstName).toEqual(mocks[0].result.data.getMyUser.firstName); + }, { timeout: 1000 }); }); it("should logout the user if the BE API call fails", async () => { From caebe5c45b90711a093e8189074f82e84f990de8 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 14 Mar 2024 16:51:28 -0400 Subject: [PATCH 09/12] feat: Split CI jobs and upgrade Node version --- .../workflows/{code-certify.yml => lint.yml} | 9 ++-- .github/workflows/test.yml | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) rename .github/workflows/{code-certify.yml => lint.yml} (86%) create mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/code-certify.yml b/.github/workflows/lint.yml similarity index 86% rename from .github/workflows/code-certify.yml rename to .github/workflows/lint.yml index ced362ea..29f5b366 100644 --- a/.github/workflows/code-certify.yml +++ b/.github/workflows/lint.yml @@ -1,4 +1,4 @@ -name: Certify Changes +name: Lint Changes on: workflow_dispatch: @@ -18,7 +18,7 @@ permissions: jobs: certify: - name: Certify Build Changes + name: Lint Changes runs-on: ubuntu-latest steps: - name: Checkout Repository @@ -27,7 +27,7 @@ jobs: - name: Setup Node uses: actions/setup-node@v4 with: - node-version: "18.x" + node-version: "20.x" cache: "npm" - name: Cache node modules @@ -44,8 +44,5 @@ jobs: - name: Run Typecheck run: npm run check:ci - - name: Run Jest - run: npm run test:ci - - name: Run Linter run: npm run lint:ci diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..17f747fc --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,45 @@ +name: Test Changes + +on: + workflow_dispatch: + pull_request: + branches: "*" + paths: + - "src/**" + - "public/**" + - "*.json" + - "*.js" + - "*.jsx" + - "*.ts" + - "*.tsx" + +permissions: + contents: read + +jobs: + certify: + name: Test Changes + runs-on: ubuntu-latest + steps: + - name: Checkout Repository + uses: actions/checkout@v3 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: "20.x" + cache: "npm" + + - name: Cache node modules + uses: actions/cache@v3 + with: + path: node_modules + key: ${{ runner.os }}-modules-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-modules- + + - name: Install Dependencies + run: npm install --legacy-peer-deps + + - name: Run Jest + run: npm run test:ci From e5b586cb32ed504c0e66c520b884d7e90c82ca71 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Fri, 15 Mar 2024 15:20:31 -0400 Subject: [PATCH 10/12] Set default tab and made workflow buttons always visible in all tabs --- src/content/dataSubmissions/DataSubmission.tsx | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/content/dataSubmissions/DataSubmission.tsx b/src/content/dataSubmissions/DataSubmission.tsx index c9a14e5c..050f794c 100644 --- a/src/content/dataSubmissions/DataSubmission.tsx +++ b/src/content/dataSubmissions/DataSubmission.tsx @@ -6,7 +6,6 @@ import { Button, Card, CardActions, - CardActionsProps, CardContent, Container, IconButton, @@ -117,13 +116,10 @@ const StyledMainContentArea = styled("div")(() => ({ padding: "21px 40px 0", })); -const StyledCardActions = styled(CardActions, { - shouldForwardProp: (prop) => prop !== "isVisible" -})(({ isVisible }) => ({ +const StyledCardActions = styled(CardActions)(() => ({ "&.MuiCardActions-root": { - visibility: isVisible ? "visible" : "hidden", paddingTop: 0, - } + }, })); const StyledTabs = styled(Tabs)(() => ({ @@ -337,7 +333,7 @@ type Props = { tab: string; }; -const DataSubmission: FC = ({ submissionId, tab }) => { +const DataSubmission: FC = ({ submissionId, tab = URLTabs.DATA_ACTIVITY }) => { usePageTitle(`Data Submission ${submissionId || ""}`); const { user } = useAuthContext(); @@ -597,7 +593,7 @@ const DataSubmission: FC = ({ submissionId, tab }) => { - + Date: Mon, 18 Mar 2024 12:56:01 -0400 Subject: [PATCH 11/12] fix: AuthCtx expectation failing --- src/components/Contexts/AuthContext.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Contexts/AuthContext.test.tsx b/src/components/Contexts/AuthContext.test.tsx index 17781a13..58171588 100644 --- a/src/components/Contexts/AuthContext.test.tsx +++ b/src/components/Contexts/AuthContext.test.tsx @@ -179,7 +179,8 @@ describe("AuthContext > AuthProvider Tests", () => { await waitFor(() => expect(screen.getByTestId("isLoggedIn").textContent).toEqual("false")); - expect(screen.getByTestId("isLoggedIn").textContent).toEqual("false"); - expect(localStorage.getItem("userDetails")).toBeNull(); + await waitFor(() => { + expect(localStorage.getItem("userDetails")).toBeNull(); + }, { timeout: 1000 }); }); }); From b63d83e119633cc7320ff5957d3be973a00b8cc2 Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 18 Mar 2024 13:18:08 -0400 Subject: [PATCH 12/12] feat: Add Coveralls coverage reporting --- .github/workflows/lint.yml | 2 +- .github/workflows/test.yml | 5 ++++- package.json | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 29f5b366..fa8e0ba0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -17,7 +17,7 @@ permissions: contents: read jobs: - certify: + lint: name: Lint Changes runs-on: ubuntu-latest steps: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 17f747fc..837e49a5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,7 +17,7 @@ permissions: contents: read jobs: - certify: + test: name: Test Changes runs-on: ubuntu-latest steps: @@ -43,3 +43,6 @@ jobs: - name: Run Jest run: npm run test:ci + + - name: Coveralls GitHub Action + uses: coverallsapp/github-action@v2.2.3 diff --git a/package.json b/package.json index 388d51bd..8fb6e631 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "start": "react-scripts start", "build": "react-scripts build", "test": "TZ=UTC react-scripts test", - "test:ci": "TZ=UTC CI=true react-scripts test --passWithNoTests", + "test:ci": "TZ=UTC CI=true react-scripts test --passWithNoTests --coverage", "eject": "react-scripts eject", "lint": "eslint . --ignore-path .gitignore", "lint:fix": "eslint --fix . --ignore-path .gitignore",