From ff9ed0d41a789cdc82782f0405c36765fe935d6d Mon Sep 17 00:00:00 2001 From: mycp98 Date: Mon, 13 May 2024 11:28:09 +0100 Subject: [PATCH 1/4] updated ukims form provider and tests --- app/forms/UkimsNumberFormProvider.scala | 12 ++-------- .../mappings/helpers/RemoveWhitespace.scala | 23 +++++++++++++++++++ app/models/StringFieldRegex.scala | 22 ++++++++++++++++++ conf/messages.en | 2 +- .../CategoryGuidanceControllerSpec.scala | 16 +++++++++++++ .../CommodityCodeControllerSpec.scala | 16 +++++++++++++ .../CountryOfOriginControllerSpec.scala | 16 +++++++++++++ test/controllers/HomePageControllerSpec.scala | 16 +++++++++++++ .../forms/CommodityCodeFormProviderSpec.scala | 16 +++++++++++++ .../CountryOfOriginFormProviderSpec.scala | 16 +++++++++++++ 10 files changed, 144 insertions(+), 11 deletions(-) create mode 100644 app/forms/mappings/helpers/RemoveWhitespace.scala create mode 100644 app/models/StringFieldRegex.scala diff --git a/app/forms/UkimsNumberFormProvider.scala b/app/forms/UkimsNumberFormProvider.scala index 3269e9492..671340da3 100644 --- a/app/forms/UkimsNumberFormProvider.scala +++ b/app/forms/UkimsNumberFormProvider.scala @@ -17,15 +17,7 @@ package forms import javax.inject.Inject - import forms.mappings.Mappings +import forms.mappings.helpers.RemoveWhitespace.removeWhitespace +import models.StringFieldRegex import play.api.data.Form - -class UkimsNumberFormProvider @Inject() extends Mappings { - - def apply(): Form[String] = - Form( - "value" -> text("ukimsNumber.error.required") - .verifying(maxLength(100, "ukimsNumber.error.length")) - ) -} diff --git a/app/forms/mappings/helpers/RemoveWhitespace.scala b/app/forms/mappings/helpers/RemoveWhitespace.scala new file mode 100644 index 000000000..083027562 --- /dev/null +++ b/app/forms/mappings/helpers/RemoveWhitespace.scala @@ -0,0 +1,23 @@ +/* + * Copyright 2024 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 forms.mappings.helpers + +object RemoveWhitespace { + + def removeWhitespace: String => String = _.filterNot(_.isWhitespace) + +} \ No newline at end of file diff --git a/app/models/StringFieldRegex.scala b/app/models/StringFieldRegex.scala new file mode 100644 index 000000000..d45214be1 --- /dev/null +++ b/app/models/StringFieldRegex.scala @@ -0,0 +1,22 @@ +/* + * Copyright 2024 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 models + +object StringFieldRegex { + val ukimsNumberRegex: String = "^(GB|XI)[0-9]{12}[0-9]{14}$" + +} diff --git a/conf/messages.en b/conf/messages.en index 7646e4d2c..e84ac5bd0 100644 --- a/conf/messages.en +++ b/conf/messages.en @@ -120,7 +120,7 @@ ukimsNumber.heading = What is your UKIMS number? ukimsNumber.hint = For example, XI47699357400020231115081800 ukimsNumber.linkText = I am not UKIMS registered ukimsNumber.error.required = Enter your UKIMS number -ukimsNumber.error.length = UkimsNumber must be 100 characters or less +ukimsNumber.error.invalidFormat = Enter your UKIMS number in the correct format ukimsNumber.checkYourAnswersLabel = ukimsNumber ukimsNumber.change.hidden = UkimsNumber diff --git a/test/controllers/CategoryGuidanceControllerSpec.scala b/test/controllers/CategoryGuidanceControllerSpec.scala index e7c872a69..5a2e1815f 100644 --- a/test/controllers/CategoryGuidanceControllerSpec.scala +++ b/test/controllers/CategoryGuidanceControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 controllers import base.SpecBase diff --git a/test/controllers/CommodityCodeControllerSpec.scala b/test/controllers/CommodityCodeControllerSpec.scala index 7d38953fa..44089e356 100644 --- a/test/controllers/CommodityCodeControllerSpec.scala +++ b/test/controllers/CommodityCodeControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 controllers import base.SpecBase diff --git a/test/controllers/CountryOfOriginControllerSpec.scala b/test/controllers/CountryOfOriginControllerSpec.scala index 9ff3fed20..3ab732f2c 100644 --- a/test/controllers/CountryOfOriginControllerSpec.scala +++ b/test/controllers/CountryOfOriginControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 controllers import base.SpecBase diff --git a/test/controllers/HomePageControllerSpec.scala b/test/controllers/HomePageControllerSpec.scala index 2868a9e53..a53523943 100644 --- a/test/controllers/HomePageControllerSpec.scala +++ b/test/controllers/HomePageControllerSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 controllers import base.SpecBase diff --git a/test/forms/CommodityCodeFormProviderSpec.scala b/test/forms/CommodityCodeFormProviderSpec.scala index 6502aeb41..36262abd0 100644 --- a/test/forms/CommodityCodeFormProviderSpec.scala +++ b/test/forms/CommodityCodeFormProviderSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 forms import forms.behaviours.StringFieldBehaviours diff --git a/test/forms/CountryOfOriginFormProviderSpec.scala b/test/forms/CountryOfOriginFormProviderSpec.scala index 49633a68c..04af7672e 100644 --- a/test/forms/CountryOfOriginFormProviderSpec.scala +++ b/test/forms/CountryOfOriginFormProviderSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2024 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 forms import forms.behaviours.StringFieldBehaviours From e729a3ba0c999635542fb31e08fcd03241663452 Mon Sep 17 00:00:00 2001 From: mycp98 Date: Mon, 13 May 2024 11:28:23 +0100 Subject: [PATCH 2/4] updated ukims form provider and tests --- app/forms/UkimsNumberFormProvider.scala | 10 ++ test/forms/UkimsNumberFormProviderSpec.scala | 138 +++++++++++++++---- test/forms/behaviours/FieldBehaviours.scala | 48 +++++-- 3 files changed, 160 insertions(+), 36 deletions(-) diff --git a/app/forms/UkimsNumberFormProvider.scala b/app/forms/UkimsNumberFormProvider.scala index 671340da3..d96664a05 100644 --- a/app/forms/UkimsNumberFormProvider.scala +++ b/app/forms/UkimsNumberFormProvider.scala @@ -21,3 +21,13 @@ import forms.mappings.Mappings import forms.mappings.helpers.RemoveWhitespace.removeWhitespace import models.StringFieldRegex import play.api.data.Form + +class UkimsNumberFormProvider @Inject() extends Mappings { + + def apply(): Form[String] = + Form( + "value" -> text("ukimsNumber.error.required") + .transform(removeWhitespace, identity[String]) + .verifying(regexp(StringFieldRegex.ukimsNumberRegex, "ukimsNumber.error.invalidFormat")) + ) +} \ No newline at end of file diff --git a/test/forms/UkimsNumberFormProviderSpec.scala b/test/forms/UkimsNumberFormProviderSpec.scala index ecbfa94a7..db93ca50e 100644 --- a/test/forms/UkimsNumberFormProviderSpec.scala +++ b/test/forms/UkimsNumberFormProviderSpec.scala @@ -17,37 +17,129 @@ package forms import forms.behaviours.StringFieldBehaviours -import play.api.data.FormError +import org.scalacheck.Gen +import play.api.data.{Form, FormError} class UkimsNumberFormProviderSpec extends StringFieldBehaviours { - val requiredKey = "ukimsNumber.error.required" - val lengthKey = "ukimsNumber.error.length" - val maxLength = 100 + private val formProvider = new UkimsNumberFormProvider() + private val form: Form[String] = formProvider() + private val requiredKey = "ukimsNumber.error.required" + private val invalidFormatKey = "ukimsNumber.error.invalidFormat" + private val fieldName = "value" - val form = new UkimsNumberFormProvider()() + ".ukimsNumber" - { - ".value" - { + "valid UKIMS number" - { - val fieldName = "value" + val ukimsNumberGenerator: Gen[String] = { + val prefixGen = Gen.oneOf("GB", "XI") + val firstDigitsGen = Gen.listOfN(12, Gen.numChar).map(_.mkString) + val lastDigitsGen = Gen.listOfN(14, Gen.numChar).map(_.mkString) - behave like fieldThatBindsValidData( - form, - fieldName, - stringsWithMaxLength(maxLength) - ) + for { + prefix <- prefixGen + firstDigits <- firstDigitsGen + lastDigits <- lastDigitsGen + } yield s"$prefix$firstDigits$lastDigits" + } - behave like fieldWithMaxLength( - form, - fieldName, - maxLength = maxLength, - lengthError = FormError(fieldName, lengthKey, Seq(maxLength)) - ) + behave like mandatoryField(form, fieldName, requiredError = FormError(fieldName, requiredKey)) - behave like mandatoryField( - form, - fieldName, - requiredError = FormError(fieldName, requiredKey) - ) + behave like fieldThatBindsValidData(form, fieldName, ukimsNumberGenerator) + + } + + "valid UKIMS number with spaces" - { + + val ukimsNumberGenerator: Gen[String] = { + val prefixGen = Gen.oneOf("GB", "XI") + val firstDigitsGen = Gen.listOfN(12, Gen.numChar).map(_.mkString) + val lastDigitsGen = Gen.listOfN(14, Gen.numChar).map(_.mkString) + + for { + prefix <- prefixGen + firstDigits <- firstDigitsGen + lastDigits <- lastDigitsGen + } yield s"$prefix $firstDigits $lastDigits" + } + + behave like mandatoryField(form, fieldName, requiredError = FormError(fieldName, requiredKey)) + + behave like fieldThatBindsValidData(form, fieldName, ukimsNumberGenerator) + + } + + "invalid UKIMS number" - { + + "invalid UKIMS number with invalid length" - { + + val invalidUkimsNumberGeneratorWithInvalidLength: Gen[String] = { + + val prefixGen = Gen.oneOf("GB", "XI") + val firstDigitsGen = Gen.listOfN(12, Gen.numChar).map(_.mkString) + val lastDigitsGen = Gen.listOfN(15, Gen.numChar).map(_.mkString) + + for { + prefix <- prefixGen + firstDigits <- firstDigitsGen + lastDigits <- lastDigitsGen + } yield s"$prefix$firstDigits$lastDigits" + } + + behave like fieldThatErrorsOnInvalidData( + form, + fieldName, + invalidUkimsNumberGeneratorWithInvalidLength, + FormError(fieldName, invalidFormatKey) + ) + } + + "invalid UKIMS number with invalid prefix" - { + + val invalidUkimsNumberGeneratorWithInvalidPrefix: Gen[String] = { + val prefixGen = Gen.oneOf("AA", "BB") + val firstDigitsGen = Gen.listOfN(12, Gen.numChar).map(_.mkString) + val lastDigitsGen = Gen.listOfN(14, Gen.numChar).map(_.mkString) + + for { + prefix <- prefixGen + firstDigits <- firstDigitsGen + lastDigits <- lastDigitsGen + } yield s"$prefix$firstDigits$lastDigits" + } + + behave like fieldThatErrorsOnInvalidData( + form, + fieldName, + invalidUkimsNumberGeneratorWithInvalidPrefix, + FormError(fieldName, invalidFormatKey) + ) + + } + + "invalid UKIMS number with special characters" - { + + val invalidUkimsNumberGeneratorWithSpecialCharacters: Gen[String] = { + val prefixGen = Gen.oneOf(" ", "A-", "B_") + val firstDigitsGen = Gen.listOfN(12, Gen.numChar).map(_.mkString) + val lastDigitsGen = Gen.listOfN(14, Gen.numChar).map(_.mkString) + + for { + prefix <- prefixGen + firstDigits <- firstDigitsGen + lastDigits <- lastDigitsGen + } yield s"$prefix$firstDigits$lastDigits" + } + + behave like fieldThatErrorsOnInvalidData( + form, + fieldName, + invalidUkimsNumberGeneratorWithSpecialCharacters, + FormError(fieldName, invalidFormatKey) + ) + + } + } } } diff --git a/test/forms/behaviours/FieldBehaviours.scala b/test/forms/behaviours/FieldBehaviours.scala index 2eed61ea3..769e0bfc9 100644 --- a/test/forms/behaviours/FieldBehaviours.scala +++ b/test/forms/behaviours/FieldBehaviours.scala @@ -24,24 +24,32 @@ import play.api.data.{Form, FormError} trait FieldBehaviours extends FormSpec with ScalaCheckPropertyChecks with Generators { - def fieldThatBindsValidData(form: Form[_], - fieldName: String, - validDataGenerator: Gen[String]): Unit = { - + def fieldThatBindsValidData(form: Form[_], fieldName: String, validDataGenerator: Gen[String]): Unit = "bind valid data" in { - forAll(validDataGenerator -> "validDataItem") { - dataItem: String => - val result = form.bind(Map(fieldName -> dataItem)).apply(fieldName) - result.value.value mustBe dataItem - result.errors mustBe empty + forAll(validDataGenerator -> "validDataItem") { dataItem: String => + val result = form.bind(Map(fieldName -> dataItem)).apply(fieldName) + result.value.value mustBe dataItem + result.errors mustBe empty } } - } - def mandatoryField(form: Form[_], - fieldName: String, - requiredError: FormError): Unit = { + def fieldThatErrorsOnInvalidData( + form: Form[_], + fieldName: String, + invalidDataGenerator: Gen[String], + invalidError: String + ): Unit = + "error on invalid data" in { + + forAll(invalidDataGenerator -> "invalidDataItem") { dataItem: String => + val result = form.bind(Map(fieldName -> dataItem)).apply(fieldName) + result.errors.size mustBe 1 + result.errors.head.message mustBe invalidError + } + } + + def mandatoryField(form: Form[_], fieldName: String, requiredError: FormError): Unit = { "not bind when key is not present at all" in { @@ -55,4 +63,18 @@ trait FieldBehaviours extends FormSpec with ScalaCheckPropertyChecks with Genera result.errors mustEqual Seq(requiredError) } } + + def fieldThatErrorsOnInvalidData( + form: Form[_], + fieldName: String, + invalidDataGenerator: Gen[String], + invalidError: FormError + ): Unit = + "error on invalid data" in { + forAll(invalidDataGenerator -> "invalidDataItem") { dataItem: String => + val result = form.bind(Map(fieldName -> dataItem)).apply(fieldName) + result.errors.size mustBe 1 + result.errors.head.message mustBe invalidError.message + } + } } From e40dcf715e72c2290a55d88cdfa1bbb8f8ebef02 Mon Sep 17 00:00:00 2001 From: mycp98 Date: Mon, 13 May 2024 11:45:32 +0100 Subject: [PATCH 3/4] deleted duplicate method and fixed controller test --- test/controllers/UkimsNumberControllerSpec.scala | 2 +- test/forms/behaviours/FieldBehaviours.scala | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/test/controllers/UkimsNumberControllerSpec.scala b/test/controllers/UkimsNumberControllerSpec.scala index 5652996b6..2dbb9e582 100644 --- a/test/controllers/UkimsNumberControllerSpec.scala +++ b/test/controllers/UkimsNumberControllerSpec.scala @@ -95,7 +95,7 @@ class UkimsNumberControllerSpec extends SpecBase with MockitoSugar { running(application) { val request = FakeRequest(POST, ukimsNumberRoute) - .withFormUrlEncodedBody(("value", "answer")) + .withFormUrlEncodedBody(("value", "XI47699357400020231115081800")) val result = route(application, request).value diff --git a/test/forms/behaviours/FieldBehaviours.scala b/test/forms/behaviours/FieldBehaviours.scala index 769e0bfc9..fb13d08a9 100644 --- a/test/forms/behaviours/FieldBehaviours.scala +++ b/test/forms/behaviours/FieldBehaviours.scala @@ -34,21 +34,6 @@ trait FieldBehaviours extends FormSpec with ScalaCheckPropertyChecks with Genera } } - def fieldThatErrorsOnInvalidData( - form: Form[_], - fieldName: String, - invalidDataGenerator: Gen[String], - invalidError: String - ): Unit = - "error on invalid data" in { - - forAll(invalidDataGenerator -> "invalidDataItem") { dataItem: String => - val result = form.bind(Map(fieldName -> dataItem)).apply(fieldName) - result.errors.size mustBe 1 - result.errors.head.message mustBe invalidError - } - } - def mandatoryField(form: Form[_], fieldName: String, requiredError: FormError): Unit = { "not bind when key is not present at all" in { From 8c010e5fb4a36f977da654489813a2cfea8502f9 Mon Sep 17 00:00:00 2001 From: mycp98 Date: Mon, 13 May 2024 13:41:23 +0100 Subject: [PATCH 4/4] changed nirmsNumber to value in NirmsNumberView --- app/views/NirmsNumberView.scala.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/NirmsNumberView.scala.html b/app/views/NirmsNumberView.scala.html index 0eb1fe888..eb6b907f1 100644 --- a/app/views/NirmsNumberView.scala.html +++ b/app/views/NirmsNumberView.scala.html @@ -43,7 +43,7 @@ @govukInput( InputViewModel( - field = form("nirmsNumber"), + field = form("value"), label = LabelViewModel(messages("nirmsNumber.heading")).asPageHeading(Large) ).withHint(Hint(content = Text(messages("nirmsNumber.hint")))) .withWidth(Fixed10)