From a980eb40c9b93badb7d577594b8a316694440c97 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Wed, 24 Sep 2025 15:57:47 +0100 Subject: [PATCH 1/3] NGR-977: allow double and compare to max value --- .../models/forms/CommonFormValidators.scala | 8 ++++++++ .../ngrraldfrontend/models/forms/RentFreePeriodForm.scala | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala index d366c6cd..65cb1f18 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala @@ -59,6 +59,14 @@ trait CommonFormValidators { Invalid(errorKey, value) } + protected def isLargerThanInt(maximum: Int, errorKey: String): Constraint[String] = + Constraint { + case str if str.toDoubleOption.getOrElse(0d) <= maximum.toDouble => + Valid + case _ => + Invalid(errorKey, maximum) + } + protected def maximumValue[A](maximum: A, errorKey: String)(implicit ev: Ordering[A]): Constraint[A] = Constraint { input => import ev._ diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala index 55755306..05f26c19 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala @@ -41,7 +41,8 @@ object RentFreePeriodForm extends CommonFormValidators { .verifying( firstError( isNotEmpty(rentFreePeriodMonths, "rentFreePeriod.months.required.error"), - regexp(wholePositiveNumberRegexp.pattern(), "rentFreePeriod.months.invalid.error") + regexp(wholePositiveNumberRegexp.pattern(), "rentFreePeriod.months.invalid.error"), + isLargerThanInt(99, "rentFreePeriod.months.maximum.error") ) ) .transform[Int](_.toInt, _.toString) From d0bd15edc4308ab22eb01987981a0c3c73b419f4 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 25 Sep 2025 10:04:14 +0100 Subject: [PATCH 2/3] NGR-977: adding validation for max characters --- ...ingSpacesOrGaragesIncludedInRentForm.scala | 31 +++++++++++++------ .../models/forms/RentFreePeriodForm.scala | 6 ++-- conf/messages | 4 ++- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala index b889a1aa..c9517c0c 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala @@ -21,6 +21,7 @@ import play.api.data.Forms.{mapping, optional, text} import play.api.data.validation.{Constraint, Invalid, Valid} import play.api.libs.json.{Json, OFormat} import uk.gov.hmrc.ngrraldfrontend.models.forms.AgreementVerbalForm.firstError +import uk.gov.hmrc.ngrraldfrontend.models.forms.RentFreePeriodForm.isLargerThanInt import uk.gov.hmrc.ngrraldfrontend.models.forms.mappings.Mappings final case class HowManyParkingSpacesOrGaragesIncludedInRentForm( @@ -66,21 +67,33 @@ object HowManyParkingSpacesOrGaragesIncludedInRentForm extends CommonFormValidat "uncoveredSpaces" -> optional( text() .transform[String](_.strip().replaceAll(",", ""), identity) - .verifying(regexp(wholePositiveNumberRegexp.pattern(), uncoveredSpacesWholeNumError)) - ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)) - .verifying(maximumValue[Int](maxValue, uncoveredSpacesTooHighError)), + .verifying( + firstError( + regexp(wholePositiveNumberRegexp.pattern(), uncoveredSpacesWholeNumError), + isLargerThanInt(maxValue, uncoveredSpacesTooHighError) + ) + ) + ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)), "coveredSpaces" -> optional( text() .transform[String](_.strip().replaceAll(",", ""), identity) - .verifying(regexp(wholePositiveNumberRegexp.pattern(), coveredSpacesWholeNumError)) - ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)) - .verifying(maximumValue[Int](maxValue, coveredSpacesTooHighError)), + .verifying( + firstError( + regexp(wholePositiveNumberRegexp.pattern(), coveredSpacesWholeNumError), + isLargerThanInt(maxValue, coveredSpacesTooHighError) + ) + ) + ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)), "garages" -> optional( text() .transform[String](_.strip().replaceAll(",", ""), identity) - .verifying(regexp(wholePositiveNumberRegexp.pattern(), garagesWholeNumError)) - ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)) - .verifying(maximumValue[Int](maxValue, garagesTooHighError)), + .verifying( + firstError( + regexp(wholePositiveNumberRegexp.pattern(), garagesWholeNumError), + isLargerThanInt(maxValue, garagesTooHighError) + ) + ) + ).transform[Int](_.map(_.toInt).getOrElse(-1), value => Some(value.toString)), ) (HowManyParkingSpacesOrGaragesIncludedInRentForm.apply)(HowManyParkingSpacesOrGaragesIncludedInRentForm.unapply) .verifying( diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala index 05f26c19..e8ebde98 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodForm.scala @@ -48,13 +48,13 @@ object RentFreePeriodForm extends CommonFormValidators { .transform[Int](_.toInt, _.toString) .verifying( firstError( - minimumValue(1, "rentFreePeriod.months.minimum.error"), - maximumValue(99, "rentFreePeriod.months.maximum.error") + minimumValue(1, "rentFreePeriod.months.minimum.error") ) ), reasons -> text() .verifying( - isNotEmpty(reasons, "rentFreePeriod.reasons.required.error") + isNotEmpty(reasons, "rentFreePeriod.reasons.required.error"), + maxLength(250, "rentFreePeriod.reasons.length.error") ) )(RentFreePeriodForm.apply)(RentFreePeriodForm.unapply) ) diff --git a/conf/messages b/conf/messages index 100cfdd7..6116235e 100644 --- a/conf/messages +++ b/conf/messages @@ -369,6 +369,7 @@ interimRentSetByTheCourt.required.error = Enter the date you started paying inte interimRentSetByTheCourt.month.required.error = Date you started paying the interim rent must include a month interimRentSetByTheCourt.year.required.error = Date you started paying the interim rent must include a year interimRentSetByTheCourt.startDate.before.1900.error = The date you started paying interim rent must be 1900 or after + #RentFreePeriod rentFreePeriod.title = Rent-free period rentFreePeriod.months.label = How many months is your rent-free period? @@ -379,4 +380,5 @@ rentFreePeriod.months.required.error = Enter how many months the rent-free perio rentFreePeriod.months.invalid.error = Rent-free period must be a number, like 6 rentFreePeriod.months.minimum.error = Rent-free period must be more more than 0 rentFreePeriod.months.maximum.error = Rent-free period must be 99 months or less -rentFreePeriod.reasons.required.error = Tell us why you have a rent-free period \ No newline at end of file +rentFreePeriod.reasons.required.error = Tell us why you have a rent-free period +rentFreePeriod.reasons.length.error = What you tell us about why you have a rent-free period must be 250 characters or less \ No newline at end of file From 4c78de60d974971a63c812f413b33360a2e7bfb4 Mon Sep 17 00:00:00 2001 From: anna-shen-hmrc Date: Thu, 25 Sep 2025 11:00:50 +0100 Subject: [PATCH 3/3] NGR-977: adding tests --- project/AppDependencies.scala | 2 +- ...pacesOrGaragesIncludedInRentFormSpec.scala | 15 +++++++++++--- .../models/forms/RentFreePeriodFormSpec.scala | 20 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/project/AppDependencies.scala b/project/AppDependencies.scala index 31a1e0b4..3d868546 100644 --- a/project/AppDependencies.scala +++ b/project/AppDependencies.scala @@ -8,7 +8,7 @@ object AppDependencies { val compile: Seq[ModuleID] = Seq( "uk.gov.hmrc" %% "bootstrap-frontend-play-30" % bootstrapVersion, - "uk.gov.hmrc" %% "play-frontend-hmrc-play-30" % "12.11.0", + "uk.gov.hmrc" %% "play-frontend-hmrc-play-30" % "12.12.0", "uk.gov.hmrc.mongo" %% "hmrc-mongo-play-30" % hmrcMongoVersion, "uk.gov.hmrc" %% "centralised-authorisation-resource-client-play-30" % "1.12.0", "com.beachape" %% "enumeratum-play" % enumeratumVersion, diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala index 2d1be97e..89c88be4 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala @@ -46,7 +46,6 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi "garages" -> "100" ) val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) - boundForm.hasErrors shouldBe false } @@ -57,7 +56,6 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi "garages" -> "100" ) val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) - boundForm.hasErrors shouldBe false } @@ -68,7 +66,6 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi "garages" -> "100" ) - val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) boundForm.hasErrors shouldBe true boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.wholeNum.error"), ArraySeq("""^\d+$"""))) @@ -97,6 +94,18 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.tooHigh.error"), ArraySeq(9999))) } + "fail to bind input is a very large number" in { + val data = Map( + "uncoveredSpaces" -> "100000000000000000", + "coveredSpaces" -> "100", + "garages" -> "100" + ) + val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.tooHigh.error"), ArraySeq(9999))) + } + "fail to bind when no input fields are entered" in { val data = Map( "uncoveredSpaces" -> "", diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodFormSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodFormSpec.scala index 194203c4..d9884bb5 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodFormSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/RentFreePeriodFormSpec.scala @@ -86,6 +86,26 @@ class RentFreePeriodFormSpec extends AnyWordSpec with Matchers { boundForm.errors should contain(FormError("rentFreePeriodMonths", List("rentFreePeriod.months.maximum.error"), ArraySeq(99))) } + "fail to bind when months is a very large number" in { + val data = Map("rentFreePeriodMonths" -> "1000000000000000000", + "reasons" -> "Any reasons" + ) + val boundForm = RentFreePeriodForm.form.bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors should contain(FormError("rentFreePeriodMonths", List("rentFreePeriod.months.maximum.error"), ArraySeq(99))) + } + + "fail to bind when reasons are longer than 250" in { + val data = Map("rentFreePeriodMonths" -> "100", + "reasons" -> "1dscsdce4343erfrfwfewfwefwefwefwefrf8988234uiml44bscbdsncsomkqmkwdqdbq3u98484nkwnfjkwenskpwekpowekjnfhwiuedojwedjkwedjknwedjkas9x9832jerfkjerfkma9090ui32nje23deioweokwjn138989wefweoijdeioiowdejknw78891329023jwenjdkde909023njknwewed8923uewdnwedwu8u23juwedw" + ) + val boundForm = RentFreePeriodForm.form.bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors should contain(FormError("reasons", List("rentFreePeriod.reasons.length.error"), ArraySeq(250))) + } + "serialize to JSON correctly" in { val form = RentFreePeriodForm(5, "Any reasons") val json = Json.toJson(form)