Skip to content

Commit 3c77112

Browse files
PIL-2855: Display Multiple Accounting Periods on Manage Group Details (#648)
Migrates the frontend to use the new **Read Subscription V2** endpoint (which returns an array of accounting periods) behind the `amendMultipleAccountingPeriods` feature flag, while retaining full backward compatibility with the existing V1 flow. ETMP are introducing a breaking change to the Display Subscription API. Replacing a singular accounting period with an array of accounting periods. This PR introduces a V2 path on the frontend that consumes the new response shape, converts it to the existing `SubscriptionLocalData` cache model, and allows both V1 and V2 data to coexist without invalidating any user's cache. This also adds the first page of the new **amend multiple accounting periods** journey -the Manage Group Details summary page now renders a multi-period view with individual period cards when the feature flag is enabled. Additionally, this work exposed a pre-existing bug in the BTN flow where the confirmation page and submission payload were using the accounting period from the subscription data rather than the period the user actually selected during the journey. This has been fixed so the entire BTN flow now consistently uses the user's chosen period. ## What changed ### Models - **`SubscriptionDataV2`** — New model representing the V2 API response with `Seq[DisplayAccountingPeriod]` (each period includes `canAmendStartDate`/`canAmendEndDate` flags). - **`DisplayAccountingPeriod`** — New model for individual accounting periods in the V2 response, with a `toAccountingPeriod` conversion helper. - **`SubscriptionLocalData`** — Two new optional fields added with defaults for backward compatibility: - `accountingPeriods: Option[Seq[DisplayAccountingPeriod]]` — populated from V2 data. - `registrationDate: Option[LocalDate]` — populated from V2 `upeDetails`. This was added so the homepage can display the registration date through the V2 path without requiring a separate homepage-specific model or a second API call (in the V1 flow, this value comes directly from `SubscriptionData.upeDetails.registrationDate`). - `subAccountingPeriod` changed from `AccountingPeriod` to `Option[AccountingPeriod] = None` — V2 data no longer synthesises a fake singular period; V1 cached data continues to populate it as `Some`. ### Connector - **`readSubscriptionV2`** — New method on `SubscriptionConnector` calling the backend V2 endpoint. Handles `404` as `NoResultFound` (consistent with V1 pattern), `422` as `UnprocessableEntityError`, and `5xx` with the existing retryable gateway error logic. ### Service - **`readSubscriptionV2AndSave`** — New method on `SubscriptionService` that calls `readSubscriptionV2`, converts the response to `SubscriptionLocalData` (populating `accountingPeriods`, `registrationDate`, and leaving `subAccountingPeriod = None`), and saves to the user-cache. - **`subscriptionDataV2ToLocalData`** — Private conversion from `SubscriptionDataV2` to `SubscriptionLocalData`. - **`amendGroupOrContactDetails`** — Updated to handle optional `subAccountingPeriod`, falling back to `currentData.accountingPeriod` from the V1 `SubscriptionData` when `None`. ### Homepage - **`HomepageController.onPageLoad`** — When `amendMultipleAccountingPeriods` is enabled, calls `readSubscriptionV2AndSave` and renders the homepage from `SubscriptionLocalData` fields. When disabled, retains the original V1 `maybeReadSubscription` + `cacheSubscription` flow. ### Manage Group Details - **`ManageGroupDetailsCheckYourAnswersController`** — When the flag is enabled, fetches V2 data and renders a multi-period summary view with individual period cards showing amendable date ranges. - **`ManageGroupDetailsMultiPeriodView`** — New view template for rendering multiple accounting periods with Change links. ### BTN Flow Decoupled the BTN flow from `subAccountingPeriod` entirely — all BTN controllers now source the accounting period from the user's chosen period (`BTNChooseAccountingPeriodPage` in user answers): - **`BTNConfirmationController`** — Confirmation page dates come from the chosen period, not subscription data. - **`CheckYourAnswersController`** — Both `onPageLoad` (summary display) and `onSubmit` (BTN payload) use the chosen period. Falls back to `subAccountingPeriod` only on the CYA summary as a last resort, with `JourneyRecovery` redirect if neither is available. - **`BTNAccountingPeriodController`** — Audit event uses the period from `selectAccountingPeriod` context directly. - **`BTNStatusAction`** — Reads `BTNChooseAccountingPeriodPage` from user answers for the already-submitted audit. ## Feature flag All V2 behaviour is gated behind `features.amendMultipleAccountingPeriods` (defaults to `false`). When `false`, the frontend behaves identically to before this PR. --------- Co-authored-by: JamesMMiller <7511223+JamesMMiller@users.noreply.github.com>
1 parent bacd31d commit 3c77112

29 files changed

+1802
-188
lines changed

app/connectors/SubscriptionConnector.scala

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import models.subscription.*
2323
import org.apache.pekko.Done
2424
import play.api.Logging
2525
import play.api.http.Status.*
26-
import play.api.libs.json.{JsValue, Json}
26+
import play.api.libs.json.*
2727
import play.api.libs.ws.JsonBodyWritables.writeableOf_JsValue
2828
import uk.gov.hmrc.http.*
2929
import uk.gov.hmrc.http.HttpErrorFunctions.is2xx
@@ -97,6 +97,29 @@ class SubscriptionConnector @Inject() (val config: FrontendAppConfig, val http:
9797
}
9898
}
9999

100+
/** Read Subscription V2: returns multiple accounting periods with canAmend flags. */
101+
def readSubscriptionV2(
102+
userId: String,
103+
plrReference: String
104+
)(using hc: HeaderCarrier, ec: ExecutionContext): Future[SubscriptionDataV2] = {
105+
val url = s"${config.pillar2BaseUrl}/report-pillar2-top-up-taxes/subscription/v2/read-subscription/$userId/$plrReference"
106+
http
107+
.get(url"$url")
108+
.execute[HttpResponse]
109+
.flatMap {
110+
case response if response.status == OK =>
111+
Future.successful(Json.parse(response.body).as[SubscriptionDataV2])
112+
case response if response.status == UNPROCESSABLE_ENTITY =>
113+
Future.failed(UnprocessableEntityError)
114+
case notFoundResponse if notFoundResponse.status == NOT_FOUND =>
115+
Future.failed(NoResultFound)
116+
case e =>
117+
logger.warn(s"Connection issue when calling read subscription v2 with status: ${e.status}")
118+
if RetryableGatewayError.retryableStatuses(e.status) then Future.failed(RetryableGatewayError)
119+
else Future.failed(InternalIssueError)
120+
}
121+
}
122+
100123
def getSubscriptionCache(
101124
userId: String
102125
)(using hc: HeaderCarrier, ec: ExecutionContext): Future[Option[SubscriptionLocalData]] =
@@ -105,7 +128,12 @@ class SubscriptionConnector @Inject() (val config: FrontendAppConfig, val http:
105128
.execute[HttpResponse]
106129
.map {
107130
case response if response.status == 200 =>
108-
Some(Json.parse(response.body).as[SubscriptionLocalData])
131+
Json.parse(response.body).validate[SubscriptionLocalData] match {
132+
case JsSuccess(data, _) => Some(data)
133+
case JsError(errors) =>
134+
logger.warn(s"Read subscription cache parse error for user $userId: $errors")
135+
None
136+
}
109137
case e =>
110138
logger.warn(s"Connection issue when calling read subscription with status: ${e.status} ${e.body}")
111139
None

app/controllers/HomepageController.scala

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import cats.data.OptionT
2020
import config.FrontendAppConfig
2121
import connectors.UserAnswersConnectors
2222
import controllers.actions.{DataRetrievalAction, IdentifierAction}
23+
import models.*
2324
import models.DueAndOverdueReturnBannerScenario
2425
import models.DueAndOverdueReturnBannerScenario.*
2526
import models.UnprocessableEntityError
@@ -28,9 +29,8 @@ import models.financialdata.PaymentState.*
2829
import models.financialdata.{AccountActivityData, FinancialData, PaymentState}
2930
import models.obligationsandsubmissions.*
3031
import models.requests.OptionalDataRequest
32+
import models.subscription.*
3133
import models.subscription.AccountStatus.{ActiveAccount, InactiveAccount}
32-
import models.subscription.{AccountStatus, ReadSubscriptionRequestParameters, SubscriptionData}
33-
import models.{BtnBanner, DynamicNotificationAreaState, RetryableGatewayError}
3434
import pages.*
3535
import play.api.Logging
3636
import play.api.i18n.I18nSupport
@@ -90,19 +90,46 @@ class HomepageController @Inject() (
9090
result <-
9191
OptionT.liftF {
9292
def attemptHomepageLoad(attempt: Int): Future[Result] = {
93-
val homepageFuture = subscriptionService
94-
.maybeReadSubscription(referenceNumber)
95-
.flatMap {
96-
case Some(_) =>
97-
subscriptionService
98-
.cacheSubscription(ReadSubscriptionRequestParameters(request.userId, referenceNumber))
99-
.flatMap(displayHomepage(_, referenceNumber))
100-
case None =>
101-
Future.successful(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad()))
102-
}
103-
.recover { case UnprocessableEntityError =>
104-
Redirect(controllers.routes.RegistrationInProgressController.onPageLoad(referenceNumber))
105-
}
93+
val homepageFuture =
94+
if appConfig.amendMultipleAccountingPeriods then
95+
subscriptionService
96+
.readSubscriptionV2AndSave(request.userId, referenceNumber)
97+
.flatMap { localData =>
98+
renderHomepage(
99+
organisationName = localData.organisationName.getOrElse(""),
100+
registrationDate = localData.registrationDate.getOrElse(LocalDate.now()),
101+
accountStatus = localData.accountStatus.getOrElse(ActiveAccount),
102+
plrReference = referenceNumber
103+
)
104+
}
105+
.recover {
106+
case NoResultFound =>
107+
Redirect(controllers.routes.JourneyRecoveryController.onPageLoad())
108+
case UnprocessableEntityError =>
109+
Redirect(controllers.routes.RegistrationInProgressController.onPageLoad(referenceNumber))
110+
}
111+
else
112+
subscriptionService
113+
.maybeReadSubscription(referenceNumber)
114+
.flatMap {
115+
case Some(_) =>
116+
subscriptionService
117+
.cacheSubscription(ReadSubscriptionRequestParameters(request.userId, referenceNumber))
118+
.flatMap { subData =>
119+
renderHomepage(
120+
organisationName = subData.upeDetails.organisationName,
121+
registrationDate = subData.upeDetails.registrationDate,
122+
accountStatus = subData.accountStatus.getOrElse(ActiveAccount),
123+
plrReference = referenceNumber
124+
)
125+
}
126+
case None =>
127+
Future.successful(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad()))
128+
}
129+
.recover { case UnprocessableEntityError =>
130+
Redirect(controllers.routes.RegistrationInProgressController.onPageLoad(referenceNumber))
131+
}
132+
106133
homepageFuture.recoverWith {
107134
case RetryableGatewayError if attempt + 1 < appConfig.homepageRetryMaxAttempts =>
108135
logger.warn(
@@ -123,11 +150,15 @@ class HomepageController @Inject() (
123150
} yield result).getOrElse(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad()))
124151
}
125152

126-
private def displayHomepage(subscriptionData: SubscriptionData, plrReference: String)(using
153+
private def renderHomepage(
154+
organisationName: String,
155+
registrationDate: LocalDate,
156+
accountStatus: AccountStatus,
157+
plrReference: String
158+
)(using
127159
request: OptionalDataRequest[?],
128160
hc: HeaderCarrier
129-
): Future[Result] = {
130-
val accountStatus = subscriptionData.accountStatus.getOrElse(ActiveAccount)
161+
): Future[Result] =
131162
sessionRepository.get(request.userId).flatMap { maybeUserAnswers =>
132163
maybeUserAnswers.getOrElse(UserAnswers(request.userId))
133164
for {
@@ -155,8 +186,8 @@ class HomepageController @Inject() (
155186
val btnBanner = determineBtnBanner(accountStatus, notificationArea)
156187
Ok(
157188
homepageView(
158-
subscriptionData.upeDetails.organisationName,
159-
subscriptionData.upeDetails.registrationDate.toDateFormat,
189+
organisationName,
190+
registrationDate.toDateFormat,
160191
btnBanner,
161192
returnsStatus,
162193
paymentsStatus,
@@ -168,7 +199,6 @@ class HomepageController @Inject() (
168199
)
169200
}
170201
}
171-
}
172202

173203
def getDueOrOverdueReturnsStatus(obligationsAndSubmissions: ObligationsAndSubmissionsSuccess): Option[DueAndOverdueReturnBannerScenario] = {
174204

app/controllers/actions/BTNStatusAction.scala

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import controllers.btn.routes.*
2020
import models.btn.BTNStatus
2121
import models.longrunningsubmissions.LongRunningSubmission.BTN
2222
import models.requests.SubscriptionDataRequest
23-
import models.subscription.SubscriptionLocalData
24-
import pages.EntitiesInsideOutsideUKPage
23+
import models.subscription.{AccountingPeriod, SubscriptionLocalData}
24+
import pages.{BTNChooseAccountingPeriodPage, EntitiesInsideOutsideUKPage}
2525
import play.api.Logging
2626
import play.api.mvc.Results.Redirect
2727
import play.api.mvc.{ActionRefiner, RequestHeader, Result}
@@ -55,19 +55,25 @@ class BTNStatusAction @Inject() (
5555
.map { maybeAnswers =>
5656
(
5757
maybeAnswers.flatMap(_.get(BTNStatus)),
58-
maybeAnswers.flatMap(_.get(EntitiesInsideOutsideUKPage))
58+
maybeAnswers.flatMap(_.get(EntitiesInsideOutsideUKPage)),
59+
maybeAnswers.flatMap(_.get(BTNChooseAccountingPeriodPage))
5960
)
6061
}
61-
.flatMap { case (btnStatus, entitiesInsideOutsideUk) =>
62+
.flatMap { case (btnStatus, entitiesInsideOutsideUk, chosenPeriod) =>
6263
btnStatus match {
6364
case Some(BTNStatus.submitted) =>
64-
auditService
65-
.auditBtnAlreadySubmitted(
66-
subscriptionData.plrReference,
67-
subscriptionData.subAccountingPeriod,
68-
entitiesInsideOutsideUk.getOrElse(false)
69-
)
70-
.map(_ => Left(Redirect(CheckYourAnswersController.cannotReturnKnockback)))
65+
chosenPeriod match {
66+
case Some(period) =>
67+
auditService
68+
.auditBtnAlreadySubmitted(
69+
subscriptionData.plrReference,
70+
AccountingPeriod(period.startDate, period.endDate),
71+
entitiesInsideOutsideUk.getOrElse(false)
72+
)
73+
.map(_ => Left(Redirect(CheckYourAnswersController.cannotReturnKnockback)))
74+
case None =>
75+
Future.successful(Left(Redirect(CheckYourAnswersController.cannotReturnKnockback)))
76+
}
7177
case Some(BTNStatus.processing) => Future.successful(Left(Redirect(controllers.routes.WaitingRoomController.onPageLoad(BTN))))
7278
case _ => Future.successful(Right(request))
7379
}

app/controllers/btn/BTNAccountingPeriodController.scala

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import cats.syntax.functor.*
2020
import config.FrontendAppConfig
2121
import controllers.actions.*
2222
import controllers.filteredAccountingPeriodDetails
23+
import models.obligationsandsubmissions.AccountingPeriodDetails
2324
import models.obligationsandsubmissions.SubmissionType.{BTN, UKTR_AMEND, UKTR_CREATE}
2425
import models.requests.ObligationsAndSubmissionsSuccessDataRequest
25-
import models.{MneOrDomestic, Mode}
26-
import pages.{EntitiesInsideOutsideUKPage, SubMneOrDomesticPage}
26+
import models.subscription.AccountingPeriod
27+
import models.{MneOrDomestic, Mode, UserAnswers}
28+
import pages.{BTNChooseAccountingPeriodPage, EntitiesInsideOutsideUKPage, SubMneOrDomesticPage}
2729
import play.api.Logging
2830
import play.api.i18n.I18nSupport
2931
import play.api.mvc.*
@@ -35,6 +37,7 @@ import views.html.btn.{BTNAccountingPeriodView, BTNAlreadyInPlaceView, BTNReturn
3537

3638
import javax.inject.{Inject, Named}
3739
import scala.concurrent.{ExecutionContext, Future}
40+
import scala.util.{Failure, Success}
3841

3942
class BTNAccountingPeriodController @Inject() (
4043
val controllerComponents: MessagesControllerComponents,
@@ -60,36 +63,10 @@ class BTNAccountingPeriodController @Inject() (
6063
given ObligationsAndSubmissionsSuccessDataRequest[AnyContent] = request
6164
sessionRepository.get(request.userId).flatMap {
6265
case Some(userAnswers) =>
63-
Future
64-
.successful(btnAccountingPeriodService.selectAccountingPeriod(userAnswers, filteredAccountingPeriodDetails))
65-
.flatMap { period =>
66-
btnAccountingPeriodService
67-
.outcome(userAnswers, period, filteredAccountingPeriodDetails, btnTypes = Set(BTN), uktrTypes = Set(UKTR_CREATE, UKTR_AMEND))
68-
.match {
69-
case BTNAccountingPeriodService.Outcome.BtnAlreadySubmitted =>
70-
auditService
71-
.auditBtnAlreadySubmitted(
72-
request.subscriptionLocalData.plrReference,
73-
request.subscriptionLocalData.subAccountingPeriod,
74-
entitiesInsideOutsideUk = userAnswers.get(EntitiesInsideOutsideUKPage).getOrElse(false)
75-
)
76-
.as(Ok(btnAlreadyInPlaceView()))
77-
case BTNAccountingPeriodService.Outcome.UktrReturnAlreadySubmitted =>
78-
Future.successful(Ok(viewReturnSubmitted(request.isAgent, period)))
79-
case BTNAccountingPeriodService.Outcome.ShowAccountingPeriod(summaryList, hasMultipleAccountingPeriods, currentAP) =>
80-
Future.successful(
81-
Ok(
82-
accountingPeriodView(
83-
summaryList,
84-
mode,
85-
request.isAgent,
86-
request.subscriptionLocalData.organisationName,
87-
hasMultipleAccountingPeriods,
88-
currentAP
89-
)
90-
)
91-
)
92-
}
66+
val period = btnAccountingPeriodService.selectAccountingPeriod(userAnswers, filteredAccountingPeriodDetails)
67+
ensurePeriodPersisted(userAnswers, period)
68+
.flatMap { answersToUse =>
69+
renderOutcome(request, answersToUse, period, mode)
9370
}
9471
.recover { case _ =>
9572
Redirect(controllers.btn.routes.BTNProblemWithServiceController.onPageLoad)
@@ -100,6 +77,50 @@ class BTNAccountingPeriodController @Inject() (
10077
}
10178
}
10279

80+
private def ensurePeriodPersisted(userAnswers: UserAnswers, period: AccountingPeriodDetails): Future[UserAnswers] =
81+
userAnswers.get(BTNChooseAccountingPeriodPage) match {
82+
case Some(_) => Future.successful(userAnswers)
83+
case None =>
84+
userAnswers.set(BTNChooseAccountingPeriodPage, period) match {
85+
case Success(updated) => sessionRepository.set(updated).map(_ => updated)
86+
case Failure(_) => Future.successful(userAnswers)
87+
}
88+
}
89+
90+
private def renderOutcome(
91+
request: ObligationsAndSubmissionsSuccessDataRequest[AnyContent],
92+
userAnswers: UserAnswers,
93+
period: AccountingPeriodDetails,
94+
mode: Mode
95+
)(using ObligationsAndSubmissionsSuccessDataRequest[AnyContent]): Future[Result] =
96+
btnAccountingPeriodService
97+
.outcome(userAnswers, period, filteredAccountingPeriodDetails, btnTypes = Set(BTN), uktrTypes = Set(UKTR_CREATE, UKTR_AMEND))
98+
.match {
99+
case BTNAccountingPeriodService.Outcome.BtnAlreadySubmitted =>
100+
auditService
101+
.auditBtnAlreadySubmitted(
102+
request.subscriptionLocalData.plrReference,
103+
AccountingPeriod(period.startDate, period.endDate),
104+
entitiesInsideOutsideUk = userAnswers.get(EntitiesInsideOutsideUKPage).getOrElse(false)
105+
)
106+
.as(Ok(btnAlreadyInPlaceView()))
107+
case BTNAccountingPeriodService.Outcome.UktrReturnAlreadySubmitted =>
108+
Future.successful(Ok(viewReturnSubmitted(request.isAgent, period)))
109+
case BTNAccountingPeriodService.Outcome.ShowAccountingPeriod(summaryList, hasMultipleAccountingPeriods, currentAP) =>
110+
Future.successful(
111+
Ok(
112+
accountingPeriodView(
113+
summaryList,
114+
mode,
115+
request.isAgent,
116+
request.subscriptionLocalData.organisationName,
117+
hasMultipleAccountingPeriods,
118+
currentAP
119+
)
120+
)
121+
)
122+
}
123+
103124
def onSubmit(mode: Mode): Action[AnyContent] = (identify andThen getSubscriptionData).async { request =>
104125
request.maybeSubscriptionLocalData
105126
.flatMap(_.get(SubMneOrDomesticPage))

app/controllers/btn/BTNConfirmationController.scala

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
2828
import utils.DateTimeUtils.toDateAtTimeFormat
2929
import views.html.btn.BTNConfirmationView
3030

31-
import java.time.LocalDate
3231
import javax.inject.{Inject, Named}
3332
import scala.concurrent.ExecutionContext
3433

@@ -48,33 +47,31 @@ class BTNConfirmationController @Inject() (
4847
given ObligationsAndSubmissionsSuccessDataRequest[AnyContent] = request
4948
sessionRepository.get(request.userId).map {
5049
case Some(userAnswers) =>
51-
val accountingPeriodStartDate: LocalDate = request.subscriptionLocalData.subAccountingPeriod.startDate
52-
val accountingPeriodEndDate: LocalDate = request.subscriptionLocalData.subAccountingPeriod.endDate
53-
val plrRef: Option[String] = userAnswers.get(PlrReferencePage)
54-
55-
userAnswers.get(BtnConfirmationPage).fold(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad())) { submittedAt =>
56-
val showUnderEnquiryWarning = userAnswers
57-
.get(BTNChooseAccountingPeriodPage)
58-
.exists { chosenPeriod =>
59-
val accountingPeriods = filteredAccountingPeriodDetails
60-
chosenPeriod.underEnquiry ||
61-
accountingPeriods
62-
.filter(_.startDate.isAfter(chosenPeriod.startDate))
63-
.exists(_.underEnquiry)
64-
}
50+
(for {
51+
chosenPeriod <- userAnswers.get(BTNChooseAccountingPeriodPage)
52+
submittedAt <- userAnswers.get(BtnConfirmationPage)
53+
} yield {
54+
val plrRef = userAnswers.get(PlrReferencePage)
55+
val showUnderEnquiryWarning = {
56+
val accountingPeriods = filteredAccountingPeriodDetails
57+
chosenPeriod.underEnquiry ||
58+
accountingPeriods
59+
.filter(_.startDate.isAfter(chosenPeriod.startDate))
60+
.exists(_.underEnquiry)
61+
}
6562

6663
Ok(
6764
view(
6865
request.subscriptionLocalData.organisationName,
6966
plrRef,
7067
submittedAt.toDateAtTimeFormat,
71-
accountingPeriodStartDate,
72-
accountingPeriodEndDate,
68+
chosenPeriod.startDate,
69+
chosenPeriod.endDate,
7370
request.isAgent,
7471
showUnderEnquiryWarning
7572
)
7673
)
77-
}
74+
}).getOrElse(Redirect(controllers.routes.JourneyRecoveryController.onPageLoad()))
7875

7976
case None => Redirect(controllers.routes.JourneyRecoveryController.onPageLoad())
8077
}

0 commit comments

Comments
 (0)