From 3bad7372475fb4b53010837b23a34b37e4442528 Mon Sep 17 00:00:00 2001 From: Theo Sanderson Date: Wed, 21 Feb 2024 16:53:40 +0000 Subject: [PATCH] address code review --- .../DownloadDialog/DownloadDialog.tsx | 2 +- .../DownloadDialog/DownloadForm.tsx | 2 +- .../components/Submission/DataUploadForm.tsx | 328 +++++++++--------- .../components/Submission/DateChangeModal.tsx | 87 ++++- .../components/Submission/RevisionForm.tsx | 5 +- .../Submission/SubmissionForm.spec.tsx | 14 +- .../components/Submission/SubmissionForm.tsx | 3 +- 7 files changed, 275 insertions(+), 166 deletions(-) diff --git a/website/src/components/SearchPage/DownloadDialog/DownloadDialog.tsx b/website/src/components/SearchPage/DownloadDialog/DownloadDialog.tsx index 794961145..101b9ff43 100644 --- a/website/src/components/SearchPage/DownloadDialog/DownloadDialog.tsx +++ b/website/src/components/SearchPage/DownloadDialog/DownloadDialog.tsx @@ -73,7 +73,7 @@ export const DownloadDialog: FC = ({ /> I agree to the {/* TODO(862) */} - + data use terms . diff --git a/website/src/components/SearchPage/DownloadDialog/DownloadForm.tsx b/website/src/components/SearchPage/DownloadDialog/DownloadForm.tsx index 56f1a848a..23a9cdb4b 100644 --- a/website/src/components/SearchPage/DownloadDialog/DownloadForm.tsx +++ b/website/src/components/SearchPage/DownloadDialog/DownloadForm.tsx @@ -80,7 +80,7 @@ export const DownloadForm: FC = ({ referenceGenomesSequenceNa <> Yes, include restricted data
({/* TODO(862) */} - + What does it mean? ) diff --git a/website/src/components/Submission/DataUploadForm.tsx b/website/src/components/Submission/DataUploadForm.tsx index 9e5778b7f..114506605 100644 --- a/website/src/components/Submission/DataUploadForm.tsx +++ b/website/src/components/Submission/DataUploadForm.tsx @@ -9,7 +9,12 @@ import { getClientLogger } from '../../clientLogger.ts'; import { routes } from '../../routes.ts'; import { backendApi } from '../../services/backendApi.ts'; import { backendClientHooks } from '../../services/serviceHooks.ts'; -import { type DataUseTermsType, openDataUseTermsType, restrictedDataUseTermsType } from '../../types/backend.ts'; +import { + type DataUseTermsType, + openDataUseTermsType, + restrictedDataUseTermsType, + type Group, +} from '../../types/backend.ts'; import type { ClientConfig } from '../../types/runtimeConfig.ts'; import { dateTimeInMonths } from '../../utils/DateTimeInMonths.tsx'; import { createAuthorizationHeader } from '../../utils/createAuthorizationHeader.ts'; @@ -29,13 +34,152 @@ type DataUploadFormProps = { organism: string; clientConfig: ClientConfig; action: Action; - groupsOfUser: any[]; // figure out what to do here + groupsOfUser: Group[]; onSuccess: () => void; onError: (message: string) => void; }; const logger = getClientLogger('DataUploadForm'); +const DataUseTerms = ({ + dataUseTermsType, + setDataUseTermsType, + restrictedUntil, + setRestrictedUntil, +}: { + dataUseTermsType: DataUseTermsType; + setDataUseTermsType: (dataUseTermsType: DataUseTermsType) => void; + restrictedUntil: DateTime; + setRestrictedUntil: (restrictedUntil: DateTime) => void; +}) => { + const [dateChangeModalOpen, setDateChangeModalOpen] = useState(false); + + return ( +
+ {dateChangeModalOpen && ( + + )} +
+

Terms of use

+

Specify how your data can be used

+
+
+
+ +
+
+
+ setDataUseTermsType(openDataUseTermsType)} + type='radio' + checked={dataUseTermsType === openDataUseTermsType} + className='h-4 w-4 border-gray-300 text-iteal-600 focus:ring-iteal-600' + /> + +
+
+ Anyone can use and share the data (though we believe researchers should exercise + scientific etiquette, including the importance of citation). Data will be released to + the INSDC databases shortly after submission.{' '} + + Find out more + + . +
+ +
+ setDataUseTermsType(restrictedDataUseTermsType)} + type='radio' + checked={dataUseTermsType === restrictedDataUseTermsType} + className='h-4 w-4 border-gray-300 text-iteal-600 focus:ring-iteal-600' + /> + +
+ +
+ Data will be restricted for a period of time. The sequences will be available but there + will be limitations on how they can be used by others.{' '} + + Find out more + + . +
+ {dataUseTermsType === restrictedDataUseTermsType && ( +
+ Data will be restricted until {restrictedUntil.toFormat('yyyy-MM-dd')}.{' '} + +
+ )} +
+
+
+
+
+ ); +}; + +const DevExampleData = ({ + setExampleEntries, + exampleEntries, + metadataFile, + sequenceFile, + handleLoadExampleData, +}: { + setExampleEntries: (entries: number) => void; + exampleEntries: number | undefined; + metadataFile: File | null; + sequenceFile: File | null; + handleLoadExampleData: () => void; +}) => { + return ( +

+ Add dev example data +
+ setExampleEntries(parseInt(event.target.value, 10))} + className='w-32' + /> + {' '} +
+ {metadataFile && sequenceFile && Example data loaded} +

+ ); +}; + const GroupSelector = ({ groupNames, selectedGroupName, @@ -57,16 +201,21 @@ const GroupSelector = ({ - + {groupNames.map((groupName) => ( {({ active }) => ( )} @@ -78,9 +227,7 @@ const GroupSelector = ({ ); }; -// disable eslint to allow Icon prop: - -const UploadForm = ({ +const UploadComponent = ({ setFile, name, title, @@ -207,9 +354,6 @@ const InnerDataUploadForm = ({ const [metadataFile, setMetadataFile] = useState(null); const [sequenceFile, setSequenceFile] = useState(null); const [exampleEntries, setExampleEntries] = useState(10); - const metadataFileInputRef = useRef(null); - const sequenceFileInputRef = useRef(null); - // initial license change date is 6 months from now const noGroup = useMemo(() => groupsOfUser.length === 0, [groupsOfUser]); @@ -219,7 +363,6 @@ const InnerDataUploadForm = ({ ); const [dataUseTermsType, setDataUseTermsType] = useState(openDataUseTermsType); const [restrictedUntil, setRestrictedUntil] = useState(dateTimeInMonths(6)); - const [dateChangeModalOpen, setDateChangeModalOpen] = useState(false); const handleLoadExampleData = async () => { const { metadataFileContent, revisedMetadataFileContent, sequenceFileContent } = getExampleData(exampleEntries); @@ -248,10 +391,6 @@ const InnerDataUploadForm = ({ switch (action) { case 'submit': const groupName = selectedGroupName ?? groupsOfUser[0].groupName; - if (groupName === undefined) { - onError('Please select a group'); - return; - } submit({ metadataFile, sequenceFile, @@ -267,34 +406,6 @@ const InnerDataUploadForm = ({ } }; - useEffect(() => { - const interval = setInterval(() => { - // Check for files which are no longer readable - which generally indicates the file has been edited since being - // selected in the UI - and clear these. - metadataFile - ?.slice(0, 1) - .arrayBuffer() - .catch(() => { - setMetadataFile(null); - if (metadataFileInputRef.current) { - metadataFileInputRef.current.value = ''; - } - }); - - sequenceFile - ?.slice(0, 1) - .arrayBuffer() - .catch(() => { - setSequenceFile(null); - if (sequenceFileInputRef.current) { - sequenceFileInputRef.current.value = ''; - } - }); - }, 500); - - return () => clearInterval(interval); - }, [metadataFile, sequenceFile]); - if (noGroup) { return (
@@ -318,15 +429,6 @@ const InnerDataUploadForm = ({ return (
- {dateChangeModalOpen && ( - - )} group.groupName)} selectedGroupName={selectedGroupName} @@ -350,41 +452,27 @@ const InnerDataUploadForm = ({ )} For more information on the format in which data should be uploaded and the required metadata, please refer to our{' '} - + help pages .

{organism.startsWith('dummy-organism') && action === 'submit' && ( -

- Add dev example data -
- setExampleEntries(parseInt(event.target.value, 10))} - className='w-32' - /> - {' '} -
- {metadataFile && sequenceFile && ( - Example data loaded - )} -

+ )}
-
-
{action !== 'revise' && ( -
-
-

Terms of use

-

Specify how your data can be used

-
-
-
- -
-
-
- setDataUseTermsType(openDataUseTermsType)} - type='radio' - checked={dataUseTermsType === openDataUseTermsType} - className='h-4 w-4 border-gray-300 text-iteal-600 focus:ring-iteal-600' - /> - -
-
- Anyone can use and share the data (though we believe researchers should - exercise scientific etiquette, including the importance of citation). Data - will be released to the INSDC databases shortly after submission.{' '} - - Find out more - - . -
- -
- setDataUseTermsType(restrictedDataUseTermsType)} - type='radio' - checked={dataUseTermsType === restrictedDataUseTermsType} - className='h-4 w-4 border-gray-300 text-iteal-600 focus:ring-iteal-600' - /> - -
- -
- Data will be restricted for a period of time. The sequences will be - available but there will be limitations on how they can be used by others.{' '} - - Find out more - - . -
- {dataUseTermsType === restrictedDataUseTermsType && ( -
- Data will be restricted until{' '} - {restrictedUntil.toFormat('yyyy-MM-dd')}.{' '} - -
- )} -
-
-
-
-
+ )}