From 1c5973893d52dd63426f233e82c52fabbb902d35 Mon Sep 17 00:00:00 2001 From: Annie Hawes Date: Mon, 18 Nov 2024 18:33:55 +0000 Subject: [PATCH 1/3] [TDRD-87] Row validation error report download compatibility tweaks --- ...DraftMetadataChecksResultsController.scala | 10 +++-- app/services/DraftMetadataService.scala | 2 +- ...tMetadataChecksResultsControllerSpec.scala | 45 ++++++++++++++++--- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/app/controllers/DraftMetadataChecksResultsController.scala b/app/controllers/DraftMetadataChecksResultsController.scala index 3ed2593e7..5e39a4cbb 100644 --- a/app/controllers/DraftMetadataChecksResultsController.scala +++ b/app/controllers/DraftMetadataChecksResultsController.scala @@ -8,7 +8,7 @@ import graphql.codegen.GetConsignmentStatus.getConsignmentStatus.GetConsignment import org.pac4j.play.scala.SecurityComponents import play.api.i18n.{I18nSupport, Lang, MessagesApi} import play.api.mvc.{Action, AnyContent, Request} -import services.FileError.SCHEMA_VALIDATION +import services.FileError.{SCHEMA_VALIDATION, UTF_8, ROW_VALIDATION} import services.Statuses._ import services._ import viewsapi.Caching.preventCaching @@ -134,15 +134,19 @@ class DraftMetadataChecksResultsController @Inject() ( reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken) errorReport <- draftMetadataService.getErrorReport(consignmentId) } yield { + val orderingPrioritisingRowErrors: Ordering[Error] = + Ordering.by[Error, (Boolean, String)](e => (!e.validationProcess.equals(s"$ROW_VALIDATION"), e.validationProcess)) val errorList = errorReport.fileError match { case SCHEMA_VALIDATION => errorReport.validationErrors.flatMap { validationErrors => val data = validationErrors.data.map(metadata => metadata.name -> metadata.value).toMap - validationErrors.errors.map(error => List(data(filePath), error.property, data(error.property), error.message)) + validationErrors.errors.toList + .sorted(orderingPrioritisingRowErrors) + .map(error => List(validationErrors.assetId, error.property, data.getOrElse(error.property, ""), error.message)) } case _ => Nil } - val header: List[String] = List(filePath, "Field", "Value", "Error Message") + val header: List[String] = List(filePath, "Column", "Value", "Error Message") val excelFile = ExcelUtils.writeExcel(s"Error report for $reference", header :: errorList) val dateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH-mm-ss") val currentDateTime = dateTimeFormatter.format(LocalDateTime.now()) diff --git a/app/services/DraftMetadataService.scala b/app/services/DraftMetadataService.scala index 4dc837c4d..b43a8385b 100644 --- a/app/services/DraftMetadataService.scala +++ b/app/services/DraftMetadataService.scala @@ -17,7 +17,7 @@ import scala.concurrent.{ExecutionContext, Future} object FileError extends Enumeration { type FileError = Value - val UTF_8, INVALID_CSV, DUPLICATE_HEADER, SCHEMA_REQUIRED, SCHEMA_VALIDATION, VIRUS, UNKNOWN, NONE = Value + val UTF_8, INVALID_CSV, ROW_VALIDATION, DUPLICATE_HEADER, SCHEMA_REQUIRED, SCHEMA_VALIDATION, VIRUS, UNKNOWN, NONE = Value } case class Error(validationProcess: String, property: String, errorKey: String, message: String) diff --git a/test/controllers/DraftMetadataChecksResultsControllerSpec.scala b/test/controllers/DraftMetadataChecksResultsControllerSpec.scala index 6d885a9e5..4a7f14597 100644 --- a/test/controllers/DraftMetadataChecksResultsControllerSpec.scala +++ b/test/controllers/DraftMetadataChecksResultsControllerSpec.scala @@ -246,9 +246,9 @@ class DraftMetadataChecksResultsControllerSpec extends FrontEndTestHelper { setConsignmentTypeResponse(wiremockServer, "standard") setConsignmentReferenceResponse(wiremockServer) - val error = Error("BASE_SCHEMA", "FOI exemption code", "enum", "BASE_SCHEMA.foi_exmption_code.enum") + val error = Error("BASE_SCHEMA", "FOI exemption code", "enum", "BASE_SCHEMA.foi_exemption_code.enum") val metadata = List(Metadata("FOI exemption code", "abcd"), Metadata("Filepath", "/aa/bb/faq")) - val errorFileData = ErrorFileData(consignmentId, date = "2024-12-12", FileError.SCHEMA_VALIDATION, List(ValidationErrors("assetId", Set(error), metadata))) + val errorFileData = ErrorFileData(consignmentId, date = "2024-12-12", FileError.SCHEMA_VALIDATION, List(ValidationErrors("/aa/bb/faq", Set(error), metadata))) val response = instantiateController(errorFileData = Some(errorFileData)) .downloadErrorReport(consignmentId)(FakeRequest(GET, s"/consignment/$consignmentId/draft-metadata/download-report")) @@ -261,14 +261,14 @@ class DraftMetadataChecksResultsControllerSpec extends FrontEndTestHelper { rows.length must equal(2) rows.head.getCell(0).asString must equal("Filepath") - rows.head.getCell(1).asString must equal("Field") + rows.head.getCell(1).asString must equal("Column") rows.head.getCell(2).asString must equal("Value") rows.head.getCell(3).asString must equal("Error Message") rows(1).getCell(0).asString must equal("/aa/bb/faq") rows(1).getCell(1).asString must equal("FOI exemption code") rows(1).getCell(2).asString must equal("abcd") - rows(1).getCell(3).asString must equal("BASE_SCHEMA.foi_exmption_code.enum") + rows(1).getCell(3).asString must equal("BASE_SCHEMA.foi_exemption_code.enum") } "download the excel file without error data when the error type is not SCHEMA_VALIDATION" in { @@ -291,10 +291,45 @@ class DraftMetadataChecksResultsControllerSpec extends FrontEndTestHelper { rows.length must equal(1) rows.head.getCell(0).asString must equal("Filepath") - rows.head.getCell(1).asString must equal("Field") + rows.head.getCell(1).asString must equal("Column") rows.head.getCell(2).asString must equal("Value") rows.head.getCell(3).asString must equal("Error Message") } + + "download the excel file with row validation errors at the top where present" in { + setConsignmentTypeResponse(wiremockServer, "standard") + setConsignmentReferenceResponse(wiremockServer) + val schemaError = Error("BASE_SCHEMA", "FOI exemption code", "enum", "BASE_SCHEMA.foi_exmption_code.enum") + val rowValidationError = Error("ROW_VALIDATION", "", "unknown", "This file was listed in your metadata file but does not match to one of your uploaded files") + val metadata = List(Metadata("FOI exemption code", "abcd"), Metadata("Filepath", "/aa/bb/faq")) + val errorFileData = + ErrorFileData(consignmentId, date = "2024-12-12", FileError.SCHEMA_VALIDATION, List(ValidationErrors("/aa/bb/faq", Set(schemaError, rowValidationError), metadata))) + val response = instantiateController(errorFileData = Some(errorFileData)) + .downloadErrorReport(consignmentId)(FakeRequest(GET, s"/consignment/$consignmentId/draft-metadata/download-report")) + + val responseByteArray: ByteString = contentAsBytes(response) + val bufferedSource = new ByteArrayInputStream(responseByteArray.toArray) + val wb: ReadableWorkbook = new ReadableWorkbook(bufferedSource) + val ws: Sheet = wb.getFirstSheet + val rows: List[Row] = ws.read.asScala.toList + + rows.length must equal(3) + + rows.head.getCell(0).asString must equal("Filepath") + rows.head.getCell(1).asString must equal("Column") + rows.head.getCell(2).asString must equal("Value") + rows.head.getCell(3).asString must equal("Error Message") + + rows(1).getCell(0).asString must equal("/aa/bb/faq") + rows(1).getCell(1).asString must equal("") + rows(1).getCell(2).asString must equal("") + rows(1).getCell(3).asString must equal("This file was listed in your metadata file but does not match to one of your uploaded files") + + rows(2).getCell(0).asString must equal("/aa/bb/faq") + rows(2).getCell(1).asString must equal("FOI exemption code") + rows(2).getCell(2).asString must equal("abcd") + rows(2).getCell(3).asString must equal("BASE_SCHEMA.foi_exmption_code.enum") + } } private def instantiateController( From 21752c0c6eb125c638a4427b82b679555ce3741f Mon Sep 17 00:00:00 2001 From: Annie Hawes Date: Tue, 19 Nov 2024 13:25:36 +0000 Subject: [PATCH 2/3] Review feedback --- ...DraftMetadataChecksResultsController.scala | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/app/controllers/DraftMetadataChecksResultsController.scala b/app/controllers/DraftMetadataChecksResultsController.scala index 5e39a4cbb..3a2ca2869 100644 --- a/app/controllers/DraftMetadataChecksResultsController.scala +++ b/app/controllers/DraftMetadataChecksResultsController.scala @@ -4,11 +4,10 @@ import auth.TokenSecurity import configuration.{ApplicationConfig, KeycloakConfiguration} import controllers.util.ExcelUtils import controllers.util.MetadataProperty.filePath -import graphql.codegen.GetConsignmentStatus.getConsignmentStatus.GetConsignment import org.pac4j.play.scala.SecurityComponents import play.api.i18n.{I18nSupport, Lang, MessagesApi} import play.api.mvc.{Action, AnyContent, Request} -import services.FileError.{SCHEMA_VALIDATION, UTF_8, ROW_VALIDATION} +import services.FileError.{FileError, ROW_VALIDATION, SCHEMA_VALIDATION, UTF_8} import services.Statuses._ import services._ import viewsapi.Caching.preventCaching @@ -129,32 +128,33 @@ class DraftMetadataChecksResultsController @Inject() ( } } - def downloadErrorReport(consignmentId: UUID): Action[AnyContent] = standardUserAndTypeAction(consignmentId) { implicit request: Request[AnyContent] => - for { - reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken) - errorReport <- draftMetadataService.getErrorReport(consignmentId) - } yield { - val orderingPrioritisingRowErrors: Ordering[Error] = - Ordering.by[Error, (Boolean, String)](e => (!e.validationProcess.equals(s"$ROW_VALIDATION"), e.validationProcess)) - val errorList = errorReport.fileError match { - case SCHEMA_VALIDATION => - errorReport.validationErrors.flatMap { validationErrors => - val data = validationErrors.data.map(metadata => metadata.name -> metadata.value).toMap - validationErrors.errors.toList - .sorted(orderingPrioritisingRowErrors) - .map(error => List(validationErrors.assetId, error.property, data.getOrElse(error.property, ""), error.message)) - } - case _ => Nil + def downloadErrorReport(consignmentId: UUID, priorityErrorType: FileError = ROW_VALIDATION): Action[AnyContent] = standardUserAndTypeAction(consignmentId) { + implicit request: Request[AnyContent] => + for { + reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken) + errorReport <- draftMetadataService.getErrorReport(consignmentId) + } yield { + val orderingPrioritisingRowErrors: Ordering[Error] = + Ordering.by[Error, (Boolean, String)](e => (!e.validationProcess.equals(s"$priorityErrorType"), e.validationProcess)) + val errorList = errorReport.fileError match { + case SCHEMA_VALIDATION => + errorReport.validationErrors.flatMap { validationErrors => + val data = validationErrors.data.map(metadata => metadata.name -> metadata.value).toMap + validationErrors.errors.toList + .sorted(orderingPrioritisingRowErrors) + .map(error => List(validationErrors.assetId, error.property, data.getOrElse(error.property, ""), error.message)) + } + case _ => Nil + } + val header: List[String] = List(filePath, "Column", "Value", "Error Message") + val excelFile = ExcelUtils.writeExcel(s"Error report for $reference", header :: errorList) + val dateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH-mm-ss") + val currentDateTime = dateTimeFormatter.format(LocalDateTime.now()) + val excelContentType = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + Ok(excelFile) + .as(excelContentType) + .withHeaders("Content-Disposition" -> s"attachment; filename=ErrorReport-${reference}-$currentDateTime.xlsx") } - val header: List[String] = List(filePath, "Column", "Value", "Error Message") - val excelFile = ExcelUtils.writeExcel(s"Error report for $reference", header :: errorList) - val dateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH-mm-ss") - val currentDateTime = dateTimeFormatter.format(LocalDateTime.now()) - val excelContentType = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" - Ok(excelFile) - .as(excelContentType) - .withHeaders("Content-Disposition" -> s"attachment; filename=ErrorReport-${reference}-$currentDateTime.xlsx") - } } } From d91edb368fe26e1a4d5ecca88491ec4393fcf448 Mon Sep 17 00:00:00 2001 From: Annie Hawes Date: Tue, 19 Nov 2024 14:17:50 +0000 Subject: [PATCH 3/3] Further review feedback --- app/controllers/DraftMetadataChecksResultsController.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/DraftMetadataChecksResultsController.scala b/app/controllers/DraftMetadataChecksResultsController.scala index 3a2ca2869..5cdb5834c 100644 --- a/app/controllers/DraftMetadataChecksResultsController.scala +++ b/app/controllers/DraftMetadataChecksResultsController.scala @@ -134,14 +134,14 @@ class DraftMetadataChecksResultsController @Inject() ( reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken) errorReport <- draftMetadataService.getErrorReport(consignmentId) } yield { - val orderingPrioritisingRowErrors: Ordering[Error] = + val errorPriorityOrdering: Ordering[Error] = Ordering.by[Error, (Boolean, String)](e => (!e.validationProcess.equals(s"$priorityErrorType"), e.validationProcess)) val errorList = errorReport.fileError match { case SCHEMA_VALIDATION => errorReport.validationErrors.flatMap { validationErrors => val data = validationErrors.data.map(metadata => metadata.name -> metadata.value).toMap validationErrors.errors.toList - .sorted(orderingPrioritisingRowErrors) + .sorted(errorPriorityOrdering) .map(error => List(validationErrors.assetId, error.property, data.getOrElse(error.property, ""), error.message)) } case _ => Nil