Merge pull request #1281 from hmrc/fset-3352

FSET-3352 pass mark report fix

app/controllers/ReportingController.scala

@@ -1206,15 +1206,19 @@ class ReportingController @Inject() (cc: ControllerComponents,
       for {
         application <- applications
         appId = UniqueIdentifier(application.applicationId)
-        fsac = fsacResults.find(_.applicationId == appId)
-        overallFsacScoreOpt = fsac.map(res => AssessmentScoreCalculator.fetchExerciseAverages(res, appId.toString).overallScore)
-        sift = siftResults.find(_.applicationId == application.applicationId)
-        q <- questionnaires.get(application.applicationId)
-        fsb <- fsbScoresAndFeedback.get(application.applicationId)
+        fsacOpt = fsacResults.find(_.applicationId == appId)
+        overallFsacScoreOpt = fsacOpt.flatMap(assessmentScores =>
+          // It is possible that only some of the fsac scores have been reviewed at this stage so we need to handle optional data
+          AssessmentScoreCalculator.fetchExerciseAveragesOpt(assessmentScores).map(_.overallScore))
+        siftOpt = siftResults.find(_.applicationId == application.applicationId)
+        questionnaire <- questionnaires.get(application.applicationId)
+        fsbOpt <- fsbScoresAndFeedback.get(application.applicationId)
         emailOpt = emails.find(_.userId == application.userId)
       } yield {
         OnlineTestPassMarkReportItem(
-          ApplicationForOnlineTestPassMarkReportItem(application, fsac, overallFsacScoreOpt, sift, fsb), q, emailOpt.map(_.email).getOrElse("")
+          ApplicationForOnlineTestPassMarkReportItem(application, fsacOpt, overallFsacScoreOpt, siftOpt, fsbOpt),
+          questionnaire,
+          emailOpt.map(_.email).getOrElse("")
         )
       }
     }

app/model/assessmentscores/AssessmentScoresAllExercises.scala

@@ -31,27 +31,39 @@ case class AssessmentScoresAllExercises(
                                        ) {
 
   def exercise1OverallAvg(appId: String): Double = {
+    exercise1OverallAvgOpt.getOrElse(throw new Exception(s"Error fetching fsac exercise1 overall average for appId: $appId"))
+  }
+
+  def exercise1OverallAvgOpt: Option[Double] = {
     exercise1.flatMap(ex =>
       for {
         average <- ex.overallAverage
       } yield average
-    ).getOrElse(throw new Exception(s"Error fetching fsac exercise1 overall average for appId: $appId"))
+    )
   }
 
   def exercise2OverallAvg(appId: String): Double = {
+    exercise2OverallAvgOpt.getOrElse(throw new Exception(s"Error fetching fsac exercise2 overall average for appId: $appId"))
+  }
+
+  def exercise2OverallAvgOpt: Option[Double] = {
     exercise2.flatMap(ex =>
       for {
         average <- ex.overallAverage
       } yield average
-    ).getOrElse(throw new Exception(s"Error fetching fsac exercise2 overall average for appId: $appId"))
+    )
   }
 
   def exercise3OverallAvg(appId: String): Double = {
+    exercise3OverallAvgOpt.getOrElse(throw new Exception(s"Error fetching fsac exercise3 overall average for appId: $appId"))
+  }
+
+  def exercise3OverallAvgOpt: Option[Double] = {
     exercise3.flatMap(ex =>
       for {
         average <- ex.overallAverage
       } yield average
-    ).getOrElse(throw new Exception(s"Error fetching fsac exercise3 overall average for appId: $appId"))
+    )
   }
 
   def toExchange: AssessmentScoresAllExercisesExchange = AssessmentScoresAllExercisesExchange(

app/services/evaluation/AssessmentScoreCalculator.scala

@@ -41,4 +41,39 @@ trait AssessmentScoreCalculator {
       scores.exercise3OverallAvg(appId),
       overallScore.toDouble)
   }
+
+  def fetchExerciseAveragesOpt(scores: AssessmentScoresAllExercises): Option[ExerciseAverageResult] = {
+    // The overall score is the individual exercise averages summed
+    val exerciseScores = List(
+      scores.exercise1OverallAvgOpt,
+      scores.exercise2OverallAvgOpt,
+      scores.exercise3OverallAvgOpt
+    )
+
+    val decimalPlaces = 4
+    val overallScoreOpt = exerciseScores.foldLeft(
+      Option(BigDecimal.apply(0d).setScale(decimalPlaces, BigDecimal.RoundingMode.HALF_UP)))((acc, cur) =>
+        for ( a <- acc; c <- cur) yield {
+          val bd = BigDecimal.apply(c)
+          a + bd
+        } )
+
+    overallScoreOpt match {
+      case Some(overallScore) =>
+        // If the overallScore is populated then all the data is populated
+        for {
+          ex1Avg <- scores.exercise1OverallAvgOpt
+          ex2Avg <- scores.exercise2OverallAvgOpt
+          ex3Avg <- scores.exercise3OverallAvgOpt
+        } yield {
+          ExerciseAverageResult(
+            ex1Avg,
+            ex2Avg,
+            ex3Avg,
+            overallScore.toDouble
+          )
+        }
+      case None => None
+    }
+  }
 }

test/services/evaluation/AssessmentScoreCalculatorSpec.scala

@@ -49,7 +49,8 @@ class AssessmentScoreCalculatorSpec extends UnitSpec {
         ))
       )
 
-      val result = AssessmentScoreCalculatorUnderTest.fetchExerciseAverages(assessmentScores, applicationId.toString)
+      val result1 = AssessmentScoreCalculatorUnderTest.fetchExerciseAverages(assessmentScores, applicationId.toString)
+      val result2 = AssessmentScoreCalculatorUnderTest.fetchExerciseAveragesOpt(assessmentScores)
 
       val expected = ExerciseAverageResult(
         exercise1Average = 3.125,
@@ -58,7 +59,51 @@ class AssessmentScoreCalculatorSpec extends UnitSpec {
         overallScore = 7.625
       )
 
-      result mustBe expected
+      result1 mustBe expected
+      result2 mustBe Some(expected)
+    }
+
+    "Throw an exception when attempting to calculate the overall score and not expecting the data to be missing" in {
+      val applicationId = UniqueIdentifier.randomUniqueIdentifier
+      val assessmentScores = AssessmentScoresAllExercises(
+        applicationId,
+        exercise1 = Some(AssessmentScoresExercise(
+          attended = true,
+          updatedBy = updatedBy,
+          overallAverage = Some(3.125)
+        )),
+        exercise2 = Some(AssessmentScoresExercise(
+          attended = true,
+          updatedBy = updatedBy,
+          overallAverage = Some(2.0)
+        )),
+        exercise3 = None
+      )
+
+      intercept[Exception] {
+        AssessmentScoreCalculatorUnderTest.fetchExerciseAverages(assessmentScores, applicationId.toString)
+      }
+    }
+
+    "Handle a missing exercise correctly when expecting optional data" in {
+      val applicationId = UniqueIdentifier.randomUniqueIdentifier
+      val assessmentScores = AssessmentScoresAllExercises(
+        applicationId,
+        exercise1 = Some(AssessmentScoresExercise(
+          attended = true,
+          updatedBy = updatedBy,
+          overallAverage = Some(3.125)
+        )),
+        exercise2 = Some(AssessmentScoresExercise(
+          attended = true,
+          updatedBy = updatedBy,
+          overallAverage = Some(2.0)
+        )),
+        exercise3 = None
+      )
+
+      val result = AssessmentScoreCalculatorUnderTest.fetchExerciseAveragesOpt(assessmentScores)
+      result mustBe None
     }
   }
 }