Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[PSR-1792] Add pre-pop flag to other assets #1089

Merged
merged 1 commit into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,12 @@ class OtherAssetsCYAController @Inject()(

def onSubmit(srn: Srn, index: Max5000, mode: Mode): Action[AnyContent] =
identifyAndRequireData(srn).async { implicit request =>
val prePopulated = request.userAnswers.get(OtherAssetsPrePopulated(srn, index))
for {
updatedUserAnswers <- Future.fromTry(
request.userAnswers
.set(OtherAssetsHeldPage(srn), true)
.setWhen(prePopulated.isDefined)(OtherAssetsPrePopulated(srn, index), true)
.set(OtherAssetsCompleted(srn, index), SectionCompleted)
)
_ <- saveService.save(updatedUserAnswers)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ class OtherAssetsListController @Inject()(
def buildOtherAssets(index: Max5000): Either[Result, OtherAssetsData] =
for {
nameOfOtherAsset <- requiredPage(WhatIsOtherAssetPage(srn, index))
} yield OtherAssetsData(index, nameOfOtherAsset)
canRemove = request.userAnswers.get(OtherAssetsPrePopulated(srn, index)).isEmpty
} yield OtherAssetsData(index, nameOfOtherAsset, canRemove)

if (isPrePopulation) {
for {
Expand Down Expand Up @@ -278,7 +279,7 @@ object OtherAssetsListController {
List()
case (list, _) =>
list.map {
case OtherAssetsData(index, nameOfOtherAssets) =>
case OtherAssetsData(index, nameOfOtherAssets, canRemove) =>
val otherAssetsMessage = Message("otherAssets.list.row", nameOfOtherAssets)
(mode, viewOnlyViewModel) match {
case (ViewOnlyMode, Some(ViewOnlyViewModel(_, year, current, previous, _))) =>
Expand All @@ -293,14 +294,20 @@ object OtherAssetsListController {
url = routes.OtherAssetsCheckAndUpdateController.onPageLoad(srn, index).url,
hiddenText = Message("site.check.param", Message("loansList.row", otherAssetsMessage))
)
case _ =>
case _ if canRemove =>
ListRow(
text = otherAssetsMessage,
changeUrl = routes.OtherAssetsCYAController.onPageLoad(srn, index, CheckMode).url,
changeHiddenText = Message("otherAssets.list.row.change.hiddenText", otherAssetsMessage),
removeUrl = routes.RemoveOtherAssetController.onPageLoad(srn, index, NormalMode).url,
removeHiddenText = Message("otherAssets.list.row.remove.hiddenText", otherAssetsMessage)
)
case _ =>
ListRow(
text = otherAssetsMessage,
changeUrl = routes.OtherAssetsCYAController.onPageLoad(srn, index, CheckMode).url,
changeHiddenText = Message("otherAssets.list.row.change.hiddenText", otherAssetsMessage)
)
}
}
}
Expand Down Expand Up @@ -465,6 +472,7 @@ object OtherAssetsListController {

case class OtherAssetsData(
index: Max5000,
nameOfOtherAssets: String
nameOfOtherAssets: String,
canRemove: Boolean
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package controllers.nonsipp.otherassetsheld
import services.{PsrSubmissionService, SaveService}
import viewmodels.implicits._
import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
import pages.nonsipp.otherassetsheld.{RemoveOtherAssetPage, WhatIsOtherAssetPage}
import pages.nonsipp.otherassetsheld.{OtherAssetsPrePopulated, RemoveOtherAssetPage, WhatIsOtherAssetPage}
import controllers.actions._
import navigation.Navigator
import forms.YesNoPageFormProvider
Expand Down Expand Up @@ -54,21 +54,25 @@ class RemoveOtherAssetController @Inject()(

def onPageLoad(srn: Srn, index: Max5000, mode: Mode): Action[AnyContent] =
identifyAndRequireData(srn) { implicit request =>
(
for {
whatIsOtherAsset <- request.userAnswers.get(WhatIsOtherAssetPage(srn, index)).getOrRedirectToTaskList(srn)
} yield {
val preparedForm =
request.userAnswers.fillForm(RemoveOtherAssetPage(srn, index), form)
Ok(
view(
preparedForm,
RemoveOtherAssetController
.viewModel(srn, index, whatIsOtherAsset, mode)
if (request.userAnswers.get(OtherAssetsPrePopulated(srn, index)).isDefined) {
Redirect(controllers.routes.UnauthorisedController.onPageLoad())
} else {
(
for {
whatIsOtherAsset <- request.userAnswers.get(WhatIsOtherAssetPage(srn, index)).getOrRedirectToTaskList(srn)
} yield {
val preparedForm =
request.userAnswers.fillForm(RemoveOtherAssetPage(srn, index), form)
Ok(
view(
preparedForm,
RemoveOtherAssetController
.viewModel(srn, index, whatIsOtherAsset, mode)
)
)
)
}
).merge
}
).merge
}
}

def onSubmit(srn: Srn, index: Max5000, mode: Mode): Action[AnyContent] =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package pages.nonsipp.memberdetails class MemberDetailsPrePopulated {

}
28 changes: 28 additions & 0 deletions app/pages/nonsipp/otherassetsheld/OtherAssetsPrePopulated.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2025 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 pages.nonsipp.otherassetsheld

import utils.RefinedUtils.RefinedIntOps
import pages.QuestionPage
import config.RefinedTypes.Max5000
import models.SchemeId.Srn
import play.api.libs.json.JsPath

case class OtherAssetsPrePopulated(srn: Srn, index: Max5000) extends QuestionPage[Boolean] {
override def path: JsPath = Paths.otherAssets \ toString \ index.arrayIndex.toString
override def toString: String = "otherAssetsPrePopulated"
}
25 changes: 21 additions & 4 deletions app/prepop/OtherAssetsPrePopulationProcessor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ package prepop

import pages.nonsipp.otherassetsdisposal.{OtherAssetsDisposalPage, OtherAssetsDisposalProgress}
import pages.nonsipp.otherassetsheld._
import models.UserAnswers.SensitiveJsObject
import config.RefinedTypes.OneTo5000
import config.RefinedTypes.{Max5000, OneTo5000}
import models.SchemeId.Srn
import pages.nonsipp.otherassetsdisposal.Paths.assetsDisposed
import eu.timepit.refined.refineV
import models.UserAnswers
import utils.ListUtils.ListOps
import models.UserAnswers.SensitiveJsObject
import pages.nonsipp.otherassetsheld.Paths.otherAssets
import play.api.libs.json.{JsObject, JsSuccess}

import scala.util.{Success, Try}
import scala.util.Try

import javax.inject.{Inject, Singleton}

Expand All @@ -52,7 +53,23 @@ class OtherAssetsPrePopulationProcessor @Inject()() {
.flatMap(_.transform((Paths.otherAssetsTransactions \ "movableSchedule29A").prune(_)))
.flatMap(_.transform((Paths.otherAssetsTransactions \ "totalIncomeOrReceipts").prune(_))) match {
case JsSuccess(value, _) =>
Success(currentUA.copy(data = SensitiveJsObject(value.deepMerge(currentUA.data.decryptedValue))))
val uaWithOtherAssetsData =
currentUA.copy(data = SensitiveJsObject(value.deepMerge(currentUA.data.decryptedValue)))

val updatedUA = uaWithOtherAssetsData
.get(WhatIsOtherAssetPages(srn))
.map(_.keys.toList)
.toList
.flatten
.refine[Max5000.Refined]
.map(index => OtherAssetsPrePopulated(srn, index))
.foldLeft(Try(uaWithOtherAssetsData)) {
case (ua, otherAssetsPrePopulated) => {
ua.flatMap(_.set(otherAssetsPrePopulated, false))
}
}

updatedUA
case _ => Try(currentUA)
}

Expand Down
18 changes: 16 additions & 2 deletions app/transformations/OtherAssetsTransformer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class OtherAssetsTransformer @Inject() extends Transformer {
assetDescription <- request.userAnswers.get(WhatIsOtherAssetPage(srn, index))
methodOfHolding <- request.userAnswers.get(WhyDoesSchemeHoldAssetsPage(srn, index))
costOfAsset <- request.userAnswers.get(CostOfOtherAssetPage(srn, index))
prePopulated = request.userAnswers.get(OtherAssetsPrePopulated(srn, index))
} yield {

val optNoneTransferRelatedDetails = buildOptNoneTransferRelatedDetails(methodOfHolding, srn, index)
Expand All @@ -91,7 +92,7 @@ class OtherAssetsTransformer @Inject() extends Transformer {
val optTotalIncomeOrReceipts = request.userAnswers.get(IncomeFromAssetPage(srn, index))

OtherAssetTransaction(
prePopulated = None,
prePopulated = prePopulated,
assetDescription = assetDescription,
methodOfHolding = methodOfHolding,
optDateOfAcqOrContrib = optNoneTransferRelatedDetails.map(_._2),
Expand Down Expand Up @@ -544,6 +545,16 @@ class OtherAssetsTransformer @Inject() extends Transformer {

val otherAssetsCompleted = OtherAssetsCompleted(srn, index) -> SectionCompleted

val otherAssetsPrePopulated = indexes
.filter(
index => {
otherAssetTransactions(index.value - 1).prePopulated.nonEmpty
}
)
.map(
index => OtherAssetsPrePopulated(srn, index) -> otherAssetTransactions(index.value - 1).prePopulated.get
)

val triedUA = for {
ua0 <- ua
ua1 <- ua0.set(assetDescription._1, assetDescription._2)
Expand All @@ -563,11 +574,14 @@ class OtherAssetsTransformer @Inject() extends Transformer {
ua15 <- optUKPartnershipTuple.map(t => ua14.set(t._2._1, t._2._2)).getOrElse(Try(ua14))
ua16 <- optOther.map(t => ua15.set(t._1, t._2)).getOrElse(Try(ua15))
ua17 <- ua16.set(otherAssetsCompleted._1, otherAssetsCompleted._2)
ua18 <- otherAssetsPrePopulated.foldLeft(Try(ua17)) {
case (ua, (page, value)) => ua.flatMap(_.set(page, value))
}
} yield {
buildOptDisposedOtherAssetsUA(
index,
srn,
ua17,
ua18,
otherAssetTransaction.optOtherAssetDisposed
)
}
Expand Down
13 changes: 11 additions & 2 deletions app/utils/nonsipp/check/OtherAssetsCheckStatusUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ object OtherAssetsCheckStatusUtils {
}

/**
* This method determines whether or not a Other Assets record needs to be checked. A record needs checking if any of
* the pre-populated-then-cleared answers are missing & all of the other answers are present.
* This method determines whether or not a Other Assets record needs to be checked. A record only needs to be checked
* if its OtherAssetsPrePopulated field is false.
* @param userAnswers the answers provided by the user, from which we get the Other Assets record
* @param srn the Scheme Reference Number, used for the .get calls
* @param recordIndex the index of the record being checked
Expand All @@ -70,6 +70,15 @@ object OtherAssetsCheckStatusUtils {
userAnswers: UserAnswers,
srn: Srn,
recordIndex: Max5000
): Boolean =
userAnswers.get(OtherAssetsPrePopulated(srn, recordIndex)) match {
case Some(checked) => !checked
case None => checkotherAssetsRecordLegacy(userAnswers, srn, recordIndex) // non-pre-pop
}
def checkotherAssetsRecordLegacy(
userAnswers: UserAnswers,
srn: Srn,
recordIndex: Max5000
): Boolean = {
val anyPrePopClearedAnswersMissing: Boolean = (
userAnswers.get(IsAssetTangibleMoveablePropertyPage(srn, recordIndex)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ class OtherAssetsListControllerSpec extends ControllerBaseSpec {
.unsafeSet(WhyDoesSchemeHoldAssetsPage(srn, index3of5000), Transfer)
.unsafeSet(CostOfOtherAssetPage(srn, index3of5000), money)
.unsafeSet(OtherAssetsCompleted(srn, index3of5000), SectionCompleted)
.unsafeSet(OtherAssetsPrePopulated(srn, index3of5000), false)

private val userAnswersChecked = completedUserAnswers
.unsafeSet(WhatIsOtherAssetPage(srn, index3of5000), "otherAssetsChecked")
.unsafeSet(WhyDoesSchemeHoldAssetsPage(srn, index3of5000), Transfer)
.unsafeSet(CostOfOtherAssetPage(srn, index3of5000), money)
.unsafeSet(OtherAssetsCompleted(srn, index3of5000), SectionCompleted)
.unsafeSet(OtherAssetsPrePopulated(srn, index3of5000), true)

private val page = 1

Expand All @@ -76,29 +84,42 @@ class OtherAssetsListControllerSpec extends ControllerBaseSpec {
private val otherAssetsData: List[OtherAssetsData] = List(
OtherAssetsData(
index1of5000,
"nameOfOtherAsset1"
"nameOfOtherAsset1",
true
),
OtherAssetsData(
index2of5000,
"nameOfOtherAsset2"
"nameOfOtherAsset2",
true
)
)

private val otherAssetsDataToCheck: List[OtherAssetsData] = List(
OtherAssetsData(
index3of5000,
"nameOfOtherAsset3"
"nameOfOtherAsset3",
false
)
)

private val otherAssetsDataChanged: List[OtherAssetsData] = List(
OtherAssetsData(
index1of5000,
"changedNameOfOtherAsset"
"changedNameOfOtherAsset",
true
),
OtherAssetsData(
index2of5000,
"nameOfOtherAsset2"
"nameOfOtherAsset2",
true
)
)

private val otherAssetsDataChecked: List[OtherAssetsData] = List(
OtherAssetsData(
index3of5000,
"otherAssetsChecked",
false
)
)

Expand Down Expand Up @@ -187,7 +208,24 @@ class OtherAssetsListControllerSpec extends ControllerBaseSpec {
isPrePop = true
)
)
}.withName("PrePop Journey"))
}.withName("PrePop Journey Not Checked"))

act.like(renderViewWithPrePopSession(onPageLoad, userAnswersChecked) { implicit app => implicit request =>
injected[ListView].apply(
form(new YesNoPageFormProvider()),
viewModel(
srn = srn,
page = page,
mode = NormalMode,
otherAssets = otherAssetsData ++ otherAssetsDataChecked,
otherAssetsToCheck = Nil,
schemeName = schemeName,
viewOnlyViewModel = None,
showBackLink = true,
isPrePop = true
)
)
}.withName("PrePop Journey Checked"))

act.like(
renderPrePopView(onPageLoad, OtherAssetsListPage(srn), true, completedUserAnswers) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
package controllers.nonsipp.otherassetsheld

import services.PsrSubmissionService
import pages.nonsipp.otherassetsheld.WhatIsOtherAssetPage
import pages.nonsipp.otherassetsheld.{OtherAssetsPrePopulated, WhatIsOtherAssetPage}
import controllers.nonsipp.otherassetsheld.RemoveOtherAssetController._
import play.api.inject.bind
import views.html.YesNoPageView
import eu.timepit.refined.refineMV
import forms.YesNoPageFormProvider
import models.NormalMode
import models.{NormalMode, UserAnswers}
import org.mockito.ArgumentMatchers.any
import play.api.inject.guice.GuiceableModule
import org.mockito.Mockito._
Expand All @@ -46,6 +46,9 @@ class RemoveOtherAssetControllerSpec extends ControllerBaseSpec {
bind[PsrSubmissionService].toInstance(mockPsrSubmissionService)
)

val prePopUserAnswersChecked: UserAnswers = userAnswers.unsafeSet(OtherAssetsPrePopulated(srn, refineMV(1)), true)
val prePopUserAnswersNotChecked: UserAnswers = userAnswers.unsafeSet(OtherAssetsPrePopulated(srn, refineMV(1)), false)

override protected def beforeEach(): Unit =
reset(mockPsrSubmissionService)

Expand Down Expand Up @@ -77,5 +80,15 @@ class RemoveOtherAssetControllerSpec extends ControllerBaseSpec {

act.like(journeyRecoveryPage(onSubmit).updateName("onSubmit" + _))

act.like(
redirectToPage(onPageLoad, controllers.routes.UnauthorisedController.onPageLoad(), prePopUserAnswersChecked)
.updateName(_ + " - Block removing checked Pre-pop other assets")
)

act.like(
redirectToPage(onPageLoad, controllers.routes.UnauthorisedController.onPageLoad(), prePopUserAnswersNotChecked)
.updateName(_ + " - Block removing unchecked Pre-pop other assets")
)

}
}
Loading