diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala index ee6bba3b..b8ddcff4 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/CommonFormValidators.scala @@ -24,6 +24,8 @@ import scala.util.Try trait CommonFormValidators { val amountRegex: Pattern = Pattern.compile("([0-9]+\\.[0-9]+|[0-9]+)") + val wholePositiveNumberRegexp: Pattern = Pattern.compile("^\\d+$") + protected def firstError[A](constraints: Constraint[A]*): Constraint[A] = Constraint { input => diff --git a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala index 345565c6..2bd6103f 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentForm.scala @@ -17,7 +17,7 @@ package uk.gov.hmrc.ngrraldfrontend.models.forms import play.api.data.Form -import play.api.data.Forms.{mapping, optional} +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 @@ -35,7 +35,7 @@ object HowManyParkingSpacesOrGaragesIncludedInRentForm extends CommonFormValidat private lazy val fieldRequired = "howManyParkingSpacesOrGaragesIncludedInRent.error.required" private lazy val uncoveredSpacesWholeNumError = "howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.wholeNum.error" private lazy val coveredSpacesWholeNumError = "howManyParkingSpacesOrGaragesIncludedInRent.coveredSpaces.wholeNum.error" - private lazy val garagesSpacesWholeNumError = "howManyParkingSpacesOrGaragesIncludedInRent.garages.wholeNum.error" + private lazy val garagesWholeNumError = "howManyParkingSpacesOrGaragesIncludedInRent.garages.wholeNum.error" private lazy val uncoveredSpacesTooHighError = "howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.tooHigh.error" private lazy val coveredSpacesTooHighError = "howManyParkingSpacesOrGaragesIncludedInRent.coveredSpaces.tooHigh.error" private lazy val garagesTooHighError = "howManyParkingSpacesOrGaragesIncludedInRent.garages.tooHigh.error" @@ -63,29 +63,25 @@ object HowManyParkingSpacesOrGaragesIncludedInRentForm extends CommonFormValidat val form: Form[HowManyParkingSpacesOrGaragesIncludedInRentForm] = { Form( mapping( - "uncoveredSpaces" -> int( - requiredKey = allFieldsRequiredError, - wholeNumberKey = uncoveredSpacesWholeNumError, - nonNumericKey = uncoveredSpacesWholeNumError, - ).verifying( - maximumValue(9999, uncoveredSpacesTooHighError)), - "coveredSpaces" -> - int( - requiredKey = allFieldsRequiredError, - wholeNumberKey = coveredSpacesWholeNumError, - nonNumericKey = coveredSpacesWholeNumError, - ).verifying( - maximumValue(9999, coveredSpacesTooHighError) - ), - "garages" -> - int( - requiredKey = allFieldsRequiredError, - wholeNumberKey = garagesSpacesWholeNumError, - nonNumericKey = garagesSpacesWholeNumError, - ).verifying( - maximumValue(9999, garagesTooHighError) - ) - ) + "uncoveredSpaces" -> optional( + text() + .transform[String](_.strip().replaceAll(",", ""), identity) + .verifying(regexp(wholePositiveNumberRegexp.pattern(), uncoveredSpacesWholeNumError)) + ).transform[Int](_.map(_.toInt).getOrElse(0), value => Some(value.toString)) + .verifying(maximumValue[Int](maxValue, uncoveredSpacesTooHighError)), + "coveredSpaces" -> optional( + text() + .transform[String](_.strip().replaceAll(",", ""), identity) + .verifying(regexp(wholePositiveNumberRegexp.pattern(), coveredSpacesWholeNumError)) + ).transform[Int](_.map(_.toInt).getOrElse(0), value => Some(value.toString)) + .verifying(maximumValue[Int](maxValue, coveredSpacesTooHighError)), + "garages" -> optional( + text() + .transform[String](_.strip().replaceAll(",", ""), identity) + .verifying(regexp(wholePositiveNumberRegexp.pattern(), garagesWholeNumError)) + ).transform[Int](_.map(_.toInt).getOrElse(0), value => Some(value.toString)) + .verifying(maximumValue[Int](maxValue, garagesTooHighError)), + ) (HowManyParkingSpacesOrGaragesIncludedInRentForm.apply)(HowManyParkingSpacesOrGaragesIncludedInRentForm.unapply) .verifying( firstError( 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 c550f297..af5ef29e 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Formatters.scala @@ -36,33 +36,4 @@ trait Formatters { Map(key -> value) } - private[mappings] def intFormatter( - isRequired: Boolean, - requiredKey: String, - wholeNumberKey: String, - nonNumericKey: String, - args: Seq[String] = Seq.empty - ): Formatter[Int] = new Formatter[Int] { - val decimalRegexp = """^-?(\d*\.\d*)$""" - override def bind(key: String, data: Map[String, String]) = { - val rawValue = data.get(key).map(_.trim).filter(_.nonEmpty) - rawValue match { - case None => - if (isRequired) { - Left(Seq(FormError(key, requiredKey, args))) - } else { - Left(Seq.empty) // Or Right(defaultValue) if you want to provide a default - } - - case Some(s) if s.matches(decimalRegexp) => - Left(Seq(FormError(key, wholeNumberKey, args))) - case Some(s) => - nonFatalCatch - .either(s.replace(",", "").toInt) - .left.map(_ => Seq(FormError(key, nonNumericKey, args))) - } - } - override def unbind(key: String, value: Int) = - Map(key -> value.toString) - } } 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 aa8fdb54..fd7dfbf5 100644 --- a/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala +++ b/app/uk/gov/hmrc/ngrraldfrontend/models/forms/mappings/Mappings.scala @@ -22,12 +22,4 @@ 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 int( - isRequired: Boolean = true, - requiredKey: String = "error.required", - wholeNumberKey: String = "error.wholeNumber", - nonNumericKey: String = "error.nonNumeric", - args: Seq[String] = Seq.empty): FieldMapping[Int] = - of(intFormatter(isRequired, requiredKey, wholeNumberKey, nonNumericKey, args)) } diff --git a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala index 83bebafa..5c9f3744 100644 --- a/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala +++ b/test/uk/gov/hmrc/ngrraldfrontend/models/forms/HowManyParkingSpacesOrGaragesIncludedInRentFormSpec.scala @@ -39,7 +39,7 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi boundForm.value shouldBe Some(HowManyParkingSpacesOrGaragesIncludedInRentForm(uncoveredSpaces = 100, coveredSpaces = 100, garages = 100)) } - "fail to bind empty input" in { + "bind empty input" in { val data = Map( "uncoveredSpaces" -> "", "coveredSpaces" -> "100", @@ -47,8 +47,31 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi ) val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) + boundForm.hasErrors shouldBe false + } + + "bind field with commas input" in { + val data = Map( + "uncoveredSpaces" -> "1,000", + "coveredSpaces" -> "100", + "garages" -> "100" + ) + val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) + + boundForm.hasErrors shouldBe false + } + + "fail to bind field with decimal place" in { + val data = Map( + "uncoveredSpaces" -> "10.5", + "coveredSpaces" -> "100", + "garages" -> "100" + ) + + + val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) boundForm.hasErrors shouldBe true - boundForm.errors should contain(FormError("uncoveredSpaces", "howManyParkingSpacesOrGaragesIncludedInRent.allFields.error.required")) + boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.wholeNum.error"), ArraySeq("""^\d+$"""))) } "fail to bind non-numeric input" in { @@ -59,7 +82,7 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi ) val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) - boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.wholeNum.error"), List())) + boundForm.errors shouldBe List(FormError("uncoveredSpaces", List("howManyParkingSpacesOrGaragesIncludedInRent.uncoveredSpaces.wholeNum.error"), ArraySeq("""^\d+$"""))) } "fail to bind input greater than 9,999" in { @@ -74,6 +97,18 @@ class HowManyParkingSpacesOrGaragesIncludedInRentFormSpec extends AnyWordSpec wi 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" -> "", + "coveredSpaces" -> "", + "garages" -> "" + ) + val boundForm = HowManyParkingSpacesOrGaragesIncludedInRentForm.form.bind(data) + + boundForm.hasErrors shouldBe true + boundForm.errors shouldBe List(FormError("", "howManyParkingSpacesOrGaragesIncludedInRent.error.required")) + } + "fail to bind when input fields are all 0" in { val data = Map( "uncoveredSpaces" -> "0",