From 0386e2efabf0b9a77f9cedeeda367ffdfc7d1759 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Thu, 2 Jan 2025 16:55:12 -0500 Subject: [PATCH 1/3] Update SubmissionContext to call submissionQCResults API in parallel with getSubmission to check for orphaned file errors --- .../Contexts/SubmissionContext.test.tsx | 260 ++++++++++++++++-- src/components/Contexts/SubmissionContext.tsx | 44 ++- src/graphql/submissionQCResults.ts | 61 ++-- 3 files changed, 320 insertions(+), 45 deletions(-) diff --git a/src/components/Contexts/SubmissionContext.test.tsx b/src/components/Contexts/SubmissionContext.test.tsx index 04a84006a..4d0e3775d 100644 --- a/src/components/Contexts/SubmissionContext.test.tsx +++ b/src/components/Contexts/SubmissionContext.test.tsx @@ -3,7 +3,13 @@ import { act, render, renderHook, waitFor } from "@testing-library/react"; import { MockedProvider, MockedResponse } from "@apollo/client/testing"; import { GraphQLError } from "graphql"; import { SubmissionCtxStatus, SubmissionProvider, useSubmissionContext } from "./SubmissionContext"; -import { GET_SUBMISSION, GetSubmissionInput, GetSubmissionResp } from "../../graphql"; +import { + GET_SUBMISSION, + GetSubmissionInput, + GetSubmissionResp, + SUBMISSION_QC_RESULTS, + SubmissionQCResultsResp, +} from "../../graphql"; const mockStartPolling = jest.fn(); const mockStopPolling = jest.fn(); @@ -100,7 +106,23 @@ describe("useSubmissionContext", () => { }, ]; - expect(() => render()).not.toThrow(); + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: null, + }, + }, + }, + ]; + + expect(() => + render() + ).not.toThrow(); }); }); @@ -119,8 +141,23 @@ describe("SubmissionProvider", () => { error: new Error("Network error"), }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: null, + }, + }, + }, + ]; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); await waitFor(() => expect(getByTestId("ctx-status")).toHaveTextContent("ERROR")); }); @@ -137,8 +174,23 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: null, + }, + }, + }, + ]; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); await waitFor(() => expect(getByTestId("ctx-status")).toHaveTextContent("ERROR")); }); @@ -159,8 +211,23 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: null, + }, + }, + }, + ]; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); await waitFor(() => expect(getByTestId("ctx-status")).toHaveTextContent("ERROR")); }); @@ -188,8 +255,26 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); await waitFor(() => expect(getByTestId("ctx-status")).toHaveTextContent("LOADED")); }); @@ -223,10 +308,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + render(); - await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(1)); + // Should poll getSubmission + submissionQCResults + await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(2)); expect(mockStopPolling).not.toHaveBeenCalled(); }); @@ -259,10 +361,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + render(); - await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(1)); + // Should stop polling getSubmission + submissionQCResults + await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(2)); expect(mockStartPolling).not.toHaveBeenCalled(); }); @@ -295,10 +414,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + render(); - await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(1)); + // Should poll getSubmission + submissionQCResults + await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(2)); expect(mockStopPolling).not.toHaveBeenCalled(); }); @@ -331,10 +467,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + // Should stop polling getSubmission + submissionQCResults + render(); - await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(2)); expect(mockStartPolling).not.toHaveBeenCalled(); }); @@ -363,10 +516,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + render(); - await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(1)); + // Should poll getSubmission + submissionQCResults + await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(2)); expect(mockStopPolling).not.toHaveBeenCalled(); }); @@ -395,10 +565,27 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; - render(); + render(); - await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(1)); + // Should stop polling getSubmission + submissionQCResults + await waitFor(() => expect(mockStopPolling).toHaveBeenCalledTimes(2)); expect(mockStartPolling).not.toHaveBeenCalled(); }); @@ -430,10 +617,26 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; const { result } = renderHook(() => useSubmissionContext(), { wrapper: ({ children }) => ( - + {children} ), @@ -447,6 +650,7 @@ describe("SubmissionProvider", () => { startPolling(1000); }); + await waitFor(() => expect(mockStartPolling).toHaveBeenCalledTimes(2)); expect(mockStartPolling).toHaveBeenCalledWith(1000); act(() => { @@ -479,10 +683,26 @@ describe("SubmissionProvider", () => { }, }, ]; + const qcMocks: MockedResponse>[] = [ + { + request: { + query: SUBMISSION_QC_RESULTS, + }, + variableMatcher: () => true, + result: { + data: { + submissionQCResults: { + total: 0, + results: [], + }, + }, + }, + }, + ]; const { result } = renderHook(() => useSubmissionContext(), { wrapper: ({ children }) => ( - + {children} ), diff --git a/src/components/Contexts/SubmissionContext.tsx b/src/components/Contexts/SubmissionContext.tsx index 13e216db0..0d0fb4404 100644 --- a/src/components/Contexts/SubmissionContext.tsx +++ b/src/components/Contexts/SubmissionContext.tsx @@ -1,7 +1,13 @@ import React, { FC, createContext, useCallback, useContext, useMemo, useState } from "react"; import { ApolloError, ApolloQueryResult, useQuery } from "@apollo/client"; import { cloneDeep, isEqual } from "lodash"; -import { GetSubmissionResp, GET_SUBMISSION, GetSubmissionInput } from "../../graphql"; +import { + GetSubmissionResp, + GET_SUBMISSION, + GetSubmissionInput, + SubmissionQCResultsResp, + SUBMISSION_QC_RESULTS, +} from "../../graphql"; import { compareNodeStats, Logger } from "../../utils"; export type SubmissionCtxState = { @@ -17,6 +23,14 @@ export type SubmissionCtxState = { * The error returned by the query */ error: ApolloError | null; + /** + * The partial Validation Results data + */ + qcData?: SubmissionQCResultsResp | null; + /** + * The error returned by the Validation Results query + */ + qcError?: ApolloError | null; /** * Initiates polling for the query at the specified interval */ @@ -126,18 +140,32 @@ export const SubmissionProvider: FC = ({ _id, children }: Provide if (!isValidating && !hasUploadingBatches && !isDeleting) { stopApolloPolling(); + stopQCPolling(); setIsPolling(false); } else { startApolloPolling(1000); + startQCPolling(1000); setIsPolling(true); } }, onError: (e) => { Logger.error("Error fetching submission data", e); }, - variables: { id: _id }, + }); + + const { + data: qcData, + error: qcError, + startPolling: startQCPolling, + stopPolling: stopQCPolling, + } = useQuery>(SUBMISSION_QC_RESULTS, { + variables: { id: _id, partial: true }, context: { clientName: "backend" }, fetchPolicy: "cache-and-network", + notifyOnNetworkStatusChange: true, + onError: (err) => { + Logger.error("Error fetching submission validation results data", err); + }, }); const status: SubmissionCtxStatus = useMemo(() => { @@ -174,30 +202,34 @@ export const SubmissionProvider: FC = ({ _id, children }: Provide const startPolling = useCallback( (interval: number) => { startApolloPolling(interval); + startQCPolling(interval); setIsPolling(true); }, - [startApolloPolling] + [startApolloPolling, startQCPolling] ); /** - * Wrapper function to stop polling for the submission + * Wrapper function to stop polling for the submission and QC results */ const stopPolling = useCallback(() => { stopApolloPolling(); + stopQCPolling(); setIsPolling(false); - }, [stopApolloPolling]); + }, [stopApolloPolling, stopQCPolling]); const value: SubmissionCtxState = useMemo( () => ({ status, data: memoedData, error, + qcData, + qcError, startPolling, stopPolling, refetch, updateQuery, }), - [status, memoedData, error, startPolling, stopPolling, refetch, updateQuery] + [status, memoedData, error, startPolling, stopPolling, refetch, updateQuery, qcData, qcError] ); return {children}; diff --git a/src/graphql/submissionQCResults.ts b/src/graphql/submissionQCResults.ts index b8f8019df..9e5235cdc 100644 --- a/src/graphql/submissionQCResults.ts +++ b/src/graphql/submissionQCResults.ts @@ -1,5 +1,38 @@ import gql from "graphql-tag"; +// The base QCResult model used for all submissionQCResults queries +const BaseQCResultFragment = gql` + fragment BaseQCResultFragment on QCResult { + errors { + title + description + } + } +`; + +// The extended QCResult model which includes all fields +const FullQCResultFragment = gql` + fragment QCResultFragment on QCResult { + submissionID + type + validationType + batchID + displayID + submittedID + severity + uploadedDate + validatedDate + errors { + title + description + } + warnings { + title + description + } + } +`; + export const query = gql` query submissionQCResults( $id: ID! @@ -10,6 +43,7 @@ export const query = gql` $offset: Int $orderBy: String $sortDirection: String + $partial: Boolean = false ) { submissionQCResults( _id: $id @@ -23,28 +57,17 @@ export const query = gql` ) { total results { - submissionID - type - validationType - batchID - displayID - submittedID - severity - uploadedDate - validatedDate - errors { - title - description - } - warnings { - title - description - } + ...BaseQCResultFragment + ...QCResultFragment @skip(if: $partial) } } } + ${FullQCResultFragment} + ${BaseQCResultFragment} `; -export type Response = { - submissionQCResults: ValidationResult; +export type Response = { + submissionQCResults: ValidationResult< + IsPartial extends true ? Pick : QCResult + >; }; From 4e5230ce7ae0bb21514485b38d8870d3600ac07f Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Thu, 2 Jan 2025 16:56:47 -0500 Subject: [PATCH 2/3] Update Data Submission submit button config to accept QC Results in logic --- .../ExportValidationButton.test.tsx | 7 ++ src/config/SubmitButtonConfig.ts | 14 ++- .../dataSubmissions/DataSubmission.tsx | 13 ++- src/utils/dataSubmissionUtils.test.ts | 90 ++++++++++--------- src/utils/dataSubmissionUtils.ts | 20 +++-- 5 files changed, 87 insertions(+), 57 deletions(-) diff --git a/src/components/DataSubmissions/ExportValidationButton.test.tsx b/src/components/DataSubmissions/ExportValidationButton.test.tsx index 41bfd704e..f93883ddc 100644 --- a/src/components/DataSubmissions/ExportValidationButton.test.tsx +++ b/src/components/DataSubmissions/ExportValidationButton.test.tsx @@ -119,6 +119,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", @@ -175,6 +176,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: "example-dynamic-filename-id", sortDirection: "asc", orderBy: "displayID", @@ -245,6 +247,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", @@ -298,6 +301,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", @@ -371,6 +375,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", @@ -408,6 +413,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", @@ -447,6 +453,7 @@ describe("ExportValidationButton cases", () => { request: { query: SUBMISSION_QC_RESULTS, variables: { + partial: false, id: submissionID, sortDirection: "asc", orderBy: "displayID", diff --git a/src/config/SubmitButtonConfig.ts b/src/config/SubmitButtonConfig.ts index 9607b6d66..cf6b9dc58 100644 --- a/src/config/SubmitButtonConfig.ts +++ b/src/config/SubmitButtonConfig.ts @@ -11,12 +11,12 @@ export type SubmitButtonCondition = { * If false, then submit button remains disabled. Otherwise, the current * condition is satisfied */ - check: (submission: Submission) => boolean; + check: (submission: Submission, qcResults: Pick[]) => boolean; /** * Optionally checks the pre-condition to determine whether this condition * is applicable in the current submission state */ - preConditionCheck?: (submission: Submission) => boolean; + preConditionCheck?: (submission: Submission, qcResults: Pick[]) => boolean; /** * The text that will display on the tooltip for the submit button */ @@ -29,6 +29,11 @@ export type SubmitButtonCondition = { export type AdminOverrideCondition = Omit; +/** + * The title of the error message that represents an orphaned file + */ +const ORPHANED_FILE_ERROR_TITLE = "Orphaned file found"; + /** * Configuration of conditions used to determine whether the submit button * should be enabled for a submission. Each condition checks a specific state @@ -49,8 +54,9 @@ export const SUBMIT_BUTTON_CONDITIONS: SubmitButtonCondition[] = [ required: true, }, { - _identifier: "Submission should not have submission level errors, such as orphaned files", - check: (s) => !s.fileErrors?.length, + _identifier: "Submission should not have orphaned files", + check: (_, qcResults) => + !qcResults?.some((qc) => qc.errors?.find((err) => err.title === ORPHANED_FILE_ERROR_TITLE)), tooltip: TOOLTIP_TEXT.SUBMISSION_ACTIONS.SUBMIT.DISABLED.NEW_DATA_OR_VALIDATION_ERRORS, required: true, }, diff --git a/src/content/dataSubmissions/DataSubmission.tsx b/src/content/dataSubmissions/DataSubmission.tsx index ed16d0b47..76fe8e91f 100644 --- a/src/content/dataSubmissions/DataSubmission.tsx +++ b/src/content/dataSubmissions/DataSubmission.tsx @@ -163,7 +163,7 @@ const DataSubmission: FC = ({ submissionId, tab = URLTabs.UPLOAD_ACTIVITY const { user } = useAuthContext(); const { enqueueSnackbar } = useSnackbar(); const { lastSearchParams } = useSearchParamsContext(); - const { data, error, refetch: getSubmission } = useSubmissionContext(); + const { data, error, refetch: getSubmission, qcData, qcError } = useSubmissionContext(); const dataSubmissionListPageUrl = `/data-submissions${ lastSearchParams?.["/data-submissions"] ?? "" @@ -189,7 +189,7 @@ const DataSubmission: FC = ({ submissionId, tab = URLTabs.UPLOAD_ACTIVITY return { enabled: false }; } - return shouldEnableSubmit(data.getSubmission, user?.role); + return shouldEnableSubmit(data.getSubmission, qcData?.submissionQCResults?.results, user?.role); }, [data?.getSubmission, user, hasUploadingBatches, SubmitDataSubmissionRoles]); const releaseInfo: ReleaseInfo = useMemo( () => shouldDisableRelease(data?.getSubmission), @@ -251,8 +251,15 @@ const DataSubmission: FC = ({ submissionId, tab = URLTabs.UPLOAD_ACTIVITY navigate(dataSubmissionListPageUrl, { state: { error: "Oops! An error occurred while retrieving that Data Submission." }, }); + } else if (qcError) { + navigate(dataSubmissionListPageUrl, { + state: { + error: + "There was an issue while retrieving the validation results for that Data Submission.", + }, + }); } - }, [error]); + }, [error, qcError]); return ( diff --git a/src/utils/dataSubmissionUtils.test.ts b/src/utils/dataSubmissionUtils.test.ts index fc0026541..486f1fa74 100644 --- a/src/utils/dataSubmissionUtils.test.ts +++ b/src/utils/dataSubmissionUtils.test.ts @@ -38,6 +38,8 @@ const baseSubmission: Submission = { collaborators: [], }; +const baseQCResults: QCResult[] = []; + describe("General Submit", () => { it("should disable submit without isAdminOverride when user role is not Admin but there are validation errors", () => { const submission: Submission = { @@ -45,7 +47,7 @@ describe("General Submit", () => { metadataValidationStatus: "Error", fileValidationStatus: "Error", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -56,7 +58,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Error", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -67,7 +69,7 @@ describe("General Submit", () => { metadataValidationStatus: "Error", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -78,7 +80,7 @@ describe("General Submit", () => { metadataValidationStatus: "Warning", fileValidationStatus: "Error", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -89,7 +91,7 @@ describe("General Submit", () => { metadataValidationStatus: "Error", fileValidationStatus: "Warning", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -101,7 +103,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: null, }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -112,7 +114,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -123,7 +125,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, undefined); + const result = utils.shouldEnableSubmit(submission, baseQCResults, undefined); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -134,7 +136,7 @@ describe("General Submit", () => { metadataValidationStatus: null, fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -145,7 +147,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: null, }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -158,7 +160,7 @@ describe("General Submit", () => { fileValidationStatus: null, intention: "Delete", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -170,7 +172,7 @@ describe("General Submit", () => { fileValidationStatus: null, intention: "New/Update", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -182,7 +184,7 @@ describe("General Submit", () => { fileValidationStatus: "Passed", intention: "Delete", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -194,7 +196,7 @@ describe("General Submit", () => { fileValidationStatus: "Error", intention: "Delete", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -205,7 +207,7 @@ describe("General Submit", () => { metadataValidationStatus: null, fileValidationStatus: null, }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -216,7 +218,7 @@ describe("General Submit", () => { metadataValidationStatus: "Validating", fileValidationStatus: "Validating", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -227,7 +229,7 @@ describe("General Submit", () => { metadataValidationStatus: "Validating", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -238,7 +240,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Validating", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -249,7 +251,7 @@ describe("General Submit", () => { metadataValidationStatus: "New", fileValidationStatus: "New", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -260,7 +262,7 @@ describe("General Submit", () => { metadataValidationStatus: "New", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -271,7 +273,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "New", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -282,7 +284,7 @@ describe("General Submit", () => { metadataValidationStatus: "Warning", fileValidationStatus: "Warning", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -293,7 +295,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -304,7 +306,7 @@ describe("General Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Warning", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -315,13 +317,13 @@ describe("General Submit", () => { metadataValidationStatus: "Warning", fileValidationStatus: "Warning", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); it("should disable submit when submission is null", () => { - const result = utils.shouldEnableSubmit(null, "Submitter"); + const result = utils.shouldEnableSubmit(null, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -334,7 +336,7 @@ describe("Admin Submit", () => { metadataValidationStatus: "Error", fileValidationStatus: "Error", }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(true); }); @@ -345,7 +347,7 @@ describe("Admin Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Passed", }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(false); }); @@ -356,7 +358,7 @@ describe("Admin Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: null, }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -367,7 +369,7 @@ describe("Admin Submit", () => { metadataValidationStatus: null, fileValidationStatus: null, }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(false); }); @@ -379,7 +381,7 @@ describe("Admin Submit", () => { fileValidationStatus: null, intention: "Delete", }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(true); }); @@ -405,7 +407,7 @@ describe("Admin Submit", () => { }, ], }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -431,7 +433,11 @@ describe("Admin Submit", () => { }, ], }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const qcResults: Pick[] = [ + { errors: [{ title: "Orphaned file found", description: "" }] }, + ]; + const result = utils.shouldEnableSubmit(submission, qcResults, "Submitter"); + expect(result._identifier).toBe("Submission should not have orphaned files"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -444,7 +450,7 @@ describe("Admin Submit", () => { fileValidationStatus: null, intention: "Delete", }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(true); }); @@ -456,7 +462,7 @@ describe("Admin Submit", () => { metadataValidationStatus: "Passed", fileValidationStatus: "New", }; - const result = utils.shouldEnableSubmit(submission, "Admin"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Admin"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -470,7 +476,7 @@ describe("Submit > Submission Type/Intention", () => { metadataValidationStatus: "Error", fileValidationStatus: "Error", }; - const result = utils.shouldEnableSubmit(submission, "Submitter"); + const result = utils.shouldEnableSubmit(submission, baseQCResults, "Submitter"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -503,13 +509,13 @@ describe("shouldAllowAdminOverride", () => { }); it("should disable submit without isAdminOverride when submission is null", () => { - const result = utils.shouldAllowAdminOverride(null); + const result = utils.shouldAllowAdminOverride(null, baseQCResults); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); it("should disable submit without isAdminOverride when submission is undefined", () => { - const result = utils.shouldAllowAdminOverride(undefined); + const result = utils.shouldAllowAdminOverride(undefined, baseQCResults); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); }); @@ -521,7 +527,7 @@ describe("shouldAllowAdminOverride", () => { metadataValidationStatus: "Error", fileValidationStatus: null, }; - const result = utils.shouldAllowAdminOverride(submission); + const result = utils.shouldAllowAdminOverride(submission, baseQCResults); expect(result._identifier).toBe("Admin Override - Submission has validation errors"); expect(result.enabled).toBe(true); expect(result.isAdminOverride).toBe(true); @@ -534,7 +540,7 @@ describe("shouldAllowAdminOverride", () => { metadataValidationStatus: "Passed", fileValidationStatus: null, }; - const result = utils.shouldAllowAdminOverride(submission); + const result = utils.shouldAllowAdminOverride(submission, baseQCResults); expect(result._identifier).toBe(undefined); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); @@ -547,7 +553,7 @@ describe("shouldAllowAdminOverride", () => { metadataValidationStatus: "Passed", fileValidationStatus: "Validating", }; - const result = utils.shouldAllowAdminOverride(submission); + const result = utils.shouldAllowAdminOverride(submission, baseQCResults); expect(result._identifier).toBe("Validation should not currently be running"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); @@ -563,7 +569,7 @@ describe("shouldAllowAdminOverride", () => { fileValidationStatus: "Passed", }; - const result = utils.shouldAllowAdminOverride(submission); + const result = utils.shouldAllowAdminOverride(submission, baseQCResults); expect(result._identifier).toBe("test-identifier-1"); expect(result.enabled).toBe(false); expect(result.isAdminOverride).toBe(false); diff --git a/src/utils/dataSubmissionUtils.ts b/src/utils/dataSubmissionUtils.ts index 8f6db9172..8edfdf0ba 100644 --- a/src/utils/dataSubmissionUtils.ts +++ b/src/utils/dataSubmissionUtils.ts @@ -13,6 +13,7 @@ import { safeParse } from "./jsonUtils"; */ export const shouldEnableSubmit = ( submission: Submission, + qcResults: Pick[], userRole: UserRole ): SubmitButtonResult => { if (!submission || !userRole) { @@ -22,7 +23,7 @@ export const shouldEnableSubmit = ( // Check for potential Admin override const isAdmin = userRole === "Admin"; if (isAdmin) { - const adminOverrideResult = shouldAllowAdminOverride(submission); + const adminOverrideResult = shouldAllowAdminOverride(submission, qcResults); if (adminOverrideResult.enabled) { return { ...adminOverrideResult }; } @@ -31,11 +32,11 @@ export const shouldEnableSubmit = ( // Skip required conditions already checked if user is Admin role const failedCondition = SUBMIT_BUTTON_CONDITIONS.find((condition) => { const preConditionMet = condition.preConditionCheck - ? condition.preConditionCheck(submission) + ? condition.preConditionCheck(submission, qcResults) : true; // Return true if preCondition is met and main condition fails - return preConditionMet && !condition.check(submission); + return preConditionMet && !condition.check(submission, qcResults); }); // If no failed conditions, enable submit @@ -63,7 +64,10 @@ export const shouldEnableSubmit = ( * @returns {SubmitButtonResult} - Returns an object indicating whether the admin override is allowed, * and an optional tooltip explaining why the override is not allowed. */ -export const shouldAllowAdminOverride = (submission: Submission): SubmitButtonResult => { +export const shouldAllowAdminOverride = ( + submission: Submission, + qcResults: Pick[] +): SubmitButtonResult => { if (!submission) { return { enabled: false, isAdminOverride: false }; } @@ -72,11 +76,11 @@ export const shouldAllowAdminOverride = (submission: Submission): SubmitButtonRe const requiredConditions = SUBMIT_BUTTON_CONDITIONS.filter((condition) => condition.required); const failedRequiredCondition = requiredConditions.find((condition) => { const preConditionMet = condition.preConditionCheck - ? condition.preConditionCheck(submission) + ? condition.preConditionCheck(submission, qcResults) : true; // Return true if preCondition is met and main condition fails - return preConditionMet && !condition.check(submission); + return preConditionMet && !condition.check(submission, qcResults); }); // If failed required condition, then disable submit buton and show tooltip if available @@ -92,11 +96,11 @@ export const shouldAllowAdminOverride = (submission: Submission): SubmitButtonRe // Check if current conditions allow for an admin override const overrideCondition = ADMIN_OVERRIDE_CONDITIONS.find((condition) => { const preConditionMet = condition.preConditionCheck - ? condition.preConditionCheck(submission) + ? condition.preConditionCheck(submission, qcResults) : true; // Return true if preCondition is met and main condition passes - return preConditionMet && condition.check(submission); + return preConditionMet && condition.check(submission, qcResults); }); // If Admin override, then enable submit button with tooltip if available From db0ccaa209b04f96245f9ed3ac1ab7abdc8852c8 Mon Sep 17 00:00:00 2001 From: Alejandro-Vega Date: Thu, 2 Jan 2025 17:06:13 -0500 Subject: [PATCH 3/3] Add back query variables --- src/components/Contexts/SubmissionContext.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/Contexts/SubmissionContext.tsx b/src/components/Contexts/SubmissionContext.tsx index 0d0fb4404..1c7122047 100644 --- a/src/components/Contexts/SubmissionContext.tsx +++ b/src/components/Contexts/SubmissionContext.tsx @@ -128,6 +128,9 @@ export const SubmissionProvider: FC = ({ _id, children }: Provide updateQuery, } = useQuery(GET_SUBMISSION, { notifyOnNetworkStatusChange: true, + variables: { id: _id }, + context: { clientName: "backend" }, + fetchPolicy: "cache-and-network", onCompleted: (d) => { const isValidating = d?.getSubmission?.fileValidationStatus === "Validating" ||