Merge pull request #4047 from nationalarchives/Revert-of-TDRD-309

Revert of tdrd 309

app/configuration/ApplicationConfig.scala

@@ -32,6 +32,4 @@ class ApplicationConfig @Inject() (configuration: Configuration) {
   val draftMetadataFileName: String = configuration.get[String]("draftMetadata.fileName")
 
   val notificationSnsTopicArn: String = get("notificationSnsTopicArn")
-
-  val fileChecksTotalTimoutInSeconds: Int = configuration.get[Int]("fileChecksTotalTimoutInSeconds")
 }

app/controllers/FileChecksController.scala

@@ -7,14 +7,14 @@ import graphql.codegen.types.ConsignmentStatusInput
 import io.circe.syntax._
 import org.pac4j.play.scala.SecurityComponents
 import play.api.i18n.I18nSupport
-import play.api.mvc._
-import services.Statuses._
-import services._
+import play.api.mvc.{Action, AnyContent, Request, RequestHeader}
+import services.Statuses.{CompletedValue, CompletedWithIssuesValue, UploadType}
+import services.{BackendChecksService, ConsignmentService, ConsignmentStatusService}
 import viewsapi.Caching.preventCaching
 
 import java.util.UUID
 import javax.inject.{Inject, Singleton}
-import scala.concurrent.{ExecutionContext, Future, blocking}
+import scala.concurrent.{ExecutionContext, Future}
 
 @Singleton
 class FileChecksController @Inject() (
@@ -24,16 +24,14 @@ class FileChecksController @Inject() (
     val consignmentService: ConsignmentService,
     val applicationConfig: ApplicationConfig,
     val backendChecksService: BackendChecksService,
-    val consignmentStatusService: ConsignmentStatusService,
-    val confirmTransferService: ConfirmTransferService,
-    val consignmentExportService: ConsignmentExportService
+    val consignmentStatusService: ConsignmentStatusService
 )(implicit val ec: ExecutionContext)
     extends TokenSecurity
     with I18nSupport {
 
   private def getFileChecksProgress(request: Request[AnyContent], consignmentId: UUID)(implicit requestHeader: RequestHeader): Future[FileChecksProgress] = {
     consignmentService
-      .fileCheckProgress(consignmentId, request.token.bearerAccessToken)
+      .getConsignmentFileChecks(consignmentId, request.token.bearerAccessToken)
       .map { fileCheckProgress =>
         FileChecksProgress(
           fileCheckProgress.totalFiles,
@@ -95,24 +93,6 @@ class FileChecksController @Inject() (
     } yield result
   }
 
-  def judgmentCompleteTransfer(consignmentId: UUID): Action[AnyContent] = judgmentTypeAction(consignmentId) { implicit request: Request[AnyContent] =>
-    val token = request.token.bearerAccessToken
-    for {
-      reference <- consignmentService.getConsignmentRef(consignmentId, token)
-      result <-
-        (
-          for {
-            _ <- waitForFileChecksToBeCompleted(consignmentId)
-            result <- JudgmentCompleteTransfer(consignmentId)
-          } yield result
-        ).recover { case _: Exception =>
-          Ok(views.html.uploadInProgress(consignmentId, reference, "Uploading your records", request.token.name, isJudgmentUser = true)).uncache()
-        }
-    } yield {
-      result
-    }
-  }
-
   private def handleSuccessfulUpload(consignmentId: UUID, reference: String, isJudgmentUser: Boolean)(implicit request: Request[AnyContent]) = {
     val token = request.token.bearerAccessToken
     (for {
@@ -145,66 +125,10 @@ class FileChecksController @Inject() (
             .uncache()
         }
       }
-    }).recover { case _: Exception =>
+    }).recover { case exception: Exception =>
       Ok(views.html.uploadInProgress(consignmentId, reference, "Uploading your records", request.token.name, isJudgmentUser)).uncache()
     }
   }
-
-  private def JudgmentCompleteTransfer(consignmentId: UUID)(implicit request: Request[AnyContent]): Future[Result] = {
-    val pageTitle = "Results of checks"
-    for {
-      consignmentStatuses <- consignmentStatusService.getConsignmentStatuses(consignmentId, request.token.bearerAccessToken)
-      reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken)
-      exportStatus = consignmentStatusService.getStatusValues(consignmentStatuses, ExportType).values.headOption.flatten
-      result <- exportStatus match {
-        case Some(InProgressValue.value) | Some(CompletedValue.value) | Some(FailedValue.value) =>
-          Future(Ok(views.html.transferAlreadyCompleted(consignmentId, reference, request.token.name, isJudgmentUser = true)).uncache())
-        case None =>
-          for {
-            fileCheck <- consignmentService.fileCheckProgress(consignmentId, request.token.bearerAccessToken)
-            result <-
-              if (fileCheck.allChecksSucceeded) {
-                val token: BearerAccessToken = request.token.bearerAccessToken
-                val legalCustodyTransferConfirmation = FinalTransferConfirmationData(transferLegalCustody = true)
-                for {
-                  _ <- confirmTransferService.addFinalTransferConfirmation(consignmentId, token, legalCustodyTransferConfirmation)
-                  _ <- consignmentExportService.updateTransferInitiated(consignmentId, request.token.bearerAccessToken)
-                  _ <- consignmentExportService.triggerExport(consignmentId, request.token.bearerAccessToken.toString)
-                  res <- Future(Redirect(routes.TransferCompleteController.judgmentTransferComplete(consignmentId)).uncache())
-                } yield res
-              } else {
-                Future(Ok(views.html.fileChecksResultsFailed(request.token.name, pageTitle, reference, isJudgmentUser = true)).uncache())
-              }
-          } yield result
-        case _ =>
-          throw new IllegalStateException(s"Unexpected Export status: $exportStatus for consignment $consignmentId")
-      }
-    } yield result
-  }
-
-  private def waitForFileChecksToBeCompleted(consignmentId: UUID)(implicit request: Request[AnyContent]): Future[Unit] = {
-    val totalSleepTime = applicationConfig.fileChecksTotalTimoutInSeconds * 1000 // Total sleep time in milliseconds
-    val interval = 5 * 1000 // Interval time in milliseconds (5 seconds)
-    val intervals = totalSleepTime / interval
-
-    def checkStatus(remainingIntervals: Int): Future[Unit] = {
-      if (remainingIntervals <= 0) {
-        Future.unit
-      } else {
-        getFileChecksProgress(request, consignmentId).flatMap { progress =>
-          if (progress.isComplete) {
-            Future.unit
-          } else {
-            Future {
-              blocking(Thread.sleep(interval))
-            }.flatMap(_ => checkStatus(remainingIntervals - 1))
-          }
-        }
-      }
-    }
-
-    checkStatus(intervals)
-  }
 }
 
 case class FileChecksProgress(totalFiles: Int, avMetadataProgressPercentage: Int, checksumProgressPercentage: Int, ffidMetadataProgressPercentage: Int) {

app/controllers/FileChecksResultsController.scala

@@ -32,7 +32,7 @@ class FileChecksResultsController @Inject() (
     val pageTitle = "Results of your checks"
 
     for {
-      fileCheck <- consignmentService.fileCheckProgress(consignmentId, request.token.bearerAccessToken)
+      fileCheck <- consignmentService.getConsignmentFileChecks(consignmentId, request.token.bearerAccessToken)
       parentFolder = fileCheck.parentFolder.getOrElse(throw new IllegalStateException(s"No parent folder found for consignment: '$consignmentId'"))
       reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken)
     } yield {
@@ -57,5 +57,38 @@ class FileChecksResultsController @Inject() (
       }
     }
   }
+
+  def judgmentFileCheckResultsPage(consignmentId: UUID): Action[AnyContent] = judgmentTypeAction(consignmentId) { implicit request: Request[AnyContent] =>
+    val pageTitle = "Results of checks"
+    for {
+      consignmentStatuses <- consignmentStatusService.getConsignmentStatuses(consignmentId, request.token.bearerAccessToken)
+      reference <- consignmentService.getConsignmentRef(consignmentId, request.token.bearerAccessToken)
+      exportStatus = consignmentStatusService.getStatusValues(consignmentStatuses, ExportType).values.headOption.flatten
+      result <- exportStatus match {
+        case Some(InProgressValue.value) | Some(CompletedValue.value) | Some(FailedValue.value) =>
+          Future(Ok(views.html.transferAlreadyCompleted(consignmentId, reference, request.token.name, isJudgmentUser = true)).uncache())
+        case None =>
+          for {
+            fileCheck <- consignmentService.getConsignmentFileChecks(consignmentId, request.token.bearerAccessToken)
+            result <-
+              if (fileCheck.allChecksSucceeded) {
+                val token: BearerAccessToken = request.token.bearerAccessToken
+                val legalCustodyTransferConfirmation = FinalTransferConfirmationData(transferLegalCustody = true)
+                for {
+                  _ <- confirmTransferService.addFinalTransferConfirmation(consignmentId, token, legalCustodyTransferConfirmation)
+                  _ <- consignmentExportService.updateTransferInitiated(consignmentId, request.token.bearerAccessToken)
+                  _ <- consignmentExportService.triggerExport(consignmentId, request.token.bearerAccessToken.toString)
+                  res <- Future(Redirect(routes.TransferCompleteController.judgmentTransferComplete(consignmentId)).uncache())
+                } yield res
+              } else {
+                Future(Ok(views.html.fileChecksResultsFailed(request.token.name, pageTitle, reference, isJudgmentUser = true)).uncache())
+              }
+          } yield result
+        case _ =>
+          throw new IllegalStateException(s"Unexpected Export status: $exportStatus for consignment $consignmentId")
+      }
+    } yield result
+  }
 }
+
 case class ConsignmentFolderInfo(numberOfFiles: Int, parentFolder: String)

app/controllers/ViewTransfersController.scala

@@ -132,7 +132,7 @@ class ViewTransfersController @Inject() (
     }
 
     val resultsUrl = if (judgmentType) {
-      routes.FileChecksController.judgmentCompleteTransfer(consignmentId).url
+      routes.FileChecksResultsController.judgmentFileCheckResultsPage(consignmentId).url
     } else {
       routes.FileChecksResultsController.fileCheckResultsPage(consignmentId).url
     }

app/services/ConsignmentService.scala

@@ -22,7 +22,7 @@ import graphql.codegen.GetConsignmentsForMetadataReview.{getConsignmentsForMetad
 import graphql.codegen.GetFileCheckProgress.{getFileCheckProgress => gfcp}
 import graphql.codegen.UpdateConsignmentSeriesId.updateConsignmentSeriesId
 import graphql.codegen.types._
-import graphql.codegen.{AddConsignment, GetConsignmentFilesMetadata}
+import graphql.codegen.{AddConsignment, GetConsignmentFilesMetadata, GetFileCheckProgress}
 import services.ApiErrorHandling._
 import services.ConsignmentService.{File, StatusTag}
 import uk.gov.nationalarchives.tdr.keycloak.Token
@@ -38,6 +38,7 @@ class ConsignmentService @Inject() (val graphqlConfiguration: GraphQLConfigurati
   private val getConsignmentClient = graphqlConfiguration.getClient[getConsignment.Data, getConsignment.Variables]()
   private val getConsignmentFilesMetadataClient = graphqlConfiguration.getClient[gcfm.Data, gcfm.Variables]()
   private val addConsignmentClient = graphqlConfiguration.getClient[addConsignment.Data, addConsignment.Variables]()
+  private val getConsignmentFileCheckClient = graphqlConfiguration.getClient[gfcp.Data, gfcp.Variables]()
   private val getConsignmentFolderDetailsClient = graphqlConfiguration.getClient[getConsignmentFolderDetails.Data, getConsignmentFolderDetails.Variables]()
   private val getConsignmentSummaryClient = graphqlConfiguration.getClient[getConsignmentSummary.Data, getConsignmentSummary.Variables]()
   private val getConsignmentReferenceClient = graphqlConfiguration.getClient[getConsignmentReference.Data, getConsignmentReference.Variables]()
@@ -121,6 +122,13 @@ class ConsignmentService @Inject() (val graphqlConfiguration: GraphQLConfigurati
       .map(data => data.addConsignment)
   }
 
+  def getConsignmentFileChecks(consignmentId: UUID, token: BearerAccessToken): Future[gfcp.GetConsignment] = {
+    val variables: gfcp.Variables = new GetFileCheckProgress.getFileCheckProgress.Variables(consignmentId)
+
+    sendApiRequest(getConsignmentFileCheckClient, gfcp.document, token, variables)
+      .map(data => data.getConsignment.get)
+  }
+
   def getConsignmentFolderInfo(consignmentId: UUID, token: BearerAccessToken): Future[getConsignmentFolderDetails.GetConsignment] = {
     val variables: getConsignmentFolderDetails.Variables = new getConsignmentFolderDetails.Variables(consignmentId)
 

app/views/fileChecksProgressAlreadyConfirmed.scala.html

@@ -21,7 +21,7 @@ <h1 class="govuk-heading-l">@title</h1>
 
             <div class="govuk-button-group">
                 <a role="button" data-prevent-double-click="true" class="govuk-button" data-module="govuk-button"
-                    href="@{if(isJudgmentUser) {routes.FileChecksController.judgmentCompleteTransfer(consignmentId)}
+                    href="@{if(isJudgmentUser) {routes.FileChecksResultsController.judgmentFileCheckResultsPage(consignmentId)}
                             else {routes.FileChecksResultsController.fileCheckResultsPage(consignmentId)}}">
                     Continue
                 </a>

app/views/judgment/judgmentFileChecksProgress.scala.html

@@ -20,7 +20,7 @@ <h1 class="govuk-heading-l">Checking your upload</h1>
             </p>
 
             <!--        Form to redirect user once file checks have completed. It sends consignmentId to record results' placeholder page -->
-            @form(routes.FileChecksController.judgmentCompleteTransfer(consignmentId), Symbol("id") -> "continue-transfer") { }
+            @form(routes.FileChecksResultsController.judgmentFileCheckResultsPage(consignmentId), Symbol("id") -> "file-checks-form") { }
         </div>
         @transferReference(consignmentRef, isJudgmentUser = true)
     </div>

build.sbt

@@ -66,8 +66,3 @@ excludeDependencies ++= Seq(
 )
 
 pipelineStages := Seq(digest)
-
-excludeDependencies ++= Seq(
-  ExclusionRule(organization = "com.typesafe.akka"),
-  ExclusionRule(organization = "com.typesafe.play")
-)

conf/application.base.conf

@@ -66,5 +66,3 @@ draftMetadata {
 }
 
 notificationSnsTopicArn = ${NOTIFICATION_SNS_TOPIC_ARN}
-fileChecksTotalTimoutInSeconds = 480
-fileChecksTotalTimoutInSeconds = ${?FILE_CHECKS_TOTAL_TIMEOUT_IN_SECONDS}

conf/application.local-base.conf

@@ -23,5 +23,3 @@ featureAccessBlock {
   blockDraftMetadataUpload=false
   blockMetadataReview=false
 }
-
-notificationSnsTopicArn = "arn:test-arn"

conf/routes

@@ -75,7 +75,8 @@ GET         /judgment/help
 GET         /judgment/:consignmentId/before-uploading                                                    controllers.BeforeUploadingController.beforeUploading(consignmentId: java.util.UUID)
 GET         /judgment/:consignmentId/upload                                                              controllers.UploadController.judgmentUploadPage(consignmentId: java.util.UUID)
 GET         /judgment/:consignmentId/file-checks                                                         controllers.FileChecksController.judgmentFileChecksPage(consignmentId: java.util.UUID, uploadFailed: Option[String])
-GET         /judgment/:consignmentId/continue-transfer                                                   controllers.FileChecksController.judgmentCompleteTransfer(consignmentId: java.util.UUID)
+POST        /judgment/:consignmentId/file-check-progress                                                 controllers.FileChecksController.fileCheckProgress(consignmentId: java.util.UUID)
+GET         /judgment/:consignmentId/file-checks-results                                                 controllers.FileChecksResultsController.judgmentFileCheckResultsPage(consignmentId: java.util.UUID)
 GET         /judgment/:consignmentId/transfer-complete                                                   controllers.TransferCompleteController.judgmentTransferComplete(consignmentId: java.util.UUID)
 
 # Routes for TNA-User

npm/src/checks/index.ts

@@ -19,26 +19,21 @@ export class Checks {
     isJudgmentUser: boolean,
     goToNextPage: (formId: string) => void
   ) => {
-    if (isJudgmentUser) {
-      goToNextPage("#continue-transfer")
-    } else {
-      const intervalId: ReturnType<typeof setInterval> = setInterval(
-        async () => {
-          const fileChecksProgress: IFileCheckProgress | Error =
-            await getFileChecksProgress()
-          if (!isError(fileChecksProgress)) {
-            const checksCompleted = haveFileChecksCompleted(fileChecksProgress)
-            if (checksCompleted) {
-              clearInterval(intervalId)
-              displayChecksCompletedBanner("file-checks")
-            }
-          } else {
-            return fileChecksProgress
-          }
-        },
-        5000
-      )
-    }
+    const intervalId: ReturnType<typeof setInterval> = setInterval(async () => {
+      const fileChecksProgress: IFileCheckProgress | Error =
+        await getFileChecksProgress()
+      if (!isError(fileChecksProgress)) {
+        const checksCompleted = haveFileChecksCompleted(fileChecksProgress)
+        if (checksCompleted) {
+          clearInterval(intervalId)
+          isJudgmentUser
+            ? goToNextPage("#file-checks-form")
+            : displayChecksCompletedBanner("file-checks")
+        }
+      } else {
+        return fileChecksProgress
+      }
+    }, 5000)
   }
 
   updateDraftMetadataValidationProgress: () => void | Error = () => {

npm/test/update-check-progress.test.ts

@@ -99,20 +99,6 @@ const mockDisplayChecksHaveCompletedBanner: () => void = () =>
         () => {}
     )
 
-test("'updateFileCheckProgress' calls goToNextPage for a judgment user without checks", async () => {
-  const consignmentId = "e25438db-4bfb-41c9-8fff-6f2e4cca6421"
-  mockGetCheckProgress.getConsignmentId.mockImplementation(
-      () => consignmentId
-  )
-
-  checks.updateFileCheckProgress(true, mockGoToNextPage)
-  await jest.runOnlyPendingTimers()
-
-  expect(mockGetCheckProgress.getFileChecksProgress).not.toHaveBeenCalled()
-  expect(haveFileChecksCompleted).not.toHaveBeenCalled()
-  expect(mockGoToNextPage).toHaveBeenCalled()
-})
-
 test("'updateFileCheckProgress' calls setInterval correctly", async () => {
   jest.spyOn(global, "setInterval")
   await checks.updateFileCheckProgress(false, mockGoToNextPage)
@@ -279,3 +265,58 @@ test("'updateDraftMetadataValidationProgress' shows a standard user, no banner a
   expect(hasDraftMetadataValidationCompleted).toBeCalled()
   expect(displayChecksCompletedBanner).not.toBeCalled()
 })
+
+test("'updateFileCheckProgress' calls goToNextPage for a judgment user, if all checks are complete", async () => {
+  const consignmentId = "e25438db-4bfb-41c9-8fff-6f2e4cca6421"
+  mockGetCheckProgress.getConsignmentId.mockImplementation(
+      () => consignmentId
+  )
+
+  mockGetFileChecksProgress("complete")
+
+  mockVerifyChecksHaveCompleted.haveFileChecksCompleted.mockImplementation(
+      () => true
+  )
+
+  checks.updateFileCheckProgress(true, mockGoToNextPage)
+  await jest.runOnlyPendingTimers()
+
+  expect(haveFileChecksCompleted).toBeCalled()
+  expect(mockGoToNextPage).toHaveBeenCalled()
+})
+
+test("'updateFileCheckProgress' does not call goToNextPage for a judgment user if the checks are in progress", async () => {
+  const consignmentId = "e25438db-4bfb-41c9-8fff-6f2e4cca6421"
+  mockGetCheckProgress.getConsignmentId.mockImplementation(
+      () => consignmentId
+  )
+  mockGetFileChecksProgress("inProgress")
+
+  mockVerifyChecksHaveCompleted.haveFileChecksCompleted.mockImplementation(
+      () => false
+  )
+
+  checks.updateFileCheckProgress(true, mockGoToNextPage)
+  await jest.runOnlyPendingTimers()
+
+  expect(haveFileChecksCompleted).toBeCalled()
+  expect(mockGoToNextPage).not.toHaveBeenCalled()
+})
+
+test("'updateFileCheckProgress' does not call goToNextPage for a judgment user if no file checks information is returned", async () => {
+  const consignmentId = "e25438db-4bfb-41c9-8fff-6f2e4cca6421"
+  mockGetCheckProgress.getConsignmentId.mockImplementation(
+      () => consignmentId
+  )
+  mockGetFileChecksProgress("noData")
+
+  mockVerifyChecksHaveCompleted.haveFileChecksCompleted.mockImplementation(
+      () => false
+  )
+
+  checks.updateFileCheckProgress(true, mockGoToNextPage)
+  await jest.runOnlyPendingTimers()
+
+  expect(haveFileChecksCompleted).toBeCalled()
+  expect(mockGoToNextPage).not.toHaveBeenCalled()
+})