From ef7d7df3d0d300a8e0c0d6726b1b9e349722871a Mon Sep 17 00:00:00 2001 From: Mahmoud Date: Tue, 20 Sep 2022 20:27:37 -0500 Subject: [PATCH] Display errors before warnings Extend hitbox for downloading templates to the entire button and not the text Update error page with download example link, extend design-system dropdown, set max height and overflow scroll Update upload loader Refactor error page - initial pass at breaking out the error page Add successes and errors section headers Reverse the loading and animation, replace with loading spinner and static text Fix copy Update comments, change component names, update affected components Fix hasErrorsOrWarnings logic More clean up - add comments Fix warnings, move loading logic positioning, add useEffect to update selected system to fix bug Update copy, update copy for 0 errors, and remove TODO Move error/warning desc to own func Change invalid metric to other, always render continue button fix lint warning Change Continue to Review button --- .../DataUpload/DataUpload.styles.tsx | 74 ++++- .../src/components/DataUpload/DataUpload.tsx | 97 ++++-- .../DataUpload/InstructionsTemplate.tsx | 18 +- .../components/DataUpload/SystemSelection.tsx | 16 +- .../DataUpload/UploadErrorsWarnings.tsx | 286 +++++++++++------- publisher/src/components/DataUpload/types.ts | 8 +- publisher/src/components/Loading/Loading.tsx | 2 +- publisher/src/components/Menu/Menu.styles.tsx | 11 +- publisher/src/components/Menu/Menu.tsx | 7 +- 9 files changed, 341 insertions(+), 178 deletions(-) diff --git a/publisher/src/components/DataUpload/DataUpload.styles.tsx b/publisher/src/components/DataUpload/DataUpload.styles.tsx index 09e30ffd7..61f71e0f2 100644 --- a/publisher/src/components/DataUpload/DataUpload.styles.tsx +++ b/publisher/src/components/DataUpload/DataUpload.styles.tsx @@ -153,7 +153,7 @@ export const ButtonWrapper = styled.div` margin: 13px 0; `; -export const UploadErrorButtonWrapper = styled(ButtonWrapper)` +export const ErrorWarningButtonWrapper = styled(ButtonWrapper)` width: 100%; justify-content: space-between; @@ -258,7 +258,7 @@ export const Button = styled.div<{ type?: ButtonTypes }>` } `; -export const DownloadTemplateBox = styled.div` +export const DownloadTemplateBox = styled.a` ${typography.sizeCSS.normal}; display: flex; align-items: center; @@ -270,7 +270,15 @@ export const DownloadTemplateBox = styled.div` border: 1px solid ${palette.highlight.grey4}; border-radius: 4px; - a { + &:hover { + background: ${palette.highlight.grey1}; + } +`; + +export const DownloadTemplateSystem = styled.div` + color: ${palette.solid.darkgrey}; + + span { ${typography.sizeCSS.small}; display: block; width: fit-content; @@ -360,7 +368,7 @@ export const DragDropContainer = styled.div<{ dragging?: boolean }>` color: ${palette.solid.white}; `; -export const UserPromptContainer = styled.div` +export const Container = styled.div` width: 100%; min-height: 100%; display: flex; @@ -370,7 +378,7 @@ export const UserPromptContainer = styled.div` padding-bottom: 80px; `; -export const UserPromptWrapper = styled.div` +export const Wrapper = styled.div` width: 100%; max-width: 50%; display: flex; @@ -379,11 +387,11 @@ export const UserPromptWrapper = styled.div` align-items: flex-start; `; -export const UserPromptTitle = styled.div` +export const Title = styled.div` ${typography.sizeCSS.title}; `; -export const UserPromptDescription = styled.div` +export const ErrorWarningDescription = styled.div` ${typography.sizeCSS.medium}; margin: 8px 0; @@ -392,13 +400,20 @@ export const UserPromptDescription = styled.div` } `; -export const UserPromptErrorContainer = styled.div` +export const MessagesContainer = styled.div` width: 100%; margin-top: 19px; `; -export const UserPromptError = styled.div` - margin-bottom: 40px; +export const Message = styled.div``; + +export const SectionHeader = styled.div` + ${typography.sizeCSS.title}; + margin: 10px 0; + + &:not(:first-child) { + margin: 40px 0 10px 0; + } `; export const MetricTitle = styled.div` @@ -418,12 +433,12 @@ export const MetricTitle = styled.div` } `; -export const ErrorIconWrapper = styled.div` +export const IconWrapper = styled.div` display: flex; align-items: center; gap: 5px; `; -export const ErrorMessageWrapper = styled.div` +export const MessageBody = styled.div` ${typography.sizeCSS.medium}; width: 100%; display: flex; @@ -431,15 +446,15 @@ export const ErrorMessageWrapper = styled.div` align-items: center; `; -export const ErrorMessageTitle = styled.div` +export const MessageTitle = styled.div` display: block; `; -export const ErrorMessageDescription = styled.div` +export const MessageSubtitle = styled.div` display: block; `; -export const ErrorAdditionalInfo = styled.div` +export const MessageDescription = styled.div` ${typography.sizeCSS.normal}; margin: 8px 0 13px 0; `; @@ -530,3 +545,32 @@ export const OrangeText = styled.span` export const StrikethroughText = styled.span` text-decoration: line-through; `; + +export const BlueText = styled.span` + color: ${palette.solid.blue}; +`; + +export const DataUploadLoading = styled.div` + width: 100%; + height: 100%; + display: flex; + flex-direction: column; + justify-content: center; + align-items: center; +`; + +export const LoadingHeader = styled.div` + ${typography.sizeCSS.large}; + display: flex; + margin: 20px 0 5px 0; + gap: 3px; +`; + +export const LoadingSubheader = styled.div` + ${typography.sizeCSS.normal}; +`; + +export const CheckIcon = styled.img` + width: 16px; + margin-right: 5px; +`; diff --git a/publisher/src/components/DataUpload/DataUpload.tsx b/publisher/src/components/DataUpload/DataUpload.tsx index 82352c7da..314f67215 100644 --- a/publisher/src/components/DataUpload/DataUpload.tsx +++ b/publisher/src/components/DataUpload/DataUpload.tsx @@ -16,19 +16,22 @@ // ============================================================================= import { observer } from "mobx-react-lite"; -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { useNavigate } from "react-router-dom"; import { AgencySystems } from "../../shared/types"; import { useStore } from "../../stores"; import logoImg from "../assets/jc-logo-vector.png"; import { Logo, LogoContainer } from "../Header"; -import { Loading } from "../Loading"; +import { Loader } from "../Loading"; import { showToast } from "../Toast"; import { Button, DataUploadContainer, DataUploadHeader, + DataUploadLoading, + LoadingHeader, + LoadingSubheader, SystemSelection, UploadFile, } from "."; @@ -36,6 +39,7 @@ import { DataUploadResponseBody, ErrorsWarningsMetrics, MetricErrors, + UploadedMetric, } from "./types"; import { UploadErrorsWarnings } from "./UploadErrorsWarnings"; @@ -82,6 +86,7 @@ export const systemToTemplateSpreadsheetFileName: { [system: string]: string } = export const DataUpload: React.FC = observer(() => { const { userStore, reportStore } = useStore(); const navigate = useNavigate(); + // eslint-disable-next-line react-hooks/exhaustive-deps const userSystems = userStore.currentAgency?.systems.filter( (system) => !EXCLUDED_SYSTEMS.includes(system) @@ -93,7 +98,7 @@ export const DataUpload: React.FC = observer(() => { const [selectedFile, setSelectedFile] = useState(); const [selectedSystem, setSelectedSystem] = useState< AgencySystems | undefined - >(userSystems.length === 1 ? userSystems[0] : undefined); + >(); const handleFileUpload = async ( file: File, @@ -118,12 +123,15 @@ export const DataUpload: React.FC = observer(() => { const data = await response?.json(); const errorsWarningsAndMetrics = processUploadResponseBody(data); + const hasErrorsOrWarnings = + (errorsWarningsAndMetrics.preIngestErrors && + errorsWarningsAndMetrics.preIngestErrors.length > 0) || + errorsWarningsAndMetrics.errorSheetsAndSuccessfulMetrics.errorSheets + .length > 0 || + errorsWarningsAndMetrics.errorSheetsAndSuccessfulMetrics.hasWarnings; setIsLoading(false); - if ( - errorsWarningsAndMetrics.errorCount || - errorsWarningsAndMetrics.warningCount - ) { + if (hasErrorsOrWarnings) { return setErrorsWarningsMetrics(errorsWarningsAndMetrics); } @@ -137,35 +145,56 @@ export const DataUpload: React.FC = observer(() => { const processUploadResponseBody = ( data: DataUploadResponseBody ): ErrorsWarningsMetrics => { - const metricErrors = data.metrics.reduce( - (acc, metric) => [...acc, ...metric.sheets], - [] as MetricErrors[] - ); - const errorWarningCount = metricErrors.reduce( - (acc, sheet) => { - sheet.messages.forEach((msg) => { - if (msg.type === "ERROR") acc.errorCount += 1; - if (msg.type === "WARNING") acc.warningCount += 1; + const errorSheetsAndSuccessfulMetrics = data.metrics.reduce( + (acc, metric) => { + /** + * Peek into the `messages` array to look for any error messages within + * the sheet and return `true` if no errors are found + */ + const noErrorsInCurrentSheet = + metric.sheets.filter( + (sheet) => + sheet.messages.filter((msg) => msg.type === "ERROR").length > 0 + ).length === 0; + + if (metric.sheets.length === 0 || noErrorsInCurrentSheet) { + acc.successfulMetrics.push(metric); + } + + metric.sheets.forEach((sheet) => { + sheet.messages.forEach((message) => { + if (message.type === "ERROR") { + acc.errorSheets.push(sheet); + } + if (message.type === "WARNING" && acc.hasWarnings === false) { + acc.hasWarnings = true; + } + }); }); + return acc; }, { - errorCount: 0, - warningCount: 0, + successfulMetrics: [] as UploadedMetric[], + errorSheets: [] as MetricErrors[], + hasWarnings: false, } ); + /** + * Pre-Ingest errors: errors that are not associated with a metric. + * @example: user uploads an excel file that contains a sheet not associated + * with a metric. + */ if (data.pre_ingest_errors) { - errorWarningCount.errorCount += data.pre_ingest_errors.length; return { - metricErrors, - ...errorWarningCount, + errorSheetsAndSuccessfulMetrics, metrics: data.metrics, preIngestErrors: data.pre_ingest_errors, }; } - return { metricErrors, ...errorWarningCount, metrics: data.metrics }; + return { errorSheetsAndSuccessfulMetrics, metrics: data.metrics }; }; const handleSystemSelection = (file: File, system: AgencySystems) => { @@ -181,14 +210,6 @@ export const DataUpload: React.FC = observer(() => { setSelectedSystem(userSystems.length === 1 ? userSystems[0] : undefined); }; - if (isLoading) { - return ( - - - - ); - } - const renderCurrentUploadStep = (): JSX.Element => { /** * There are ~4 steps in the upload phase before reaching the metrics confirmation page. @@ -241,6 +262,22 @@ export const DataUpload: React.FC = observer(() => { ); }; + useEffect(() => { + setSelectedSystem(userSystems.length === 1 ? userSystems[0] : undefined); + }, [userSystems]); + + if (isLoading) { + return ( + + + + We are processing your data... + This might take a few minutes. + + + ); + } + return ( diff --git a/publisher/src/components/DataUpload/InstructionsTemplate.tsx b/publisher/src/components/DataUpload/InstructionsTemplate.tsx index 19844e8d4..959fc0cad 100644 --- a/publisher/src/components/DataUpload/InstructionsTemplate.tsx +++ b/publisher/src/components/DataUpload/InstructionsTemplate.tsx @@ -22,6 +22,7 @@ import { ReactComponent as SpreadsheetIcon } from "../assets/microsoft-excel-ico import { ButtonWrapper, DownloadTemplateBox, + DownloadTemplateSystem, systemToTemplateSpreadsheetFileName, } from "."; @@ -202,18 +203,17 @@ export const GeneralInstructions: React.FC< const systemFileName = systemToTemplateSpreadsheetFileName[system]; return ( - + - + {systemName} - - Download - - + Download + ); })} diff --git a/publisher/src/components/DataUpload/SystemSelection.tsx b/publisher/src/components/DataUpload/SystemSelection.tsx index 550ad15e3..8d50869e3 100644 --- a/publisher/src/components/DataUpload/SystemSelection.tsx +++ b/publisher/src/components/DataUpload/SystemSelection.tsx @@ -21,12 +21,12 @@ import { AgencySystems } from "../../shared/types"; import { removeSnakeCase } from "../../utils"; import { ReactComponent as CheckIcon } from "../assets/check-icon.svg"; import { + Container, FileName, SelectSystemOptions, SystemName, - UserPromptContainer, - UserPromptTitle, - UserPromptWrapper, + Title, + Wrapper, } from "."; type SystemSelectionProps = { @@ -41,13 +41,13 @@ export const SystemSelection: React.FC = ({ handleSystemSelection, }) => { return ( - - + + {selectedFile.name} - Which system is this data for? + Which system is this data for? {userSystems.map((system) => ( @@ -60,7 +60,7 @@ export const SystemSelection: React.FC = ({ ))} - - + + ); }; diff --git a/publisher/src/components/DataUpload/UploadErrorsWarnings.tsx b/publisher/src/components/DataUpload/UploadErrorsWarnings.tsx index be00e6f07..3c0b88b7d 100644 --- a/publisher/src/components/DataUpload/UploadErrorsWarnings.tsx +++ b/publisher/src/components/DataUpload/UploadErrorsWarnings.tsx @@ -20,25 +20,28 @@ import { useNavigate } from "react-router-dom"; import { removeSnakeCase } from "../../utils"; import { ReactComponent as ErrorIcon } from "../assets/error-icon.svg"; +import checkIcon from "../assets/status-check-icon.png"; import { ReactComponent as WarningIcon } from "../assets/warning-icon.svg"; import { + BlueText, Button, - ErrorAdditionalInfo, - ErrorIconWrapper, - ErrorMessageDescription, - ErrorMessageTitle, - ErrorMessageWrapper, + CheckIcon, + Container, + ErrorWarningButtonWrapper, + ErrorWarningDescription, + IconWrapper, + Message, + MessageBody, + MessageDescription, + MessagesContainer, + MessageSubtitle, + MessageTitle, MetricTitle, - OrangeText, RedText, + SectionHeader, systemToTemplateSpreadsheetFileName, - UploadErrorButtonWrapper, - UserPromptContainer, - UserPromptDescription, - UserPromptError, - UserPromptErrorContainer, - UserPromptTitle, - UserPromptWrapper, + Title, + Wrapper, } from "."; import { ErrorsWarningsMetrics } from "./types"; @@ -52,115 +55,182 @@ export const UploadErrorsWarnings: React.FC = ({ selectedSystem, resetToNewUpload, }) => { - const { errorCount, warningCount, metrics, metricErrors, preIngestErrors } = + const { metrics, errorSheetsAndSuccessfulMetrics, preIngestErrors } = errorsWarningsMetrics; const navigate = useNavigate(); const systemFileName = selectedSystem && systemToTemplateSpreadsheetFileName[selectedSystem]; - const hasWarningsOnly = !!warningCount && !errorCount; + const successCount = errorSheetsAndSuccessfulMetrics.successfulMetrics.length; + /** If there are pre-ingest errors, include them in the error count */ + const errorCount = preIngestErrors + ? errorSheetsAndSuccessfulMetrics.errorSheets.length + + preIngestErrors?.length + : errorSheetsAndSuccessfulMetrics.errorSheets.length; const renderMessages = () => { return ( <> - {metricErrors.map((sheet) => ( - - - {sheet.display_name} {sheet.sheet_name} - - - {sheet.display_name && - sheet.messages?.map((message) => ( - - - {message.type === "ERROR" ? : } - - - {message.title} - - {message.subtitle} - - - - - {message.description} - - - ))} - - ))} - - {preIngestErrors?.map((message) => ( - - - - {message.type === "ERROR" ? : } - - - {message.title} - - {message.subtitle} - - - - {message.description} - - ))} + {/* Errors */} + {errorCount > 0 && ( + <> + Errors + {errorSheetsAndSuccessfulMetrics.errorSheets.map((sheet) => ( + + + {sheet.display_name} {sheet.sheet_name} + + + {sheet.display_name && + sheet.messages.map((message) => ( + + + {message.type === "ERROR" ? ( + + ) : ( + + )} + + + {message.title} + {message.subtitle} + + + + {message.description} + + + ))} + + ))} + + {preIngestErrors && preIngestErrors.length > 0 && ( + + Other + {preIngestErrors.map((message) => ( + + + {message.type === "ERROR" ? ( + + ) : ( + + )} + + + {message.title} + {message.subtitle} + + + + {message.description} + + + ))} + + )} + + )} + + {/* Successful Metrics */} + {successCount > 0 && ( + <> + Successes + {errorSheetsAndSuccessfulMetrics.successfulMetrics.map((metric) => ( + + + + {metric.display_name} + + + {metric.sheets.map((sheet) => ( + + {sheet.messages.map((message) => ( + + + {message.type === "ERROR" ? ( + + ) : ( + + )} + + + {message.title} + + {message.subtitle} + + + + + {message.description} + + + ))} + + ))} + + ))} + + )} ); }; - return ( - - - {/* Error/Warning Header */} - {hasWarningsOnly ? ( + const renderErrorWarningTitle = () => { + return ( + <> + {errorCount === 0 && ( <> - - We found {warningCount} warning - {warningCount > 1 ? "s" : ""}, but you can still proceed. - - - We ran into a few discrepancies between the uploaded data and the - Justice Counts format for the{" "} - - - {selectedSystem && - removeSnakeCase(selectedSystem).toLowerCase()} - - {" "} - system, but we did our best to resolve them. Please review the - warnings and determine if it is safe to proceed. If not, resolve - the warnings in your file and reupload. - + {successCount} metric + {successCount === 0 || successCount > 1 ? "s" : ""} were uploaded + successfully. - ) : ( + )} + {errorCount > 0 && ( <> - - Uh oh, we found {errorCount} error - {errorCount > 1 ? "s" : ""}. - - - We ran into a few discrepancies between the uploaded data and the - Justice Counts format for the{" "} - - - {selectedSystem && - removeSnakeCase(selectedSystem).toLowerCase()} - - {" "} - system. - + We found {errorCount} error + {errorCount > 1 ? "s" : ""}, and {successCount}{" "} + metric + {successCount === 0 || successCount > 1 ? "s" : ""} were uploaded + successfully. )} + + ); + }; + + const renderErrorWarningDescription = () => { + return ( + <> + We ran into a few discrepancies between the uploaded data and the + Justice Counts format for the{" "} + + {selectedSystem && removeSnakeCase(selectedSystem).toLowerCase()} + {" "} + system ( + + download example + + ) + {errorCount > 0 + ? `. To continue, please resolve the errors in your file and + reupload.` + : `, but we did our best to resolve them. Please review the + warnings and determine if it is safe to proceed. If not, + resolve the warnings in your file and reupload.`} + + ); + }; + + return ( + + + {/* Error/Warning Header */} + {renderErrorWarningTitle()} + + {renderErrorWarningDescription()} + {/* Action Button(s) */} - + - + {/* Messages */} - {renderMessages()} - - + {renderMessages()} + + ); }; diff --git a/publisher/src/components/DataUpload/types.ts b/publisher/src/components/DataUpload/types.ts index 4d5517394..4f048caf3 100644 --- a/publisher/src/components/DataUpload/types.ts +++ b/publisher/src/components/DataUpload/types.ts @@ -42,9 +42,11 @@ export type MetricErrors = { }; export type ErrorsWarningsMetrics = { - errorCount: number; - warningCount: number; metrics: UploadedMetric[]; - metricErrors: MetricErrors[]; + errorSheetsAndSuccessfulMetrics: { + successfulMetrics: UploadedMetric[]; + errorSheets: MetricErrors[]; + hasWarnings: boolean; + }; preIngestErrors?: ErrorWarningMessage[]; }; diff --git a/publisher/src/components/Loading/Loading.tsx b/publisher/src/components/Loading/Loading.tsx index 05cb7d487..78483e8cb 100644 --- a/publisher/src/components/Loading/Loading.tsx +++ b/publisher/src/components/Loading/Loading.tsx @@ -40,7 +40,7 @@ const loadingSpriteAnimation = keyframes` } `; -const Loader = styled.div` +export const Loader = styled.div` height: ${loaderWidth}px; width: ${loaderWidth}px; background-image: url(${sprite}); diff --git a/publisher/src/components/Menu/Menu.styles.tsx b/publisher/src/components/Menu/Menu.styles.tsx index 61fa095e8..7c5dd829a 100644 --- a/publisher/src/components/Menu/Menu.styles.tsx +++ b/publisher/src/components/Menu/Menu.styles.tsx @@ -14,7 +14,11 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . // ============================================================================= -import { DropdownMenuItem, DropdownToggle } from "@recidiviz/design-system"; +import { + DropdownMenu, + DropdownMenuItem, + DropdownToggle, +} from "@recidiviz/design-system"; import styled from "styled-components/macro"; import { HEADER_BAR_HEIGHT, palette, typography } from "../GlobalStyles"; @@ -90,6 +94,11 @@ export const ExtendedDropdownToggle = styled(DropdownToggle)<{ } `; +export const ExtendedDropdownMenu = styled(DropdownMenu)` + max-height: 50vh; + overflow-y: scroll; +`; + export const ExtendedDropdownMenuItem = styled(DropdownMenuItem)<{ highlight?: boolean; noPadding?: boolean; diff --git a/publisher/src/components/Menu/Menu.tsx b/publisher/src/components/Menu/Menu.tsx index 92715cc94..bdb88a74f 100644 --- a/publisher/src/components/Menu/Menu.tsx +++ b/publisher/src/components/Menu/Menu.tsx @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . // ============================================================================= -import { Dropdown, DropdownMenu } from "@recidiviz/design-system"; +import { Dropdown } from "@recidiviz/design-system"; import { observer } from "mobx-react-lite"; import React, { useEffect, useState } from "react"; import { useLocation, useNavigate } from "react-router-dom"; @@ -23,6 +23,7 @@ import { Permission } from "../../shared/types"; import { useStore } from "../../stores"; import { Button } from "../DataUpload"; import { + ExtendedDropdownMenu, ExtendedDropdownMenuItem, ExtendedDropdownToggle, MenuContainer, @@ -126,7 +127,7 @@ const Menu = () => { Agencies - + {userStore.userAgencies?.map((agency) => { return ( { ); })} - + )}