From 3bcce7aaee5f93ba718adf955a8d877d0e8e8767 Mon Sep 17 00:00:00 2001 From: Ian Bolton Date: Mon, 1 Apr 2024 13:08:08 -0400 Subject: [PATCH] :bug: Add more assessment status unit tests & modify useAssessmentStatus to fix 2 regression bugs (#1811) Resolves https://issues.redhat.com/browse/MTA-2503 Resolves https://issues.redhat.com/browse/MTA-2505 Signed-off-by: Ian Bolton --- client/src/app/hooks/useAssessmentStatus.ts | 12 +- .../applications-table/applications-table.tsx | 85 +++++++-------- .../application-assessment-status.test.tsx | 103 +++++++++++++----- client/src/app/queries/assessments.ts | 15 +-- client/src/app/queries/reviews.ts | 4 +- client/src/app/test-config/test-utils.tsx | 50 ++++++--- 6 files changed, 166 insertions(+), 103 deletions(-) diff --git a/client/src/app/hooks/useAssessmentStatus.ts b/client/src/app/hooks/useAssessmentStatus.ts index df1eea447c..a8d8ebb516 100644 --- a/client/src/app/hooks/useAssessmentStatus.ts +++ b/client/src/app/hooks/useAssessmentStatus.ts @@ -67,8 +67,10 @@ export const useAssessmentStatus = (application: Application) => { (assessment: Assessment) => assessment.status === "started" || assessment.status === "empty" || - assessment.status === "complete" + (assessment.status === "complete" && + application.assessments?.length !== 0) ); + return { allArchetypesAssessed, countOfFullyAssessedArchetypes: assessedArchetypesCount, @@ -76,5 +78,11 @@ export const useAssessmentStatus = (application: Application) => { hasApplicationAssessmentInProgress, isApplicationDirectlyAssessed: isDirectlyAssessed, }; - }, [assessments, archetypes, application.id, isDirectlyAssessed]); + }, [ + assessments, + archetypes, + application.id, + application.assessments, + isDirectlyAssessed, + ]); }; diff --git a/client/src/app/pages/applications/applications-table/applications-table.tsx b/client/src/app/pages/applications/applications-table/applications-table.tsx index 379ed748c3..5599d572fa 100644 --- a/client/src/app/pages/applications/applications-table/applications-table.tsx +++ b/client/src/app/pages/applications/applications-table/applications-table.tsx @@ -75,7 +75,6 @@ import { // Queries import { Application, Assessment, Ref, Task } from "@app/api/models"; import { - ApplicationsQueryKey, useBulkDeleteApplicationMutation, useFetchApplications, } from "@app/queries/applications"; @@ -243,64 +242,54 @@ export const ApplicationsTable: React.FC = () => { onDeleteApplicationError ); - const onDeleteReviewSuccess = (name: string) => { - pushNotification({ - title: t("toastr.success.reviewDiscarded", { - application: name, - }), - variant: "success", - }); - queryClient.invalidateQueries([ApplicationsQueryKey]); - }; - const { mutate: deleteReview } = useDeleteReviewMutation( - onDeleteReviewSuccess + (name) => { + pushNotification({ + title: t("toastr.success.reviewDiscarded", { application: name }), + variant: "success", + }); + }, + (error) => { + console.error("Error while deleting review:", error); + pushNotification({ + title: getAxiosErrorMessage(error), + variant: "danger", + }); + } ); - const { mutate: deleteAssessment } = useDeleteAssessmentMutation(); - - const discardAssessment = async (application: Application) => { - try { - if (application.assessments) { - await Promise.all( - application.assessments.map(async (assessment) => { - await deleteAssessment({ - assessmentId: assessment.id, - applicationName: application.name, - }); - }) - ).then(() => { - pushNotification({ - title: t("toastr.success.assessmentDiscarded", { - application: application.name, - }), - variant: "success", - }); - queryClient.invalidateQueries([ApplicationsQueryKey]); - }); - } - } catch (error) { + const { mutate: deleteAssessment } = useDeleteAssessmentMutation( + (name) => { + pushNotification({ + title: t("toastr.success.assessmentDiscarded", { application: name }), + variant: "success", + }); + }, + (error) => { console.error("Error while deleting assessments:", error); pushNotification({ - title: getAxiosErrorMessage(error as AxiosError), + title: getAxiosErrorMessage(error), variant: "danger", }); } + ); + + const discardAssessment = async (application: Application) => { + if (application.assessments) { + application.assessments.forEach((assessment) => { + deleteAssessment({ + assessmentId: assessment.id, + applicationName: application.name, + }); + }); + } }; const discardReview = async (application: Application) => { - try { - if (application.review?.id) { - await deleteReview({ - id: application.review.id, - name: application.name, - }); - } - } catch (error) { - console.error("Error while deleting review:", error); - pushNotification({ - title: getAxiosErrorMessage(error as AxiosError), - variant: "danger", + if (application.review) { + deleteReview({ + id: application.review.id, + name: application.name, }); } }; diff --git a/client/src/app/pages/applications/components/application-assessment-status/tests/application-assessment-status.test.tsx b/client/src/app/pages/applications/components/application-assessment-status/tests/application-assessment-status.test.tsx index e410dcac36..b4dc78da78 100644 --- a/client/src/app/pages/applications/components/application-assessment-status/tests/application-assessment-status.test.tsx +++ b/client/src/app/pages/applications/components/application-assessment-status/tests/application-assessment-status.test.tsx @@ -9,6 +9,8 @@ import { } from "@app/test-config/test-utils"; import { rest } from "msw"; import { server } from "@mocks/server"; +import { assessmentsQueryKey } from "@app/queries/assessments"; +import { QueryClient } from "@tanstack/react-query"; describe("useAssessmentStatus", () => { beforeEach(() => { @@ -18,6 +20,73 @@ describe("useAssessmentStatus", () => { server.resetHandlers(); }); + it("Updates hasApplicationAssessmentInProgress to false once associated assessments are deleted", async () => { + server.use( + rest.get("/hub/assessments", (req, res, ctx) => { + return res( + ctx.json([ + createMockAssessment({ + id: 1, + application: { id: 1, name: "app1" }, + questionnaire: { id: 1, name: "questionnaire1" }, + status: "started", + sections: [], + }), + createMockAssessment({ + id: 2, + application: { id: 1, name: "app1" }, + questionnaire: { id: 2, name: "questionnaire2" }, + status: "complete", + sections: [], + }), + ]) + ); + }), + rest.get("/hub/archetypes", (req, res, ctx) => { + return res( + ctx.json([ + createMockArchetype({ + id: 1, + name: "archetype1", + applications: [], + assessed: false, + assessments: [], + }), + ]) + ); + }) + ); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + cacheTime: 1000, + }, + }, + }); + const { result, rerender } = renderHook( + () => useAssessmentStatus(createMockApplication({ id: 1, name: "app1" })), + { queryClient } + ); + + await waitFor(() => { + expect(result.current.hasApplicationAssessmentInProgress).toBe(true); + }); + + server.use( + rest.get("/hub/assessments", (req, res, ctx) => { + return res(ctx.json([])); + }) + ); + queryClient.invalidateQueries([assessmentsQueryKey]); + + rerender(createMockApplication({ id: 1, name: "app1" })); + + await waitFor(() => { + expect(result.current.hasApplicationAssessmentInProgress).toBe(false); + }); + }); + it("Correctly calculates status given one started assessment and one complete assessment for an application", async () => { server.use( rest.get("/hub/assessments", (req, res, ctx) => { @@ -55,10 +124,10 @@ describe("useAssessmentStatus", () => { ); }) ); - const { result, waitForNextUpdate } = renderHook(() => + + const { result } = renderHook(() => useAssessmentStatus(createMockApplication({ id: 1, name: "app1" })) ); - await waitForNextUpdate(); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: false, @@ -105,11 +174,8 @@ describe("useAssessmentStatus", () => { assessments: mockAssessments, }); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); - await waitForNextUpdate(); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: false, @@ -175,10 +241,7 @@ describe("useAssessmentStatus", () => { ], assessed: false, }); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); - await waitForNextUpdate(); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: false, @@ -228,10 +291,7 @@ describe("useAssessmentStatus", () => { }) ); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); - await waitForNextUpdate(); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: true, @@ -279,10 +339,7 @@ describe("useAssessmentStatus", () => { }) ); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); - await waitForNextUpdate(); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: false, @@ -331,10 +388,7 @@ describe("useAssessmentStatus", () => { }) ); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); - await waitForNextUpdate(); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); await waitFor(() => { expect(result.current).toEqual({ allArchetypesAssessed: true, @@ -398,10 +452,7 @@ describe("useAssessmentStatus", () => { }) ); - const { result, waitForNextUpdate } = renderHook(() => - useAssessmentStatus(mockApplication) - ); - await waitForNextUpdate(); + const { result } = renderHook(() => useAssessmentStatus(mockApplication)); await waitFor(() => { expect(result.current).toEqual({ diff --git a/client/src/app/queries/assessments.ts b/client/src/app/queries/assessments.ts index 44508dc353..50d321f2a8 100644 --- a/client/src/app/queries/assessments.ts +++ b/client/src/app/queries/assessments.ts @@ -23,6 +23,7 @@ import { } from "@app/api/models"; import { QuestionnairesQueryKey } from "./questionnaires"; import { ARCHETYPE_QUERY_KEY } from "./archetypes"; +import { ApplicationsQueryKey } from "./applications"; export const assessmentsQueryKey = "assessments"; export const assessmentQueryKey = "assessment"; @@ -120,20 +121,10 @@ export const useDeleteAssessmentMutation = ( archetypeId?: number; }) => { const deletedAssessment = deleteAssessment(args.assessmentId); - const isArchetype = !!args.archetypeId; - queryClient.invalidateQueries([assessmentQueryKey, args?.assessmentId]); + queryClient.invalidateQueries([ApplicationsQueryKey]); + queryClient.invalidateQueries([assessmentsQueryKey]); queryClient.invalidateQueries([ARCHETYPE_QUERY_KEY, args?.archetypeId]); - queryClient.invalidateQueries([ - assessmentsByItemIdQueryKey, - args?.archetypeId, - isArchetype, - ]); - queryClient.invalidateQueries([ - assessmentsByItemIdQueryKey, - args?.applicationId, - isArchetype, - ]); return deletedAssessment; }, diff --git a/client/src/app/queries/reviews.ts b/client/src/app/queries/reviews.ts index 1bde4905a2..a47936b5ff 100644 --- a/client/src/app/queries/reviews.ts +++ b/client/src/app/queries/reviews.ts @@ -9,6 +9,7 @@ import { } from "@app/api/rest"; import { New, Review } from "@app/api/models"; import { AxiosError } from "axios"; +import { ApplicationsQueryKey } from "./applications"; export const reviewQueryKey = "review"; export const reviewsByItemIdQueryKey = "reviewsByItemId"; @@ -85,13 +86,14 @@ export const useDeleteReviewMutation = ( onSuccess: (_, args) => { onSuccess && onSuccess(args.name); queryClient.invalidateQueries([reviewsQueryKey]); + queryClient.invalidateQueries([ApplicationsQueryKey]); }, onError: onError && onError, }); }; export const useFetchReviewById = (id?: number | string) => { - const { data, isLoading, error, isFetching } = useQuery({ + const { data, error, isFetching } = useQuery({ queryKey: [reviewQueryKey, id], queryFn: () => id === undefined ? Promise.resolve(null) : getReviewById(id), diff --git a/client/src/app/test-config/test-utils.tsx b/client/src/app/test-config/test-utils.tsx index e7e6d05461..c15f3b1f7c 100644 --- a/client/src/app/test-config/test-utils.tsx +++ b/client/src/app/test-config/test-utils.tsx @@ -4,20 +4,41 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { Application, Archetype, Assessment } from "@app/api/models"; import { RenderHookOptions, renderHook } from "@testing-library/react-hooks"; -const AllTheProviders: FC<{ children: React.ReactNode }> = ({ children }) => { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - cacheTime: 1000, +import { createContext, useContext } from "react"; + +const QueryClientContext = createContext(undefined); + +const AllTheProviders: FC<{ + children: React.ReactNode; + queryClient?: QueryClient; +}> = ({ children, queryClient }) => { + const internalQueryClient = + queryClient || + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + cacheTime: 1000, + }, }, - }, - }); + }); return ( - {children} + + {children} + ); }; +export const useQueryClientContext = () => { + const context = useContext(QueryClientContext); + if (context === undefined) { + throw new Error( + "useQueryClientContext must be used within a QueryClientContext.Provider" + ); + } + return context; +}; + const customRender = ( ui: ReactElement, options?: Omit @@ -25,19 +46,20 @@ const customRender = ( const customRenderHook = ( callback: (props: TProps) => TResult, - options?: Omit, "wrapper"> + options?: Omit, "wrapper"> & { + queryClient?: QueryClient; + } ) => { + const { queryClient, ...rest } = options || {}; const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( - {children} + {children} ); - return renderHook(callback, { wrapper: Wrapper as React.FC, ...options }); + return renderHook(callback, { wrapper: Wrapper as React.FC, ...rest }); }; -// re-export everything export * from "@testing-library/react"; -// override render method export { customRender as render }; export { customRenderHook as renderHook };