From e3951b72d3cd5a411f5a6c7bf06f34952d90ef9f Mon Sep 17 00:00:00 2001 From: ian-hoyle Date: Fri, 8 Nov 2024 12:03:56 +0000 Subject: [PATCH] Tdrd 351 handle error when user clicks upload but no file was selected (#4268) Adding a proper error page if app fails on trying to upload. Not the debug message --- .../DraftMetadataUploadController.scala | 14 ++++- .../draftMetadataUploadError.scala.html | 62 +++++++++++++++++++ .../DraftMetadataUploadControllerSpec.scala | 33 +++++++--- 3 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 app/views/draftmetadata/draftMetadataUploadError.scala.html diff --git a/app/controllers/DraftMetadataUploadController.scala b/app/controllers/DraftMetadataUploadController.scala index d4458fae4..1a2e0dce1 100644 --- a/app/controllers/DraftMetadataUploadController.scala +++ b/app/controllers/DraftMetadataUploadController.scala @@ -6,9 +6,10 @@ import cats.effect.IO.fromOption import cats.effect.unsafe.implicits.global import configuration.{ApplicationConfig, KeycloakConfiguration} import org.pac4j.play.scala.SecurityComponents +import play.api._ import play.api.i18n.I18nSupport import play.api.libs.Files.TemporaryFile -import play.api.mvc.{Action, AnyContent, MultipartFormData, Request, Result} +import play.api.mvc._ import services._ import viewsapi.Caching.preventCaching @@ -28,7 +29,8 @@ class DraftMetadataUploadController @Inject() ( val applicationConfig: ApplicationConfig )(implicit val ec: ExecutionContext) extends TokenSecurity - with I18nSupport { + with I18nSupport + with Logging { def draftMetadataUploadPage(consignmentId: UUID): Action[AnyContent] = standardUserAndTypeAction(consignmentId) { implicit request: Request[AnyContent] => if (applicationConfig.blockDraftMetadataUpload) { @@ -64,7 +66,13 @@ class DraftMetadataUploadController @Inject() ( uploadDraftMetadata .recoverWith { case error => - IO(InternalServerError(s"Unable to upload draft metadata to : $uploadBucket/$uploadKey: Error:" + error.getMessage + " stack" + error.getStackTrace.mkString)) + val errorPage = for { + reference <- consignmentService.getConsignmentRef(consignmentId, token.bearerAccessToken) + } yield { + logger.error(error.getMessage, error) + Ok(views.html.draftmetadata.draftMetadataUploadError(consignmentId, reference, frontEndInfoConfiguration.frontEndInfo, token.bearerAccessToken.getValue)).uncache() + } + IO.fromFuture(IO(errorPage)) } .unsafeToFuture() } diff --git a/app/views/draftmetadata/draftMetadataUploadError.scala.html b/app/views/draftmetadata/draftMetadataUploadError.scala.html new file mode 100644 index 000000000..ab66f28f7 --- /dev/null +++ b/app/views/draftmetadata/draftMetadataUploadError.scala.html @@ -0,0 +1,62 @@ +@import helper._ +@import views.html.partials._ +@import viewsapi.FrontEndInfo + +@import java.util.UUID +@(consignmentId: UUID, consignmentRef: String, frontEndInfo: FrontEndInfo, name: String)(implicit request: RequestHeader, messages: Messages) + +@main("Upload a metadata CSV", name = name, backLink = Some(backLink(routes.AdditionalMetadataEntryMethodController.additionalMetadataEntryMethodPage(consignmentId).url, "How would you like to enter metadata?"))) { +
+ +
+
+
+

+ There is a problem +

+
+ +
+
+
+ +

Upload a metadata CSV

+
+

In your CSV, include a header row for the column titles and one row for every record that requires metadata.

+
+
+ @downloadMetadataLink(consignmentId, "Download metadata Excel template", routes.DownloadMetadataController.downloadMetadataFile(consignmentId).url) +
+ +
+ @CSRF.formField +
+ @loggedOutErrorMessage("file-upload") + @frontEndInputs(frontEndInfo) + + +

+ Error: Select a CSV file to upload +

+ +
+ +
+
+
+ +
+
+
+ @transferReference(consignmentRef, isJudgmentUser = false) +
+} diff --git a/test/controllers/DraftMetadataUploadControllerSpec.scala b/test/controllers/DraftMetadataUploadControllerSpec.scala index 41afc759b..2c81ef6b9 100644 --- a/test/controllers/DraftMetadataUploadControllerSpec.scala +++ b/test/controllers/DraftMetadataUploadControllerSpec.scala @@ -8,6 +8,7 @@ import org.pac4j.play.scala.SecurityComponents import org.scalatest.matchers.should.Matchers._ import play.api.Configuration import play.api.Play.materializer +import play.api.libs.Files import play.api.libs.Files.SingletonTemporaryFileCreator import play.api.mvc.MultipartFormData.FilePart import play.api.mvc.{MultipartFormData, Result} @@ -117,21 +118,25 @@ class DraftMetadataUploadControllerSpec extends FrontEndTestHelper { redirect.getOrElse(s"incorrect redirect $redirect") must include regex "/consignment/*.*/draft-metadata/checks" } - "render error page when upload unsuccessful" in { + "render error page when upload unsuccessful when no file uploaded" in { val uploadServiceMock = mock[UploadService] - when(uploadServiceMock.uploadDraftMetadata(anyString, anyString, anyString)) - .thenReturn(Future.failed(new RuntimeException("Upload failed"))) + setConsignmentReferenceResponse(wiremockServer) + when(configuration.get[String]("draftMetadata.fileName")).thenReturn("wrong name") + val putObjectResponse = PutObjectResponse.builder().eTag("testEtag").build() + when(uploadServiceMock.uploadDraftMetadata(anyString, anyString, anyString)).thenReturn(Future.successful(putObjectResponse)) + val draftMetadataServiceMock = mock[DraftMetadataService] when(draftMetadataServiceMock.triggerDraftMetadataValidator(any[UUID], anyString, anyString)).thenReturn(Future.successful(true)) - val response = requestFileUpload(uploadServiceMock, draftMetadataServiceMock) + val response = requestFileUploadWithoutFile(uploadServiceMock, draftMetadataServiceMock) - playStatus(response) mustBe 500 + playStatus(response) mustBe 200 - contentAsString(response) must include("Upload failed") + contentAsString(response) must include("There is a problem") } "render error page when upload success but trigger fails" in { val uploadServiceMock = mock[UploadService] + setConsignmentReferenceResponse(wiremockServer) val putObjectResponse = PutObjectResponse.builder().eTag("testEtag").build() when(configuration.get[String]("draftMetadata.fileName")).thenReturn(uploadFilename) when(uploadServiceMock.uploadDraftMetadata(anyString, anyString, anyString)).thenReturn(Future.successful(putObjectResponse)) @@ -140,9 +145,9 @@ class DraftMetadataUploadControllerSpec extends FrontEndTestHelper { when(draftMetadataServiceMock.triggerDraftMetadataValidator(any[UUID], anyString, anyString)).thenReturn(Future.failed(new RuntimeException("Trigger failed"))) val response = requestFileUpload(uploadServiceMock, draftMetadataServiceMock) - playStatus(response) mustBe 500 + playStatus(response) mustBe 200 - contentAsString(response) must include("Trigger failed") + contentAsString(response) must include("There is a problem") } } @@ -170,6 +175,15 @@ class DraftMetadataUploadControllerSpec extends FrontEndTestHelper { ) } + private def requestFileUploadWithoutFile(uploadServiceMock: UploadService, draftMetadataServiceMock: DraftMetadataService): Future[Result] = { + val controller = instantiateDraftMetadataUploadController(blockDraftMetadataUpload = false, uploadService = uploadServiceMock, draftMetadataService = draftMetadataServiceMock) + + val formData = MultipartFormData(dataParts = Map("" -> Seq("dummy data")), files = Seq.empty[FilePart[Files.TemporaryFile]], badParts = Seq()) + + val request = FakeRequest(POST, "/consignment/1234567/draft-metadata").withCSRFToken.withBody(formData).withHeaders(FakeHeaders()) + controller.saveDraftMetadata(UUID.randomUUID()).apply(request) + } + private def requestFileUpload(uploadServiceMock: UploadService, draftMetadataServiceMock: DraftMetadataService): Future[Result] = { val controller = instantiateDraftMetadataUploadController(blockDraftMetadataUpload = false, uploadService = uploadServiceMock, draftMetadataService = draftMetadataServiceMock) @@ -183,7 +197,8 @@ class DraftMetadataUploadControllerSpec extends FrontEndTestHelper { val file = FilePart("upload", "hello.txt", Option("text/plain"), tempFile) val formData = MultipartFormData(dataParts = Map("" -> Seq("dummy data")), files = Seq(file), badParts = Seq()) - val request = FakeRequest(POST, "/consignment/1234567/draft-metadata").withBody(formData).withHeaders(FakeHeaders()) + val request = FakeRequest(POST, "/consignment/1234567/draft-metadata").withCSRFToken.withBody(formData).withHeaders(FakeHeaders()) + controller.saveDraftMetadata(UUID.randomUUID()).apply(request) } }