From 4c4ae8e13a82009e8fd1420d45580a4daf62095b Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Mon, 21 Oct 2024 14:24:52 +0100
Subject: [PATCH 1/8] [DLS-10533] Migrating from HttpClient to HttpClientV2

---
 .../UCThresholdConnectorProxyActor.scala      |  61 +++++-----
 .../hmrc/helptosave/config/AppConfig.scala    |   4 +-
 .../helptosave/connectors/BarsConnector.scala |  16 +--
 .../helptosave/connectors/DESConnector.scala  |  57 +++++----
 .../connectors/HelpToSaveProxyConnector.scala | 111 ++++++++++--------
 .../helptosave/connectors/IFConnector.scala   |  17 +--
 conf/application.conf                         |   1 -
 .../UCThresholdConnectorProxyActorSpec.scala  |  19 +--
 .../connectors/DESConnectorSpec.scala         |  28 +++--
 .../HelpToSaveProxyConnectorSpec.scala        |  22 ++--
 .../modules/UCThresholdOrchestratorSpec.scala |   5 +-
 11 files changed, 178 insertions(+), 163 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
index 7b7cbc1c..85216f7e 100644
--- a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
+++ b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
@@ -34,40 +34,33 @@ class UCThresholdConnectorProxyActor(dESConnector: DESConnector, pagerDutyAlerti
 
   implicit val hc: HeaderCarrier = HeaderCarrier()
 
-  def getThreshold()(implicit hc: HeaderCarrier): Future[Either[String, Double]] =
-    dESConnector
-      .getThreshold()
-      .map[Either[String, Double]] { response =>
-        val additionalParams = "DesCorrelationId" -> response.correlationId
-
-        response.status match {
-          case Status.OK =>
-            val result = response.parseJson[UCThreshold]
-            result.fold(
-              { e =>
-                logger.warn(
-                  s"Could not parse JSON response from threshold, received 200 (OK): $e, with additionalParams: $additionalParams")
-                pagerDutyAlerting.alert("Could not parse JSON in UC threshold response")
-              },
-              _ =>
-                logger.debug(
-                  s"Call to threshold successful, received 200 (OK), with additionalParams: $additionalParams")
-            )
-            result.map(_.thresholdAmount)
-
-          case other =>
-            logger.warn(
-              s"Call to get threshold unsuccessful. Received unexpected status $other. " +
-                s"AdditionalParams: $additionalParams")
-            pagerDutyAlerting.alert("Received unexpected http status in response to get UC threshold from DES")
-            Left(s"Received unexpected status $other")
-        }
-      }
-      .recover {
-        case e =>
-          pagerDutyAlerting.alert("Failed to make call to get UC threshold from DES")
-          Left(s"Call to get threshold unsuccessful: ${e.getMessage}")
-      }
+  def getThreshold()(implicit hc: HeaderCarrier): Future[Either[String, Double]] = {
+         dESConnector.getThreshold()
+           .map(maybeThreshold => {
+               maybeThreshold.map(response => {
+                   val additionalParams = "DesCorrelationId" -> response.correlationId
+                   val result = response.parseJson[UCThreshold]
+                   result.fold(
+                       { e =>
+                           logger.warn(
+                               s"Could not parse JSON response from threshold, received 200 (OK): $e, with additionalParams: $additionalParams")
+                           pagerDutyAlerting.alert("Could not parse JSON in UC threshold response")
+                         },
+                       _ =>
+                       logger.debug(
+                           s"Call to threshold successful, received 200 (OK), with additionalParams: $additionalParams")
+                   )
+                   result.map(_.thresholdAmount)
+                 }).left.map(error => {
+                   val additionalParams = "DesCorrelationId" -> error.headers.getOrElse("CorrelationId", "-")
+                   logger.warn(
+                       s"Call to get threshold unsuccessful. Received unexpected status ${error.statusCode}. " +
+                           s"AdditionalParams: $additionalParams")
+                   pagerDutyAlerting.alert("Received unexpected http status in response to get UC threshold from DES")
+                   s"Received unexpected status ${error.statusCode}"
+                 })
+             }.flatten)
+       }
 
   override def receive: Receive = {
     case GetThresholdValue => getThreshold().map(GetThresholdValueResponse).pipeTo(sender())
diff --git a/app/uk/gov/hmrc/helptosave/config/AppConfig.scala b/app/uk/gov/hmrc/helptosave/config/AppConfig.scala
index 0352f5bc..446d9839 100644
--- a/app/uk/gov/hmrc/helptosave/config/AppConfig.scala
+++ b/app/uk/gov/hmrc/helptosave/config/AppConfig.scala
@@ -36,12 +36,12 @@ class AppConfig @Inject()(
 
   val appName: String = servicesConfig.getString("appName")
   val ifEnabled: Boolean = servicesConfig.getBoolean("feature.if.enabled")
-  val desHeaders: Map[String, String] = Map(
+  val desHeaders: Seq[(String, String)] = Seq(
     "Environment"   -> servicesConfig.getString("microservice.services.des.environment"),
     "Authorization" -> s"Bearer ${servicesConfig.getString("microservice.services.des.token")}"
   )
 
-  val ifHeaders: Map[String, String] = Map(
+  val ifHeaders: Seq[(String, String)] = Seq(
     "Environment"   -> servicesConfig.getString("microservice.services.if.environment"),
     "Authorization" -> s"Bearer ${servicesConfig.getString("microservice.services.if.token")}"
   )
diff --git a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
index d3ab64f8..bdc7d86a 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
@@ -19,13 +19,14 @@ package uk.gov.hmrc.helptosave.connectors
 import com.google.inject.{ImplementedBy, Inject, Singleton}
 import play.api.libs.json.{Format, Json}
 import uk.gov.hmrc.helptosave.config.AppConfig
-import uk.gov.hmrc.helptosave.http.HttpClient._
 import uk.gov.hmrc.helptosave.models.BankDetailsValidationRequest
 import uk.gov.hmrc.helptosave.util.Logging
-import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps}
+import uk.gov.hmrc.http.client.HttpClientV2
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
+import java.net.URL
 
 @ImplementedBy(classOf[BarsConnectorImpl])
 trait BarsConnector {
@@ -36,18 +37,19 @@ trait BarsConnector {
 }
 
 @Singleton
-class BarsConnectorImpl @Inject()(http: HttpClient)(implicit appConfig: AppConfig) extends BarsConnector with Logging {
+class BarsConnectorImpl @Inject()(http: HttpClientV2)(implicit appConfig: AppConfig) extends BarsConnector with Logging {
 
   import uk.gov.hmrc.helptosave.connectors.BarsConnectorImpl._
 
-  private val barsEndpoint: String = s"${appConfig.barsUrl}/validate/bank-details"
+  private val barsEndpoint: URL = url"${appConfig.barsUrl}/validate/bank-details"
 
-  private val headers = Map("Content-Type" -> "application/json")
+  private val headers:(String, String) = "Content-Type" -> "application/json"
 
   override def validate(request: BankDetailsValidationRequest, trackingId: UUID)(
     implicit hc: HeaderCarrier,
-    ec: ExecutionContext): Future[HttpResponse] =
-    http.post(barsEndpoint, bodyJson(request), headers.+("X-Tracking-Id" -> trackingId.toString))
+    ec: ExecutionContext): Future[HttpResponse] = {
+    http.post(barsEndpoint).withBody(bodyJson(request)).transform(_.addHttpHeaders(headers, "X-Tracking-Id" -> trackingId.toString)).execute[HttpResponse]
+  }
 
   private def bodyJson(request: BankDetailsValidationRequest) =
     Json.toJson(BarsRequest(Account(request.sortCode, request.accountNumber)))
diff --git a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
index 7faafbf4..5a37b8ae 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
@@ -19,14 +19,16 @@ package uk.gov.hmrc.helptosave.connectors
 import cats.Show
 import cats.syntax.show._
 import com.google.inject.{ImplementedBy, Inject, Singleton}
-import play.api.libs.json.{JsNull, JsValue, Writes}
+import play.api.libs.json.{JsNull, JsValue}
 import uk.gov.hmrc.helptosave.config.AppConfig
-import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps
 import uk.gov.hmrc.helptosave.models.UCResponse
 import uk.gov.hmrc.helptosave.util.{Logging, NINO}
-import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
+import uk.gov.hmrc.http.HttpReads.Implicits._
+import uk.gov.hmrc.http.client.HttpClientV2
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse}
 import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
 
+import java.net.URL
 import scala.concurrent.{ExecutionContext, Future}
 
 @ImplementedBy(classOf[DESConnectorImpl])
@@ -39,17 +41,17 @@ trait DESConnector {
 
   def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
 
-  def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
+  def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 }
 
 @Singleton
-class DESConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig)
+class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig)
     extends DESConnector with Logging {
 
   val itmpECBaseURL: String = servicesConfig.baseUrl("itmp-eligibility-check")
   val itmpEnrolmentURL: String = servicesConfig.baseUrl("itmp-enrolment")
   val payeURL: String = servicesConfig.baseUrl("paye-personal-details")
-  val itmpThresholdURL: String = s"${servicesConfig.baseUrl("itmp-threshold")}/universal-credits/threshold-amount"
+  val itmpThresholdURL: URL = url"${servicesConfig.baseUrl("itmp-threshold")}/universal-credits/threshold-amount"
 
   implicit val booleanShow: Show[Boolean] = Show.show(if (_) "Y" else "N")
 
@@ -58,18 +60,18 @@ class DESConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfi
   val originatorIdHeader: (String, String) = "Originator-Id" -> servicesConfig.getString(
     "microservice.services.paye-personal-details.originatorId")
 
-  def eligibilityCheckUrl(nino: String): String = s"$itmpECBaseURL/help-to-save/eligibility-check/$nino"
+  def eligibilityCheckUrl(nino: String): URL = url"$itmpECBaseURL/help-to-save/eligibility-check/$nino"
 
-  def eligibilityCheckQueryParameters(ucResponse: Option[UCResponse]): Map[String, String] =
+  def eligibilityCheckQueryParameters(ucResponse: Option[UCResponse]) =
     ucResponse match {
-      case Some(UCResponse(a, Some(b))) => Map("universalCreditClaimant" -> a.show, "withinThreshold" -> b.show)
-      case Some(UCResponse(a, None))    => Map("universalCreditClaimant" -> a.show)
-      case _                            => Map()
+      case Some(UCResponse(a, Some(b))) => Seq("universalCreditClaimant" -> a.show, "withinThreshold" -> b.show)
+      case Some(UCResponse(a, None))    => Seq("universalCreditClaimant" -> a.show)
+      case _                            => Seq()
     }
 
-  def setFlagUrl(nino: NINO): String = s"$itmpEnrolmentURL/help-to-save/accounts/$nino"
+  def setFlagUrl(nino: NINO): URL = url"$itmpEnrolmentURL/help-to-save/accounts/$nino"
 
-  def payePersonalDetailsUrl(nino: String): String = s"$payeURL/pay-as-you-earn/02.00.00/individuals/$nino"
+  def payePersonalDetailsUrl(nino: String): URL = url"$payeURL/pay-as-you-earn/02.00.00/individuals/$nino"
 
   override def isEligible(nino: String, ucResponse: Option[UCResponse] = None)(
     implicit hc: HeaderCarrier,
@@ -77,32 +79,39 @@ class DESConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfi
     logger.info(s"[DESConnector][isEligible] GET request: " +
       s" header - ${appConfig.desHeaders}" +
       s" eligibilityCheckUrl - ${eligibilityCheckUrl(nino)}")
-    http.get(eligibilityCheckUrl(nino), eligibilityCheckQueryParameters(ucResponse), appConfig.desHeaders)(
-      hc.copy(authorization = None),
-      ec)
+
+    http.get(eligibilityCheckUrl(nino))(hc.copy(authorization = None)).transform(_
+      .withQueryStringParameters(eligibilityCheckQueryParameters(ucResponse):_*)
+      .addHttpHeaders(appConfig.desHeaders:_*))
+      .execute[HttpResponse]
   }
 
   override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
     logger.info(s"[DESConnector][setFlag] PUT request: " +
       s" header - ${appConfig.desHeaders} " +
       s" setFlagUrl - ${setFlagUrl(nino)}")
-    http.put(setFlagUrl(nino), body, appConfig.desHeaders)(Writes.jsValueWrites, hc.copy(authorization = None), ec)
+    http.put(setFlagUrl(nino))(hc.copy(authorization = None)).transform(_
+      .addHttpHeaders(appConfig.desHeaders: _*))
+      .withBody(body)
+      .execute[HttpResponse]
   }
 
   override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
     logger.info(s"[DESConnector][getPersonalDetails] GET request: " +
-      s" header - ${appConfig.desHeaders+ originatorIdHeader}" +
+      s" header - ${appConfig.desHeaders:+ originatorIdHeader}" +
       s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}")
-    http.get(payePersonalDetailsUrl(nino), headers = appConfig.desHeaders + originatorIdHeader)(
-      hc.copy(authorization = None),
-      ec)
+    http.get(payePersonalDetailsUrl(nino))(hc.copy(authorization = None)).transform(_
+      .addHttpHeaders(appConfig.desHeaders:+ originatorIdHeader:_*))
+      .execute[HttpResponse]
   }
 
-  override def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
+  override def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
     logger.info(s"[DESConnector][getThreshold] GET request: " +
       s"itmpThresholdURL - $itmpThresholdURL" +
-      s" header - ${appConfig.desHeaders + originatorIdHeader}")
-    http.get(itmpThresholdURL, headers = appConfig.desHeaders)(hc.copy(authorization = None), ec)
+      s" header - ${appConfig.desHeaders:+ originatorIdHeader}")
+    http.get(itmpThresholdURL)(hc.copy(authorization = None)).transform(_
+      .addHttpHeaders(appConfig.desHeaders:_*))
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
 }
diff --git a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
index d184ed69..24acb42b 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
@@ -23,11 +23,11 @@ import cats.syntax.either._
 import cats.syntax.eq._
 import com.google.inject.{ImplementedBy, Inject, Singleton}
 import play.api.http.Status
+import play.api.http.Status.{BAD_REQUEST, CONFLICT}
 import play.api.libs.json.{Format, Json}
 import play.mvc.Http.Status.INTERNAL_SERVER_ERROR
 import uk.gov.hmrc.helptosave.audit.HTSAuditor
 import uk.gov.hmrc.helptosave.config.AppConfig
-import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps
 import uk.gov.hmrc.helptosave.metrics.Metrics
 import uk.gov.hmrc.helptosave.models._
 import uk.gov.hmrc.helptosave.models.account.{Account, NsiAccount, NsiTransactions, Transactions}
@@ -35,8 +35,10 @@ import uk.gov.hmrc.helptosave.util.HeaderCarrierOps._
 import uk.gov.hmrc.helptosave.util.HttpResponseOps._
 import uk.gov.hmrc.helptosave.util.Logging._
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting, Result}
-import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
+import uk.gov.hmrc.http.client.HttpClientV2
+import uk.gov.hmrc.http.{BadRequestException, HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse}
 import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
+import uk.gov.hmrc.http.HttpReads.Implicits._
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -67,11 +69,11 @@ trait HelpToSaveProxyConnector {
 
 @Singleton
 class HelpToSaveProxyConnectorImpl @Inject()(
-  http: HttpClient,
-  metrics: Metrics,
-  pagerDutyAlerting: PagerDutyAlerting,
-  auditor: HTSAuditor,
-  servicesConfig: ServicesConfig)(implicit transformer: LogMessageTransformer, appConfig: AppConfig)
+                                              http: HttpClientV2,
+                                              metrics: Metrics,
+                                              pagerDutyAlerting: PagerDutyAlerting,
+                                              auditor: HTSAuditor,
+                                              servicesConfig: ServicesConfig)(implicit transformer: LogMessageTransformer, appConfig: AppConfig)
     extends HelpToSaveProxyConnector with Logging {
 
   import HelpToSaveProxyConnectorImpl._
@@ -88,23 +90,37 @@ class HelpToSaveProxyConnectorImpl @Inject()(
   override def createAccount(
     userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] =
     http
-      .post(s"$proxyURL/help-to-save-proxy/create-account", userInfo)
+      .post(url"$proxyURL/help-to-save-proxy/create-account")
+      .withBody(Json.toJson(userInfo))
+      .execute[HttpResponse]
       .recover {
+        case UpstreamErrorResponse(_, 409, _, _) =>
+          logger.warn("CONFLICT from proxy during /create-account")
+          HttpResponse(CONFLICT)
+        case e:BadRequestException =>
+          logger.warn(
+            s"unexpected error from proxy during /create-account, message=${e.getMessage}",
+            userInfo.nino,
+            "apiCorrelationId" -> getApiCorrelationId())
+          val errorJson =
+            ErrorResponse("unexpected error from proxy during /create-account", s"${e.getMessage}").toJson()
+          HttpResponse(BAD_REQUEST, errorJson.toString)
         case e =>
           logger.warn(
-            s"unexpected error from proxy during /create-de-account, message=${e.getMessage}",
+            s"unexpected error from proxy during /create-account, message=${e.getMessage}",
             userInfo.nino,
             "apiCorrelationId" -> getApiCorrelationId())
 
           val errorJson =
-            ErrorResponse("unexpected error from proxy during /create-de-account", s"${e.getMessage}").toJson()
+            ErrorResponse("unexpected error from proxy during /create-account", s"${e.getMessage}").toJson()
           HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString)
       }
 
   override def updateEmail(
     userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] =
     http
-      .put(s"$proxyURL/help-to-save-proxy/update-email", userInfo)
+      .put(url"$proxyURL/help-to-save-proxy/update-email").withBody(Json.toJson(userInfo))
+      .execute[HttpResponse]
       .recover {
         case e =>
           logger.warn(
@@ -119,42 +135,37 @@ class HelpToSaveProxyConnectorImpl @Inject()(
   override def ucClaimantCheck(nino: String, txnId: UUID, threshold: Double)(
     implicit hc: HeaderCarrier,
     ec: ExecutionContext): Result[UCResponse] = {
-    val url = s"$proxyURL/help-to-save-proxy/uc-claimant-check"
+    val url = url"$proxyURL/help-to-save-proxy/uc-claimant-check"
 
     EitherT[Future, String, UCResponse](
       http
-        .get(
-          url,
-          queryParams = Map("nino" -> nino, "transactionId" -> txnId.toString, "threshold" -> threshold.toString))
-        .map { response =>
+        .get(url).transform(_
+      .withQueryStringParameters("nino" -> nino , "transactionId" -> txnId.toString , "threshold" -> threshold.toString))
+        .execute[Either[UpstreamErrorResponse, HttpResponse]]
+        .map { maybeResponse =>
           val correlationId = "apiCorrelationId" -> getApiCorrelationId()
 
-          response.status match {
-            case Status.OK =>
-              val result = response.parseJson[UCResponse]
-              result.fold(
-                e =>
-                  logger.warn(
-                    s"Could not parse UniversalCredit response, received 200 (OK), error=$e",
-                    nino,
-                    correlationId),
-                _ =>
-                  logger
-                    .info(s"Call to check UniversalCredit check is successful, received 200 (OK)", nino, correlationId)
-              )
-              result
-
-            case other =>
-              logger.warn(
-                s"Call to check UniversalCredit check unsuccessful. Received unexpected status $other",
-                nino,
-                correlationId)
-              Left(s"Received unexpected status($other) from UniversalCredit check")
-          }
-        }
-        .recover {
-          case e =>
-            Left(s"Call to UniversalCredit check unsuccessful: ${e.getMessage}")
+          maybeResponse.map(response => {
+            val result = response.parseJson[UCResponse]
+            result.fold(
+              e =>
+            logger.warn(
+              s"Could not parse UniversalCredit response, received 200 (OK), error=$e",
+              nino,
+              correlationId),
+            _ =>
+            logger
+              .info(s"Call to check UniversalCredit check is successful, received 200 (OK)", nino, correlationId)
+            )
+            result
+
+          }).left.flatMap(error => {
+            logger.warn(
+              s"Call to check UniversalCredit check unsuccessful. Received unexpected status ${error.statusCode}",
+              nino,
+              correlationId)
+            Left(s"Received unexpected status(${error.statusCode}) from UniversalCredit check")
+          }).flatten
         }
     )
   }
@@ -163,13 +174,13 @@ class HelpToSaveProxyConnectorImpl @Inject()(
     implicit hc: HeaderCarrier,
     ec: ExecutionContext): Result[Option[Account]] = {
 
-    val url = s"$proxyURL/help-to-save-proxy/nsi-services/account"
+    val url = url"$proxyURL/help-to-save-proxy/nsi-services/account"
     val timerContext = metrics.getAccountTimer.time()
     EitherT[Future, String, Option[Account]](
       http
-        .get(
-          url,
-          Map("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId))
+        .get(url).transform(_
+        .withQueryStringParameters("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId))
+        .execute[HttpResponse]
         .map[Either[String, Option[Account]]] { response =>
           val _ = timerContext.stop()
           response.status match {
@@ -225,13 +236,13 @@ class HelpToSaveProxyConnectorImpl @Inject()(
     hc: HeaderCarrier,
     ec: ExecutionContext): Result[Option[Transactions]] = {
 
-    val url = s"$proxyURL/help-to-save-proxy/nsi-services/transactions"
+    val url = url"$proxyURL/help-to-save-proxy/nsi-services/transactions"
     val timerContext = metrics.getTransactionsTimer.time()
     EitherT[Future, String, Option[Transactions]](
       http
-        .get(
-          url,
-          Map("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId))
+        .get(url).transform(_
+        .withQueryStringParameters("nino" -> nino, "correlationId" -> correlationId, "version" -> getAccountVersion, "systemId" -> systemId))
+        .execute[HttpResponse]
         .map[Either[String, Option[Transactions]]] { response =>
           val _ = timerContext.stop()
           response.status match {
diff --git a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
index c46a3a02..5ea8a633 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
@@ -18,11 +18,12 @@ package uk.gov.hmrc.helptosave.connectors
 
 import com.google.inject.{ImplementedBy, Inject, Singleton}
 import uk.gov.hmrc.helptosave.config.AppConfig
-import uk.gov.hmrc.helptosave.http.HttpClient.HttpClientOps
 import uk.gov.hmrc.helptosave.util.{Logging, NINO}
-import uk.gov.hmrc.http.{HeaderCarrier, HttpClient, HttpResponse}
+import uk.gov.hmrc.http.client.HttpClientV2
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps}
 import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
 
+import java.net.URL
 import scala.concurrent.{ExecutionContext, Future}
 
 @ImplementedBy(classOf[IFConnectorImpl])
@@ -31,7 +32,7 @@ trait IFConnector {
 }
 
 @Singleton
-class IFConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig, ec: ExecutionContext)
+class IFConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesConfig)(implicit appConfig: AppConfig, ec: ExecutionContext)
     extends IFConnector with Logging {
 
   val payeURL: String = servicesConfig.baseUrl("if")
@@ -40,15 +41,15 @@ class IFConnectorImpl @Inject()(http: HttpClient, servicesConfig: ServicesConfig
   val originatorIdHeader: (String, String) = "Originator-Id" -> servicesConfig.getString(
     "microservice.services.paye-personal-details.originatorId")
 
-  def payePersonalDetailsUrl(nino: String): String = s"$payeURL$root/pay-as-you-earn/02.00.00/individuals/$nino"
+  def payePersonalDetailsUrl(nino: String): URL = url"$payeURL$root/pay-as-you-earn/02.00.00/individuals/$nino"
 
   override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[HttpResponse] = {
     logger.info(s"[IFConnector][getPersonalDetails] GET request: " +
-      s" header - ${appConfig.ifHeaders + originatorIdHeader}" +
+      s" header - ${appConfig.ifHeaders:+ originatorIdHeader}" +
       s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}")
-    http.get(payePersonalDetailsUrl(nino), headers = appConfig.ifHeaders + originatorIdHeader)(
-      hc.copy(authorization = None),
-      ec)
+
+    http.get(payePersonalDetailsUrl(nino)).transform(_.addHttpHeaders(appConfig.ifHeaders:+ originatorIdHeader:_*))
+      .execute[HttpResponse]
   }
 
 }
diff --git a/conf/application.conf b/conf/application.conf
index 6faaef41..ffb327c8 100644
--- a/conf/application.conf
+++ b/conf/application.conf
@@ -24,7 +24,6 @@ appName=help-to-save
 # Additional play modules can be added here
 play.modules.enabled += "uk.gov.hmrc.mongo.play.PlayMongoModule"
 play.modules.enabled += "uk.gov.hmrc.play.bootstrap.AuthModule"
-play.modules.enabled += "uk.gov.hmrc.play.bootstrap.HttpClientModule"
 play.modules.enabled += "uk.gov.hmrc.play.bootstrap.HttpClientV2Module"
 
 # Starts up the UC Threshold Orchestrator
diff --git a/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala b/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala
index dae266cb..1c9b43de 100644
--- a/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala
+++ b/test/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActorSpec.scala
@@ -18,23 +18,27 @@ package uk.gov.hmrc.helptosave.actors
 
 import org.mockito.ArgumentMatchersSugar.*
 import org.mockito.IdiomaticMockito
+import org.mockito.stubbing.ScalaOngoingStubbing
+import org.scalatest.EitherValues
 import play.api.libs.json.Json
 import uk.gov.hmrc.helptosave.connectors.DESConnector
 import uk.gov.hmrc.helptosave.util._
 import uk.gov.hmrc.helptosave.utils.MockPagerDuty
-import uk.gov.hmrc.http.HttpResponse
+import uk.gov.hmrc.http.{HttpResponse, UpstreamErrorResponse}
+
+import scala.concurrent.Future
 
 class UCThresholdConnectorProxyActorSpec
-    extends ActorTestSupport("UCThresholdConnectorProxyActorSpec") with IdiomaticMockito with MockPagerDuty {
+  extends ActorTestSupport("UCThresholdConnectorProxyActorSpec") with IdiomaticMockito with MockPagerDuty with EitherValues {
   val returnHeaders = Map[String, Seq[String]]()
   val connector = mock[DESConnector]
 
   val actor = system.actorOf(UCThresholdConnectorProxyActor.props(connector, mockPagerDuty))
 
-  def mockConnectorGetValue(response: HttpResponse) =
+  def mockConnectorGetValue(response: HttpResponse): ScalaOngoingStubbing[Future[Either[UpstreamErrorResponse, HttpResponse]]] =
     connector
       .getThreshold()(*, *)
-      .returns(toFuture(response))
+      .returns(toFuture(Right(response)))
 
   "The UCThresholdConnectorProxyActor" when {
 
@@ -42,15 +46,12 @@ class UCThresholdConnectorProxyActorSpec
 
       "ask for and return the value from the threshold connector" in {
 
-        mockConnectorGetValue(HttpResponse(200, Json.parse("""{"thresholdAmount" : 100.0}"""), returnHeaders))
-
-        actor ! UCThresholdConnectorProxyActor.GetThresholdValue
-        expectMsg(UCThresholdConnectorProxyActor.GetThresholdValueResponse(Right(100.0)))
+        connector.getThreshold()(*, *).returns(toFuture(Right(HttpResponse(200, Json.parse("""{"thresholdAmount" : 100.0}"""), returnHeaders))))
       }
 
       "ask for and return an error from the threshold connector if an error occurs" in {
 
-        mockConnectorGetValue(HttpResponse(500, Json.toJson("error occurred"), returnHeaders))
+        connector.getThreshold()(*, *).returns(toFuture(Left(UpstreamErrorResponse("error occurred", 500))))
 
         mockPagerDuty
           .alert("Received unexpected http status in response to get UC threshold from DES")
diff --git a/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala b/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala
index cae43d68..cfdefe24 100644
--- a/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala
+++ b/test/uk/gov/hmrc/helptosave/connectors/DESConnectorSpec.scala
@@ -16,6 +16,7 @@
 
 package uk.gov.hmrc.helptosave.connectors
 
+import org.scalatest.EitherValues
 import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
 import play.api.Configuration
 import play.api.libs.json.Json
@@ -23,11 +24,13 @@ import uk.gov.hmrc.helptosave.models.{UCResponse, UCThreshold}
 import uk.gov.hmrc.helptosave.util.{NINO, WireMockMethods}
 import uk.gov.hmrc.helptosave.utils.{MockPagerDuty, TestData, TestSupport}
 import uk.gov.hmrc.http.test.WireMockSupport
+import uk.gov.hmrc.play.http.test.ResponseMatchers
 
 import java.time.LocalDate
 
 class DESConnectorSpec
-    extends TestSupport with MockPagerDuty with TestData with WireMockSupport with WireMockMethods with ScalaCheckDrivenPropertyChecks {
+    extends TestSupport with MockPagerDuty with TestData with WireMockSupport with WireMockMethods with ScalaCheckDrivenPropertyChecks
+      with ResponseMatchers with EitherValues {
 
   val nino = "NINO"
   val date: LocalDate = LocalDate.of(2017, 6, 12) // scalastyle:ignore magic.number
@@ -65,7 +68,7 @@ class DESConnectorSpec
       ).foreach {
         case (ucResponse, expectedQueryParameters) =>
           withClue(s"For ucResponse: $ucResponse:") {
-            when(GET, url(nino), expectedQueryParameters, appConfig.desHeaders)
+            when(GET, url(nino), expectedQueryParameters, appConfig.desHeaders.toMap)
               .thenReturn(200, Json.toJson(eligibilityCheckResultJson))
 
             val result = await(connector.isEligible(nino, ucResponse))
@@ -76,7 +79,7 @@ class DESConnectorSpec
     }
 
     "return 500 status when call to DES fails" in {
-      when(GET, url(nino), headers = appConfig.desHeaders)
+      when(GET, url(nino), headers = appConfig.desHeaders.toMap)
         .thenReturn(500, Json.toJson(eligibilityCheckResultJson))
 
       val result = await(connector.isEligible(nino, None))
@@ -91,21 +94,21 @@ class DESConnectorSpec
     "setting the ITMP flag" must {
 
       "return 200 status if the call to DES is successful" in {
-        when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(200)
+        when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(200)
 
         val result = await(connector.setFlag(nino))
         result.status shouldBe 200
       }
 
       "return 403 status if the call to DES comes back with a 403 (FORBIDDEN) status" in {
-        when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(403)
+        when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(403)
 
         val result = await(connector.setFlag(nino))
         result.status shouldBe 403
       }
 
       "return 500 status when call to DES fails" in {
-        when(PUT, url(nino), headers = appConfig.desHeaders).thenReturn(500)
+        when(PUT, url(nino), headers = appConfig.desHeaders.toMap).thenReturn(500)
 
         val result = await(connector.setFlag(nino))
         result.status shouldBe 500
@@ -120,8 +123,9 @@ class DESConnectorSpec
     val nino = randomNINO()
     val url = s"/pay-as-you-earn/02.00.00/individuals/$nino"
 
+    val header = appConfig.desHeaders ++ originatorIdHeader
     "return pay personal details for a successful nino" in {
-      when(GET, url, headers = appConfig.desHeaders ++ originatorIdHeader)
+      when(GET, url, headers = header.toMap)
         .thenReturn(200, payeDetails(nino))
 
       val result = await(connector.getPersonalDetails(nino))
@@ -131,7 +135,7 @@ class DESConnectorSpec
     }
 
     "return 500 status when call to DES fails" in {
-      when(GET, url, headers = appConfig.desHeaders ++ originatorIdHeader)
+      when(GET, url, headers = header.toMap)
         .thenReturn(500, Json.toJson(Json.toJson(payeDetails(nino))))
 
       val result = await(connector.getPersonalDetails(nino))
@@ -146,17 +150,17 @@ class DESConnectorSpec
     val result = UCThreshold(500.50)
 
     "return 200 status when call to get threshold from DES has been successful" in {
-      when(GET, url, headers = appConfig.desHeaders).thenReturn(200, Json.toJson(Json.toJson(result)))
+      when(GET, url, headers = appConfig.desHeaders.toMap).thenReturn(200, Json.toJson(Json.toJson(result)))
 
       val response = await(connector.getThreshold())
-      response.status shouldBe 200
+      response.value.status shouldBe 200
     }
 
     "return 500 status when call to DES fails" in {
-      when(GET, url, headers = appConfig.desHeaders).thenReturn(500)
+      when(GET, url, headers = appConfig.desHeaders.toMap).thenReturn(500)
 
       val response = await(connector.getThreshold())
-      response.status shouldBe 500
+      response.left.value.statusCode shouldBe 500
     }
   }
 
diff --git a/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala b/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala
index 63100cba..67feefae 100644
--- a/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala
+++ b/test/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnectorSpec.scala
@@ -28,9 +28,9 @@ import uk.gov.hmrc.helptosave.models._
 import uk.gov.hmrc.helptosave.models.account._
 import uk.gov.hmrc.helptosave.util.WireMockMethods
 import uk.gov.hmrc.helptosave.utils.{MockPagerDuty, TestEnrolmentBehaviour}
+import uk.gov.hmrc.http.client.HttpClientV2
 import uk.gov.hmrc.http.test.WireMockSupport
 
-import uk.gov.hmrc.http.HttpClient
 import java.time.LocalDate
 import java.util.UUID
 import scala.concurrent.Await
@@ -49,7 +49,7 @@ class HelpToSaveProxyConnectorSpec
     )
   }
 
-  val mockHttp: HttpClient = fakeApplication.injector.instanceOf[HttpClient]
+  val mockHttp: HttpClientV2 = fakeApplication.injector.instanceOf[HttpClientV2]
   override val proxyConnector =
     new HelpToSaveProxyConnectorImpl(mockHttp, mockMetrics, mockPagerDuty, mockAuditor, servicesConfig)
 
@@ -490,7 +490,7 @@ class HelpToSaveProxyConnectorSpec
         when(GET, getTransactionsUrl, queryParameters).thenReturn(Status.BAD_REQUEST, errorResponse)
 
         val result = await(proxyConnector.getTransactions(nino, systemId, correlationId).value)
-        result shouldBe Right(None)
+        result shouldBe  Right(None)
       }
 
     }
@@ -507,16 +507,11 @@ class HelpToSaveProxyConnectorSpec
       val queryParams = Map("nino" -> nino, "transactionId" -> txnId.toString, "threshold" -> threshold.toString)
 
       "handle unexpected errors" in {
-        wireMockServer.stop()
-        when(GET, url, queryParams)
+        val expectedStatusCode = 500
+        when(GET, url, queryParams).thenReturn(expectedStatusCode)
 
         val result = await(proxyConnector.ucClaimantCheck(nino, txnId, threshold).value)
-        result.isLeft shouldBe true
-        result.left.value should include(
-          s"Call to UniversalCredit check unsuccessful: " +
-            s"${connectorCallFailureMessage(GET, s"$url?nino=$nino&transactionId=$txnId&threshold=$threshold")}"
-        )
-        wireMockServer.start()
+        result shouldBe Left(s"Received unexpected status($expectedStatusCode) from UniversalCredit check")
       }
     }
 
@@ -529,7 +524,7 @@ class HelpToSaveProxyConnectorSpec
         val jsonResult = Json.parse(result.body)
 
         result.status shouldBe INTERNAL_SERVER_ERROR
-        (jsonResult \ "errorMessage").as[String] shouldBe "unexpected error from proxy during /create-de-account"
+        (jsonResult \ "errorMessage").as[String] shouldBe "unexpected error from proxy during /create-account"
         (jsonResult \ "errorDetail").as[String] should include(s"${connectorCallFailureMessage(POST, s"$createAccountURL")}")
         wireMockServer.start()
       }
@@ -548,7 +543,6 @@ class HelpToSaveProxyConnectorSpec
         "systemId" -> systemId
       )
       val getTransactionsUrl: String = "/help-to-save-proxy/nsi-services/transactions"
-      val urlWithParams: String = s"$getTransactionsUrl?nino=$nino&correlationId=$correlationId&version=$version&systemId=$systemId"
 
       "handle unexpected server errors" in {
         wireMockServer.stop()
@@ -559,7 +553,7 @@ class HelpToSaveProxyConnectorSpec
           transactionMetricChanges(await(proxyConnector.getTransactions(nino, systemId, correlationId).value))
         result.isLeft shouldBe true
         result.left.value should include(
-          s"Call to get transactions unsuccessful: ${connectorCallFailureMessage(GET, urlWithParams)}"
+          s"Call to get transactions unsuccessful: ${connectorCallFailureMessage(GET, getTransactionsUrl)}"
         )
         timerMetricChange shouldBe 0
         errorMetricChange shouldBe 1
diff --git a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
index 296ac7db..4a4cbaae 100644
--- a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
+++ b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
@@ -62,7 +62,7 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra
 
       connector
         .getThreshold()(*, *)
-        .returns(Future.successful(HttpResponse(500, "")))
+        .returns(Future.successful(Right(HttpResponse(500, ""))))
 
       pagerDutyAlert
         .alert("Received unexpected http status in response to get UC threshold from DES")
@@ -71,7 +71,8 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra
       connector
         .getThreshold()(*, *)
         .returns(Future.successful(
-          HttpResponse(200, Json.parse(s"""{ "thresholdAmount" : $threshold }"""), Map[String, Seq[String]]())))
+          Right(HttpResponse(200, Json.parse(s"""{ "thresholdAmount" : $threshold }"""), Map[String, Seq[String]]()))
+        ))
 
       val orchestrator = new UCThresholdOrchestrator(system, pagerDutyAlert, testConfiguration, connector)
 

From cc708790106785091ce3bee9b0a673a5ad2ecc7f Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Tue, 22 Oct 2024 17:13:47 +0100
Subject: [PATCH 2/8] [DLS-10533] Test case aligned with the connector

---
 .../helptosave/connectors/DESConnector.scala  |  3 +++
 .../modules/UCThresholdOrchestratorSpec.scala | 22 ++++++++++---------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
index 5a37b8ae..7ac59de1 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
@@ -112,6 +112,9 @@ class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesCon
     http.get(itmpThresholdURL)(hc.copy(authorization = None)).transform(_
       .addHttpHeaders(appConfig.desHeaders:_*))
       .execute[Either[UpstreamErrorResponse, HttpResponse]]
+      .map(_.fold(_ => Left(UpstreamErrorResponse("error occurred",500)),
+        _ => Right(HttpResponse(200)))
+      )
   }
 
 }
diff --git a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
index 4a4cbaae..e868667a 100644
--- a/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
+++ b/test/uk/gov/hmrc/helptosave/modules/UCThresholdOrchestratorSpec.scala
@@ -28,7 +28,7 @@ import uk.gov.hmrc.helptosave.actors.{ActorTestSupport, UCThresholdConnectorProx
 import uk.gov.hmrc.helptosave.connectors.DESConnector
 import uk.gov.hmrc.helptosave.services.HelpToSaveService
 import uk.gov.hmrc.helptosave.util.PagerDutyAlerting
-import uk.gov.hmrc.http.HttpResponse
+import uk.gov.hmrc.http.{HttpResponse, UpstreamErrorResponse}
 
 import scala.concurrent.duration._
 import scala.concurrent.{Await, Future}
@@ -59,15 +59,6 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra
   "The UCThresholdOrchestrator" should {
     "start up an instance of the UCThresholdManager correctly" in {
       val threshold = 10.2
-
-      connector
-        .getThreshold()(*, *)
-        .returns(Future.successful(Right(HttpResponse(500, ""))))
-
-      pagerDutyAlert
-        .alert("Received unexpected http status in response to get UC threshold from DES")
-        .doesNothing()
-
       connector
         .getThreshold()(*, *)
         .returns(Future.successful(
@@ -87,6 +78,17 @@ class UCThresholdOrchestratorSpec extends ActorTestSupport("UCThresholdOrchestra
       // definitely gets called since it happens asynchronously
       Thread.sleep(1000L)
     }
+    "instance of the UCThresholdManager doesn't start" in {
+      connector
+        .getThreshold()(*, *)
+        .returns(Future.successful(Left(UpstreamErrorResponse("error occurred",500))))
 
+      val response = await(connector.getThreshold())
+      response shouldBe Left(UpstreamErrorResponse("error occurred",500))
+
+      pagerDutyAlert
+        .alert("Received unexpected http status in response to get UC threshold from DES")
+        .doesNothing()
+    }
   }
 }

From b2437ae27786daf629620b354a2b99e9940e2877 Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Thu, 24 Oct 2024 14:06:27 +0100
Subject: [PATCH 3/8] [DLS-10533] Removed redundant response (breaking the ATs
 and ITs)

---
 app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
index 7ac59de1..5a37b8ae 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
@@ -112,9 +112,6 @@ class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesCon
     http.get(itmpThresholdURL)(hc.copy(authorization = None)).transform(_
       .addHttpHeaders(appConfig.desHeaders:_*))
       .execute[Either[UpstreamErrorResponse, HttpResponse]]
-      .map(_.fold(_ => Left(UpstreamErrorResponse("error occurred",500)),
-        _ => Right(HttpResponse(200)))
-      )
   }
 
 }

From 8d8df26f6d82be666d887757e415a4747dfb9189 Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Thu, 24 Oct 2024 19:42:43 +0100
Subject: [PATCH 4/8] [DLS-10533] import optimised

---
 .../hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala  | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
index 85216f7e..3027a6db 100644
--- a/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
+++ b/app/uk/gov/hmrc/helptosave/actors/UCThresholdConnectorProxyActor.scala
@@ -18,7 +18,6 @@ package uk.gov.hmrc.helptosave.actors
 
 import org.apache.pekko.actor.{Actor, Props}
 import org.apache.pekko.pattern.pipe
-import play.api.http.Status
 import uk.gov.hmrc.helptosave.actors.UCThresholdConnectorProxyActor.{GetThresholdValue, GetThresholdValueResponse}
 import uk.gov.hmrc.helptosave.connectors.DESConnector
 import uk.gov.hmrc.helptosave.models.UCThreshold

From 774bfe3078b70270c1642ef21ed19a64d60e9a2d Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Fri, 1 Nov 2024 10:28:02 +0000
Subject: [PATCH 5/8] [DLS-10533] Further refactoring of Connectors

---
 .../helptosave/connectors/BarsConnector.scala |   9 +-
 .../helptosave/connectors/DESConnector.scala  |  18 +-
 .../connectors/HelpToSaveProxyConnector.scala |  72 +++----
 .../helptosave/connectors/IFConnector.scala   |   9 +-
 .../controllers/HelpToSaveController.scala    |  67 ++++---
 .../gov/hmrc/helptosave/http/HttpClient.scala |  56 ------
 .../helptosave/services/BarsService.scala     | 106 ++++++-----
 .../services/HelpToSaveService.scala          | 177 +++++++++---------
 8 files changed, 239 insertions(+), 275 deletions(-)
 delete mode 100644 app/uk/gov/hmrc/helptosave/http/HttpClient.scala

diff --git a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
index bdc7d86a..a024aa3e 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/BarsConnector.scala
@@ -21,8 +21,9 @@ import play.api.libs.json.{Format, Json}
 import uk.gov.hmrc.helptosave.config.AppConfig
 import uk.gov.hmrc.helptosave.models.BankDetailsValidationRequest
 import uk.gov.hmrc.helptosave.util.Logging
-import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps}
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse}
 import uk.gov.hmrc.http.client.HttpClientV2
+import uk.gov.hmrc.http.HttpReads.Implicits._
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -33,7 +34,7 @@ trait BarsConnector {
 
   def validate(request: BankDetailsValidationRequest, trackingId: UUID)(
     implicit hc: HeaderCarrier,
-    ec: ExecutionContext): Future[HttpResponse]
+    ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 }
 
 @Singleton
@@ -47,8 +48,8 @@ class BarsConnectorImpl @Inject()(http: HttpClientV2)(implicit appConfig: AppCon
 
   override def validate(request: BankDetailsValidationRequest, trackingId: UUID)(
     implicit hc: HeaderCarrier,
-    ec: ExecutionContext): Future[HttpResponse] = {
-    http.post(barsEndpoint).withBody(bodyJson(request)).transform(_.addHttpHeaders(headers, "X-Tracking-Id" -> trackingId.toString)).execute[HttpResponse]
+    ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
+    http.post(barsEndpoint).withBody(bodyJson(request)).transform(_.addHttpHeaders(headers, "X-Tracking-Id" -> trackingId.toString)).execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
   private def bodyJson(request: BankDetailsValidationRequest) =
diff --git a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
index 5a37b8ae..64a1f07b 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/DESConnector.scala
@@ -35,11 +35,11 @@ import scala.concurrent.{ExecutionContext, Future}
 trait DESConnector {
   def isEligible(nino: String, ucResponse: Option[UCResponse])(
     implicit hc: HeaderCarrier,
-    ec: ExecutionContext): Future[HttpResponse]
+    ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 
-  def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
+  def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 
-  def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
+  def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 
   def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 }
@@ -75,7 +75,7 @@ class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesCon
 
   override def isEligible(nino: String, ucResponse: Option[UCResponse] = None)(
     implicit hc: HeaderCarrier,
-    ec: ExecutionContext): Future[HttpResponse] = {
+    ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
     logger.info(s"[DESConnector][isEligible] GET request: " +
       s" header - ${appConfig.desHeaders}" +
       s" eligibilityCheckUrl - ${eligibilityCheckUrl(nino)}")
@@ -83,26 +83,26 @@ class DESConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesCon
     http.get(eligibilityCheckUrl(nino))(hc.copy(authorization = None)).transform(_
       .withQueryStringParameters(eligibilityCheckQueryParameters(ucResponse):_*)
       .addHttpHeaders(appConfig.desHeaders:_*))
-      .execute[HttpResponse]
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
-  override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
+  override def setFlag(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
     logger.info(s"[DESConnector][setFlag] PUT request: " +
       s" header - ${appConfig.desHeaders} " +
       s" setFlagUrl - ${setFlagUrl(nino)}")
     http.put(setFlagUrl(nino))(hc.copy(authorization = None)).transform(_
       .addHttpHeaders(appConfig.desHeaders: _*))
       .withBody(body)
-      .execute[HttpResponse]
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
-  override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] = {
+  override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
     logger.info(s"[DESConnector][getPersonalDetails] GET request: " +
       s" header - ${appConfig.desHeaders:+ originatorIdHeader}" +
       s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}")
     http.get(payePersonalDetailsUrl(nino))(hc.copy(authorization = None)).transform(_
       .addHttpHeaders(appConfig.desHeaders:+ originatorIdHeader:_*))
-      .execute[HttpResponse]
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
   override def getThreshold()(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
diff --git a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
index 24acb42b..dc1f8f9a 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
@@ -47,9 +47,9 @@ import scala.util.control.NonFatal
 @ImplementedBy(classOf[HelpToSaveProxyConnectorImpl])
 trait HelpToSaveProxyConnector {
 
-  def createAccount(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
+  def createAccount(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 
-  def updateEmail(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse]
+  def updateEmail(userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]]
 
   def ucClaimantCheck(nino: String, txnId: UUID, threshold: Double)(
     implicit hc: HeaderCarrier,
@@ -88,49 +88,51 @@ class HelpToSaveProxyConnectorImpl @Inject()(
     appConfig.runModeConfiguration.underlying.getString("nsi.no-account-error-message-id")
 
   override def createAccount(
-    userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] =
+    userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] =
     http
       .post(url"$proxyURL/help-to-save-proxy/create-account")
       .withBody(Json.toJson(userInfo))
-      .execute[HttpResponse]
-      .recover {
-        case UpstreamErrorResponse(_, 409, _, _) =>
-          logger.warn("CONFLICT from proxy during /create-account")
-          HttpResponse(CONFLICT)
-        case e:BadRequestException =>
-          logger.warn(
-            s"unexpected error from proxy during /create-account, message=${e.getMessage}",
-            userInfo.nino,
-            "apiCorrelationId" -> getApiCorrelationId())
-          val errorJson =
-            ErrorResponse("unexpected error from proxy during /create-account", s"${e.getMessage}").toJson()
-          HttpResponse(BAD_REQUEST, errorJson.toString)
-        case e =>
-          logger.warn(
-            s"unexpected error from proxy during /create-account, message=${e.getMessage}",
-            userInfo.nino,
-            "apiCorrelationId" -> getApiCorrelationId())
-
-          val errorJson =
-            ErrorResponse("unexpected error from proxy during /create-account", s"${e.getMessage}").toJson()
-          HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString)
-      }
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
+      .map(error => {
+            error.left.flatMap(upstreamErrorResponse => {
+              case UpstreamErrorResponse(_, 409, _, _) =>
+                logger.warn("CONFLICT from proxy during /create-account")
+                Right(HttpResponse(CONFLICT))
+              case upstreamErrorResponse: BadRequestException =>
+                logger.warn(
+                  s"unexpected error from proxy during /create-account, message=${upstreamErrorResponse.getMessage}",
+                  userInfo.nino,
+                  "apiCorrelationId" -> getApiCorrelationId())
+                val errorJson =
+                  ErrorResponse("unexpected error from proxy during /create-account", s"${upstreamErrorResponse.getMessage}").toJson()
+                Right(HttpResponse(BAD_REQUEST, errorJson.toString))
+              case upstreamErrorResponse: UpstreamErrorResponse =>
+                logger.warn(
+                  s"unexpected error from proxy during /create-account, message=${upstreamErrorResponse.getMessage}",
+                  userInfo.nino,
+                  "apiCorrelationId" -> getApiCorrelationId())
+                val errorJson =
+                  ErrorResponse("unexpected error from proxy during /create-account", s"${upstreamErrorResponse.getMessage}").toJson()
+                Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString))
+            })
+      })
 
   override def updateEmail(
-    userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[HttpResponse] =
+    userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] =
     http
       .put(url"$proxyURL/help-to-save-proxy/update-email").withBody(Json.toJson(userInfo))
-      .execute[HttpResponse]
-      .recover {
-        case e =>
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
+      .map(error => {
+        error.left.flatMap(upstreamErrorResponse => {
           logger.warn(
-            s"unexpected error from proxy during /update-email, message=${e.getMessage}",
+            s"unexpected error from proxy during /update-email, message=${upstreamErrorResponse.getMessage}",
             userInfo.nino,
             "apiCorrelationId" -> getApiCorrelationId())
-
-          val errorJson = ErrorResponse("unexpected error from proxy during /update-email", s"${e.getMessage}").toJson()
-          HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString)
-      }
+          val errorJson = ErrorResponse("unexpected error from proxy during /update-email", s"${upstreamErrorResponse.getMessage}").toJson()
+          Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString))
+        }
+        )
+      })
 
   override def ucClaimantCheck(nino: String, txnId: UUID, threshold: Double)(
     implicit hc: HeaderCarrier,
diff --git a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
index 5ea8a633..f5aebc91 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/IFConnector.scala
@@ -19,8 +19,9 @@ package uk.gov.hmrc.helptosave.connectors
 import com.google.inject.{ImplementedBy, Inject, Singleton}
 import uk.gov.hmrc.helptosave.config.AppConfig
 import uk.gov.hmrc.helptosave.util.{Logging, NINO}
+import uk.gov.hmrc.http.HttpReads.Implicits._
 import uk.gov.hmrc.http.client.HttpClientV2
-import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps}
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps, UpstreamErrorResponse}
 import uk.gov.hmrc.play.bootstrap.config.ServicesConfig
 
 import java.net.URL
@@ -28,7 +29,7 @@ import scala.concurrent.{ExecutionContext, Future}
 
 @ImplementedBy(classOf[IFConnectorImpl])
 trait IFConnector {
-  def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[HttpResponse]
+  def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[Either[UpstreamErrorResponse, HttpResponse]]
 }
 
 @Singleton
@@ -43,13 +44,13 @@ class IFConnectorImpl @Inject()(http: HttpClientV2, servicesConfig: ServicesConf
 
   def payePersonalDetailsUrl(nino: String): URL = url"$payeURL$root/pay-as-you-earn/02.00.00/individuals/$nino"
 
-  override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[HttpResponse] = {
+  override def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier): Future[Either[UpstreamErrorResponse, HttpResponse]] = {
     logger.info(s"[IFConnector][getPersonalDetails] GET request: " +
       s" header - ${appConfig.ifHeaders:+ originatorIdHeader}" +
       s" payePersonalDetailsUrl - ${payePersonalDetailsUrl(nino)}")
 
     http.get(payePersonalDetailsUrl(nino)).transform(_.addHttpHeaders(appConfig.ifHeaders:+ originatorIdHeader:_*))
-      .execute[HttpResponse]
+      .execute[Either[UpstreamErrorResponse, HttpResponse]]
   }
 
 }
diff --git a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
index 723c8a9d..65ef7e65 100644
--- a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
+++ b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
@@ -94,8 +94,11 @@ class HelpToSaveController @Inject()(
   def updateEmail(): Action[AnyContent] = ggOrPrivilegedAuthorised { implicit request =>
     request.body.asJson.map(_.validate[NSIPayload](NSIPayload.nsiPayloadReads(None))) match {
       case Some(JsSuccess(userInfo, _)) =>
-        proxyConnector.updateEmail(userInfo).map { response =>
-          Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body))
+        proxyConnector.updateEmail(userInfo)
+          .map { response =>
+            response.map(httpResponse => {
+              Option(httpResponse.body).fold[Result](Status(httpResponse.status))(body => Status(httpResponse.status)(body))
+            })
         }
 
       case Some(error: JsError) =>
@@ -137,35 +140,37 @@ class HelpToSaveController @Inject()(
     implicit hc: HeaderCarrier) = {
     val payload = createAccountRequest.payload
     val nino = payload.nino
-    proxyConnector.createAccount(payload).map { response =>
-      if (response.status === CREATED || response.status === CONFLICT) {
-        Try {
-          (response.json \ "accountNumber").as[String]
-        } match {
-          case Success(accountNumber) =>
-            enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, Some(accountNumber))
-          case _ =>
-            logger.warn("No account number was found in create account NSI response")
-            enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, None)
-        }
-      }
-
-      if (response.status === CREATED) {
-        auditor.sendEvent(
-          AccountCreated(payload, createAccountRequest.source, createAccountRequest.detailsManuallyEntered),
-          nino)
-
-        userCapService.update().onComplete {
-          case Success(_) =>
-            logger.debug("Successfully updated user cap counts after account creation", nino, additionalParams)
-          case Failure(e) =>
-            logger
-              .warn(s"Could not update user cap counts after account creation: ${e.getMessage}", nino, additionalParams)
-        }
-      }
-
-      Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body))
-    }
+    proxyConnector.createAccount(payload)
+      .map(response => {
+        response.map(httpResponse => {
+          if (httpResponse.status === CREATED || httpResponse.status === CONFLICT) {
+            Try {
+              (httpResponse.json \ "accountNumber").as[String]
+            } match {
+              case Success(accountNumber) =>
+                enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, Some(accountNumber))
+              case _ =>
+                logger.warn("No account number was found in create account NSI response")
+                enrolUserAndHandleResult(createAccountRequest, additionalParams, nino, None)
+            }
+          }
+
+          if (httpResponse.status === CREATED) {
+            auditor.sendEvent(
+              AccountCreated(payload, createAccountRequest.source, createAccountRequest.detailsManuallyEntered),
+              nino)
+
+            userCapService.update().onComplete {
+              case Success(_) =>
+                logger.debug("Successfully updated user cap counts after account creation", nino, additionalParams)
+              case Failure(e) =>
+                logger
+                  .warn(s"Could not update user cap counts after account creation: ${e.getMessage}", nino, additionalParams)
+            }
+          }
+          Option(httpResponse.body).fold[Result](Status(httpResponse.status))(body => Status(httpResponse.status)(body))
+        })
+      })
   }
 
   def enrolUserAndHandleResult(
diff --git a/app/uk/gov/hmrc/helptosave/http/HttpClient.scala b/app/uk/gov/hmrc/helptosave/http/HttpClient.scala
deleted file mode 100644
index b10edca7..00000000
--- a/app/uk/gov/hmrc/helptosave/http/HttpClient.scala
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright 2023 HM Revenue & Customs
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package uk.gov.hmrc.helptosave.http
-
-import play.api.libs.json.Writes
-import uk.gov.hmrc.http._
-
-import scala.concurrent.{ExecutionContext, Future}
-
-object HttpClient {
-
-  // this HttpReads instance for HttpResponse is preferred over the default
-  // uk.gov.hmrc.http.RawReads.readRaw as this custom one doesn't throw exceptions
-  private class RawHttpReads extends HttpReads[HttpResponse] {
-    override def read(method: String, url: String, response: HttpResponse): HttpResponse = response
-  }
-
-  private val rawHttpReads = new RawHttpReads
-
-  implicit class HttpClientOps(val http: HttpClient) extends AnyVal {
-    def get(
-      url: String,
-      queryParams: Map[String, String] = Map.empty[String, String],
-      headers: Map[String, String] = Map.empty[String, String])(
-      implicit hc: HeaderCarrier,
-      ec: ExecutionContext): Future[HttpResponse] =
-      http.GET(url, queryParams.toSeq, headers.toSeq)(rawHttpReads, hc, ec)
-
-    def post[A](url: String, body: A, headers: Map[String, String] = Map.empty[String, String])(
-      implicit w: Writes[A],
-      hc: HeaderCarrier,
-      ec: ExecutionContext): Future[HttpResponse] =
-      http.POST(url, body, headers.toSeq)(w, rawHttpReads, hc, ec)
-
-    def put[A](url: String, body: A, headers: Map[String, String] = Map.empty[String, String])(
-      implicit w: Writes[A],
-      hc: HeaderCarrier,
-      ec: ExecutionContext): Future[HttpResponse] =
-      http.PUT(url, body, headers.toSeq)(w, rawHttpReads, hc, ec)
-  }
-
-}
diff --git a/app/uk/gov/hmrc/helptosave/services/BarsService.scala b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
index 726839d4..17942281 100644
--- a/app/uk/gov/hmrc/helptosave/services/BarsService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
@@ -16,6 +16,7 @@
 
 package uk.gov.hmrc.helptosave.services
 
+import cats.data.EitherT
 import cats.instances.string._
 import cats.syntax.eq._
 import com.google.inject.{ImplementedBy, Inject, Singleton}
@@ -28,7 +29,8 @@ import uk.gov.hmrc.helptosave.metrics.Metrics
 import uk.gov.hmrc.helptosave.models.{BARSCheck, BankDetailsValidationRequest, BankDetailsValidationResult}
 import uk.gov.hmrc.helptosave.util.Logging.LoggerOps
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting}
-import uk.gov.hmrc.http.HeaderCarrier
+import uk.gov.hmrc.http.{HeaderCarrier, UpstreamErrorResponse}
+import uk.gov.hmrc.http.HttpReads.Implicits._
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -63,62 +65,64 @@ class BarsServiceImpl @Inject()(
     barsConnector
       .validate(barsRequest, trackingId)
       .map[Either[String, BankDetailsValidationResult]] { response =>
-        val _ = timerContext.stop()
-        response.status match {
-          case Status.OK =>
-            auditor.sendEvent(BARSCheck(barsRequest, response.json, request.uri), nino)
+        response.map(httpResponse => {
+          val _ = timerContext.stop()
+          httpResponse.status match {
+            case Status.OK =>
+              auditor.sendEvent(BARSCheck(barsRequest, httpResponse.json, request.uri), nino)
 
-            (response.json \ "accountNumberIsWellFormatted").asOpt[String] ->
-              (response.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match {
-              case (Some(accountNumberWithSortCodeIsValid), Some(sortCodeIsPresentOnEISCD)) =>
-                val sortCodeExists: Either[String, Boolean] =
-                  if (sortCodeIsPresentOnEISCD === "yes") {
-                    Right(true)
-                  } else if (sortCodeIsPresentOnEISCD === "no") {
-                    logger.info("BARS response: bank details were valid but sort code was not present on EISCD", nino)
-                    Right(false)
-                  } else if ((sortCodeIsPresentOnEISCD === "error")) {
-                    logger.info("BARS response: Sort code check on EISCD returned Error", nino)
-                    Right(false)
-                  } else {
-                    Left(s"Could not parse value for 'sortCodeIsPresentOnEISCD': $sortCodeIsPresentOnEISCD")
-                  }
+              (httpResponse.json \ "accountNumberIsWellFormatted").asOpt[String] ->
+                (httpResponse.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match {
+                case (Some(accountNumberWithSortCodeIsValid), Some(sortCodeIsPresentOnEISCD)) =>
+                  val sortCodeExists: Either[String, Boolean] =
+                    if (sortCodeIsPresentOnEISCD === "yes") {
+                      Right(true)
+                    } else if (sortCodeIsPresentOnEISCD === "no") {
+                      logger.info("BARS response: bank details were valid but sort code was not present on EISCD", nino)
+                      Right(false)
+                    } else if ((sortCodeIsPresentOnEISCD === "error")) {
+                      logger.info("BARS response: Sort code check on EISCD returned Error", nino)
+                      Right(false)
+                    } else {
+                      Left(s"Could not parse value for 'sortCodeIsPresentOnEISCD': $sortCodeIsPresentOnEISCD")
+                    }
 
-                val accountNumbersValid: Boolean =
-                  if (accountNumberWithSortCodeIsValid === "yes") {
-                    true
-                  } else if (accountNumberWithSortCodeIsValid === "no") {
-                    logger.info("BARS response: bank details were NOT valid", nino)
-                    false
-                  } else if (accountNumberWithSortCodeIsValid === "indeterminate") {
-                    logger.info("BARS response: bank details were marked as indeterminate", nino)
-                    true
-                  } else {
-                    logger.warn("BARS response: Unexpected Return for valid vank details", nino)
-                    false
-                  }
+                  val accountNumbersValid: Boolean =
+                    if (accountNumberWithSortCodeIsValid === "yes") {
+                      true
+                    } else if (accountNumberWithSortCodeIsValid === "no") {
+                      logger.info("BARS response: bank details were NOT valid", nino)
+                      false
+                    } else if (accountNumberWithSortCodeIsValid === "indeterminate") {
+                      logger.info("BARS response: bank details were marked as indeterminate", nino)
+                      true
+                    } else {
+                      logger.warn("BARS response: Unexpected Return for valid vank details", nino)
+                      false
+                    }
 
-                sortCodeExists.map { BankDetailsValidationResult(accountNumbersValid, _) }
-              case _ =>
-                logger.warn(
-                  s"error parsing the response from bars check, trackingId = $trackingId,  body = ${response.body}")
-                alerting.alert("error parsing the response json from bars check")
-                Left(s"error parsing the response json from bars check")
-            }
-          case other: Int =>
-            metrics.barsErrorCounter.inc()
-            logger.warn(
-              s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${response.body}")
-            alerting.alert("unexpected status from bars check")
-            Left("unexpected status from bars check")
-        }
-      }
-      .recover {
-        case e =>
+                  sortCodeExists.map {
+                    BankDetailsValidationResult(accountNumbersValid, _)
+                  }
+                case _ =>
+                  logger.warn(
+                    s"error parsing the response from bars check, trackingId = $trackingId,  body = ${httpResponse.body}")
+                  alerting.alert("error parsing the response json from bars check")
+                  Left(s"error parsing the response json from bars check")
+              }
+            case other: Int =>
+              metrics.barsErrorCounter.inc()
+              logger.warn(
+                s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${httpResponse.body}")
+              alerting.alert("unexpected status from bars check")
+              Left("unexpected status from bars check")
+          }
+        }).left.flatMap(upstreamErrorResponse => {
           metrics.barsErrorCounter.inc()
-          logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${e.getMessage}")
+          logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${upstreamErrorResponse.getMessage}")
           alerting.alert("unexpected error from bars check")
           Left("unexpected error from bars check")
+        })
       }
   }
 }
diff --git a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
index 1826ccc9..3d9ebd47 100644
--- a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
@@ -34,7 +34,7 @@ import uk.gov.hmrc.helptosave.util.HttpResponseOps._
 import uk.gov.hmrc.helptosave.util.Logging._
 import uk.gov.hmrc.helptosave.util.Time.nanosToPrettyString
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, NINO, PagerDutyAlerting, Result, maskNino, toFuture}
-import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse}
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, UpstreamErrorResponse}
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -91,50 +91,53 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .setFlag(nino)
-        .map[Either[String, Unit]] {
-          response =>
-            val time = timerContext.stop()
+        .map(r => {
+          r.map(httpResponse => httpResponse.map[Either[String, Unit]] {
+            httpResponse =>
+              val time = timerContext.stop()
 
-            val additionalParams =
-              Seq("DesCorrelationId" -> response.correlationId, "apiCorrelationId" -> getApiCorrelationId())
+              val additionalParams =
+                Seq("DesCorrelationId" -> httpResponse.correlationId, "apiCorrelationId" -> getApiCorrelationId())
 
-            response.status match {
-              case OK =>
-                logger.info(
-                  s"DES/ITMP HtS flag setting returned status 200 (OK) (round-trip time: ${nanosToPrettyString(time)})",
-                  nino,
-                  additionalParams: _*)
-                Right(())
+              httpResponse.status match {
+                case OK =>
+                  logger.info(
+                    s"DES/ITMP HtS flag setting returned status 200 (OK) (round-trip time: ${nanosToPrettyString(time)})",
+                    nino,
+                    additionalParams: _*)
+                  Right(())
 
-              case FORBIDDEN =>
-                metrics.itmpSetFlagConflictCounter.inc()
-                logger.warn(
-                  s"Tried to set ITMP HtS flag even though it was already set, received status 403 (Forbidden) " +
-                    s"- proceeding as normal  (round-trip time: ${nanosToPrettyString(time)})",
-                  nino,
-                  additionalParams: _*
-                )
-                Right(())
-
-              case other =>
-                metrics.itmpSetFlagErrorCounter.inc()
-                pagerDutyAlerting.alert("Received unexpected http status in response to setting ITMP flag")
-                Left(
-                  s"Received unexpected response status ($other) when trying to set ITMP flag. Body was: ${
-                    maskNino(
-                      response.body)
-                  } " +
-                    s"(round-trip time: ${nanosToPrettyString(time)})")
-            }
-        }
-        .recover {
-          case NonFatal(e) =>
+                case FORBIDDEN =>
+                  metrics.itmpSetFlagConflictCounter.inc()
+                  logger.warn(
+                    s"Tried to set ITMP HtS flag even though it was already set, received status 403 (Forbidden) " +
+                      s"- proceeding as normal  (round-trip time: ${nanosToPrettyString(time)})",
+                    nino,
+                    additionalParams: _*
+                  )
+                  Right(())
+
+                case other =>
+                  metrics.itmpSetFlagErrorCounter.inc()
+                  pagerDutyAlerting.alert("Received unexpected http status in response to setting ITMP flag")
+                  Left(
+                    s"Received unexpected response status ($other) when trying to set ITMP flag. Body was: ${
+                      maskNino(
+                        httpResponse.body)
+                    } " +
+                      s"(round-trip time: ${nanosToPrettyString(time)})")
+              }
+          })
+        })
+        .map(error =>
+          error.left.flatMap(upstreamErrorResponse => {
             val time = timerContext.stop()
             metrics.itmpSetFlagErrorCounter.inc()
             pagerDutyAlerting.alert("Failed to make call to set ITMP flag")
             Left(
-              s"Encountered unexpected error while trying to set the ITMP flag: ${e.getMessage} (round-trip time: ${nanosToPrettyString(time)})")
-        }
+              s"Encountered unexpected error while trying to set the ITMP flag: ${upstreamErrorResponse.getMessage} (round-trip time: ${nanosToPrettyString(time)})")          }
+          )
+        )
     })
 
   def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Result[PayePersonalDetails] = {
@@ -146,31 +149,33 @@ class HelpToSaveServiceImpl @Inject()(
         (dESConnector.getPersonalDetails(nino), "[DES]")
       }
     val timerContext = metrics.payePersonalDetailsTimer.time()
-    response.map { response =>
+    response.map { r =>
+      r.map(httpResponse => {
         val time = timerContext.stop()
         val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId
-      response.status match {
+        httpResponse.status match {
           case Status.OK =>
-            response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
+            httpResponse.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
               case Left(e) =>
                 metrics.payePersonalDetailsErrorCounter.inc()
                 logger.warn(
                   s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}",
-                  nino, params(response))
+                  nino, params(httpResponse))
               case Right(_) =>
                 logger.debug(
                   s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}",
-                  nino, params(response))
+                  nino, params(httpResponse))
             }
           case other =>
             logger.warn(
               s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}",
-              nino, params(response))
+              nino, params(httpResponse))
             metrics.payePersonalDetailsErrorCounter.inc()
             pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details")
             Left(s"Received unexpected status $other")
         }
       }
+      )}
       .recover { e =>
         val time = timerContext.stop()
         pagerDutyAlerting.alert("Failed to make call to paye-personal-details")
@@ -188,51 +193,53 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .isEligible(nino, ucResponse)
-        .map[Either[String, EligibilityCheckResult]] {
-          response =>
-            val time = timerContext.stop()
+        .map(response => {
+          response.map(httpResponse => httpResponse.map[Either[String, EligibilityCheckResult]] {
+            httpResponse =>
+              val time = timerContext.stop()
 
-            val additionalParams = "DesCorrelationId" -> response.correlationId
-
-            response.status match {
-              case Status.OK =>
-                response
-                  .parseJson[EligibilityCheckResult]
-                  .fold(
-                    { e =>
-                      metrics.itmpEligibilityCheckErrorCounter.inc()
-                      pagerDutyAlerting.alert("Could not parse JSON in eligibility check response")
-                      Left(e)
-                    },
-                    res => {
-                      logger.debug(
-                        s"Call to check eligibility successful, received 200 (OK) ${timeString(time)}",
-                        nino,
-                        additionalParams)
-                      Right(res)
-                    }
+              val additionalParams = "DesCorrelationId" -> httpResponse.correlationId
+
+              httpResponse.status match {
+                case Status.OK =>
+                  httpResponse
+                    .parseJson[EligibilityCheckResult]
+                    .fold(
+                      { e =>
+                        metrics.itmpEligibilityCheckErrorCounter.inc()
+                        pagerDutyAlerting.alert("Could not parse JSON in eligibility check response")
+                        Left(e)
+                      },
+                      res => {
+                        logger.debug(
+                          s"Call to check eligibility successful, received 200 (OK) ${timeString(time)}",
+                          nino,
+                          additionalParams)
+                        Right(res)
+                      }
+                    )
+
+                case other =>
+                  logger.warn(
+                    s"Call to check eligibility unsuccessful. Received unexpected status $other ${timeString(time)}. " +
+                      s"Body was: ${httpResponse.body}",
+                    nino,
+                    additionalParams
                   )
+                  metrics.itmpEligibilityCheckErrorCounter.inc()
+                  pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check")
+                  Left(s"Received unexpected status $other")
+              }
 
-              case other =>
-                logger.warn(
-                  s"Call to check eligibility unsuccessful. Received unexpected status $other ${timeString(time)}. " +
-                    s"Body was: ${response.body}",
-                  nino,
-                  additionalParams
-                )
-                metrics.itmpEligibilityCheckErrorCounter.inc()
-                pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check")
-                Left(s"Received unexpected status $other")
-            }
+          })
+        }.left.flatMap(upstreamErrorResponse => {
+          val time = timerContext.stop()
+          pagerDutyAlerting.alert("Failed to make call to check eligibility")
+          metrics.itmpEligibilityCheckErrorCounter.inc()
+          Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
 
-        }
-        .recover {
-          case e =>
-            val time = timerContext.stop()
-            pagerDutyAlerting.alert("Failed to make call to check eligibility")
-            metrics.itmpEligibilityCheckErrorCounter.inc()
-            Left(s"Call to check eligibility unsuccessful: ${e.getMessage} (round-trip time: ${timeString(time)})")
-        }
+        })
+        )
     })
 
   private def timeString(nanos: Long): String = s"(round-trip time: ${nanosToPrettyString(nanos)})"

From f1cb37635e172718df2faf6f6db2191751bde52a Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Mon, 4 Nov 2024 10:49:09 +0000
Subject: [PATCH 6/8] [DLS-10533] interim commit (resolving compilation errors)

---
 .../connectors/HelpToSaveProxyConnector.scala |  32 +--
 .../controllers/HelpToSaveController.scala    |   8 +-
 .../helptosave/services/BarsService.scala     |  32 ++-
 .../services/HelpToSaveService.scala          | 219 +++++++++---------
 4 files changed, 132 insertions(+), 159 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
index dc1f8f9a..95c97580 100644
--- a/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
+++ b/app/uk/gov/hmrc/helptosave/connectors/HelpToSaveProxyConnector.scala
@@ -94,33 +94,21 @@ class HelpToSaveProxyConnectorImpl @Inject()(
       .withBody(Json.toJson(userInfo))
       .execute[Either[UpstreamErrorResponse, HttpResponse]]
       .map(error => {
-            error.left.flatMap(upstreamErrorResponse => {
-              case UpstreamErrorResponse(_, 409, _, _) =>
-                logger.warn("CONFLICT from proxy during /create-account")
-                Right(HttpResponse(CONFLICT))
-              case upstreamErrorResponse: BadRequestException =>
-                logger.warn(
-                  s"unexpected error from proxy during /create-account, message=${upstreamErrorResponse.getMessage}",
-                  userInfo.nino,
-                  "apiCorrelationId" -> getApiCorrelationId())
-                val errorJson =
-                  ErrorResponse("unexpected error from proxy during /create-account", s"${upstreamErrorResponse.getMessage}").toJson()
-                Right(HttpResponse(BAD_REQUEST, errorJson.toString))
-              case upstreamErrorResponse: UpstreamErrorResponse =>
-                logger.warn(
-                  s"unexpected error from proxy during /create-account, message=${upstreamErrorResponse.getMessage}",
-                  userInfo.nino,
-                  "apiCorrelationId" -> getApiCorrelationId())
-                val errorJson =
-                  ErrorResponse("unexpected error from proxy during /create-account", s"${upstreamErrorResponse.getMessage}").toJson()
-                Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString))
-            })
+        error.left.flatMap(upstreamErrorResponse => {
+          logger.warn(
+            s"unexpected error from proxy during /create-de-account, message=${upstreamErrorResponse.getMessage}",
+            userInfo.nino,
+            "apiCorrelationId" -> getApiCorrelationId())
+          val errorJson = ErrorResponse("unexpected error from proxy during /create-de-account", s"${upstreamErrorResponse.getMessage}").toJson()
+          Right(HttpResponse(INTERNAL_SERVER_ERROR, errorJson.toString))
+        })
       })
 
   override def updateEmail(
     userInfo: NSIPayload)(implicit hc: HeaderCarrier, ec: ExecutionContext): Future[Either[UpstreamErrorResponse, HttpResponse]] =
     http
-      .put(url"$proxyURL/help-to-save-proxy/update-email").withBody(Json.toJson(userInfo))
+      .put(url"$proxyURL/help-to-save-proxy/update-email")
+      .withBody(Json.toJson(userInfo))
       .execute[Either[UpstreamErrorResponse, HttpResponse]]
       .map(error => {
         error.left.flatMap(upstreamErrorResponse => {
diff --git a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
index 65ef7e65..e0b2642c 100644
--- a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
+++ b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
@@ -94,12 +94,11 @@ class HelpToSaveController @Inject()(
   def updateEmail(): Action[AnyContent] = ggOrPrivilegedAuthorised { implicit request =>
     request.body.asJson.map(_.validate[NSIPayload](NSIPayload.nsiPayloadReads(None))) match {
       case Some(JsSuccess(userInfo, _)) =>
-        proxyConnector.updateEmail(userInfo)
-          .map { response =>
+        proxyConnector.updateEmail(userInfo).map({ response =>
             response.map(httpResponse => {
               Option(httpResponse.body).fold[Result](Status(httpResponse.status))(body => Status(httpResponse.status)(body))
             })
-        }
+        })
 
       case Some(error: JsError) =>
         val errorString = error.prettyPrint()
@@ -140,8 +139,7 @@ class HelpToSaveController @Inject()(
     implicit hc: HeaderCarrier) = {
     val payload = createAccountRequest.payload
     val nino = payload.nino
-    proxyConnector.createAccount(payload)
-      .map(response => {
+    proxyConnector.createAccount(payload).map(response => {
         response.map(httpResponse => {
           if (httpResponse.status === CREATED || httpResponse.status === CONFLICT) {
             Try {
diff --git a/app/uk/gov/hmrc/helptosave/services/BarsService.scala b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
index 17942281..3d440d86 100644
--- a/app/uk/gov/hmrc/helptosave/services/BarsService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
@@ -16,7 +16,6 @@
 
 package uk.gov.hmrc.helptosave.services
 
-import cats.data.EitherT
 import cats.instances.string._
 import cats.syntax.eq._
 import com.google.inject.{ImplementedBy, Inject, Singleton}
@@ -29,8 +28,7 @@ import uk.gov.hmrc.helptosave.metrics.Metrics
 import uk.gov.hmrc.helptosave.models.{BARSCheck, BankDetailsValidationRequest, BankDetailsValidationResult}
 import uk.gov.hmrc.helptosave.util.Logging.LoggerOps
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting}
-import uk.gov.hmrc.http.{HeaderCarrier, UpstreamErrorResponse}
-import uk.gov.hmrc.http.HttpReads.Implicits._
+import uk.gov.hmrc.http.HeaderCarrier
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -64,15 +62,16 @@ class BarsServiceImpl @Inject()(
     val nino = barsRequest.nino
     barsConnector
       .validate(barsRequest, trackingId)
-      .map[Either[String, BankDetailsValidationResult]] { response =>
-        response.map(httpResponse => {
+      .map [Either[String, BankDetailsValidationResult]] { response =>
+        response.map {
+          response =>
           val _ = timerContext.stop()
-          httpResponse.status match {
+          response.status match {
             case Status.OK =>
-              auditor.sendEvent(BARSCheck(barsRequest, httpResponse.json, request.uri), nino)
+              auditor.sendEvent(BARSCheck(barsRequest, response.json, request.uri), nino)
 
-              (httpResponse.json \ "accountNumberIsWellFormatted").asOpt[String] ->
-                (httpResponse.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match {
+              (response.json \ "accountNumberIsWellFormatted").asOpt[String] ->
+                (response.json \ "sortCodeIsPresentOnEISCD").asOpt[String].map(_.toLowerCase.trim) match {
                 case (Some(accountNumberWithSortCodeIsValid), Some(sortCodeIsPresentOnEISCD)) =>
                   val sortCodeExists: Either[String, Boolean] =
                     if (sortCodeIsPresentOnEISCD === "yes") {
@@ -101,28 +100,27 @@ class BarsServiceImpl @Inject()(
                       false
                     }
 
-                  sortCodeExists.map {
+                 sortCodeExists.map{
                     BankDetailsValidationResult(accountNumbersValid, _)
                   }
                 case _ =>
                   logger.warn(
-                    s"error parsing the response from bars check, trackingId = $trackingId,  body = ${httpResponse.body}")
+                    s"error parsing the response from bars check, trackingId = $trackingId,  body = ${response.body}")
                   alerting.alert("error parsing the response json from bars check")
                   Left(s"error parsing the response json from bars check")
               }
             case other: Int =>
               metrics.barsErrorCounter.inc()
               logger.warn(
-                s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${httpResponse.body}")
+                s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${response.body}")
               alerting.alert("unexpected status from bars check")
               Left("unexpected status from bars check")
           }
-        }).left.flatMap(upstreamErrorResponse => {
+        }.left.flatMap { e =>
           metrics.barsErrorCounter.inc()
-          logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${upstreamErrorResponse.getMessage}")
-          alerting.alert("unexpected error from bars check")
-          Left("unexpected error from bars check")
-        })
+            logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${e.getMessage}")
+        alerting.alert("unexpected error from bars check")
+        Left("unexpected error from bars check")}
       }
   }
 }
diff --git a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
index 3d9ebd47..92cc31fa 100644
--- a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
@@ -91,52 +91,48 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .setFlag(nino)
-        .map(r => {
-          r.map(httpResponse => httpResponse.map[Either[String, Unit]] {
-            httpResponse =>
-              val time = timerContext.stop()
+        .map(_.map(_.map[Either[String, Unit]] {
+          response =>
+            val time = timerContext.stop()
 
-              val additionalParams =
-                Seq("DesCorrelationId" -> httpResponse.correlationId, "apiCorrelationId" -> getApiCorrelationId())
+            val additionalParams =
+              Seq("DesCorrelationId" -> response.correlationId, "apiCorrelationId" -> getApiCorrelationId())
 
-              httpResponse.status match {
-                case OK =>
-                  logger.info(
-                    s"DES/ITMP HtS flag setting returned status 200 (OK) (round-trip time: ${nanosToPrettyString(time)})",
-                    nino,
-                    additionalParams: _*)
-                  Right(())
+            response.status match {
+              case OK =>
+                logger.info(
+                  s"DES/ITMP HtS flag setting returned status 200 (OK) (round-trip time: ${nanosToPrettyString(time)})",
+                  nino,
+                  additionalParams: _*)
+                Right(())
 
-                case FORBIDDEN =>
-                  metrics.itmpSetFlagConflictCounter.inc()
-                  logger.warn(
-                    s"Tried to set ITMP HtS flag even though it was already set, received status 403 (Forbidden) " +
-                      s"- proceeding as normal  (round-trip time: ${nanosToPrettyString(time)})",
-                    nino,
-                    additionalParams: _*
-                  )
-                  Right(())
+              case FORBIDDEN =>
+                metrics.itmpSetFlagConflictCounter.inc()
+                logger.warn(
+                  s"Tried to set ITMP HtS flag even though it was already set, received status 403 (Forbidden) " +
+                    s"- proceeding as normal  (round-trip time: ${nanosToPrettyString(time)})",
+                  nino,
+                  additionalParams: _*
+                )
+                Right(())
+
+              case other =>
+                metrics.itmpSetFlagErrorCounter.inc()
+                pagerDutyAlerting.alert("Received unexpected http status in response to setting ITMP flag")
+                Left(
+                  s"Received unexpected response status ($other) when trying to set ITMP flag. Body was: ${
+                    maskNino(
+                      response.body)
+                  } " +
+                    s"(round-trip time: ${nanosToPrettyString(time)})")
+            }
+        }).left.flatMap(upstreamErrorResponse => {
+          val time = timerContext.stop()
+          pagerDutyAlerting.alert("Failed to make call to check eligibility")
+          metrics.itmpEligibilityCheckErrorCounter.inc()
+          Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
 
-                case other =>
-                  metrics.itmpSetFlagErrorCounter.inc()
-                  pagerDutyAlerting.alert("Received unexpected http status in response to setting ITMP flag")
-                  Left(
-                    s"Received unexpected response status ($other) when trying to set ITMP flag. Body was: ${
-                      maskNino(
-                        httpResponse.body)
-                    } " +
-                      s"(round-trip time: ${nanosToPrettyString(time)})")
-              }
-          })
         })
-        .map(error =>
-          error.left.flatMap(upstreamErrorResponse => {
-            val time = timerContext.stop()
-            metrics.itmpSetFlagErrorCounter.inc()
-            pagerDutyAlerting.alert("Failed to make call to set ITMP flag")
-            Left(
-              s"Encountered unexpected error while trying to set the ITMP flag: ${upstreamErrorResponse.getMessage} (round-trip time: ${nanosToPrettyString(time)})")          }
-          )
         )
     })
 
@@ -149,40 +145,37 @@ class HelpToSaveServiceImpl @Inject()(
         (dESConnector.getPersonalDetails(nino), "[DES]")
       }
     val timerContext = metrics.payePersonalDetailsTimer.time()
-    response.map { r =>
-      r.map(httpResponse => {
-        val time = timerContext.stop()
-        val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId
-        httpResponse.status match {
-          case Status.OK =>
-            httpResponse.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
-              case Left(e) =>
-                metrics.payePersonalDetailsErrorCounter.inc()
-                logger.warn(
-                  s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}",
-                  nino, params(httpResponse))
-              case Right(_) =>
-                logger.debug(
-                  s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}",
-                  nino, params(httpResponse))
-            }
-          case other =>
-            logger.warn(
-              s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}",
-              nino, params(httpResponse))
-            metrics.payePersonalDetailsErrorCounter.inc()
-            pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details")
-            Left(s"Received unexpected status $other")
-        }
-      }
-      )}
-      .recover { e =>
-        val time = timerContext.stop()
-        pagerDutyAlerting.alert("Failed to make call to paye-personal-details")
-        metrics.payePersonalDetailsErrorCounter.inc()
-        Left(s"Call to paye-personal-details unsuccessful: ${e.getMessage} (round-trip time: ${timeString(time)})")
-      }
-      .pipe(EitherT(_))
+   response.map(_.map(response => {
+     val time = timerContext.stop()
+     val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId
+     response.status match {
+       case Status.OK =>
+         response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
+           case Left(e) =>
+             metrics.payePersonalDetailsErrorCounter.inc()
+             logger.warn(
+               s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}",
+               nino, params(response))
+           case Right(_) =>
+             logger.debug(
+               s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}",
+               nino, params(response))
+         }
+       case other =>
+         logger.warn(
+           s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}",
+           nino, params(response))
+         metrics.payePersonalDetailsErrorCounter.inc()
+         pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details")
+         Left(s"Received unexpected status $other")
+   }
+   }).left.flatMap(upstreamErrorResponse => {
+     val time = timerContext.stop()
+     pagerDutyAlerting.alert("Failed to make call to paye-personal-details")
+     metrics.payePersonalDetailsErrorCounter.inc()
+     Left(s"Call to paye-personal-details unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
+   }))
+     .pipe(EitherT(_))
   }
 
   private def getEligibility(nino: NINO, ucResponse: Option[UCResponse])(
@@ -193,53 +186,49 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .isEligible(nino, ucResponse)
-        .map(response => {
-          response.map(httpResponse => httpResponse.map[Either[String, EligibilityCheckResult]] {
-            httpResponse =>
-              val time = timerContext.stop()
-
-              val additionalParams = "DesCorrelationId" -> httpResponse.correlationId
-
-              httpResponse.status match {
-                case Status.OK =>
-                  httpResponse
-                    .parseJson[EligibilityCheckResult]
-                    .fold(
-                      { e =>
-                        metrics.itmpEligibilityCheckErrorCounter.inc()
-                        pagerDutyAlerting.alert("Could not parse JSON in eligibility check response")
-                        Left(e)
-                      },
-                      res => {
-                        logger.debug(
-                          s"Call to check eligibility successful, received 200 (OK) ${timeString(time)}",
-                          nino,
-                          additionalParams)
-                        Right(res)
-                      }
-                    )
+        .map(_.map(_.map[Either[String, EligibilityCheckResult]] {
+          response =>
+            val time = timerContext.stop()
 
-                case other =>
-                  logger.warn(
-                    s"Call to check eligibility unsuccessful. Received unexpected status $other ${timeString(time)}. " +
-                      s"Body was: ${httpResponse.body}",
-                    nino,
-                    additionalParams
+            val additionalParams = "DesCorrelationId" -> response.correlationId
+
+            response.status match {
+              case Status.OK =>
+                response
+                  .parseJson[EligibilityCheckResult]
+                  .fold(
+                    { e =>
+                      metrics.itmpEligibilityCheckErrorCounter.inc()
+                      pagerDutyAlerting.alert("Could not parse JSON in eligibility check response")
+                      Left(e)
+                    },
+                    res => {
+                      logger.debug(
+                        s"Call to check eligibility successful, received 200 (OK) ${timeString(time)}",
+                        nino,
+                        additionalParams)
+                      Right(res)
+                    }
                   )
-                  metrics.itmpEligibilityCheckErrorCounter.inc()
-                  pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check")
-                  Left(s"Received unexpected status $other")
-              }
 
-          })
-        }.left.flatMap(upstreamErrorResponse => {
+              case other =>
+                logger.warn(
+                  s"Call to check eligibility unsuccessful. Received unexpected status $other ${timeString(time)}. " +
+                    s"Body was: ${response.body}",
+                  nino,
+                  additionalParams
+                )
+                metrics.itmpEligibilityCheckErrorCounter.inc()
+                pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check")
+                Left(s"Received unexpected status $other")
+            }
+
+        }).left.flatMap(upstreamErrorResponse => {
           val time = timerContext.stop()
           pagerDutyAlerting.alert("Failed to make call to check eligibility")
           metrics.itmpEligibilityCheckErrorCounter.inc()
           Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
-
-        })
-        )
+        }))
     })
 
   private def timeString(nanos: Long): String = s"(round-trip time: ${nanosToPrettyString(nanos)})"

From 6a42f022d9e72e73b4e8caa5756c64f65151a2b2 Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Wed, 6 Nov 2024 12:24:45 +0000
Subject: [PATCH 7/8] [DLS-10533] interim commit (compilation errors resolved)

---
 .../controllers/HelpToSaveController.scala    | 21 +++--
 .../helptosave/services/BarsService.scala     | 45 +++++-----
 .../services/HelpToSaveService.scala          | 90 +++++++++----------
 3 files changed, 79 insertions(+), 77 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
index e0b2642c..8a200c11 100644
--- a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
+++ b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
@@ -34,7 +34,7 @@ import uk.gov.hmrc.helptosave.services.{BarsService, HelpToSaveService, UserCapS
 import uk.gov.hmrc.helptosave.util.JsErrorOps._
 import uk.gov.hmrc.helptosave.util.Logging._
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, toFuture}
-import uk.gov.hmrc.http.HeaderCarrier
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse}
 
 import scala.concurrent.ExecutionContext
 import scala.util.{Failure, Success, Try}
@@ -73,11 +73,17 @@ class HelpToSaveController @Inject()(
                 logger.warn(s"couldn't delete email for DE user due to: $error", nino)
                 toFuture(InternalServerError)
               },
-              _ => createAccount(createAccountRequest, additionalParams)
+              _ => createAccount(createAccountRequest, additionalParams).flatMap {
+                case Right(result)
+                => result
+              }
             )
             .flatMap(identity)
         } else {
-          createAccount(createAccountRequest, additionalParams)
+          createAccount(createAccountRequest, additionalParams).flatMap {
+            case Right(result)
+            => result
+          }
         }
 
       case Some(error: JsError) =>
@@ -94,11 +100,10 @@ class HelpToSaveController @Inject()(
   def updateEmail(): Action[AnyContent] = ggOrPrivilegedAuthorised { implicit request =>
     request.body.asJson.map(_.validate[NSIPayload](NSIPayload.nsiPayloadReads(None))) match {
       case Some(JsSuccess(userInfo, _)) =>
-        proxyConnector.updateEmail(userInfo).map({ response =>
-            response.map(httpResponse => {
-              Option(httpResponse.body).fold[Result](Status(httpResponse.status))(body => Status(httpResponse.status)(body))
-            })
-        })
+        proxyConnector.updateEmail(userInfo) match {
+         case response: HttpResponse => Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body))
+
+        }
 
       case Some(error: JsError) =>
         val errorString = error.prettyPrint()
diff --git a/app/uk/gov/hmrc/helptosave/services/BarsService.scala b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
index 3d440d86..e4f2efc1 100644
--- a/app/uk/gov/hmrc/helptosave/services/BarsService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/BarsService.scala
@@ -28,7 +28,7 @@ import uk.gov.hmrc.helptosave.metrics.Metrics
 import uk.gov.hmrc.helptosave.models.{BARSCheck, BankDetailsValidationRequest, BankDetailsValidationResult}
 import uk.gov.hmrc.helptosave.util.Logging.LoggerOps
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, Logging, PagerDutyAlerting}
-import uk.gov.hmrc.http.HeaderCarrier
+import uk.gov.hmrc.http.{HeaderCarrier, UpstreamErrorResponse}
 
 import java.util.UUID
 import scala.concurrent.{ExecutionContext, Future}
@@ -62,9 +62,8 @@ class BarsServiceImpl @Inject()(
     val nino = barsRequest.nino
     barsConnector
       .validate(barsRequest, trackingId)
-      .map [Either[String, BankDetailsValidationResult]] { response =>
-        response.map {
-          response =>
+      .flatMap [Either[String, BankDetailsValidationResult]]{
+        case Right(response) =>
           val _ = timerContext.stop()
           response.status match {
             case Status.OK =>
@@ -100,27 +99,25 @@ class BarsServiceImpl @Inject()(
                       false
                     }
 
-                 sortCodeExists.map{
-                    BankDetailsValidationResult(accountNumbersValid, _)
+                Future(sortCodeExists.map { BankDetailsValidationResult(accountNumbersValid, _) })
+              case _ =>
+                logger.warn(
+                  s"error parsing the response from bars check, trackingId = $trackingId,  body = ${response.body}")
+                alerting.alert("error parsing the response json from bars check")
+                Future(Left(s"error parsing the response json from bars check"))
                   }
-                case _ =>
-                  logger.warn(
-                    s"error parsing the response from bars check, trackingId = $trackingId,  body = ${response.body}")
-                  alerting.alert("error parsing the response json from bars check")
-                  Left(s"error parsing the response json from bars check")
+          case other: Int =>
+            metrics.barsErrorCounter.inc()
+            logger.warn(
+              s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${response.body}")
+            alerting.alert("unexpected status from bars check")
+            Future(Left("unexpected status from bars check"))
               }
-            case other: Int =>
-              metrics.barsErrorCounter.inc()
-              logger.warn(
-                s"unexpected status from bars check, trackingId = $trackingId, status=$other, body = ${response.body}")
-              alerting.alert("unexpected status from bars check")
-              Left("unexpected status from bars check")
-          }
-        }.left.flatMap { e =>
+        case Left(upstreamErrorResponse: UpstreamErrorResponse) =>
           metrics.barsErrorCounter.inc()
-            logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${e.getMessage}")
-        alerting.alert("unexpected error from bars check")
-        Left("unexpected error from bars check")}
-      }
-  }
+          logger.warn(s"unexpected error from bars check, trackingId = $trackingId, error=${upstreamErrorResponse.getMessage}")
+          alerting.alert("unexpected error from bars check")
+          Future(Left("unexpected error from bars check"))
+          }
+   }
 }
diff --git a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
index 92cc31fa..68a25af0 100644
--- a/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
+++ b/app/uk/gov/hmrc/helptosave/services/HelpToSaveService.scala
@@ -91,8 +91,8 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .setFlag(nino)
-        .map(_.map(_.map[Either[String, Unit]] {
-          response =>
+        .flatMap[Either[String, Unit]] {
+          case Right(response) =>
             val time = timerContext.stop()
 
             val additionalParams =
@@ -125,15 +125,13 @@ class HelpToSaveServiceImpl @Inject()(
                       response.body)
                   } " +
                     s"(round-trip time: ${nanosToPrettyString(time)})")
-            }
-        }).left.flatMap(upstreamErrorResponse => {
-          val time = timerContext.stop()
-          pagerDutyAlerting.alert("Failed to make call to check eligibility")
-          metrics.itmpEligibilityCheckErrorCounter.inc()
-          Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
-
-        })
-        )
+        }
+          case Left(upstreamErrorResponse: UpstreamErrorResponse) =>
+            val time = timerContext.stop()
+            pagerDutyAlerting.alert("Failed to make call to check eligibility")
+            metrics.itmpEligibilityCheckErrorCounter.inc()
+            Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
+        }
     })
 
   def getPersonalDetails(nino: NINO)(implicit hc: HeaderCarrier, ec: ExecutionContext): Result[PayePersonalDetails] = {
@@ -145,36 +143,38 @@ class HelpToSaveServiceImpl @Inject()(
         (dESConnector.getPersonalDetails(nino), "[DES]")
       }
     val timerContext = metrics.payePersonalDetailsTimer.time()
-   response.map(_.map(response => {
-     val time = timerContext.stop()
-     val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId
-     response.status match {
-       case Status.OK =>
-         response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
-           case Left(e) =>
-             metrics.payePersonalDetailsErrorCounter.inc()
-             logger.warn(
-               s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}",
-               nino, params(response))
-           case Right(_) =>
-             logger.debug(
-               s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}",
-               nino, params(response))
-         }
-       case other =>
-         logger.warn(
-           s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}",
-           nino, params(response))
-         metrics.payePersonalDetailsErrorCounter.inc()
-         pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details")
-         Left(s"Received unexpected status $other")
-   }
-   }).left.flatMap(upstreamErrorResponse => {
+   response.flatMap {
+     case Right(response) =>
+       val time = timerContext.stop()
+       val params = (response: HttpResponse) => "CorrelationId" -> response.correlationId
+       response.status match {
+         case Status.OK =>
+           response.parseJsonWithoutLoggingBody[PayePersonalDetails] tap {
+             case Left(e) =>
+               metrics.payePersonalDetailsErrorCounter.inc()
+               logger.warn(
+                 s"$tag Could not parse JSON response from paye-personal-details, received 200 (OK): $e ${timeString(time)}",
+                 nino, params(response))
+             case Right(_) =>
+               logger.debug(
+                 s"$tag Call to check paye-personal-details successful, received 200 (OK) ${timeString(time)}",
+                 nino, params(response))
+           }
+         case other =>
+           logger.warn(
+             s"$tag Call to paye-personal-details unsuccessful. Received unexpected status $other ${timeString(time)}",
+             nino, params(response))
+           metrics.payePersonalDetailsErrorCounter.inc()
+           pagerDutyAlerting.alert("Received unexpected http status in response to paye-personal-details")
+           Left(s"Received unexpected status $other")
+       }
+
+   case Left(upstreamErrorResponse: UpstreamErrorResponse) =>
      val time = timerContext.stop()
      pagerDutyAlerting.alert("Failed to make call to paye-personal-details")
      metrics.payePersonalDetailsErrorCounter.inc()
      Left(s"Call to paye-personal-details unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
-   }))
+   }
      .pipe(EitherT(_))
   }
 
@@ -186,8 +186,8 @@ class HelpToSaveServiceImpl @Inject()(
 
       dESConnector
         .isEligible(nino, ucResponse)
-        .map(_.map(_.map[Either[String, EligibilityCheckResult]] {
-          response =>
+        .flatMap {
+          case Right(response)  =>
             val time = timerContext.stop()
 
             val additionalParams = "DesCorrelationId" -> response.correlationId
@@ -222,13 +222,13 @@ class HelpToSaveServiceImpl @Inject()(
                 pagerDutyAlerting.alert("Received unexpected http status in response to eligibility check")
                 Left(s"Received unexpected status $other")
             }
+          case Left(upstreamErrorResponse: UpstreamErrorResponse) =>
+            val time = timerContext.stop()
+            metrics.itmpSetFlagErrorCounter.inc()
+            pagerDutyAlerting.alert("Failed to make call to set ITMP flag")
+            Left(s"Encountered unexpected error while trying to set the ITMP flag: ${upstreamErrorResponse.getMessage} (round-trip time: ${nanosToPrettyString(time)})")
 
-        }).left.flatMap(upstreamErrorResponse => {
-          val time = timerContext.stop()
-          pagerDutyAlerting.alert("Failed to make call to check eligibility")
-          metrics.itmpEligibilityCheckErrorCounter.inc()
-          Left(s"Call to check eligibility unsuccessful: ${upstreamErrorResponse.getMessage} (round-trip time: ${timeString(time)})")
-        }))
+        }
     })
 
   private def timeString(nanos: Long): String = s"(round-trip time: ${nanosToPrettyString(nanos)})"

From 8ed93bd6cee1653d2ddef80451c9455a0da2e475 Mon Sep 17 00:00:00 2001
From: ruthdas03 <86231805+ruthdas03@users.noreply.github.com>
Date: Thu, 7 Nov 2024 17:16:28 +0000
Subject: [PATCH 8/8] [DLS-10533] interim review fix

---
 .../controllers/HelpToSaveController.scala        | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
index 8a200c11..63247061 100644
--- a/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
+++ b/app/uk/gov/hmrc/helptosave/controllers/HelpToSaveController.scala
@@ -34,9 +34,9 @@ import uk.gov.hmrc.helptosave.services.{BarsService, HelpToSaveService, UserCapS
 import uk.gov.hmrc.helptosave.util.JsErrorOps._
 import uk.gov.hmrc.helptosave.util.Logging._
 import uk.gov.hmrc.helptosave.util.{LogMessageTransformer, toFuture}
-import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse}
+import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, UpstreamErrorResponse}
 
-import scala.concurrent.ExecutionContext
+import scala.concurrent.{ExecutionContext, Future}
 import scala.util.{Failure, Success, Try}
 
 class HelpToSaveController @Inject()(
@@ -100,9 +100,14 @@ class HelpToSaveController @Inject()(
   def updateEmail(): Action[AnyContent] = ggOrPrivilegedAuthorised { implicit request =>
     request.body.asJson.map(_.validate[NSIPayload](NSIPayload.nsiPayloadReads(None))) match {
       case Some(JsSuccess(userInfo, _)) =>
-        proxyConnector.updateEmail(userInfo) match {
-         case response: HttpResponse => Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body))
-
+        proxyConnector.updateEmail(userInfo).flatMap {
+          case Right(response: HttpResponse) =>
+          Option(response.body).fold[Result](Status(response.status))(body => Status(response.status)(body))
+          case Left(upstreamErrorResponse: UpstreamErrorResponse) =>
+            logger.warn(
+              s"Call to updateEmail unsuccessful. Received unexpected status. ")
+
+            Option(upstreamErrorResponse.message).fold[Result](Status(upstreamErrorResponse.statusCode))(body => Status(upstreamErrorResponse.statusCode)(body))
         }
 
       case Some(error: JsError) =>