VEIOSS-379 Build View Submitted Returns Pages for Multiple IOSS Accou…

…nts (#87)

* VEIOSS-379 Build View Submitted Returns Pages for Multiple IOSS Accounts

* Complete return-registration-selection page

* Add /view-returns-multiple-reg

* Fix test

* Remove stub

* Clean

* Skip selection when only one previous registration

* Fix test

* Add id to href links

* Clean

* Clean

* Add index

* Clean

* Add submitted-returns-history id

* Add ReturnRegistrationSelectionControllerSpec

* Add ViewReturnsMultipleRegControllerSpec

* Add SelectedPreviousRegistrationRepositorySpec

* Add ReturnRegistrationSelectionFormProviderSpec

* Rename previousRegistration.submittedReturnsHistory viewReturnsMultipleReg

* Add previousIossNumber to makePayment

* Add onPageLoadForIossNumber and makePaymentForIossNumber

* Remove println in test

* Factor onPageLoad and onPageLoadForIossNumber tests

* Add onPageLoadForIossNumber tests

* Add test

* Add PaymentController tests

* Clean

app/connectors/FinancialDataConnector.scala

@@ -38,28 +38,23 @@ class FinancialDataConnector @Inject()(
 
   private val baseUrl = config.get[Service]("microservice.services.ioss-returns")
 
-  private def financialDataUrl(date: LocalDate) = s"$baseUrl/financial-data/get/$date"
-
-  private val prepareFinancialDataUrl = s"$baseUrl/financial-data/prepare"
-
-  private def chargeUrl(period: Period) = s"$baseUrl/financial-data/charge/$period"
-
   def getFinancialData(date: LocalDate)(implicit hc: HeaderCarrier): Future[FinancialData] = {
-    val url = financialDataUrl(date)
-    http.GET[FinancialData](
-      url
-    )
+    val url = s"$baseUrl/financial-data/get/$date"
+    http.GET[FinancialData](url)
   }
 
   def prepareFinancialData()(implicit hc: HeaderCarrier): Future[PrepareDataResponse] = {
-    val url = prepareFinancialDataUrl
-    http.GET[PrepareDataResponse](
-      url
-    )
+    val url = s"$baseUrl/financial-data/prepare"
+    http.GET[PrepareDataResponse](url)
   }
 
   def getCharge(period: Period)(implicit hc: HeaderCarrier): Future[ChargeResponse] = {
-    val url = chargeUrl(period)
+    val url = s"$baseUrl/financial-data/charge/$period"
+    http.GET[ChargeResponse](url)
+  }
+
+  def getChargeForIossNumber(period: Period, iossNumber: String)(implicit hc: HeaderCarrier): Future[ChargeResponse] = {
+    val url = s"$baseUrl/financial-data/charge/$period/$iossNumber"
     http.GET[ChargeResponse](url)
   }
 

app/connectors/VatReturnConnector.scala

@@ -45,4 +45,8 @@ class VatReturnConnector @Inject()(config: Configuration, httpClient: HttpClient
   def get(period: Period)(implicit hc: HeaderCarrier): Future[EtmpVatReturnResponse] = {
     httpClient.GET[EtmpVatReturnResponse](url = s"$baseUrl/return/$period")
   }
+
+  def getForIossNumber(period: Period, iossNumber: String)(implicit hc: HeaderCarrier): Future[EtmpVatReturnResponse] = {
+    httpClient.GET[EtmpVatReturnResponse](url = s"$baseUrl/return/$period/$iossNumber")
+  }
 }

app/controllers/actions/IdentifierAction.scala

@@ -21,11 +21,11 @@ import config.FrontendAppConfig
 import controllers.routes
 import logging.Logging
 import models.requests.IdentifierRequest
-import play.api.mvc._
 import play.api.mvc.Results._
+import play.api.mvc._
 import services.AccountService
-import uk.gov.hmrc.auth.core._
 import uk.gov.hmrc.auth.core.AffinityGroup.{Individual, Organisation}
+import uk.gov.hmrc.auth.core._
 import uk.gov.hmrc.auth.core.retrieve._
 import uk.gov.hmrc.auth.core.retrieve.v2.Retrievals
 import uk.gov.hmrc.domain.Vrn
@@ -99,10 +99,9 @@ class IdentifierAction @Inject()(
                                         credentials: Credentials,
                                         vrn: Vrn,
                                         iossNumber: String
-                                      )(implicit hc: HeaderCarrier): Either[Result, IdentifierRequest[A]] = {
+                                      ): Either[Result, IdentifierRequest[A]] = {
     val identifierRequest = IdentifierRequest(request, credentials, vrn, iossNumber)
     Right(identifierRequest)
-
   }
 
   private def checkConfidenceAndGetResponse[A](
@@ -111,7 +110,7 @@ class IdentifierAction @Inject()(
                                                 vrn: Vrn,
                                                 iossNumber: String,
                                                 confidence: ConfidenceLevel
-                                              )(implicit hc: HeaderCarrier): Either[Result, IdentifierRequest[A]] = {
+                                              ): Either[Result, IdentifierRequest[A]] = {
     if (confidence >= ConfidenceLevel.L200) {
       getSuccessfulResponse(request, credentials, vrn, iossNumber)
     } else {

app/controllers/payments/PaymentController.scala

@@ -19,32 +19,52 @@ package controllers.payments
 import config.Service
 import controllers.actions._
 import models.Period
-import pages.Waypoints
+import pages.{JourneyRecoveryPage, Waypoints}
 import play.api.Configuration
 import play.api.i18n.I18nSupport
-import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
-import services.PaymentsService
+import play.api.mvc.{Action, AnyContent, MessagesControllerComponents, Result}
+import services.{PaymentsService, PreviousRegistrationService}
+import uk.gov.hmrc.http.HeaderCarrier
 import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
 
 import javax.inject.Inject
-import scala.concurrent.ExecutionContext
+import scala.concurrent.{ExecutionContext, Future}
 
 class PaymentController @Inject()(
                                    cc: AuthenticatedControllerComponents,
                                    config: Configuration,
                                    paymentsService: PaymentsService,
+                                   previousRegistrationService: PreviousRegistrationService
                                  )(implicit ec: ExecutionContext) extends FrontendBaseController with I18nSupport {
 
   protected val controllerComponents: MessagesControllerComponents = cc
   private val paymentsBaseUrl: Service = config.get[Service]("microservice.services.pay-api")
 
-  def makePayment(waypoints: Waypoints, period: Period, amountInPence: Long): Action[AnyContent] = cc.authAndGetOptionalData().async {
-    implicit request =>
-      val amountOwed = BigDecimal(amountInPence) / 100
+  def makePayment(waypoints: Waypoints, period: Period, amountInPence: Long): Action[AnyContent] = {
+    cc.authAndGetOptionalData().async { implicit request =>
+      makePayment(period, amountInPence, request.iossNumber)
+    }
+  }
 
-      paymentsService.makePayment(request.iossNumber, period, amountOwed).map {
-        case Right(value) => Redirect(value.nextUrl)
-        case _ => Redirect(s"$paymentsBaseUrl/pay/service-unavailable")
+  def makePaymentForIossNumber(waypoints: Waypoints, period: Period, amountInPence: Long, iossNumber: String): Action[AnyContent] = {
+    cc.authAndGetOptionalData().async { implicit request =>
+      previousRegistrationService.getPreviousRegistrations().flatMap { previousRegistrations =>
+        val validIossNumbers: Seq[String] = request.iossNumber :: previousRegistrations.map(_.iossNumber)
+        if (validIossNumbers.contains(iossNumber)) {
+          makePayment(period, amountInPence, iossNumber)
+        } else {
+          Future.successful(Redirect(JourneyRecoveryPage.route(waypoints)))
+        }
       }
+    }
+  }
+
+  private def makePayment(period: Period, amountInPence: Long, iossNumber: String)(implicit hc: HeaderCarrier): Future[Result] = {
+    val amountOwed = BigDecimal(amountInPence) / 100
+
+    paymentsService.makePayment(iossNumber, period, amountOwed).map {
+      case Right(value) => Redirect(value.nextUrl)
+      case _ => Redirect(s"$paymentsBaseUrl/pay/service-unavailable")
+    }
   }
 }

app/controllers/previousReturns/ReturnRegistrationSelectionController.scala

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2024 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 controllers.previousReturns
+
+import controllers.actions._
+import forms.ReturnRegistrationSelectionFormProvider
+import logging.Logging
+import pages.Waypoints
+import play.api.data.Form
+import play.api.i18n.{I18nSupport, MessagesApi}
+import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
+import repositories.SelectedPreviousRegistrationRepository
+import services.PreviousRegistrationService
+import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
+import viewmodels.previousReturns.{PreviousRegistration, SelectedPreviousRegistration}
+import views.html.previousReturns.ReturnRegistrationSelectionView
+
+import javax.inject.Inject
+import scala.concurrent.{ExecutionContext, Future}
+
+class ReturnRegistrationSelectionController @Inject()(
+                                                       override val messagesApi: MessagesApi,
+                                                       cc: AuthenticatedControllerComponents,
+                                                       formProvider: ReturnRegistrationSelectionFormProvider,
+                                                       view: ReturnRegistrationSelectionView,
+                                                       previousRegistrationService: PreviousRegistrationService,
+                                                       selectedPreviousRegistrationRepository: SelectedPreviousRegistrationRepository
+                                                     )(implicit ec: ExecutionContext) extends FrontendBaseController with I18nSupport with Logging {
+
+  protected val controllerComponents: MessagesControllerComponents = cc
+
+  def onPageLoad(waypoints: Waypoints): Action[AnyContent] = cc.authAndGetOptionalData().async {
+    implicit request =>
+      for {
+        previousRegistrations <- previousRegistrationService.getPreviousRegistrations()
+        selectedPreviousRegistration <- selectedPreviousRegistrationRepository.get(request.userId)
+      } yield {
+        val form: Form[PreviousRegistration] = formProvider(previousRegistrations)
+
+        val preparedForm = selectedPreviousRegistration match {
+          case None => form
+          case Some(value) => form.fill(value.previousRegistration)
+        }
+
+        previousRegistrations match {
+          case Nil => Redirect(controllers.routes.JourneyRecoveryController.onPageLoad())
+          case _ :: Nil => Redirect(controllers.previousReturns.routes.ViewReturnsMultipleRegController.onPageLoad())
+          case _ => Ok(view(waypoints, preparedForm, previousRegistrations))
+        }
+      }
+  }
+
+  def onSubmit(waypoints: Waypoints): Action[AnyContent] = cc.authAndGetOptionalData().async {
+    implicit request =>
+      previousRegistrationService.getPreviousRegistrations().flatMap { previousRegistrations =>
+        val form: Form[PreviousRegistration] = formProvider(previousRegistrations)
+
+        form.bindFromRequest().fold(
+          formWithErrors => Future.successful(BadRequest(view(waypoints, formWithErrors, previousRegistrations))),
+          value =>
+            selectedPreviousRegistrationRepository.set(SelectedPreviousRegistration(request.userId, value)).map { _ =>
+              Redirect(controllers.previousReturns.routes.ViewReturnsMultipleRegController.onPageLoad())
+            }
+        )
+      }
+  }
+}

app/controllers/previousReturns/SubmittedReturnForPeriodController.scala

@@ -16,14 +16,18 @@
 
 package controllers.previousReturns
 
+import connectors.FinancialDataHttpParser.ChargeResponse
+import connectors.VatReturnHttpParser.EtmpVatReturnResponse
 import connectors.{FinancialDataConnector, VatReturnConnector}
 import controllers.actions._
 import logging.Logging
 import models.Period
 import models.etmp.{EtmpExclusion, EtmpExclusionReason, EtmpVatReturn}
+import models.requests.OptionalDataRequest
 import pages.{JourneyRecoveryPage, Waypoints}
 import play.api.i18n.{I18nSupport, Messages, MessagesApi}
-import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
+import play.api.mvc.{Action, AnyContent, MessagesControllerComponents, Result}
+import services.PreviousRegistrationService
 import uk.gov.hmrc.govukfrontend.views.viewmodels.content.HtmlContent
 import uk.gov.hmrc.govukfrontend.views.viewmodels.summarylist.{Card, CardTitle, SummaryList, SummaryListRow}
 import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
@@ -33,26 +37,45 @@ import views.html.previousReturns.SubmittedReturnForPeriodView
 
 import java.time.{Clock, LocalDate}
 import javax.inject.Inject
-import scala.concurrent.ExecutionContext
+import scala.concurrent.{ExecutionContext, Future}
 
 class SubmittedReturnForPeriodController @Inject()(
                                                     override val messagesApi: MessagesApi,
                                                     clock: Clock,
                                                     cc: AuthenticatedControllerComponents,
                                                     vatReturnConnector: VatReturnConnector,
                                                     financialDataConnector: FinancialDataConnector,
+                                                    previousRegistrationService: PreviousRegistrationService,
                                                     view: SubmittedReturnForPeriodView
                                                   )(implicit ec: ExecutionContext) extends FrontendBaseController with I18nSupport with Logging {
 
   protected val controllerComponents: MessagesControllerComponents = cc
 
-  def onPageLoad(waypoints: Waypoints, period: Period): Action[AnyContent] = cc.authAndGetOptionalData().async {
-    implicit request => {
-      for {
-        etmpVatReturnResult <- vatReturnConnector.get(period)
-        getChargeResult <- financialDataConnector.getCharge(period)
-      } yield (etmpVatReturnResult, getChargeResult)
-    }.map {
+  def onPageLoad(waypoints: Waypoints, period: Period): Action[AnyContent] = cc.authAndGetOptionalData().async { implicit request =>
+    for {
+      etmpVatReturnResponse <- vatReturnConnector.get(period)
+      chargeResponse <- financialDataConnector.getCharge(period)
+    } yield onPageLoad(waypoints, period, etmpVatReturnResponse, chargeResponse)
+  }
+
+  def onPageLoadForIossNumber(waypoints: Waypoints, period: Period, iossNumber: String): Action[AnyContent] = cc.authAndGetOptionalData().async {
+    implicit request =>
+      previousRegistrationService.getPreviousRegistrations().flatMap { previousRegistrations =>
+        val validIossNumbers: Seq[String] = request.iossNumber :: previousRegistrations.map(_.iossNumber)
+        if (validIossNumbers.contains(iossNumber)) {
+          for {
+            etmpVatReturnResponse <- vatReturnConnector.getForIossNumber(period, iossNumber)
+            chargeResponse <- financialDataConnector.getChargeForIossNumber(period, iossNumber)
+          } yield onPageLoad(waypoints, period, etmpVatReturnResponse, chargeResponse)
+        } else {
+          Future.successful(Redirect(JourneyRecoveryPage.route(waypoints)))
+        }
+      }
+  }
+
+  private def onPageLoad(waypoints: Waypoints, period: Period, etmpVatReturnResponse: EtmpVatReturnResponse, chargeResponse: ChargeResponse)
+                        (implicit request: OptionalDataRequest[AnyContent]): Result = {
+    (etmpVatReturnResponse, chargeResponse) match {
       case (Right(etmpVatReturn), chargeResponse) =>
         val maybeCharge = chargeResponse.fold(_ => None, charge => charge)
 

app/controllers/previousReturns/SubmittedReturnsHistoryController.scala

@@ -16,28 +16,23 @@
 
 package controllers.previousReturns
 
-import connectors.VatReturnConnector
 import controllers.actions._
 import logging.Logging
-import models.Period
-import models.payments.{Payment, PaymentStatus}
 import pages.Waypoints
 import play.api.i18n.{I18nSupport, MessagesApi}
 import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
-import services.{ObligationsService, PaymentsService}
-import uk.gov.hmrc.http.HeaderCarrier
+import services.{PeriodWithFinancialDataService, PreviousRegistrationService}
 import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
 import views.html.previousReturns.SubmittedReturnsHistoryView
 
 import javax.inject.Inject
-import scala.concurrent.{ExecutionContext, Future}
+import scala.concurrent.ExecutionContext
 
 class SubmittedReturnsHistoryController @Inject()(
                                                    override val messagesApi: MessagesApi,
                                                    cc: AuthenticatedControllerComponents,
-                                                   paymentsService: PaymentsService,
-                                                   obligationsService: ObligationsService,
-                                                   vatReturnConnector: VatReturnConnector,
+                                                   periodWithFinancialDataService: PeriodWithFinancialDataService,
+                                                   previousRegistrationService: PreviousRegistrationService,
                                                    view: SubmittedReturnsHistoryView
                                                  )(implicit ec: ExecutionContext) extends FrontendBaseController with I18nSupport with Logging {
 
@@ -47,54 +42,8 @@ class SubmittedReturnsHistoryController @Inject()(
     implicit request =>
 
       for {
-        obligations <- obligationsService.getFulfilledObligations(request.iossNumber)
-        preparedFinancialData <- paymentsService.prepareFinancialData()
-        periods = obligations.map(_.periodKey).map(Period.fromKey)
-        allUnpaidPayments = preparedFinancialData.duePayments ++ preparedFinancialData.overduePayments ++ preparedFinancialData.excludedPayments
-        periodWithFinancialData <- getPeriodWithFinancialData(periods, allUnpaidPayments)
-      } yield {
-
-        Ok(view(waypoints, periodWithFinancialData))
-      }
-  }
-
-  private def getPeriodWithFinancialData(periods: Seq[Period], allUnpaidPayments: List[Payment])
-                                        (implicit hc: HeaderCarrier): Future[Map[Int, Seq[(Period, Payment)]]] = {
-    val futurePeriods = Future(periods)
-
-    for {
-      periods <- futurePeriods
-      allPaymentsForPeriod <- getAllPaymentsForPeriods(periods, allUnpaidPayments)
-    } yield allPaymentsForPeriod.flatten.groupBy(_._1.year)
-  }
-
-  private def getAllPaymentsForPeriods(periods: Seq[Period], allUnpaidPayments: List[Payment])
-                                      (implicit hc: HeaderCarrier): Future[Seq[Map[Period, Payment]]] = {
-
-    Future.sequence(periods.map { period =>
-      allUnpaidPayments.find(_.period == period) match {
-        case Some(payment) =>
-          Future(Map(period -> payment))
-        case _ =>
-          vatReturnConnector.get(period).map {
-            case Right(vatReturn) =>
-              val paymentStatus = if (vatReturn.correctionPreviousVATReturn.isEmpty && vatReturn.goodsSupplied.isEmpty) {
-                PaymentStatus.NilReturn
-              } else {
-                PaymentStatus.Paid
-              }
-              Map(period -> Payment(
-                period = period,
-                amountOwed = 0,
-                dateDue = period.paymentDeadline,
-                paymentStatus = paymentStatus
-              ))
-            case Left(error) =>
-              val exception = new IllegalStateException(s"Unable to get vat return for calculating amount owed ${error.body}")
-              logger.error(exception.getMessage, exception)
-              throw exception
-          }
-      }
-    })
+        periodWithFinancialData <- periodWithFinancialDataService.getPeriodWithFinancialData(request.iossNumber)
+        previousRegistrations <- previousRegistrationService.getPreviousRegistrations()
+      } yield Ok(view(waypoints, periodWithFinancialData, previousRegistrations))
   }
 }