From 8fbc9d042f00b391e02d4ee43b0d4dffec5f9bad Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Tue, 7 Oct 2025 17:22:10 +0100 Subject: [PATCH 1/9] NGR-1130: adding what rent includes for Total Occupancy Cost Lease --- ...tIncludesRatesWaterServiceController.scala | 98 +++++++++++++++++++ .../WhatYourRentIncludesController.scala | 20 ++-- .../models/WhatYourRentIncludes.scala | 6 +- .../forms/WhatYourRentIncludesForm.scala | 26 ++--- .../models/forms/mappings/Formatters.scala | 13 +++ .../models/forms/mappings/Mappings.scala | 6 ++ .../navigation/Navigator.scala | 16 ++- .../views/WhatYourRentIncludesView.scala.html | 16 ++- conf/app.routes | 6 ++ 9 files changed, 174 insertions(+), 33 deletions(-) create mode 100644 app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala new file mode 100644 index 00000000..efd9732a --- /dev/null +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala @@ -0,0 +1,98 @@ +/* + * 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.controllers + +import play.api.i18n.I18nSupport +import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} +import uk.gov.hmrc.ngrraldfrontend.actions.{AuthRetrievals, DataRetrievalAction} +import uk.gov.hmrc.ngrraldfrontend.config.AppConfig +import uk.gov.hmrc.ngrraldfrontend.models.components.NGRRadio.buildRadios +import uk.gov.hmrc.ngrraldfrontend.models.forms.WhatYourRentIncludesForm +import uk.gov.hmrc.ngrraldfrontend.models.forms.WhatYourRentIncludesForm.{answerToForm, form, formToAnswers} +import uk.gov.hmrc.ngrraldfrontend.models.{Mode, UserAnswers, WhatYourRentIncludes} +import uk.gov.hmrc.ngrraldfrontend.navigation.Navigator +import uk.gov.hmrc.ngrraldfrontend.pages.{WhatIsYourRentBasedOnPage, WhatYourRentIncludesPage} +import uk.gov.hmrc.ngrraldfrontend.repo.SessionRepository +import uk.gov.hmrc.ngrraldfrontend.views.html.WhatYourRentIncludesView +import uk.gov.hmrc.ngrraldfrontend.views.html.components.InputText +import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController + +import javax.inject.{Inject, Singleton} +import scala.concurrent.{ExecutionContext, Future} + +@Singleton +class WhatRentIncludesRatesWaterServiceController @Inject()(whatYourRentIncludesView: WhatYourRentIncludesView, + authenticate: AuthRetrievals, + inputText: InputText, + getData: DataRetrievalAction, + sessionRepository: SessionRepository, + navigator: Navigator, + mcc: MessagesControllerComponents)(implicit appConfig: AppConfig, ec: ExecutionContext) + extends FrontendController(mcc) with I18nSupport { + + def show(mode: Mode): Action[AnyContent] = { + (authenticate andThen getData).async { implicit request => + val preparedForm = request.userAnswers.getOrElse(UserAnswers(request.credId)).get(WhatYourRentIncludesPage) match { + case Some(value) => answerToForm(value, isOTCLease = true) + case None => form(isOTCLease = true) + } + Future.successful(Ok(whatYourRentIncludesView( + form = preparedForm, + radios1 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio1(preparedForm, inputText)), + radios2 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio2), + radios3 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio3), + radios4 = None, + radios5 = None, + radios6 = None, + propertyAddress = request.property.addressFull, + mode = mode + ))) + } + } + + def submit(mode: Mode): Action[AnyContent] = + (authenticate andThen getData).async { implicit request => + form(isOTCLease = true).bindFromRequest().fold( + formWithErrors => { + val correctedFormErrors = formWithErrors.errors.map { formError => + (formError.key, formError.messages) match + case ("", messages) => + formError.copy(key = "bedroomNumbers") + case _ => formError + } + val formWithCorrectedErrors = formWithErrors.copy(errors = correctedFormErrors) + Future.successful(BadRequest(whatYourRentIncludesView( + form = formWithCorrectedErrors, + radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithCorrectedErrors, inputText)), + radios2 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio2), + radios3 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio3), + radios4 = None, + radios5 = None, + radios6 = None, + propertyAddress = request.property.addressFull, + mode = mode + ))) + }, + whatYourRentIncludesForm => + for { + updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) + .set(WhatYourRentIncludesPage, formToAnswers(whatYourRentIncludesForm, isOTCLease = false))) + _ <- sessionRepository.set(updatedAnswers) + } yield Redirect(navigator.nextPage(WhatYourRentIncludesPage, mode, updatedAnswers)) + ) + } +} diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala index 23a8d75e..c66948a8 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala @@ -47,17 +47,17 @@ class WhatYourRentIncludesController @Inject()(whatYourRentIncludesView: WhatYou def show(mode: Mode): Action[AnyContent] = { (authenticate andThen getData).async { implicit request => val preparedForm = request.userAnswers.getOrElse(UserAnswers(request.credId)).get(WhatYourRentIncludesPage) match { - case Some(value) => answerToForm(value) - case None => form + case Some(value) => answerToForm(value, isOTCLease = false) + case None => form(isOTCLease = false) } Future.successful(Ok(whatYourRentIncludesView( form = preparedForm, radios1 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio1(preparedForm, inputText)), radios2 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio2), radios3 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio3), - radios4 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio4), - radios5 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio5), - radios6 = buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio6), + radios4 = Some(buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio4)), + radios5 = Some(buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio5)), + radios6 = Some(buildRadios(preparedForm, WhatYourRentIncludesForm.ngrRadio6)), propertyAddress = request.property.addressFull, mode = mode ))) @@ -66,7 +66,7 @@ class WhatYourRentIncludesController @Inject()(whatYourRentIncludesView: WhatYou def submit(mode: Mode): Action[AnyContent] = (authenticate andThen getData).async { implicit request => - form.bindFromRequest().fold( + form(isOTCLease = false).bindFromRequest().fold( formWithErrors => { val correctedFormErrors = formWithErrors.errors.map { formError => (formError.key, formError.messages) match @@ -80,9 +80,9 @@ class WhatYourRentIncludesController @Inject()(whatYourRentIncludesView: WhatYou radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithCorrectedErrors, inputText)), radios2 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio2), radios3 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio3), - radios4 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio4), - radios5 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio5), - radios6 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio6), + radios4 = Some(buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio4)), + radios5 = Some(buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio5)), + radios6 = Some(buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio6)), propertyAddress = request.property.addressFull, mode = mode ))) @@ -90,7 +90,7 @@ class WhatYourRentIncludesController @Inject()(whatYourRentIncludesView: WhatYou whatYourRentIncludesForm => for { updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) - .set(WhatYourRentIncludesPage, formToAnswers(whatYourRentIncludesForm))) + .set(WhatYourRentIncludesPage, formToAnswers(whatYourRentIncludesForm, isOTCLease = false))) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(WhatYourRentIncludesPage, mode, updatedAnswers)) ) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludes.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludes.scala index 2ca63465..115856f4 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludes.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludes.scala @@ -22,9 +22,9 @@ case class WhatYourRentIncludes( livingAccommodation: Boolean, rentPartAddress: Boolean, rentEmptyShell: Boolean, - rentIncBusinessRates: Boolean, - rentIncWaterCharges: Boolean, - rentIncService: Boolean, + rentIncBusinessRates: Option[Boolean], + rentIncWaterCharges: Option[Boolean], + rentIncService: Option[Boolean], bedroomNumbers: Option[Int] ) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala index 3bb5fd1a..434554be 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala @@ -120,25 +120,25 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { whatYourRentIncludesForm.bedroomNumbers )) - def answerToForm(whatYourRentIncludes: WhatYourRentIncludes): Form[WhatYourRentIncludesForm] = - form.fill(WhatYourRentIncludesForm( + def answerToForm(whatYourRentIncludes: WhatYourRentIncludes, isOTCLease: Boolean): Form[WhatYourRentIncludesForm] = + form(isOTCLease = false).fill(WhatYourRentIncludesForm( livingAccommodationRadio = whatYourRentIncludes.livingAccommodation.toString, rentPartAddressRadio = whatYourRentIncludes.rentPartAddress.toString, rentEmptyShellRadio = whatYourRentIncludes.rentEmptyShell.toString, - rentIncBusinessRatesRadio = whatYourRentIncludes.rentIncBusinessRates.toString, - rentIncWaterChargesRadio = whatYourRentIncludes.rentIncWaterCharges.toString, - rentIncServiceRadio = whatYourRentIncludes.rentIncService.toString, + rentIncBusinessRatesRadio = whatYourRentIncludes.rentIncBusinessRates.map(_.toString).getOrElse(""), + rentIncWaterChargesRadio = whatYourRentIncludes.rentIncWaterCharges.map(_.toString).getOrElse(""), + rentIncServiceRadio = whatYourRentIncludes.rentIncService.map(_.toString).getOrElse(""), bedroomNumbers = whatYourRentIncludes.bedroomNumbers.map(_.toString) )) - def formToAnswers(whatYourRentIncludesForm: WhatYourRentIncludesForm): WhatYourRentIncludes = + def formToAnswers(whatYourRentIncludesForm: WhatYourRentIncludesForm, isOTCLease: Boolean): WhatYourRentIncludes = WhatYourRentIncludes( livingAccommodation = whatYourRentIncludesForm.livingAccommodationRadio.toBoolean, rentPartAddress = whatYourRentIncludesForm.rentPartAddressRadio.toBoolean, rentEmptyShell = whatYourRentIncludesForm.rentEmptyShellRadio.toBoolean, - rentIncBusinessRates = whatYourRentIncludesForm.rentIncBusinessRatesRadio.toBoolean, - rentIncWaterCharges = whatYourRentIncludesForm.rentIncWaterChargesRadio.toBoolean, - rentIncService = whatYourRentIncludesForm.rentIncServiceRadio.toBoolean, + rentIncBusinessRates = if(isOTCLease) None else whatYourRentIncludesForm.rentIncBusinessRatesRadio.toBooleanOption, + rentIncWaterCharges = if(isOTCLease) None else whatYourRentIncludesForm.rentIncWaterChargesRadio.toBooleanOption, + rentIncService = if(isOTCLease) None else whatYourRentIncludesForm.rentIncServiceRadio.toBooleanOption, bedroomNumbers = whatYourRentIncludesForm.bedroomNumbers match { case Some(value) if whatYourRentIncludesForm.livingAccommodationRadio == "true" => Some(value.toInt) case _ => None @@ -165,15 +165,15 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { Valid ) - def form: Form[WhatYourRentIncludesForm] = { + def form(isOTCLease: Boolean): Form[WhatYourRentIncludesForm] = { Form( mapping( livingAccommodationRadio -> radioText(livingAccommodationRadioError), rentPartAddressRadio -> radioText(rentPartAddressRadioError), rentEmptyShellRadio -> radioText(rentEmptyShellRadioError), - rentIncBusinessRatesRadio -> radioText(rentIncBusinessRatesRadioError), - rentIncWaterChargesRadio -> radioText(rentIncWaterChargesRadioError), - rentIncServiceRadio -> radioText(rentIncServiceRadioError), + rentIncBusinessRatesRadio -> optionalRadioText(rentIncBusinessRatesRadioError, isOTCLease), + rentIncWaterChargesRadio -> optionalRadioText(rentIncWaterChargesRadioError, isOTCLease), + rentIncServiceRadio -> optionalRadioText(rentIncServiceRadioError, isOTCLease), bedroomNumbers -> optional(text().transform[String](_.strip(), identity)) )(WhatYourRentIncludesForm.apply)(WhatYourRentIncludesForm.unapply) .verifying(isBedroomNumberValid) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala index af5ef29e..e94647d2 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala @@ -35,5 +35,18 @@ trait Formatters { override def unbind(key: String, value: String): Map[String, String] = Map(key -> value) } + + private[mappings] def optionalStringFormatter(args: Seq[String] = Seq.empty): Formatter[String] = new Formatter[String] { + + override def bind(key: String, data: Map[String, String]): Either[Seq[FormError], String] = + println(Console.YELLOW_B + data + Console.RESET) + data.get(key) match { + case None => Right("") + case Some(s) => Right(s.trim) + } + + override def unbind(key: String, value: String): Map[String, String] = + Map(key -> value) + } } diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala index fd7dfbf5..17cdde3e 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala @@ -22,4 +22,10 @@ import play.api.data.Forms.of trait Mappings extends Formatters { protected def radioText(errorKey: String = "error.required", args: Seq[String] = Seq.empty): FieldMapping[String] = of(stringFormatter(errorKey, args)) + + protected def optionalRadioText(errorKey: String = "error.required", isOptional: Boolean, args: Seq[String] = Seq.empty): FieldMapping[String] = + if (!isOptional) + of(stringFormatter(errorKey, args)) + else + of(optionalStringFormatter(args)) } diff --git a/app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala b/app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala index d0c845ce..3171fbec 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala @@ -56,6 +56,8 @@ class Navigator @Inject()() { case Some(_) => uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) case None => uk.gov.hmrc.ngrraldfrontend.controllers.routes.HowMuchIsTotalAnnualRentController.show(NormalMode) } + case "TotalOccupancyCost" if answers.get(TellUsAboutRentPage).nonEmpty => + uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatRentIncludesRatesWaterServiceController.show(NormalMode) case _ => answers.get(TellUsAboutRentPage) match { case Some(value) => uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) case None => uk.gov.hmrc.ngrraldfrontend.controllers.routes.AgreedRentChangeController.show(NormalMode) @@ -119,9 +121,19 @@ class Navigator @Inject()() { //TODO ADD A TECHNICAL DIFFICULTIES PAGE case None => ??? } - case RentDatesAgreePage => _ => uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) + case RentDatesAgreePage => answers => + answers.get(WhatIsYourRentBasedOnPage) match + case Some(value) if value.rentBased == "TotalOccupancyCost" => + uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatRentIncludesRatesWaterServiceController.show(NormalMode) + case _ => + uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) case WhatYourRentIncludesPage => _ => uk.gov.hmrc.ngrraldfrontend.controllers.routes.DoesYourRentIncludeParkingController.show(NormalMode) - case RentDatesAgreeStartPage => _ => uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) + case RentDatesAgreeStartPage => answers => + answers.get(WhatIsYourRentBasedOnPage) match + case Some(value) if value.rentBased == "TotalOccupancyCost" => + uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatRentIncludesRatesWaterServiceController.show(NormalMode) + case _ => + uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.show(NormalMode) case DoesYourRentIncludeParkingPage => answers => answers.get(DoesYourRentIncludeParkingPage) match { case Some(value) => value match { diff --git a/app/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesView.scala.html b/app/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesView.scala.html index 99bbb5f4..289cfa3d 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesView.scala.html +++ b/app/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesView.scala.html @@ -30,10 +30,10 @@ saveAndContinueButton: saveAndContinueButton ) -@(form:Form[WhatYourRentIncludesForm], radios1: Radios, radios2: Radios, radios3: Radios, radios4: Radios, radios5: Radios, radios6: Radios, propertyAddress: String, mode: Mode)(implicit request: RequestHeader, messages: Messages, appConfig: AppConfig) +@(form:Form[WhatYourRentIncludesForm], radios1: Radios, radios2: Radios, radios3: Radios, radios4: Option[Radios], radios5: Option[Radios], radios6: Option[Radios], propertyAddress: String, mode: Mode)(implicit request: RequestHeader, messages: Messages, appConfig: AppConfig) @layout(pageTitle = Some(messages("whatYourRentIncludes.title")), showBackLink = true, fullWidth = true) { - @formHelper(action = uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.submit(mode), Symbol("autoComplete") -> "off") { + @formHelper(action = if(radios4.nonEmpty){uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatYourRentIncludesController.submit(mode)} else {uk.gov.hmrc.ngrraldfrontend.controllers.routes.WhatRentIncludesRatesWaterServiceController.submit(mode)}, Symbol("autoComplete") -> "off") { @if(form.errors.nonEmpty) { @govukErrorSummary(ErrorSummaryViewModel(form)) } @@ -48,9 +48,15 @@

@messages("whatYourRentIncludes.title")

)) @govukRadios(radios2) @govukRadios(radios3) - @govukRadios(radios4) - @govukRadios(radios5) - @govukRadios(radios6) + @if(radios4.nonEmpty) { + @govukRadios(radios4.get) + } + @if(radios5.nonEmpty) { + @govukRadios(radios5.get) + } + @if(radios6.nonEmpty) { + @govukRadios(radios6.get) + } @saveAndContinueButton(msg = messages("service.continue"), isStartButton = false) diff --git a/conf/app.routes b/conf/app.routes index d2f59741..7ddf8e8f 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -112,6 +112,12 @@ POST /what-rent-includes uk.gov.hmrc.n GET /what-rent-includes/change uk.gov.hmrc.ngrraldfrontend.controllers.WhatYourRentIncludesController.show(mode: Mode = CheckMode) POST /what-rent-includes/change uk.gov.hmrc.ngrraldfrontend.controllers.WhatYourRentIncludesController.submit(mode: Mode = CheckMode) +#What your rent includes for Total Occupancy Cost lease +GET /what-rent-includes-rates-water-service uk.gov.hmrc.ngrraldfrontend.controllers.WhatRentIncludesRatesWaterServiceController.show(mode: Mode = NormalMode) +POST /what-rent-includes-rates-water-service uk.gov.hmrc.ngrraldfrontend.controllers.WhatRentIncludesRatesWaterServiceController.submit(mode: Mode = NormalMode) +GET /what-rent-includes-rates-water-service/change uk.gov.hmrc.ngrraldfrontend.controllers.WhatRentIncludesRatesWaterServiceController.show(mode: Mode = CheckMode) +POST /what-rent-includes-rates-water-service/change uk.gov.hmrc.ngrraldfrontend.controllers.WhatRentIncludesRatesWaterServiceController.submit(mode: Mode = CheckMode) + #Does your rent include parking spaces or garages GET /does-rent-include-parking-spaces-or-garages uk.gov.hmrc.ngrraldfrontend.controllers.DoesYourRentIncludeParkingController.show(mode: Mode = NormalMode) POST /does-rent-include-parking-spaces-or-garages uk.gov.hmrc.ngrraldfrontend.controllers.DoesYourRentIncludeParkingController.submit(mode: Mode = NormalMode) From b0d2e6e5c12761805be537a332e3feb7b52c4357 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Wed, 8 Oct 2025 13:39:32 +0100 Subject: [PATCH 2/9] NGR-1130: refactory bedroom number validation --- ...tIncludesRatesWaterServiceController.scala | 2 +- .../forms/WhatYourRentIncludesForm.scala | 49 ++++++++++--------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala index efd9732a..7cc405ec 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala @@ -90,7 +90,7 @@ class WhatRentIncludesRatesWaterServiceController @Inject()(whatYourRentIncludes whatYourRentIncludesForm => for { updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) - .set(WhatYourRentIncludesPage, formToAnswers(whatYourRentIncludesForm, isOTCLease = false))) + .set(WhatYourRentIncludesPage, formToAnswers(whatYourRentIncludesForm, isOTCLease = true))) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(WhatYourRentIncludesPage, mode, updatedAnswers)) ) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala index 434554be..3b783e34 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala @@ -16,9 +16,10 @@ package uk.gov.hmrc.ngrraldfrontend.models.forms -import play.api.data.Forms.{mapping, optional, text} +import play.api.data.Forms.{mapping, of, optional, text} +import play.api.data.format.Formatter import play.api.data.validation.{Constraint, Invalid, Valid} -import play.api.data.{Form, Forms} +import play.api.data.{Form, FormError, Forms} import play.api.i18n.Messages import play.api.libs.json.{Json, OFormat} import uk.gov.hmrc.ngrraldfrontend.models.WhatYourRentIncludes @@ -145,25 +146,30 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { } ) - private def isBedroomNumberValid[A]: Constraint[A] = - Constraint((input: A) => - val whatYourRentIncludesForm = input.asInstanceOf[WhatYourRentIncludesForm] - val bedroomNumber: Option[String] = whatYourRentIncludesForm.bedroomNumbers - if (whatYourRentIncludesForm.livingAccommodationRadio.equals("true")) { - if (bedroomNumber.isEmpty) - Invalid("whatYourRentIncludes.bedroom.number.required.error") - else if (!bedroomNumber.get.matches(wholePositiveNumberRegexp.pattern())) - Invalid("whatYourRentIncludes.bedroom.number.invalid.error") - else if (bedroomNumber.get.toDoubleOption.getOrElse(0d) > 99) - Invalid("whatYourRentIncludes.bedroom.number.maximum.error") - else if (bedroomNumber.get.toLong < 1) - Invalid("whatYourRentIncludes.bedroom.number.minimum.error") - else - Valid + private def bedroomFormatter(args: Seq[String] = Seq.empty): Formatter[Option[String]] = new Formatter[Option[String]] { + override def bind(key: String, data: Map[String, String]): Either[Seq[FormError], Option[String]] = + val hasLivingAccommodation = data.get(livingAccommodationRadio).exists(_ == "true") + data.get(key) match { + case None if hasLivingAccommodation => Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.required.error", args))) + case Some(s) if hasLivingAccommodation => isBedroomNumberValid(s.trim, key, args) + case Some(s) => Right(Some(s)) } - else - Valid - ) + + override def unbind(key: String, value: Option[String]): Map[String, String] = + Map(key -> value.getOrElse("")) + } + + private def isBedroomNumberValid(bedroomNumber: String, key: String, args: Seq[String] = Seq.empty) = + if (bedroomNumber.isEmpty) + Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.required.error", args))) + else if (!bedroomNumber.matches(wholePositiveNumberRegexp.pattern())) + Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.invalid.error", args))) + else if (bedroomNumber.toDoubleOption.getOrElse(0d) > 99) + Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.maximum.error", args))) + else if (bedroomNumber.toLong < 1) + Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.minimum.error", args))) + else + Right(Some(bedroomNumber)) def form(isOTCLease: Boolean): Form[WhatYourRentIncludesForm] = { Form( @@ -174,9 +180,8 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { rentIncBusinessRatesRadio -> optionalRadioText(rentIncBusinessRatesRadioError, isOTCLease), rentIncWaterChargesRadio -> optionalRadioText(rentIncWaterChargesRadioError, isOTCLease), rentIncServiceRadio -> optionalRadioText(rentIncServiceRadioError, isOTCLease), - bedroomNumbers -> optional(text().transform[String](_.strip(), identity)) + bedroomNumbers -> of(bedroomFormatter()) )(WhatYourRentIncludesForm.apply)(WhatYourRentIncludesForm.unapply) - .verifying(isBedroomNumberValid) ) } } From 69f6aa1aff1613a4ce6f2cb4741ff9f9a161bbd0 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Wed, 8 Oct 2025 16:04:49 +0100 Subject: [PATCH 3/9] NGR-1130: adding tests --- ...tIncludesRatesWaterServiceController.scala | 11 +- .../WhatYourRentIncludesController.scala | 11 +- .../forms/WhatYourRentIncludesForm.scala | 1 + .../models/forms/mappings/Formatters.scala | 1 - ...ludesRatesWaterServiceControllerSpec.scala | 229 ++++++++++++++++++ .../WhatYourRentIncludesControllerSpec.scala | 1 + .../ngrraldfrontend/helpers/TestData.scala | 6 +- .../models/WhatYourRentIncludesSpec.scala | 12 +- .../forms/WhatYourRentIncludesFormSpec.scala | 122 ++++++++-- .../views/WhatYourRentIncludesViewSpec.scala | 8 +- 10 files changed, 350 insertions(+), 52 deletions(-) create mode 100644 test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala index 7cc405ec..94f46fb9 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceController.scala @@ -68,16 +68,9 @@ class WhatRentIncludesRatesWaterServiceController @Inject()(whatYourRentIncludes (authenticate andThen getData).async { implicit request => form(isOTCLease = true).bindFromRequest().fold( formWithErrors => { - val correctedFormErrors = formWithErrors.errors.map { formError => - (formError.key, formError.messages) match - case ("", messages) => - formError.copy(key = "bedroomNumbers") - case _ => formError - } - val formWithCorrectedErrors = formWithErrors.copy(errors = correctedFormErrors) Future.successful(BadRequest(whatYourRentIncludesView( - form = formWithCorrectedErrors, - radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithCorrectedErrors, inputText)), + form = formWithErrors, + radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithErrors, inputText)), radios2 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio2), radios3 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio3), radios4 = None, diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala index c66948a8..cf25d91f 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesController.scala @@ -68,16 +68,9 @@ class WhatYourRentIncludesController @Inject()(whatYourRentIncludesView: WhatYou (authenticate andThen getData).async { implicit request => form(isOTCLease = false).bindFromRequest().fold( formWithErrors => { - val correctedFormErrors = formWithErrors.errors.map { formError => - (formError.key, formError.messages) match - case ("", messages) => - formError.copy(key = "bedroomNumbers") - case _ => formError - } - val formWithCorrectedErrors = formWithErrors.copy(errors = correctedFormErrors) Future.successful(BadRequest(whatYourRentIncludesView( - form = formWithCorrectedErrors, - radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithCorrectedErrors, inputText)), + form = formWithErrors, + radios1 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio1(formWithErrors, inputText)), radios2 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio2), radios3 = buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio3), radios4 = Some(buildRadios(formWithErrors, WhatYourRentIncludesForm.ngrRadio4)), diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala index 3b783e34..c3fc55a6 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala @@ -153,6 +153,7 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { case None if hasLivingAccommodation => Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.required.error", args))) case Some(s) if hasLivingAccommodation => isBedroomNumberValid(s.trim, key, args) case Some(s) => Right(Some(s)) + case None => Right(None) } override def unbind(key: String, value: Option[String]): Map[String, String] = diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala index e94647d2..8cf28590 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala @@ -39,7 +39,6 @@ trait Formatters { private[mappings] def optionalStringFormatter(args: Seq[String] = Seq.empty): Formatter[String] = new Formatter[String] { override def bind(key: String, data: Map[String, String]): Either[Seq[FormError], String] = - println(Console.YELLOW_B + data + Console.RESET) data.get(key) match { case None => Right("") case Some(s) => Right(s.trim) diff --git a/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala new file mode 100644 index 00000000..21ace61a --- /dev/null +++ b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala @@ -0,0 +1,229 @@ +/* + * 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.controllers + +import org.jsoup.Jsoup +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.when +import play.api.http.Status.{BAD_REQUEST, OK, SEE_OTHER} +import play.api.test.FakeRequest +import play.api.test.Helpers.{await, contentAsString, defaultAwaitTimeout, redirectLocation, status} +import uk.gov.hmrc.auth.core.Nino +import uk.gov.hmrc.http.{HeaderNames, NotFoundException} +import uk.gov.hmrc.ngrraldfrontend.helpers.ControllerSpecSupport +import uk.gov.hmrc.ngrraldfrontend.models.AgreementType.NewAgreement +import uk.gov.hmrc.ngrraldfrontend.models.registration.CredId +import uk.gov.hmrc.ngrraldfrontend.models.{AuthenticatedUserRequest, NormalMode, UserAnswers} +import uk.gov.hmrc.ngrraldfrontend.pages.WhatYourRentIncludesPage +import uk.gov.hmrc.ngrraldfrontend.views.html.WhatYourRentIncludesView +import uk.gov.hmrc.ngrraldfrontend.views.html.components.NGRCharacterCountComponent + +import scala.concurrent.Future + +class WhatRentIncludesRatesWaterServiceControllerSpec extends ControllerSpecSupport { + val pageTitle = "What your rent includes" + val view: WhatYourRentIncludesView = inject[WhatYourRentIncludesView] + val mockNGRCharacterCountComponent: NGRCharacterCountComponent = inject[NGRCharacterCountComponent] + val controllerNoProperty: WhatRentIncludesRatesWaterServiceController = new WhatRentIncludesRatesWaterServiceController( + view, + fakeAuth, + mockInputText, + fakeData(None), + mockSessionRepository, + mockNavigator, + mcc)(mockConfig, ec) + val controllerProperty: Option[UserAnswers] => WhatRentIncludesRatesWaterServiceController = answers => new WhatRentIncludesRatesWaterServiceController( + view, + fakeAuth, + mockInputText, + fakeDataProperty(Some(property), answers), + mockSessionRepository, + mockNavigator, + mcc)(mockConfig, ec) + val whatYourRentIncludesAnswersAllYes: Option[UserAnswers] = UserAnswers("id").set(WhatYourRentIncludesPage, whatYourRentIncludesModelAllYes).toOption + val whatYourRentIncludesAnswersAllNo: Option[UserAnswers] = UserAnswers("id").set(WhatYourRentIncludesPage, whatYourRentIncludesModelAllNo).toOption + + "Tell us about what your rent includes rates water service controller" must { + "method show" must { + "Return OK and the correct view" in { + val result = controllerProperty(None).show(NormalMode)(authenticatedFakeRequest) + status(result) mustBe OK + val content = contentAsString(result) + content must include(pageTitle) + } + "Return OK and the correct view with prepopulated answers all Yes" in { + val result = controllerProperty(whatYourRentIncludesAnswersAllYes).show(NormalMode)(authenticatedFakeRequest) + status(result) mustBe OK + val content = contentAsString(result) + val document = Jsoup.parse(content) + document.select("input[type=radio][name=livingAccommodationRadio][value=true]").hasAttr("checked") mustBe true + document.select("input[type=radio][name=rentPartAddressRadio][value=true]").hasAttr("checked") mustBe true + document.select("input[type=radio][name=rentEmptyShellRadio][value=true]").hasAttr("checked") mustBe true + document.select("input[type=text][name=bedroomNumbers]").attr("value") mustBe "5" + document.select("input[type=radio][name=rentIncBusinessRatesRadio]").isEmpty mustBe true + document.select("input[type=radio][name=rentIncWaterChargesRadio]").isEmpty mustBe true + document.select("input[type=radio][name=rentIncServiceRadio]").isEmpty mustBe true + } + "Return OK and the correct view with prepopulated answers all No" in { + val result = controllerProperty(whatYourRentIncludesAnswersAllNo).show(NormalMode)(authenticatedFakeRequest) + status(result) mustBe OK + val content = contentAsString(result) + val document = Jsoup.parse(content) + document.select("input[type=radio][name=livingAccommodationRadio][value=false]").hasAttr("checked") mustBe true + document.select("input[type=radio][name=rentPartAddressRadio][value=false]").hasAttr("checked") mustBe true + document.select("input[type=radio][name=rentEmptyShellRadio][value=false]").hasAttr("checked") mustBe true + document.select("input[type=radio][name=rentIncBusinessRatesRadio]").isEmpty mustBe true + document.select("input[type=radio][name=rentIncWaterChargesRadio]").isEmpty mustBe true + document.select("input[type=radio][name=rentIncServiceRadio]").isEmpty mustBe true + } + "Return NotFoundException when property is not found in the mongo" in { + when(mockNGRConnector.getLinkedProperty(any[CredId])(any())).thenReturn(Future.successful(None)) + val exception = intercept[NotFoundException] { + await(controllerNoProperty.show(NormalMode)(authenticatedFakeRequest)) + } + exception.getMessage contains "Could not find answers in backend mongo" mustBe true + } + } + + "method submit" must { + "Return SEE_OTHER and the correct view after submitting with all radio buttons selected" in { + when(mockSessionRepository.set(any())).thenReturn(Future.successful(true)) + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "6" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/does-rent-include-parking-spaces-or-garages") + }) + status(result) mustBe SEE_OTHER + redirectLocation(result) mustBe Some(routes.DoesYourRentIncludeParkingController.show(NormalMode).url) + } + "Return SEE_OTHER and the correct view after submitting with all radio buttons selected but no bedrooms" in { + when(mockSessionRepository.set(any())).thenReturn(Future.successful(true)) + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "false", + "rentPartAddressRadio" -> "true", + "rentEmptyShellRadio" -> "false" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/does-rent-include-parking-spaces-or-garages") + }) + status(result) mustBe SEE_OTHER + redirectLocation(result) mustBe Some(routes.DoesYourRentIncludeParkingController.show(NormalMode).url) + } + "Return Form with Errors when no radio button is selected" in { + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/what-rent-includes") + }) + status(result) mustBe BAD_REQUEST + val content = contentAsString(result) + content must include(pageTitle) + content must include("Select yes if your rent includes any living accommodation") + } + "Return Form with Errors when bedroom numbers is not provide" in { + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/what-rent-includes") + }) + status(result) mustBe BAD_REQUEST + val content = contentAsString(result) + content must include(pageTitle) + content must include("Enter how many bedrooms the living accommodation has") + } + "Return Form with Errors when bedroom numbers is not numeric" in { + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "AS&" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/what-rent-includes") + }) + status(result) mustBe BAD_REQUEST + val content = contentAsString(result) + content must include(pageTitle) + content must include("How many bedrooms must be a number, like 6") + } + "Return Form with Errors when bedroom numbers is less than 1" in { + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "0" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/what-rent-includes") + }) + status(result) mustBe BAD_REQUEST + val content = contentAsString(result) + content must include(pageTitle) + content must include("How many bedrooms must be 1 or more") + } + "Return Form with Errors when bedroom numbers is greater than 99" in { + val result = controllerProperty(None).submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "100" + ) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some("")))) + result.map(result => { + result.header.headers.get("Location") mustBe Some("/ngr-rald-frontend/what-rent-includes") + }) + status(result) mustBe BAD_REQUEST + val content = contentAsString(result) + content must include(pageTitle) + content must include("How many bedrooms must be 99 or less") + } + "Return Exception if no address is in the mongo" in { + + val exception = intercept[NotFoundException] { + await(controllerNoProperty.submit(NormalMode)(AuthenticatedUserRequest(FakeRequest(routes.WhatRentIncludesRatesWaterServiceController.submit(NormalMode)) + .withFormUrlEncodedBody(("what-type-of-agreement-radio", "")) + .withHeaders(HeaderNames.authorisation -> "Bearer 1"), None, None, None, Some(property), credId = Some(credId.value), None, None, nino = Nino(true, Some(""))))) + } + exception.getMessage contains "Could not find answers in backend mongo" mustBe true + } + } + } +} diff --git a/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesControllerSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesControllerSpec.scala index 40eeada8..1b0d5dfb 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesControllerSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatYourRentIncludesControllerSpec.scala @@ -75,6 +75,7 @@ class WhatYourRentIncludesControllerSpec extends ControllerSpecSupport { document.select("input[type=radio][name=rentIncBusinessRatesRadio][value=true]").hasAttr("checked") mustBe true document.select("input[type=radio][name=rentIncWaterChargesRadio][value=true]").hasAttr("checked") mustBe true document.select("input[type=radio][name=rentIncServiceRadio][value=true]").hasAttr("checked") mustBe true + document.select("input[type=text][name=bedroomNumbers]").attr("value") mustBe "5" } "Return OK and the correct view with prepopulated answers all No" in { val result = controllerProperty(whatYourRentIncludesAnswersAllNo).show(NormalMode)(authenticatedFakeRequest) diff --git a/test/uk/gov/hmrc/ngrraldfrontend/helpers/TestData.scala b/test/uk/gov/hmrc/ngrraldfrontend/helpers/TestData.scala index 7e269f43..44c68ea0 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/helpers/TestData.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/helpers/TestData.scala @@ -144,11 +144,11 @@ trait TestData { val rentDatesAgreeStartModel: RentDatesAgreeStart = RentDatesAgreeStart("2025-01-01", "2025-02-02") - val whatYourRentIncludesModelAllYes: WhatYourRentIncludes = WhatYourRentIncludes(true,true,true,true,true,true,Some(5)) + val whatYourRentIncludesModelAllYes: WhatYourRentIncludes = WhatYourRentIncludes(true, true, true, Some(true), Some(true), Some(true), Some(5)) - val whatYourRentIncludesModelAllNo: WhatYourRentIncludes = WhatYourRentIncludes(false,false,false,false,false,false,None) + val whatYourRentIncludesModelAllNo: WhatYourRentIncludes = WhatYourRentIncludes(false, false, false, Some(false), Some(false), Some(false), None) - val rentBasedOnModel: RentBasedOn = RentBasedOn("Other",Some("The rent was agreed")) + val rentBasedOnModel: RentBasedOn = RentBasedOn("Other", Some("The rent was agreed")) val repairsAndInsuranceModel = RepairsAndInsurance("You", "Landlord", "YouAndLandlord") } \ No newline at end of file diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludesSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludesSpec.scala index 57e6a70c..5d38b346 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludesSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/WhatYourRentIncludesSpec.scala @@ -28,9 +28,9 @@ class WhatYourRentIncludesSpec extends PlaySpec { livingAccommodation = true, rentPartAddress = false, rentEmptyShell = true, - rentIncBusinessRates = false, - rentIncWaterCharges = true, - rentIncService = false, + rentIncBusinessRates = Some(false), + rentIncWaterCharges = Some(true), + rentIncService = Some(false), bedroomNumbers = Some(6) ) @@ -69,9 +69,9 @@ class WhatYourRentIncludesSpec extends PlaySpec { livingAccommodation = false, rentPartAddress = true, rentEmptyShell = false, - rentIncBusinessRates = true, - rentIncWaterCharges = false, - rentIncService = true, + rentIncBusinessRates = Some(true), + rentIncWaterCharges = Some(false), + rentIncService = Some(true), bedroomNumbers = None ) diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesFormSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesFormSpec.scala index 7d9b181b..c9d484cd 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesFormSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesFormSpec.scala @@ -19,11 +19,12 @@ package uk.gov.hmrc.ngrraldfrontend.models.forms import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec import play.api.data.FormError +import play.api.libs.json.Json class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "WhatYourRentIncludesForm" should { - "bind successfully with all radio inputs selected" in { + "bind successfully with all radio inputs selected for not OTC lease" in { val data = Map( "livingAccommodationRadio" -> "true", "rentPartAddressRadio" -> "false", @@ -33,12 +34,25 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe false boundForm.value shouldBe Some(WhatYourRentIncludesForm("true", "false", "true", "false", "false", "true", Some("6"))) } + "bind successfully with all radio inputs selected for OTC lease" in { + val data = Map( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "bedroomNumbers" -> "6" + ) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = true).bind(data) + + boundForm.hasErrors shouldBe false + boundForm.value shouldBe Some(WhatYourRentIncludesForm("true", "false", "true", "", "", "", Some("6"))) + } + "fail to bind when livingAccommodationRadio input is missing" in { val data = Map( "livingAccommodationRadio" -> "", @@ -49,11 +63,41 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("livingAccommodationRadio", List("whatYourRentIncludes.radio.1.required"))) } + "fail to bind when no radios are selected for OTC lease" in { + val data = Map( + "livingAccommodationRadio" -> "", + "rentPartAddressRadio" -> "", + "rentEmptyShellRadio" -> "", + "bedroomNumbers" -> "" + ) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = true).bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors.size shouldBe 3 + boundForm.errors should contain(FormError("livingAccommodationRadio", List("whatYourRentIncludes.radio.1.required"))) + boundForm.errors should contain(FormError("rentPartAddressRadio", List("whatYourRentIncludes.radio.2.required"))) + boundForm.errors should contain(FormError("rentEmptyShellRadio", List("whatYourRentIncludes.radio.3.required"))) + } + "fail to bind when 2 radios are unselected and bedroom numbers is missing for OTC lease" in { + val data = Map( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "", + "rentEmptyShellRadio" -> "", + "bedroomNumbers" -> "" + ) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = true).bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors.size shouldBe 3 + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.required.error"))) + boundForm.errors should contain(FormError("rentPartAddressRadio", List("whatYourRentIncludes.radio.2.required"))) + boundForm.errors should contain(FormError("rentEmptyShellRadio", List("whatYourRentIncludes.radio.3.required"))) + } "fail to bind when rentPartAddressRadio input is missing" in { val data = Map( "livingAccommodationRadio" -> "true", @@ -64,7 +108,7 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("rentPartAddressRadio", List("whatYourRentIncludes.radio.2.required"))) @@ -79,7 +123,7 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("rentEmptyShellRadio", List("whatYourRentIncludes.radio.3.required"))) @@ -94,7 +138,7 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("rentIncBusinessRatesRadio", List("whatYourRentIncludes.radio.4.required"))) @@ -109,7 +153,7 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("rentIncWaterChargesRadio", List("whatYourRentIncludes.radio.5.required"))) @@ -124,7 +168,7 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "", "bedroomNumbers" -> "6" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true boundForm.errors should contain(FormError("rentIncServiceRadio", List("whatYourRentIncludes.radio.6.required"))) @@ -139,10 +183,10 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.required.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.required.error"))) } "fail to bind when bedroomNumbers input is falset numeric" in { val data = Map( @@ -154,10 +198,10 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "A^&" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.invalid.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.invalid.error"))) } "fail to bind when bedroomNumbers input is mines" in { val data = Map( @@ -169,10 +213,10 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "-2" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.invalid.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.invalid.error"))) } "fail to bind when bedroomNumbers input is less than 1" in { val data = Map( @@ -184,10 +228,10 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "0" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.minimum.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.minimum.error"))) } "fail to bind when bedroomNumbers input is greater than 99" in { val data = Map( @@ -199,10 +243,10 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "100" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.maximum.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.maximum.error"))) } "fail to bind when bedroomNumbers input is 30 digits long" in { val data = Map( @@ -214,10 +258,48 @@ class WhatYourRentIncludesFormSpec extends AnyWordSpec with Matchers { "rentIncServiceRadio" -> "true", "bedroomNumbers" -> "123123123123123123123123345678" ) - val boundForm = WhatYourRentIncludesForm.form.bind(data) + val boundForm = WhatYourRentIncludesForm.form(isOTCLease = false).bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("", List("whatYourRentIncludes.bedroom.number.maximum.error"))) + boundForm.errors should contain(FormError("bedroomNumbers", List("whatYourRentIncludes.bedroom.number.maximum.error"))) + } + + "serialize to JSON correctly" in { + val form = WhatYourRentIncludesForm("true", "false", "true", "false", "false", "true", Some("6")) + val json = Json.toJson(form) + + json shouldBe Json.obj( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "rentIncBusinessRatesRadio" -> "false", + "rentIncWaterChargesRadio" -> "false", + "rentIncServiceRadio" -> "true", + "bedroomNumbers" -> "6" + ) + } + + "deserialize from JSON correctly" in { + val json = Json.obj( + "livingAccommodationRadio" -> "true", + "rentPartAddressRadio" -> "false", + "rentEmptyShellRadio" -> "true", + "rentIncBusinessRatesRadio" -> "false", + "rentIncWaterChargesRadio" -> "false", + "rentIncServiceRadio" -> "true", + "bedroomNumbers" -> "6" + ) + val result = json.validate[WhatYourRentIncludesForm] + + result.isSuccess shouldBe true + result.get shouldBe WhatYourRentIncludesForm("true", "false", "true", "false", "false", "true", Some("6")) + } + + "fail deserialization if businessRatesBillRadio is missing" in { + val json = Json.obj() + val result = json.validate[WhatYourRentIncludesForm] + + result.isError shouldBe true } } } diff --git a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala index 2c7d2a78..1599b3a5 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala @@ -68,14 +68,14 @@ class WhatYourRentIncludesViewSpec extends ViewBaseSpec { val address = "5 Brixham Marina, Berry Head Road, Brixham, Devon, TQ5 9BW" val mockInputText: InputText = inject[InputText] - val form = WhatYourRentIncludesForm.form.fillAndValidate(WhatYourRentIncludesForm("true", "true", "true", "true", "true", "true", Some("6"))) + val form = WhatYourRentIncludesForm.form(isOTCLease = false).fillAndValidate(WhatYourRentIncludesForm("true", "true", "true", "true", "true", "true", Some("6"))) private val radio1: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio1(form, inputText)) private val radio2: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio2) private val radio3: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio3) - private val radio4: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio4) - private val radio5: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio5) - private val radio6: Radios = buildRadios(form, WhatYourRentIncludesForm.ngrRadio6) + private val radio4: Option[Radios] = Some(buildRadios(form, WhatYourRentIncludesForm.ngrRadio4)) + private val radio5: Option[Radios] = Some(buildRadios(form, WhatYourRentIncludesForm.ngrRadio5)) + private val radio6: Option[Radios] = Some(buildRadios(form, WhatYourRentIncludesForm.ngrRadio6)) "WhatYourRentIncludesView" must { val whatYourRentIncludesView = view(form, radio1, radio2, radio3, radio4, radio5, radio6, address, NormalMode) From 2921a280585e2634a5be37f168c648f71159c271 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Wed, 8 Oct 2025 16:57:14 +0100 Subject: [PATCH 4/9] NGR-1130: refactory landlord relationship validation --- .../controllers/LandlordController.scala | 14 +---- .../models/forms/LandlordForm.scala | 56 +++++++++---------- .../forms/WhatYourRentIncludesForm.scala | 2 +- conf/messages | 2 +- .../models/forms/LandlordFormSpec.scala | 10 ++-- 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/LandlordController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/LandlordController.scala index bb453c83..ea41dd05 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/LandlordController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/LandlordController.scala @@ -65,20 +65,10 @@ class LandlordController @Inject()(view: LandlordView, .bindFromRequest() .fold( formWithErrors => - val correctedFormErrors = formWithErrors.errors.map { formError => - (formError.key, formError.messages) match - case ("", messages) if messages.contains("landlord.radio.emptyText.error") => - formError.copy(key = "landlord-relationship") - case ("", messages) if messages.contains("landlord.radio.tooLong.error") => - formError.copy(key = "landlord-relationship") - case _ => - formError - } - val formWithCorrectedErrors = formWithErrors.copy(errors = correctedFormErrors) Future.successful(BadRequest(view( selectedPropertyAddress = request.property.addressFull, - formWithCorrectedErrors, - buildRadios(formWithErrors, LandlordForm.landlordRadio(formWithCorrectedErrors, ngrCharacterCountComponent)), + formWithErrors, + buildRadios(formWithErrors, LandlordForm.landlordRadio(formWithErrors, ngrCharacterCountComponent)), mode ))), landlordForm => diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordForm.scala index 37fd5bad..0e1cd0a4 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordForm.scala @@ -16,8 +16,9 @@ package uk.gov.hmrc.ngrraldfrontend.models.forms -import play.api.data.Form -import play.api.data.Forms.{mapping, optional, text} +import play.api.data.{Form, FormError} +import play.api.data.Forms.{mapping, optional, text, of} +import play.api.data.format.Formatter import play.api.data.validation.{Constraint, Invalid, Valid} import play.api.i18n.* import play.api.libs.json.{Json, OFormat} @@ -39,7 +40,7 @@ object LandlordForm extends CommonFormValidators with Mappings{ private lazy val landlordNameEmptyError = "landlord.name.empty.error" private lazy val landlordNameTooLongError = "landlord.name.empty.tooLong.error" private lazy val radioUnselectedError = "landlord.radio.empty.error" - private lazy val landlordRelationshipEmptyError = "landlord.radio.emptyText.error" + private lazy val landlordRelationshipEmptyError = "landlord.relationship.empty.error" private lazy val landlordRelationshipTooLongError = "landlord.radio.tooLong.error" private val landlord = "landlord-name-value" @@ -96,24 +97,28 @@ object LandlordForm extends CommonFormValidators with Mappings{ landlordForm.hasRelationship.toBoolean, if (landlordForm.hasRelationship.toBoolean) landlordForm.landlordRelationship else None ) - - private def isLandlordRelationshipTextEmpty[A]: Constraint[A] = - Constraint((input: A) => - val landlordForm = input.asInstanceOf[LandlordForm] - if (landlordForm.hasRelationship.equals("true") && landlordForm.landlordRelationship.getOrElse("").isBlank) - Invalid(landlordRelationshipEmptyError) - else - Valid - ) - private def isLandlordRelationshipTextMaxLength[A]: Constraint[A] = - Constraint((input: A) => - val landlordForm = input.asInstanceOf[LandlordForm] - if (landlordForm.hasRelationship.equals("true") && landlordForm.landlordRelationship.getOrElse("").length > 250) - Invalid(landlordRelationshipTooLongError) - else - Valid - ) + private def relationshipFormatter(args: Seq[String] = Seq.empty): Formatter[Option[String]] = new Formatter[Option[String]] { + override def bind(key: String, data: Map[String, String]): Either[Seq[FormError], Option[String]] = + val hasRelationship = data.get(landlordRadio).exists(_ == "true") + data.get(key) match { + case None if hasRelationship => Left(Seq(FormError(key, landlordRelationshipEmptyError, args))) + case Some(s) if hasRelationship => isLandlordRelationshipValid(s.trim, key, args) + case Some(s) => Right(Some(s)) + case None => Right(None) + } + + override def unbind(key: String, value: Option[String]): Map[String, String] = + Map(key -> value.getOrElse("")) + } + + private def isLandlordRelationshipValid(relationship: String, key: String, args: Seq[String] = Seq.empty): Either[Seq[FormError], Option[String]] = + if (relationship.isEmpty) + Left(Seq(FormError(key, landlordRelationshipEmptyError, args))) + else if (relationship.length > 250) + Left(Seq(FormError(key, landlordRelationshipTooLongError, args))) + else + Right(Some(relationship)) def form: Form[LandlordForm] = { Form( @@ -127,17 +132,8 @@ object LandlordForm extends CommonFormValidators with Mappings{ ), landlordRadio -> radioText(radioUnselectedError), - landlordRelationshipYes -> optional( - play.api.data.Forms.text - .transform[String](_.strip(), identity) - ) + landlordRelationshipYes -> of(relationshipFormatter()) )(LandlordForm.apply)(LandlordForm.unapply) - .verifying( - firstError( - isLandlordRelationshipTextEmpty, - isLandlordRelationshipTextMaxLength - ) - ) ) } } diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala index c3fc55a6..c8243b27 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/WhatYourRentIncludesForm.scala @@ -160,7 +160,7 @@ object WhatYourRentIncludesForm extends CommonFormValidators with Mappings { Map(key -> value.getOrElse("")) } - private def isBedroomNumberValid(bedroomNumber: String, key: String, args: Seq[String] = Seq.empty) = + private def isBedroomNumberValid(bedroomNumber: String, key: String, args: Seq[String] = Seq.empty): Either[Seq[FormError], Option[String]] = if (bedroomNumber.isEmpty) Left(Seq(FormError(key, "whatYourRentIncludes.bedroom.number.required.error", args))) else if (!bedroomNumber.matches(wholePositiveNumberRegexp.pattern())) diff --git a/conf/messages b/conf/messages index a06df94c..e63b7297 100644 --- a/conf/messages +++ b/conf/messages @@ -60,7 +60,7 @@ landlord.p2 = Do you have a relationship with the landlord other than as a tenan landlord.name.empty.error = Enter the landlord''s full name landlord.name.empty.tooLong.error = Landlord''s full name must be 50 characters or less landlord.radio.empty.error = Select yes if you have any relationship with landlord -landlord.radio.emptyText.error = Tell us what your relationship with the landlord is +landlord.relationship.empty.error = Tell us what your relationship with the landlord is landlord.radio.tooLong.error = Maximum character allowed is 250 landlord.radio.yes = Can you tell us what your relationship with the landlord is? landlord.radio.yes.hint = For example, the landlord is a family member, business partner, shared director or company pension fund diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordFormSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordFormSpec.scala index 629a4655..f469f472 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordFormSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/LandlordFormSpec.scala @@ -27,8 +27,7 @@ class LandlordFormSpec extends AnyFlatSpec with Matchers { val validData = Map( "landlord-name-value" -> "John Doe", - "landlord-radio" -> "false", - "landlord-relationship" -> "" + "landlord-radio" -> "false" ) "LandlordForm" should "bind valid data successfully" in { @@ -38,7 +37,7 @@ class LandlordFormSpec extends AnyFlatSpec with Matchers { boundForm.value shouldBe Some(LandlordForm("John Doe", "false", None)) } - it should "fail when landlord name is missing" in { + it should "fail when landlord name and relationship are missing" in { val data = Map( "landlord-name-value" -> "", "landlord-radio" -> "true", @@ -46,7 +45,8 @@ class LandlordFormSpec extends AnyFlatSpec with Matchers { ) val boundForm = LandlordForm.form.bind(data) - boundForm.errors shouldBe List(FormError("landlord-name-value", List("landlord.name.empty.error"), ArraySeq("landlord-name-value"))) + boundForm.errors shouldBe List(FormError("landlord-name-value", List("landlord.name.empty.error"), ArraySeq("landlord-name-value")), + FormError("landlord-relationship", List("landlord.relationship.empty.error"))) } it should "fail when landlord type (radio) is missing" in { @@ -65,7 +65,7 @@ class LandlordFormSpec extends AnyFlatSpec with Matchers { val boundForm = LandlordForm.form.bind(data) - boundForm.errors shouldBe List(FormError("", List("landlord.radio.emptyText.error"), List())) + boundForm.errors shouldBe List(FormError("landlord-relationship", List("landlord.relationship.empty.error"), List())) } it should "pass when 'Yes' is selected and description is provided" in { From 6dd6b63dd7719a6a1bad8f88cc268ea414496b29 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 9 Oct 2025 09:23:36 +0100 Subject: [PATCH 5/9] NGR-1130: remove unused imports --- .../WhatRentIncludesRatesWaterServiceControllerSpec.scala | 1 - test/uk/gov/hmrc/ngrraldfrontend/views/LandlordViewSpec.scala | 1 - .../ngrraldfrontend/views/WhatIsYourRentBasedOnViewSpec.scala | 1 - .../ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala | 1 - 4 files changed, 4 deletions(-) diff --git a/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala index 21ace61a..c402bfc2 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/controllers/WhatRentIncludesRatesWaterServiceControllerSpec.scala @@ -37,7 +37,6 @@ import scala.concurrent.Future class WhatRentIncludesRatesWaterServiceControllerSpec extends ControllerSpecSupport { val pageTitle = "What your rent includes" val view: WhatYourRentIncludesView = inject[WhatYourRentIncludesView] - val mockNGRCharacterCountComponent: NGRCharacterCountComponent = inject[NGRCharacterCountComponent] val controllerNoProperty: WhatRentIncludesRatesWaterServiceController = new WhatRentIncludesRatesWaterServiceController( view, fakeAuth, diff --git a/test/uk/gov/hmrc/ngrraldfrontend/views/LandlordViewSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/views/LandlordViewSpec.scala index f9e7b304..fe67d393 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/views/LandlordViewSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/views/LandlordViewSpec.scala @@ -25,7 +25,6 @@ import uk.gov.hmrc.ngrraldfrontend.models.components.* import uk.gov.hmrc.ngrraldfrontend.models.components.NGRRadio.buildRadios import uk.gov.hmrc.ngrraldfrontend.models.forms.LandlordForm import uk.gov.hmrc.ngrraldfrontend.views.html.LandlordView -import uk.gov.hmrc.ngrraldfrontend.views.html.components.NGRCharacterCountComponent class LandlordViewSpec extends ViewBaseSpec { lazy val view: LandlordView = inject[LandlordView] diff --git a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatIsYourRentBasedOnViewSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatIsYourRentBasedOnViewSpec.scala index a8cd826d..3aa85b04 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatIsYourRentBasedOnViewSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatIsYourRentBasedOnViewSpec.scala @@ -28,7 +28,6 @@ import uk.gov.hmrc.ngrraldfrontend.models.components.* import uk.gov.hmrc.ngrraldfrontend.models.components.NGRRadio.buildRadios import uk.gov.hmrc.ngrraldfrontend.models.forms.WhatIsYourRentBasedOnForm import uk.gov.hmrc.ngrraldfrontend.views.html.WhatIsYourRentBasedOnView -import uk.gov.hmrc.ngrraldfrontend.views.html.components.NGRCharacterCountComponent class WhatIsYourRentBasedOnViewSpec extends ViewBaseSpec { lazy val view: WhatIsYourRentBasedOnView = inject[WhatIsYourRentBasedOnView] diff --git a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala index 1599b3a5..d2b90600 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/views/WhatYourRentIncludesViewSpec.scala @@ -18,7 +18,6 @@ package uk.gov.hmrc.ngrraldfrontend.views import org.jsoup.Jsoup import org.jsoup.nodes.Document -import uk.gov.hmrc.govukfrontend.views.Aliases.{Legend, Text} import uk.gov.hmrc.govukfrontend.views.viewmodels.radios.Radios import uk.gov.hmrc.ngrraldfrontend.helpers.ViewBaseSpec import uk.gov.hmrc.ngrraldfrontend.models.NormalMode From 174354119406ae9cde7966d0b936aeace8cb9cc2 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 9 Oct 2025 09:25:10 +0100 Subject: [PATCH 6/9] NGR-1130: address PR bot --- project/AppDependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/AppDependencies.scala b/project/AppDependencies.scala index c05daa6c..9acbe726 100644 --- a/project/AppDependencies.scala +++ b/project/AppDependencies.scala @@ -3,7 +3,7 @@ import sbt.* object AppDependencies { private val bootstrapVersion = "9.19.0" - private val hmrcMongoVersion = "2.7.0" + private val hmrcMongoVersion = "2.9.0" private val enumeratumVersion = "1.9.0" val compile: Seq[ModuleID] = Seq( From 8332da475815a6aede2e85b0afe796b858a4243d Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 9 Oct 2025 09:36:06 +0100 Subject: [PATCH 7/9] NGR-1130: address PR bot --- project/plugins.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/plugins.sbt b/project/plugins.sbt index b504ab0c..fe857d29 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -4,7 +4,7 @@ resolvers += Resolver.typesafeRepo("releases") addSbtPlugin("uk.gov.hmrc" % "sbt-auto-build" % "3.24.0") addSbtPlugin("uk.gov.hmrc" % "sbt-distributables" % "2.6.0") -addSbtPlugin("org.playframework" % "sbt-plugin" % "3.0.8") +addSbtPlugin("org.playframework" % "sbt-plugin" % "3.0.9") addSbtPlugin("org.scoverage" % "sbt-scoverage" % "2.3.0") addSbtPlugin("com.github.sbt" % "sbt-gzip" % "2.0.0") addSbtPlugin("uk.gov.hmrc" % "sbt-sass-compiler" % "0.12.0") From 7240263515f953a8b81c2d6c3609382ffb3af220 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 9 Oct 2025 10:42:51 +0100 Subject: [PATCH 8/9] NGR-1130: remove rent base on answer if it's verabl agreement --- .../controllers/WhatIsYourRentBasedOnController.scala | 9 ++++++++- .../controllers/WhatTypeOfAgreementController.scala | 8 +++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatIsYourRentBasedOnController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatIsYourRentBasedOnController.scala index bbd19985..02f240c1 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatIsYourRentBasedOnController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatIsYourRentBasedOnController.scala @@ -107,7 +107,14 @@ class WhatIsYourRentBasedOnController @Inject()(view: WhatIsYourRentBasedOnView, buildRadios(formWithErrors, ngrRadio(formWithCorrectedErrors)), request.property.addressFull, mode))), rentBasedOnForm => for { - updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)).set(WhatIsYourRentBasedOnPage, RentBasedOn(rentBasedOnForm.radioValue,rentBasedOnForm.rentBasedOnOther))) + updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) + .set(WhatIsYourRentBasedOnPage, + RentBasedOn(rentBasedOnForm.radioValue, + if (rentBasedOnForm.radioValue == "Other") + rentBasedOnForm.rentBasedOnOther + else + None + ))) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(WhatIsYourRentBasedOnPage, mode, updatedAnswers)) ) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala index c6c1fa14..d32246ad 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala @@ -26,7 +26,7 @@ import uk.gov.hmrc.ngrraldfrontend.models.forms.WhatTypeOfAgreementForm import uk.gov.hmrc.ngrraldfrontend.models.forms.WhatTypeOfAgreementForm.form import uk.gov.hmrc.ngrraldfrontend.models.{Mode, UserAnswers} import uk.gov.hmrc.ngrraldfrontend.navigation.Navigator -import uk.gov.hmrc.ngrraldfrontend.pages.WhatTypeOfAgreementPage +import uk.gov.hmrc.ngrraldfrontend.pages.{WhatIsYourRentBasedOnPage, WhatTypeOfAgreementPage} import uk.gov.hmrc.ngrraldfrontend.repo.SessionRepository import uk.gov.hmrc.ngrraldfrontend.views.html.WhatTypeOfAgreementView import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController @@ -77,8 +77,10 @@ class WhatTypeOfAgreementController @Inject()(view: WhatTypeOfAgreementView, for { updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) .set(WhatTypeOfAgreementPage, whatTypeOfAgreementForm.radioValue)) - _ <- sessionRepository.set(updatedAnswers) - } yield Redirect(navigator.nextPage(WhatTypeOfAgreementPage,mode,updatedAnswers)) + //Remove Rent Based on answer + newAnswers <- Future(updatedAnswers.remove(WhatIsYourRentBasedOnPage)) + _ <- sessionRepository.set(newAnswers.get) + } yield Redirect(navigator.nextPage(WhatTypeOfAgreementPage,mode,newAnswers.get)) ) } } From 3c664499039b60b214620dd6a8207f8c48fa6bb2 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 9 Oct 2025 12:34:23 +0100 Subject: [PATCH 9/9] NGR-1130: remove rent base on answer if it's verabl agreement --- .../controllers/WhatTypeOfAgreementController.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala index d32246ad..55118e96 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/controllers/WhatTypeOfAgreementController.scala @@ -33,6 +33,7 @@ import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController import javax.inject.{Inject, Singleton} import scala.concurrent.{ExecutionContext, Future} +import scala.util.Try @Singleton class WhatTypeOfAgreementController @Inject()(view: WhatTypeOfAgreementView, @@ -77,8 +78,10 @@ class WhatTypeOfAgreementController @Inject()(view: WhatTypeOfAgreementView, for { updatedAnswers <- Future.fromTry(request.userAnswers.getOrElse(UserAnswers(request.credId)) .set(WhatTypeOfAgreementPage, whatTypeOfAgreementForm.radioValue)) - //Remove Rent Based on answer - newAnswers <- Future(updatedAnswers.remove(WhatIsYourRentBasedOnPage)) + //Remove Rent Based on answer if agreement type is verbal + newAnswers <- whatTypeOfAgreementForm.radioValue match + case "Verbal" => Future(updatedAnswers.remove(WhatIsYourRentBasedOnPage)) + case _ => Future(Try(updatedAnswers)) _ <- sessionRepository.set(newAnswers.get) } yield Redirect(navigator.nextPage(WhatTypeOfAgreementPage,mode,newAnswers.get)) )