Skip to content
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
53 changes: 53 additions & 0 deletions app/uk/gov/hmrc/ngrraldfrontend/actions/DataRetrievalAction.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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 uk.gov.hmrc.ngrraldfrontend.actions

import play.api.mvc.Results.Redirect
import play.api.mvc.{ActionTransformer, Result}
import uk.gov.hmrc.http.{HeaderCarrier, NotFoundException}
import uk.gov.hmrc.ngrraldfrontend.config.AppConfig
import uk.gov.hmrc.ngrraldfrontend.connectors.NGRConnector
import uk.gov.hmrc.ngrraldfrontend.models.AuthenticatedUserRequest
import uk.gov.hmrc.ngrraldfrontend.models.registration.CredId
import uk.gov.hmrc.ngrraldfrontend.models.requests.{IdentifierRequest, OptionalDataRequest}
import uk.gov.hmrc.ngrraldfrontend.repo.SessionRepository
import uk.gov.hmrc.play.http.HeaderCarrierConverter

import javax.inject.Inject
import scala.concurrent.{ExecutionContext, Future}

class DataRetrievalActionImpl @Inject()(
val sessionRepository: SessionRepository,
ngrConnector: NGRConnector,
appConfig: AppConfig
)(implicit val executionContext: ExecutionContext) extends DataRetrievalAction {

override protected def transform[A](request: AuthenticatedUserRequest[A]): Future[OptionalDataRequest[A]] = {

implicit val hc: HeaderCarrier = HeaderCarrierConverter.fromRequestAndSession(request, request.session)

sessionRepository.get(request.credId.getOrElse("")).flatMap { userAnswersOpt =>
ngrConnector.getLinkedProperty(CredId(request.credId.getOrElse(""))).map {
case Some(property) =>
OptionalDataRequest(request.request, request.credId.getOrElse(""), userAnswersOpt, property)
case None => throw new NotFoundException("Property not found")
}
}
}
}

trait DataRetrievalAction extends ActionTransformer[AuthenticatedUserRequest, OptionalDataRequest]

This file was deleted.

4 changes: 2 additions & 2 deletions app/uk/gov/hmrc/ngrraldfrontend/config/AppConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ trait AppConfig {
val ngrLoginRegistrationHost: String
val ngrDashboardUrl: String
val ngrLogoutUrl: String
val timeToLive: String
def getString(key: String): String
val cacheTtl: Long
}

@Singleton
class FrontendAppConfig @Inject()(config: Configuration, servicesConfig: ServicesConfig) extends AppConfig {
override val features = new Features()(config)
override val nextGenerationRatesHost: String = servicesConfig.baseUrl("next-generation-rates")
override val ngrLoginRegistrationHost: String = servicesConfig.baseUrl("ngr-login-register-frontend")
override val timeToLive: String = servicesConfig.getString("time-to-live.time")
override val ngrDashboardUrl: String = s"$dashboardHost/ngr-dashboard-frontend/dashboard"
override val ngrLogoutUrl: String = s"$dashboardHost/ngr-dashboard-frontend/signout"
override val cacheTtl: Long = config.get[Int]("mongodb.timeToLiveInSeconds")

def getString(key: String): String =
config.getOptional[String](key).filter(!_.isBlank).getOrElse(throwConfigNotFoundError(key))
Expand Down
6 changes: 5 additions & 1 deletion app/uk/gov/hmrc/ngrraldfrontend/config/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
package uk.gov.hmrc.ngrraldfrontend.config

import com.google.inject.AbstractModule
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, AuthRetrievalsImpl}
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, AuthRetrievalsImpl, DataRetrievalAction, DataRetrievalActionImpl}

import java.time.{Clock, ZoneOffset}

class Module extends AbstractModule {

override def configure(): Unit = {
bind(classOf[AppConfig]).to(classOf[FrontendAppConfig]).asEagerSingleton()
bind(classOf[AuthRetrievals]).to(classOf[AuthRetrievalsImpl]).asEagerSingleton()
bind(classOf[DataRetrievalAction]).to(classOf[DataRetrievalActionImpl]).asEagerSingleton()
bind(classOf[Clock]).toInstance(Clock.systemDefaultZone.withZone(ZoneOffset.UTC))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class NGRConnector @Inject()(http: HttpClientV2,
getPropertyLinkingUserAnswers(credId)
.map {
case Some(propertyLinkingUserAnswers) => Some(propertyLinkingUserAnswers.vmvProperty)
case None => throw new NotFoundException("failed to find propertyLinkingUserAnswers from backend mongo")
case None => None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thinking if we deal with the exception here we don't have to do it anywhere else this is called. And maybe in the future we should show that page was talking about to not show kibana errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its been changed to None, as it was firing the exception in this file and not hitting the data retrieval action to hit the exception there. I had to do this in order to get the tests to work if a property wasn't found

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ package uk.gov.hmrc.ngrraldfrontend.controllers

import play.api.i18n.I18nSupport
import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
import uk.gov.hmrc.http.NotFoundException
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, PropertyLinkingAction}
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, DataRetrievalAction}
import uk.gov.hmrc.ngrraldfrontend.config.AppConfig
import uk.gov.hmrc.ngrraldfrontend.models.{Mode, UserAnswers}
import uk.gov.hmrc.ngrraldfrontend.models.components.NGRRadio.buildRadios
import uk.gov.hmrc.ngrraldfrontend.models.forms.AgreedRentChangeForm
import uk.gov.hmrc.ngrraldfrontend.models.forms.AgreedRentChangeForm.form
import uk.gov.hmrc.ngrraldfrontend.models.registration.CredId
import uk.gov.hmrc.ngrraldfrontend.repo.RaldRepo
import uk.gov.hmrc.ngrraldfrontend.navigation.Navigator
import uk.gov.hmrc.ngrraldfrontend.pages.AgreedRentChangePage
import uk.gov.hmrc.ngrraldfrontend.repo.SessionRepository
import uk.gov.hmrc.ngrraldfrontend.views.html.AgreedRentChangeView
import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController

Expand All @@ -35,44 +36,44 @@ import scala.concurrent.{ExecutionContext, Future}
@Singleton
class AgreedRentChangeController @Inject()(agreedRentChangeView: AgreedRentChangeView,
authenticate: AuthRetrievals,
hasLinkedProperties: PropertyLinkingAction,
raldRepo: RaldRepo,
getData: DataRetrievalAction,
sessionRepository: SessionRepository,
navigator: Navigator,
mcc: MessagesControllerComponents)(implicit appConfig: AppConfig, ec: ExecutionContext)
extends FrontendController(mcc) with I18nSupport {


def show: Action[AnyContent] = {
(authenticate andThen hasLinkedProperties).async { implicit request =>
request.propertyLinking.map(property =>
def show(mode: Mode): Action[AnyContent] = {
(authenticate andThen getData).async { implicit request =>
val preparedForm = request.userAnswers.getOrElse(UserAnswers(request.credId)).get(AgreedRentChangePage) match {
case None => form
case Some(value) => form.fill(AgreedRentChangeForm(value))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this man! big improvement!

Future.successful(Ok(agreedRentChangeView(
form = form,
radios = buildRadios(form, AgreedRentChangeForm.ngrRadio(form)),
propertyAddress = property.addressFull,
)))).getOrElse(throw new NotFoundException("Couldn't find property in mongo"))
form = preparedForm,
radios = buildRadios(preparedForm, AgreedRentChangeForm.ngrRadio(preparedForm)),
propertyAddress = request.property.addressFull,
mode = mode
)))
}
}

def submit: Action[AnyContent] =
(authenticate andThen hasLinkedProperties).async { implicit request =>
def submit(mode: Mode): Action[AnyContent] =
(authenticate andThen getData).async { implicit request =>
form.bindFromRequest().fold(
formWithErrors => {
request.propertyLinking.map(property =>
Future.successful(BadRequest(agreedRentChangeView(
form = formWithErrors,
radios = buildRadios(formWithErrors, AgreedRentChangeForm.ngrRadio(formWithErrors)),
propertyAddress = property.addressFull
)))).getOrElse(throw new NotFoundException("Couldn't find property in mongo"))
propertyAddress = request.property.addressFull,
mode = mode
)))
},
radioValue =>
raldRepo.insertAgreedRentChange(
credId = CredId(request.credId.getOrElse("")),
agreedRentChange = radioValue.radioValue
)
if (radioValue.radioValue == "Yes") {
Future.successful(Redirect(routes.ProvideDetailsOfFirstSecondRentPeriodController.show.url))
} else {
Future.successful(Redirect(routes.HowMuchIsTotalAnnualRentController.show.url))
}
for {
updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)).set(AgreedRentChangePage, radioValue.radioValue))
_ <- sessionRepository.set(updatedAnswers)
} yield Redirect(navigator.nextPage(AgreedRentChangePage, mode, updatedAnswers))
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ import play.api.mvc.{Action, AnyContent, MessagesControllerComponents}
import uk.gov.hmrc.govukfrontend.views.Aliases.*
import uk.gov.hmrc.govukfrontend.views.html.components.ErrorMessage
import uk.gov.hmrc.govukfrontend.views.viewmodels.dateinput.DateInput
import uk.gov.hmrc.http.NotFoundException
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, PropertyLinkingAction}
import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, DataRetrievalAction}
import uk.gov.hmrc.ngrraldfrontend.config.AppConfig
import uk.gov.hmrc.ngrraldfrontend.models.{Mode, Agreement, NGRDate, UserAnswers}
import uk.gov.hmrc.ngrraldfrontend.models.components.*
import uk.gov.hmrc.ngrraldfrontend.models.components.NGRRadio.buildRadios
import uk.gov.hmrc.ngrraldfrontend.models.forms.AgreementForm
import uk.gov.hmrc.ngrraldfrontend.models.forms.AgreementForm.form
import uk.gov.hmrc.ngrraldfrontend.models.registration.CredId
import uk.gov.hmrc.ngrraldfrontend.repo.RaldRepo
import uk.gov.hmrc.ngrraldfrontend.navigation.Navigator
import uk.gov.hmrc.ngrraldfrontend.pages.AgreementPage
import uk.gov.hmrc.ngrraldfrontend.repo.SessionRepository
import uk.gov.hmrc.ngrraldfrontend.views.html.AgreementView
import uk.gov.hmrc.ngrraldfrontend.views.html.components.{DateTextFields, NGRCharacterCountComponent}
import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController
Expand All @@ -42,10 +43,11 @@ import scala.concurrent.{ExecutionContext, Future}
class AgreementController @Inject()(view: AgreementView,
authenticate: AuthRetrievals,
dateTextFields: DateTextFields,
hasLinkedProperties: PropertyLinkingAction,
raldRepo: RaldRepo,
ngrCharacterCountComponent: NGRCharacterCountComponent,
mcc: MessagesControllerComponents
mcc: MessagesControllerComponents,
getData: DataRetrievalAction,
navigator: Navigator,
sessionRepository: SessionRepository
)(implicit appConfig: AppConfig, ec: ExecutionContext) extends FrontendController(mcc) with I18nSupport {

def dateInput()(implicit messages: Messages): DateInput = DateInput(
Expand Down Expand Up @@ -133,22 +135,36 @@ class AgreementController @Inject()(view: AgreementView,
}


def show: Action[AnyContent] = {
(authenticate andThen hasLinkedProperties).async { implicit request =>
request.propertyLinking.map(property =>
Future.successful(Ok(view(
selectedPropertyAddress = property.addressFull,
form,
dateInput(),
buildRadios(form, openEndedRadio(form)),
buildRadios(form, breakClauseRadio(form))
)))
).getOrElse(throw new NotFoundException("Couldn't find property in mongo"))
def show(mode: Mode): Action[AnyContent] = {
(authenticate andThen getData).async { implicit request =>
val preparedForm = request.userAnswers.getOrElse(UserAnswers(request.credId)).get(AgreementPage) match {
case None => form
case Some(value) => form.fill(AgreementForm(NGRDate.fromString(value.agreementStart), if (value.isOpenEnded) {
"YesOpenEnded"
} else {
"NoOpenEnded"
}, value.openEndedDate match {
case Some(value) => Some(NGRDate.fromString(value))
case None => None
}, if (value.haveBreakClause) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking storing things as boolean is making our code more annoying then good, we should raise this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think in the future we should have some kind of standard. A lot of the radio buttons are strings which are just simple Yes or No answers. So I think we should be storing them as booleans as it makes it easier to prepopulate the form rather than having to put in the name of the radio value. If you get me. For example what your rent includes page really should just be booleans for each value not the whole value that you have to pass back

"YesBreakClause"
} else {
"NoBreakClause"
}, value.breakClauseInfo))
}
Future.successful(Ok(view(
selectedPropertyAddress = request.property.addressFull,
preparedForm,
dateInput(),
buildRadios(preparedForm, openEndedRadio(preparedForm)),
buildRadios(preparedForm, breakClauseRadio(preparedForm)),
mode = mode
)))
}
}

def submit: Action[AnyContent] = {
(authenticate andThen hasLinkedProperties).async { implicit request =>
def submit(mode: Mode): Action[AnyContent] = {
(authenticate andThen getData).async { implicit request =>
form
.bindFromRequest()
.fold(
Expand All @@ -166,24 +182,30 @@ class AgreementController @Inject()(view: AgreementView,
}
val formWithCorrectedErrors = formWithErrors.copy(errors = correctedFormErrors)

request.propertyLinking.map(property =>
Future.successful(BadRequest(view(
selectedPropertyAddress = property.addressFull,
selectedPropertyAddress = request.property.addressFull,
formWithCorrectedErrors,
dateInput(),
buildRadios(formWithErrors, openEndedRadio(formWithCorrectedErrors)),
buildRadios(formWithErrors, breakClauseRadio(formWithCorrectedErrors))
)))).getOrElse(throw new NotFoundException("Couldn't find property in mongo")),
buildRadios(formWithErrors, breakClauseRadio(formWithCorrectedErrors)),
mode = mode
))),
agreementForm =>
raldRepo.insertAgreement(
CredId(request.credId.getOrElse("")),
agreementForm.agreementStart.makeString,
agreementForm.openEndedRadio,
val answers = Agreement(agreementForm.agreementStart.makeString,
agreementForm.openEndedRadio match {
case answer if(answer == "YesOpenEnded") => true
case _ => false
},
agreementForm.openEndedDate.map(value => value.makeString),
agreementForm.breakClauseRadio,
agreementForm.breakClauseInfo,
)
Future.successful(Redirect(routes.WhatIsYourRentBasedOnController.show.url))
agreementForm.breakClauseRadio match {
case openEndedRadio if(openEndedRadio == "YesBreakClause") => true
case _ => false
},
agreementForm.breakClauseInfo)
for {
updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)).set(AgreementPage, answers))
_ <- sessionRepository.set(updatedAnswers)
} yield Redirect(navigator.nextPage(AgreementPage, mode, updatedAnswers))
)
}
}
Expand Down
Loading