From 5c883b0909e9c3e4fbe2a818991e56fb6a56ec83 Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Fri, 22 Sep 2023 16:53:46 -0400 Subject: [PATCH] :sparkles: Reports - Current Landscape update risk handling Custom assessments changes risk and confidence to be accessed from `Assessment` and not `Application`. The reports page in general, and the `Landscape` component in specific need to be updated to use risk values from `Assessment`. Summary of Change: - `Reports` - Fetch `Questionnaires` and `Assessments` to allow easy use in the Questionnaire select menu - Adjust the `Card`s to be clickable and selectable when we provide custom actions (to avoid a console warning) - `Landscape` - Use data from props instead of fetching any additional data. The containing component is now responsible for controlling the source data. - Aggregate risk data into buckets matching `Risk` options - Setup responsive layout so the donut charts wrap nicely when the view becomes narrow - `Donut` - Force `id` to be provided - Set the width to `200px` - Make sure content lines up centered in its container - Deprecated `useFetchRisks()` Enhancement: https://github.com/konveyor/enhancements/blob/90b827b68cc367284a66bf66f087d5c263487e05/enhancements/assessment-module/README.md#changes-in-the-application-reports-view Part Of: #1305 Follow Up: #1374 Signed-off-by: Scott J Dickerson --- client/public/locales/en/translation.json | 4 +- client/public/locales/es/translation.json | 2 - .../reports/components/landscape/donut.tsx | 46 ++-- .../components/landscape/landscape.tsx | 151 ++++++------- client/src/app/pages/reports/reports.tsx | 199 ++++++++++-------- client/src/app/queries/risks.ts | 14 +- 6 files changed, 223 insertions(+), 193 deletions(-) diff --git a/client/public/locales/en/translation.json b/client/public/locales/en/translation.json index fc6872bb39..fa98976fa6 100644 --- a/client/public/locales/en/translation.json +++ b/client/public/locales/en/translation.json @@ -87,8 +87,8 @@ "noDataStateBody": "Create a new {{what}} to start seeing data here.", "noDataStateTitle": "No {{what}} available", "Nquestions": "{{n}} questions", - "ofTotalApplications": "Of {{count}} application", - "ofTotalApplications_plural": "Of {{count}} applications", + "ofTotalAssessments": "Of {{count}} assessment", + "ofTotalAssessments_plural": "Of {{count}} assessments", "selectMany": "Select {{what}}", "selectOne": "Select a {{what}}", "selectAn": "Select an {{what}}", diff --git a/client/public/locales/es/translation.json b/client/public/locales/es/translation.json index 12f96255f9..2304a0cccb 100644 --- a/client/public/locales/es/translation.json +++ b/client/public/locales/es/translation.json @@ -68,8 +68,6 @@ "noDataStateBody": "Cree un(a) nuevo(a) {{what}} para empezar a ver datos acá.", "noDataStateTitle": "No existen {{what}} disponibles", "Nquestions": "{{n}} preguntas", - "ofTotalApplications": "De {{count}} aplicación", - "ofTotalApplications_plural": "De {{count}} aplicaciones", "selectMany": "Seleccione {{what}}", "selectOne": "Seleccione un {{what}}", "selectAn": "Seleccione una {{what}}", diff --git a/client/src/app/pages/reports/components/landscape/donut.tsx b/client/src/app/pages/reports/components/landscape/donut.tsx index 5c0f4bacce..6d40bada99 100644 --- a/client/src/app/pages/reports/components/landscape/donut.tsx +++ b/client/src/app/pages/reports/components/landscape/donut.tsx @@ -4,9 +4,16 @@ import { useTranslation } from "react-i18next"; import { ChartDonut } from "@patternfly/react-charts"; import { global_palette_black_300 as black } from "@patternfly/react-tokens"; -import { Stack, StackItem, Text, TextContent } from "@patternfly/react-core"; +import { + Bullseye, + Stack, + StackItem, + Text, + TextContent, +} from "@patternfly/react-core"; export interface IDonutProps { + id: string; value: number; total: number; color: string; @@ -15,6 +22,7 @@ export interface IDonutProps { } export const Donut: React.FC = ({ + id, value, total, color, @@ -24,24 +32,26 @@ export const Donut: React.FC = ({ const { t } = useTranslation(); return ( - - - `${datum.x}: ${datum.y}`} - colorScale={[color, black.value]} - /> + + + + `${datum.x}: ${datum.y}`} + colorScale={[color, black.value]} + /> + - + {riskLabel} {riskDescription} diff --git a/client/src/app/pages/reports/components/landscape/landscape.tsx b/client/src/app/pages/reports/components/landscape/landscape.tsx index 7c500cd922..2e5e5709c0 100644 --- a/client/src/app/pages/reports/components/landscape/landscape.tsx +++ b/client/src/app/pages/reports/components/landscape/landscape.tsx @@ -1,37 +1,29 @@ -import React, { useContext, useMemo } from "react"; +import React, { useMemo } from "react"; import { useTranslation } from "react-i18next"; - -import { Skeleton, Split, SplitItem } from "@patternfly/react-core"; - -import { ConditionalRender } from "@app/components/ConditionalRender"; -import { StateError } from "@app/components/StateError"; +import { Flex, FlexItem, Skeleton } from "@patternfly/react-core"; import { RISK_LIST } from "@app/Constants"; -import { Assessment, AssessmentRisk } from "@app/api/models"; - -import { ApplicationSelectionContext } from "../../application-selection-context"; -import { NoApplicationSelectedEmptyState } from "../no-application-selected-empty-state"; +import { Assessment, Questionnaire } from "@app/api/models"; +import { ConditionalRender } from "@app/components/ConditionalRender"; import { Donut } from "./donut"; -import { useFetchRisks } from "@app/queries/risks"; -interface ILandscapeData { - low: number; - medium: number; - high: number; +interface IAggregateRiskData { + green: number; + yellow: number; + red: number; + unknown: number; unassessed: number; + assessmentCount: number; } -const extractLandscapeData = ( - totalApps: number, - data: AssessmentRisk[] -): ILandscapeData => { +const aggregateRiskData = (assessments: Assessment[]): IAggregateRiskData => { let low = 0; let medium = 0; let high = 0; - let unassessed = 0; + let unknown = 0; - data.forEach((elem) => { - switch (elem.risk) { + assessments?.forEach((assessment) => { + switch (assessment.risk) { case "green": low++; break; @@ -41,48 +33,49 @@ const extractLandscapeData = ( case "red": high++; break; + case "unknown": + unknown++; + break; } }); - unassessed = totalApps - low - medium - high; - return { low, medium, high, unassessed }; + return { + green: low, + yellow: medium, + red: high, + unknown, + unassessed: assessments.length - low - medium - high, + assessmentCount: assessments.length, + }; }; interface ILandscapeProps { + /** + * The selected questionnaire or `null` if _all questionnaires_ is selected. + */ + questionnaire: Questionnaire | null; + + /** + * The set of assessments for the selected questionnaire. Risk values will be + * aggregated from the individual assessment risks. + */ assessments: Assessment[]; } -export const Landscape: React.FC = ({ assessments }) => { +export const Landscape: React.FC = ({ + questionnaire, + assessments, +}) => { const { t } = useTranslation(); - // Context - const { allItems: applications } = useContext(ApplicationSelectionContext); - - const { - risks: assessmentRisks, - isFetching, - error: fetchError, - } = useFetchRisks(applications.map((app) => app.id!)); - - const landscapeData = useMemo(() => { - if (applications.length > 0 && assessmentRisks) { - return extractLandscapeData(applications.length, assessmentRisks); - } else { - return undefined; - } - }, [applications, assessmentRisks]); - - if (fetchError) { - return ; - } - - if (!isFetching && !landscapeData) { - return ; - } + const landscapeData = useMemo( + () => aggregateRiskData(assessments), + [assessments] + ); return ( @@ -90,44 +83,52 @@ export const Landscape: React.FC = ({ assessments }) => { } > {landscapeData && ( - - + + - - + + - - + + - - + + - - + + )} ); diff --git a/client/src/app/pages/reports/reports.tsx b/client/src/app/pages/reports/reports.tsx index 3a085a639d..dcc23d17a3 100644 --- a/client/src/app/pages/reports/reports.tsx +++ b/client/src/app/pages/reports/reports.tsx @@ -1,4 +1,4 @@ -import React, { useState } from "react"; +import React, { useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { Bullseye, @@ -9,10 +9,7 @@ import { CardExpandableContent, CardHeader, CardTitle, - Flex, - FlexItem, MenuToggle, - MenuToggleElement, PageSection, PageSectionVariants, Popover, @@ -29,46 +26,65 @@ import { } from "@patternfly/react-core"; import HelpIcon from "@patternfly/react-icons/dist/esm/icons/help-icon"; +import { Questionnaire } from "@app/api/models"; +import { useFetchApplications } from "@app/queries/applications"; +import { useFetchAssessments } from "@app/queries/assessments"; +import { useFetchQuestionnaires } from "@app/queries/questionnaires"; + import { AppPlaceholder } from "@app/components/AppPlaceholder"; import { ConditionalRender } from "@app/components/ConditionalRender"; import { StateError } from "@app/components/StateError"; + import { ApplicationSelectionContextProvider } from "./application-selection-context"; import { Landscape } from "./components/landscape"; import { AdoptionPlan } from "./components/adoption-plan"; import { IdentifiedRisksTable } from "./components/identified-risks-table"; -import { useFetchApplications } from "@app/queries/applications"; -import { useFetchAssessments } from "@app/queries/assessments"; -import { Ref } from "@app/api/models"; -const ALL_QUESTIONNAIRES = "All questionnaires"; +const ALL_QUESTIONNAIRES = -1; export const Reports: React.FC = () => { - // i18 const { t } = useTranslation(); - const [isQuestionnaireSelectOpen, setIsQuestionnaireSelectOpen] = - React.useState(false); - - const [selectedQuestionnaire, setSelectedQuestionnaire] = - React.useState("All questionnaires"); - const { assessments, isFetching: isAssessmentsFetching, fetchError: assessmentsFetchError, } = useFetchAssessments(); - // Cards + const { + questionnaires, + isFetching: isQuestionnairesFetching, + fetchError: questionnairesFetchError, + } = useFetchQuestionnaires(); + + const questionnairesById = useMemo( + () => + questionnaires.reduce>((byId, q) => { + byId[q.id] = q; + return byId; + }, {}), + [questionnaires] + ); + + const { + data: applications, + isFetching: isApplicationsFetching, + error: applicationsFetchError, + } = useFetchApplications(); + + const [isQuestionnaireSelectOpen, setIsQuestionnaireSelectOpen] = + React.useState(false); + + const [selectedQuestionnaireId, setSelectedQuestionnaireId] = + React.useState(ALL_QUESTIONNAIRES); + const [isAdoptionCandidateTable, setIsAdoptionCandidateTable] = useState(true); + const [isAdoptionPlanOpen, setAdoptionPlanOpen] = useState(false); + const [isRiskCardOpen, setIsRiskCardOpen] = useState(false); - const { - data: applications, - isFetching, - error: fetchError, - } = useFetchApplications(); const pageHeaderSection = ( @@ -77,7 +93,11 @@ export const Reports: React.FC = () => { ); - if (fetchError) { + if ( + applicationsFetchError || + assessmentsFetchError || + questionnairesFetchError + ) { return ( <> {pageHeaderSection} @@ -88,116 +108,117 @@ export const Reports: React.FC = () => { ); } - const toggleQuestionnaire = (toggleRef: React.Ref) => ( - { - setIsQuestionnaireSelectOpen(!isQuestionnaireSelectOpen); - }} - isExpanded={isQuestionnaireSelectOpen} - > - {selectedQuestionnaire} - - ); - const onSelectQuestionnaire = ( _event: React.MouseEvent | undefined, value: string | number | undefined ) => { - setSelectedQuestionnaire(value as string); + setSelectedQuestionnaireId(value as number); setIsQuestionnaireSelectOpen(false); }; - const answeredQuestionnaires: Ref[] = - isAssessmentsFetching || assessmentsFetchError + const answeredQuestionnaires: Questionnaire[] = + isAssessmentsFetching || isQuestionnairesFetching ? [] : assessments - .reduce((questionnaires: Ref[], assessment) => { - if ( - !questionnaires - .map((ref) => ref.id) - .includes(assessment.questionnaire.id) - ) { - assessment.questionnaire && - questionnaires.push(assessment.questionnaire); - } - return questionnaires; - }, []) - .sort((a, b) => { - if (a.name > b.name) return 1; - if (b.name > a.name) return -1; - return 0; - }); + .map((assessment) => assessment?.questionnaire?.id ?? -1) + .filter((id, index, ids) => id > 0 && ids.indexOf(id) !== -1) + .map((id) => questionnairesById[id]) + .sort((a, b) => a.name.localeCompare(b.name)); return ( <> {pageHeaderSection} - }> + } + > - - - - - - - {t("terms.currentLandscape")} - - - - + + setIsQuestionnaireSelectOpen(false) } - toggle={toggleQuestionnaire} + toggle={(toggleRef) => ( + { + setIsQuestionnaireSelectOpen( + !isQuestionnaireSelectOpen + ); + }} + isExpanded={isQuestionnaireSelectOpen} + > + {selectedQuestionnaireId === ALL_QUESTIONNAIRES + ? "All questionnaires" + : questionnairesById[selectedQuestionnaireId] + ?.name} + + )} shouldFocusToggleOnSelect > - + All questionnaires - {answeredQuestionnaires.map( - (answeredQuestionnaire, index) => ( + {...answeredQuestionnaires.map( + (answeredQuestionnaire) => ( {answeredQuestionnaire.name} ) )} - - + ), + }} + > + + {t("terms.currentLandscape")} + - - - assessment.questionnaire.name === - selectedQuestionnaire - ) - } - /> - + + questionnaire.id === selectedQuestionnaireId + ) + } + /> - + { - const { data, refetch, isFetching, error } = useQuery( - ["assessmentrisks", applicationIDs], - async () => { + const { data, refetch, isFetching, error } = useQuery({ + queryKey: ["assessmentrisks", applicationIDs], + queryFn: async () => { if (applicationIDs.length > 0) // return (await getAssessmentLandscape(applicationIDs)).data; //TODO see if we still need this return []; else return []; }, - { - onError: (error) => console.log("error, ", error), - } - ); + onError: (error) => console.log("error, ", error), + }); + return { risks: data || [], isFetching,