From 71b4c2e865234404186e9e04b37a8717b0976612 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Wed, 13 Mar 2024 17:37:41 -0400 Subject: [PATCH 1/5] Fixed error dialog horizontal scrolling --- src/content/dataSubmissions/ErrorDialog.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/content/dataSubmissions/ErrorDialog.tsx b/src/content/dataSubmissions/ErrorDialog.tsx index cb01130e..6edf4d09 100644 --- a/src/content/dataSubmissions/ErrorDialog.tsx +++ b/src/content/dataSubmissions/ErrorDialog.tsx @@ -88,6 +88,7 @@ const StyledErrorItem = styled(Typography)({ fontStyle: "normal", fontWeight: 400, lineHeight: "22px", + wordBreak: "break-word" }); const StyledErrors = styled(Stack)({ From 427cb1332b066c7ee2882ade85e2d97902cca988 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Wed, 13 Mar 2024 17:38:54 -0400 Subject: [PATCH 2/5] Updated shouldDisableSubmit utility and tests to include submission level errors such as orphaned Data Files --- src/utils/dataSubmissionUtils.test.ts | 302 ++++++++++++++++++++++---- src/utils/dataSubmissionUtils.ts | 7 +- 2 files changed, 262 insertions(+), 47 deletions(-) diff --git a/src/utils/dataSubmissionUtils.test.ts b/src/utils/dataSubmissionUtils.test.ts index d15997ba..a8580cc7 100644 --- a/src/utils/dataSubmissionUtils.test.ts +++ b/src/utils/dataSubmissionUtils.test.ts @@ -1,166 +1,378 @@ -import * as utils from './dataSubmissionUtils'; -import { SubmitInfo } from './dataSubmissionUtils'; +import * as utils from "./dataSubmissionUtils"; +import { SubmitInfo } from "./dataSubmissionUtils"; -describe('General Submit', () => { - it('should disable submit without isAdminOverride when user role is not Admin but there are validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Error', 'Error', 'Submitter'); +const baseSubmission: Submission = { + _id: "1234", + name: "test123", + submitterID: "1", + submitterName: "User", + organization: undefined, + dataCommons: "", + modelVersion: "", + studyAbbreviation: "", + dbGaPID: "", + bucketName: "", + rootPath: "", + status: "New", + metadataValidationStatus: null, + fileValidationStatus: null, + fileErrors: [], + history: [], + conciergeName: "", + conciergeEmail: "", + createdAt: "", + updatedAt: "", +}; + +describe("General Submit", () => { + it("should disable submit without isAdminOverride when user role is not Admin but there are validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Error", + fileValidationStatus: "Error", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when metadata passed but files has validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Error', 'Submitter'); + it("should disable submit when metadata passed but files has validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Error", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when files passed but metadata has validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Error', 'Passed', 'Submitter'); + it("should disable submit when files passed but metadata has validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Error", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when metadata validation is "Warning" and file validation is "Error"', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Warning', 'Error', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Warning", + fileValidationStatus: "Error", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when metadata validation is "Error" and file validation is "Warning"', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Error', 'Warning', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Error", + fileValidationStatus: "Warning", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should not disable submit when user role is not Admin and there are no validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Passed', 'Submitter'); + it("should not disable submit when user role is not Admin and there are no validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when user role is undefined', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Passed', undefined); + it("should disable submit when user role is undefined", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, undefined); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when metadata validation is null', () => { - const result: SubmitInfo = utils.shouldDisableSubmit(null, 'Passed', 'Submitter'); + it("should disable submit when metadata validation is null", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: null, + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when file validation is null', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', null, 'Submitter'); + it("should disable submit when file validation is null", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: null, + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should disable submit when both metadata validation and file validation are null', () => { - const result: SubmitInfo = utils.shouldDisableSubmit(null, null, 'Submitter'); + it("should disable submit when both metadata validation and file validation are null", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: null, + fileValidationStatus: null, + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when both validations are in "Validating" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Validating', 'Validating', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Validating", + fileValidationStatus: "Validating", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when metadata validation is in "Validating" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Validating', 'Passed', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Validating", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when file validation is in "Validating" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Validating', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Validating", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when both validations are in "New" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('New', 'New', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "New", + fileValidationStatus: "New", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when metadata validation is in "New" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('New', 'Passed', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "New", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should disable submit when file validation is in "New" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'New', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "New", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); - it('should allow submit when there are validation warnings', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Warning', 'Warning', 'Submitter'); + it("should allow submit when there are validation warnings", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Warning", + fileValidationStatus: "Warning", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); it('should allow submit when both validations are in "Passed" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Passed', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); it('should allow submit when both validations are in "Warning" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Warning', 'Warning', 'Submitter'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Warning", + fileValidationStatus: "Warning", + }; + const result: SubmitInfo = utils.shouldDisableSubmit( + submission, + "Submitter" + ); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); }); -describe('Admin Submit', () => { - it('should allow submit with isAdminOverride when there are validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Error', 'Error', 'Admin'); +describe("Admin Submit", () => { + it("should allow submit with isAdminOverride when there are validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Error", + fileValidationStatus: "Error", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); - it('should allow submit without isAdminOverride when there are no validation errors', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', 'Passed', 'Admin'); + it("should allow submit without isAdminOverride when there are no validation errors", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); - it('should allow submit with isAdminOverride but null data files', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Passed', null, 'Admin'); + it("should allow submit with isAdminOverride but null data files", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: null, + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); - it('should allow submit with isAdminOverride but null metadata', () => { - const result: SubmitInfo = utils.shouldDisableSubmit(null, 'Passed', 'Admin'); + it("should allow submit with isAdminOverride but null metadata", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: null, + fileValidationStatus: "Passed", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); - it('should disable submit without isAdminOverride when null metadata and null data files', () => { - const result: SubmitInfo = utils.shouldDisableSubmit(null, null, 'Admin'); + it("should disable submit without isAdminOverride when null metadata and null data files", () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: null, + fileValidationStatus: null, + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(true); expect(result.isAdminOverride).toBe(false); }); it('should allow submit without isAdminOverride when both validations are in "Warning" state', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Warning', 'Warning', 'Admin'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Warning", + fileValidationStatus: "Warning", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(false); }); it('should allow submit with isAdminOverride when metadata validation is "Warning" and file validation is "Error"', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Warning', 'Error', 'Admin'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Warning", + fileValidationStatus: "Error", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); it('should allow submit with isAdminOverride when metadata validation is "Error" and file validation is "Warning"', () => { - const result: SubmitInfo = utils.shouldDisableSubmit('Error', 'Warning', 'Admin'); + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Error", + fileValidationStatus: "Warning", + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); diff --git a/src/utils/dataSubmissionUtils.ts b/src/utils/dataSubmissionUtils.ts index bd2899c5..58a00fb7 100644 --- a/src/utils/dataSubmissionUtils.ts +++ b/src/utils/dataSubmissionUtils.ts @@ -12,13 +12,13 @@ export type SubmitInfo = { * @returns {SubmitInfo} Info indicating whether or not to disable submit, as well as if it is due to an admin override */ export const shouldDisableSubmit = ( - metadataValidationStatus: ValidationStatus, - fileValidationStatus: ValidationStatus, + submission: Submission, userRole: User["role"], ): SubmitInfo => { if (!userRole) { return { disable: true, isAdminOverride: false }; } + const { metadataValidationStatus, fileValidationStatus, fileErrors } = submission; const isAdmin = userRole === "Admin"; const isMissingBoth = !metadataValidationStatus && !fileValidationStatus; @@ -26,15 +26,18 @@ export const shouldDisableSubmit = ( const isValidating = metadataValidationStatus === "Validating" || fileValidationStatus === "Validating"; const hasNew = metadataValidationStatus === "New" || fileValidationStatus === "New"; const hasError = metadataValidationStatus === "Error" || fileValidationStatus === "Error"; + const hasSubmissionLevelErrors = fileErrors?.length > 0; const isAdminOverride = isAdmin && !isValidating && !isMissingBoth && !hasNew + && !hasSubmissionLevelErrors && (hasError || isMissingOne); const disable = isValidating || isMissingBoth || hasNew + || hasSubmissionLevelErrors || (userRole !== "Admin" && (hasError || isMissingOne)); return { disable, isAdminOverride }; From 72376031db3bcf851772df78c0255886dd363550 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Wed, 13 Mar 2024 17:39:27 -0400 Subject: [PATCH 3/5] Updated the utility function params --- src/content/dataSubmissions/DataSubmission.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/content/dataSubmissions/DataSubmission.tsx b/src/content/dataSubmissions/DataSubmission.tsx index c9a14e5c..65e8afc3 100644 --- a/src/content/dataSubmissions/DataSubmission.tsx +++ b/src/content/dataSubmissions/DataSubmission.tsx @@ -372,11 +372,7 @@ const DataSubmission: FC = ({ submissionId, tab }) => { return { disable: true, isAdminOverride: false }; } - return shouldDisableSubmit( - data.getSubmission.metadataValidationStatus, - data.getSubmission.fileValidationStatus, - user?.role - ); + return shouldDisableSubmit(data.getSubmission, user?.role); }, [data?.getSubmission, user] ); From a540e82439db23c83020c5484f83f2e0ffe95e42 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Wed, 20 Mar 2024 13:32:20 -0400 Subject: [PATCH 4/5] Added submit data submission tests for submission level errors such as orphaned data files --- src/utils/dataSubmissionUtils.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/utils/dataSubmissionUtils.test.ts b/src/utils/dataSubmissionUtils.test.ts index a8580cc7..255bb4d8 100644 --- a/src/utils/dataSubmissionUtils.test.ts +++ b/src/utils/dataSubmissionUtils.test.ts @@ -376,4 +376,28 @@ describe("Admin Submit", () => { expect(result.disable).toBe(false); expect(result.isAdminOverride).toBe(true); }); + + it('should not allow submit with isAdminOverride when Submission level errors exist', () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: null, + fileValidationStatus: "Error", + fileErrors: [{ title: "", description: "" }] + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Admin"); + expect(result.disable).toBe(true); + expect(result.isAdminOverride).toBe(false); + }); + + it('should not allow submitter to submit when Submission level errors exist', () => { + const submission: Submission = { + ...baseSubmission, + metadataValidationStatus: "Passed", + fileValidationStatus: "Passed", + fileErrors: [{ title: "", description: "" }] + }; + const result: SubmitInfo = utils.shouldDisableSubmit(submission, "Submitter"); + expect(result.disable).toBe(true); + expect(result.isAdminOverride).toBe(false); + }); }); From 4152ac831716eb3d684ef13167379daf4dcfcf69 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Wed, 20 Mar 2024 13:42:15 -0400 Subject: [PATCH 5/5] Updated comments --- src/utils/dataSubmissionUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/utils/dataSubmissionUtils.ts b/src/utils/dataSubmissionUtils.ts index 58a00fb7..1201f0cc 100644 --- a/src/utils/dataSubmissionUtils.ts +++ b/src/utils/dataSubmissionUtils.ts @@ -6,8 +6,7 @@ export type SubmitInfo = { /** * Determines whether submit for a submission should be disabled based on its validation statuses and user role * - * @param {ValidationStatus} metadataValidationStatus - The metadata validation status of the submission - * @param {ValidationStatus} fileValidationStatus - The file validation status of the submission + * @param {Submission} submission - The Data Submission * @param {User["role"]} userRole - The role of the user * @returns {SubmitInfo} Info indicating whether or not to disable submit, as well as if it is due to an admin override */