From 78f1cf3a05b3bfc2c889ef0c56fb70a7607afdf3 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Tue, 10 Mar 2026 16:49:59 +0000 Subject: [PATCH 01/11] feat: add rollnumber, add tests and add check your details page, add encrypt session details transfer. rename save and fetch methods in details service --- app/config/startup/VerifyCrypto.scala | 2 +- .../CheckBankDetailsController.scala | 78 +++++++++++ .../ChooseAccountTypeController.scala | 2 +- .../HasBankAccountController.scala | 2 +- .../NoUKBankAccountController.scala | 2 +- .../UkBankAccountDetailsController.scala | 81 +++++++----- app/forms/BankAccountDetailsForms.scala | 21 ++- app/models/BankAccountDetails.scala | 15 ++- .../BankAccountDetailsSessionFormat.scala | 45 +++++++ app/services/BankAccountDetailsService.scala | 19 +-- .../CheckBankDetailsView.scala.html | 75 +++++++++++ .../EnterCompanyBankAccountDetails.scala.html | 48 ++++--- conf/app.routes | 6 +- conf/application.conf | 5 + conf/messages | 24 +++- test/fixtures/VatRegistrationFixture.scala | 3 +- test/forms/BankAccountDetailsFormSpec.scala | 83 ++++++++++++ test/models/BankAccountDetailsSpec.scala | 20 ++- .../BankAccountDetailsSessionFormatSpec.scala | 105 +++++++++++++++ .../BankAccountDetailsServiceSpec.scala | 6 +- .../CheckBankDetailsViewSpec.scala | 123 ++++++++++++++++++ .../CompanyBankDetailsViewSpec.scala | 18 ++- 22 files changed, 696 insertions(+), 87 deletions(-) create mode 100644 app/controllers/bankdetails/CheckBankDetailsController.scala create mode 100644 app/models/bars/BankAccountDetailsSessionFormat.scala create mode 100644 app/views/bankdetails/CheckBankDetailsView.scala.html create mode 100644 test/models/bars/BankAccountDetailsSessionFormatSpec.scala create mode 100644 test/views/bankdetails/CheckBankDetailsViewSpec.scala diff --git a/app/config/startup/VerifyCrypto.scala b/app/config/startup/VerifyCrypto.scala index c074d4215..e0ee6cc0b 100644 --- a/app/config/startup/VerifyCrypto.scala +++ b/app/config/startup/VerifyCrypto.scala @@ -16,7 +16,7 @@ package config.startup -import uk.gov.hmrc.crypto.ApplicationCrypto +import uk.gov.hmrc.play.bootstrap.frontend.filters.crypto.ApplicationCrypto import javax.inject.{Inject, Singleton} diff --git a/app/controllers/bankdetails/CheckBankDetailsController.scala b/app/controllers/bankdetails/CheckBankDetailsController.scala new file mode 100644 index 000000000..efda04755 --- /dev/null +++ b/app/controllers/bankdetails/CheckBankDetailsController.scala @@ -0,0 +1,78 @@ +/* + * Copyright 2026 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.bankdetails + +import config.{AuthClientConnector, BaseControllerComponents, FrontendAppConfig} +import controllers.BaseController +import featuretoggle.FeatureSwitch.UseNewBarsVerify +import featuretoggle.FeatureToggleSupport.isEnabled +import models.BankAccountDetails +import models.bars.BankAccountDetailsSessionFormat +import play.api.Configuration +import play.api.libs.json.Format +import play.api.mvc.{Action, AnyContent} +import services.{BankAccountDetailsService, SessionService} +import uk.gov.hmrc.crypto.SymmetricCryptoFactory +import views.html.bankdetails.CheckBankDetailsView + +import javax.inject.Inject +import scala.concurrent.{ExecutionContext, Future} + +class CheckBankDetailsController @Inject() ( + val authConnector: AuthClientConnector, + val bankAccountDetailsService: BankAccountDetailsService, + val sessionService: SessionService, + configuration: Configuration, + view: CheckBankDetailsView +)(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) + extends BaseController { + + private val encrypter = + SymmetricCryptoFactory.aesCryptoFromConfig("json.encryption", configuration.underlying) + + private implicit val encryptedFormat: Format[BankAccountDetails] = + BankAccountDetailsSessionFormat.format(encrypter) + + private val sessionKey = "bankAccountDetails" + + def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + if (isEnabled(UseNewBarsVerify)) { + sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { + case Some(details) => Ok(view(details)) + case None => Redirect(routes.UkBankAccountDetailsController.show) + } + } else { + Future.successful(Redirect(routes.HasBankAccountController.show)) + } + } + + def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + if (isEnabled(UseNewBarsVerify)) { + sessionService.fetchAndGet[BankAccountDetails](sessionKey).flatMap { + case Some(details) => + bankAccountDetailsService.saveEnteredBankAccountDetails(details).flatMap { + case true => sessionService.remove.map(_ => Redirect(controllers.routes.TaskListController.show.url)) + case false => Future.successful(Redirect(routes.UkBankAccountDetailsController.show)) + + } + case None => Future.successful(Redirect(routes.UkBankAccountDetailsController.show)) + } + } else { + Future.successful(Redirect(routes.HasBankAccountController.show)) + } + } +} diff --git a/app/controllers/bankdetails/ChooseAccountTypeController.scala b/app/controllers/bankdetails/ChooseAccountTypeController.scala index c0d76ead0..1de0d142d 100644 --- a/app/controllers/bankdetails/ChooseAccountTypeController.scala +++ b/app/controllers/bankdetails/ChooseAccountTypeController.scala @@ -39,7 +39,7 @@ class ChooseAccountTypeController @Inject() (val authConnector: AuthClientConnec def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => if (isEnabled(UseNewBarsVerify)) { - bankAccountDetailsService.fetchBankAccountDetails.map { bankDetails => + bankAccountDetailsService.getBankAccount.map { bankDetails => val filledForm = bankDetails .flatMap(_.bankAccountType) .fold(ChooseAccountTypeForm.form)(ChooseAccountTypeForm.form.fill) diff --git a/app/controllers/bankdetails/HasBankAccountController.scala b/app/controllers/bankdetails/HasBankAccountController.scala index d843e2615..704846c66 100644 --- a/app/controllers/bankdetails/HasBankAccountController.scala +++ b/app/controllers/bankdetails/HasBankAccountController.scala @@ -45,7 +45,7 @@ class HasBankAccountController @Inject()(val authConnector: AuthClientConnector, Future.successful(Redirect(controllers.flatratescheme.routes.JoinFlatRateSchemeController.show)) case _ => for { - bankDetails <- bankAccountDetailsService.fetchBankAccountDetails + bankDetails <- bankAccountDetailsService.getBankAccount filledForm = bankDetails.map(_.isProvided).fold(hasBankAccountForm)(hasBankAccountForm.fill) } yield Ok(view(filledForm)) } diff --git a/app/controllers/bankdetails/NoUKBankAccountController.scala b/app/controllers/bankdetails/NoUKBankAccountController.scala index 24d230e51..0a2639ee9 100644 --- a/app/controllers/bankdetails/NoUKBankAccountController.scala +++ b/app/controllers/bankdetails/NoUKBankAccountController.scala @@ -40,7 +40,7 @@ class NoUKBankAccountController @Inject()(noUKBankAccountView: NoUkBankAccount, implicit request => implicit profile => for { - optBankAccountDetails <- bankAccountDetailsService.fetchBankAccountDetails + optBankAccountDetails <- bankAccountDetailsService.getBankAccount form = optBankAccountDetails.flatMap(_.reason).fold(NoUKBankAccountForm.form)(NoUKBankAccountForm.form.fill) } yield Ok(noUKBankAccountView(form)) } diff --git a/app/controllers/bankdetails/UkBankAccountDetailsController.scala b/app/controllers/bankdetails/UkBankAccountDetailsController.scala index 4a7b2c48e..9db4347b4 100644 --- a/app/controllers/bankdetails/UkBankAccountDetailsController.scala +++ b/app/controllers/bankdetails/UkBankAccountDetailsController.scala @@ -18,49 +18,68 @@ package controllers.bankdetails import config.{AuthClientConnector, BaseControllerComponents, FrontendAppConfig} import controllers.BaseController +import featuretoggle.FeatureSwitch.UseNewBarsVerify +import featuretoggle.FeatureToggleSupport.isEnabled import forms.EnterBankAccountDetailsForm import forms.EnterBankAccountDetailsForm.{form => enterBankAccountDetailsForm} +import models.BankAccountDetails +import models.bars.BankAccountDetailsSessionFormat +import play.api.libs.json.Format import play.api.mvc.{Action, AnyContent} +import play.api.Configuration import services.{BankAccountDetailsService, SessionService} +import uk.gov.hmrc.crypto.SymmetricCryptoFactory import views.html.bankdetails.EnterCompanyBankAccountDetails import javax.inject.Inject import scala.concurrent.{ExecutionContext, Future} -class UkBankAccountDetailsController @Inject()(val authConnector: AuthClientConnector, - val bankAccountDetailsService: BankAccountDetailsService, - val sessionService: SessionService, - view: EnterCompanyBankAccountDetails) - (implicit appConfig: FrontendAppConfig, - val executionContext: ExecutionContext, - baseControllerComponents: BaseControllerComponents) extends BaseController { +class UkBankAccountDetailsController @Inject() ( + val authConnector: AuthClientConnector, + val bankAccountDetailsService: BankAccountDetailsService, + val sessionService: SessionService, + configuration: Configuration, + view: EnterCompanyBankAccountDetails +)(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) + extends BaseController { - def show: Action[AnyContent] = isAuthenticatedWithProfile { - implicit request => - implicit profile => - for { - bankDetails <- bankAccountDetailsService.fetchBankAccountDetails - filledForm = bankDetails.flatMap(_.details).fold(enterBankAccountDetailsForm)(enterBankAccountDetailsForm.fill) - } yield Ok(view(filledForm)) + private val encrypter = + SymmetricCryptoFactory.aesCryptoFromConfig("json.encryption", configuration.underlying) + + private implicit val encryptedFormat: Format[BankAccountDetails] = + BankAccountDetailsSessionFormat.format(encrypter) + + private val sessionKey = "bankAccountDetails" + + def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + if (isEnabled(UseNewBarsVerify)) { + sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { + case Some(details) => Ok(view(enterBankAccountDetailsForm.fill(details))) + case None => Ok(view(enterBankAccountDetailsForm)) + } + } else { + for { + bankDetails <- bankAccountDetailsService.getBankAccount + filledForm = bankDetails.flatMap(_.details).fold(enterBankAccountDetailsForm)(enterBankAccountDetailsForm.fill) + } yield Ok(view(filledForm)) + } } - def submit: Action[AnyContent] = isAuthenticatedWithProfile { - implicit request => - implicit profile => - enterBankAccountDetailsForm.bindFromRequest().fold( - formWithErrors => - Future.successful(BadRequest(view(formWithErrors))), - accountDetails => - bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { accountDetailsValid => - if (accountDetailsValid) { - Redirect(controllers.routes.TaskListController.show.url) - } - else { - val invalidDetails = EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails) - BadRequest(view(invalidDetails)) - } + def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + enterBankAccountDetailsForm + .bindFromRequest() + .fold( + formWithErrors => Future.successful(BadRequest(view(formWithErrors))), + accountDetails => + if (isEnabled(UseNewBarsVerify)) + sessionService.cache[BankAccountDetails](sessionKey, accountDetails).map { _ => + Redirect(routes.CheckBankDetailsController.show) } - ) + else + bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { + case true => Redirect(controllers.routes.TaskListController.show.url) + case false => BadRequest(view(EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) + } + ) } - } diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index bfe9a8f77..c2b1e1471 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -55,6 +55,7 @@ object EnterBankAccountDetailsForm { val ACCOUNT_NAME = "accountName" val ACCOUNT_NUMBER = "accountNumber" val SORT_CODE = "sortCode" + val ROLL_NUMBER = "rollNumber" val accountNameEmptyKey = "validation.companyBankAccount.name.missing" val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" @@ -63,6 +64,7 @@ object EnterBankAccountDetailsForm { val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" + val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" val invalidAccountReputationKey = "sortCodeAndAccountGroup" val invalidAccountReputationMessage = "validation.companyBankAccount.invalidCombination" @@ -71,6 +73,7 @@ object EnterBankAccountDetailsForm { private val accountNameMaxLength = 60 private val accountNumberRegex = """[0-9]{6,8}""".r private val sortCodeRegex = """[0-9]{6}""".r + private val rollNumberMaxLength = 25 val form = Form[BankAccountDetails]( mapping( @@ -93,11 +96,19 @@ object EnterBankAccountDetailsForm { stopOnFail( mandatory(sortCodeEmptyKey), matchesRegex(sortCodeRegex, sortCodeInvalidKey) - )) - )((accountName, accountNumber, sortCode) => BankAccountDetails.apply(accountName, accountNumber, sortCode, None))(bankAccountDetails => - BankAccountDetails.unapply(bankAccountDetails).map { case (accountName, accountNumber, sortCode, _) => - (accountName, accountNumber, sortCode) - }) + )), + ROLL_NUMBER -> optional( + text + .transform(removeSpaces, identity[String]) + .verifying( + maxLength(rollNumberMaxLength, rollNumberInvalidKey) + ) + ) + )((accountName, accountNumber, sortCode, rollNumber) => BankAccountDetails.apply(accountName, accountNumber, sortCode, rollNumber))( + bankAccountDetails => + BankAccountDetails.unapply(bankAccountDetails).map { case (accountName, accountNumber, sortCode, rollNumber, _) => + (accountName, accountNumber, sortCode, rollNumber) + }) ) val formWithInvalidAccountReputation: Form[BankAccountDetails] = diff --git a/app/models/BankAccountDetails.scala b/app/models/BankAccountDetails.scala index 45dae9ea0..2551f4c1d 100644 --- a/app/models/BankAccountDetails.scala +++ b/app/models/BankAccountDetails.scala @@ -26,7 +26,11 @@ case class BankAccount(isProvided: Boolean, reason: Option[NoUKBankAccount], bankAccountType: Option[BankAccountType] = None) -case class BankAccountDetails(name: String, number: String, sortCode: String, status: Option[BankAccountDetailsStatus] = None) +case class BankAccountDetails(name: String, + number: String, + sortCode: String, + rollNumber: Option[String] = None, + status: Option[BankAccountDetailsStatus] = None) object BankAccountDetails { implicit val accountReputationWrites: OWrites[BankAccountDetails] = new OWrites[BankAccountDetails] { @@ -42,10 +46,11 @@ object BankAccountDetails { def bankSeq(bankAccount: BankAccountDetails): Seq[String] = Seq( - bankAccount.name, - bankAccount.number, - bankAccount.sortCode - ) + Some(bankAccount.name), + Some(bankAccount.number), + Some(bankAccount.sortCode), + bankAccount.rollNumber + ).flatten } object BankAccount { diff --git a/app/models/bars/BankAccountDetailsSessionFormat.scala b/app/models/bars/BankAccountDetailsSessionFormat.scala new file mode 100644 index 000000000..96a86316f --- /dev/null +++ b/app/models/bars/BankAccountDetailsSessionFormat.scala @@ -0,0 +1,45 @@ +/* + * Copyright 2026 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.bars + +import models.api.BankAccountDetailsStatus +import models.BankAccountDetails +import play.api.libs.functional.syntax._ +import play.api.libs.json._ +import uk.gov.hmrc.crypto.{Decrypter, Encrypter} +import uk.gov.hmrc.crypto.Sensitive.SensitiveString +import uk.gov.hmrc.crypto.json.JsonEncryption + +object BankAccountDetailsSessionFormat { + + def format(encrypter: Encrypter with Decrypter): Format[BankAccountDetails] = { + implicit val crypto: Encrypter with Decrypter = encrypter + + implicit val sensitiveStringFormat: Format[SensitiveString] = + JsonEncryption.sensitiveEncrypterDecrypter(SensitiveString.apply) + + ( + (__ \ "name").format[String] and + (__ \ "sortCode").format[SensitiveString] + .bimap(_.decryptedValue, SensitiveString.apply) and + (__ \ "number").format[SensitiveString] + .bimap(_.decryptedValue, SensitiveString.apply) and + (__ \ "rollNumber").formatNullable[String] and + (__ \ "status").formatNullable[BankAccountDetailsStatus] + )(BankAccountDetails.apply, unlift(BankAccountDetails.unapply)) + } +} \ No newline at end of file diff --git a/app/services/BankAccountDetailsService.scala b/app/services/BankAccountDetailsService.scala index 4cf669d95..01812b2c4 100644 --- a/app/services/BankAccountDetailsService.scala +++ b/app/services/BankAccountDetailsService.scala @@ -20,6 +20,7 @@ import connectors.RegistrationApiConnector import models._ import models.bars.BankAccountType import models.api.{IndeterminateStatus, InvalidStatus, ValidStatus} +import play.api.libs.json.Json import uk.gov.hmrc.http.HeaderCarrier import javax.inject.{Inject, Singleton} @@ -30,18 +31,18 @@ import play.api.mvc.Request class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiConnector, val bankAccountRepService: BankAccountReputationService) { - def fetchBankAccountDetails(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = { + def getBankAccount(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = { regApiConnector.getSection[BankAccount](profile.registrationId) } - def saveBankAccountDetails(bankAccount: BankAccount) + def saveBankAccount(bankAccount: BankAccount) (implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = { regApiConnector.replaceSection[BankAccount](profile.registrationId, bankAccount) } def saveHasCompanyBankAccount(hasBankAccount: Boolean) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { - val bankAccount = fetchBankAccountDetails map { + val bankAccount = getBankAccount map { case Some(BankAccount(oldHasBankAccount, _, _, _)) if oldHasBankAccount != hasBankAccount => BankAccount(hasBankAccount, None, None, None) case Some(bankAccountDetails) => @@ -50,13 +51,13 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo BankAccount(hasBankAccount, None, None, None) } - bankAccount flatMap saveBankAccountDetails + bankAccount flatMap saveBankAccount } def saveEnteredBankAccountDetails(accountDetails: BankAccountDetails) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[Boolean] = { for { - existing <- fetchBankAccountDetails + existing <- getBankAccount result <- bankAccountRepService.validateBankDetails(accountDetails).flatMap { case status@(ValidStatus | IndeterminateStatus) => val bankAccount = BankAccount( @@ -65,7 +66,7 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo reason = None, bankAccountType = existing.flatMap(_.bankAccountType) ) - saveBankAccountDetails(bankAccount) map (_ => true) + saveBankAccount(bankAccount) map (_ => true) case InvalidStatus => Future.successful(false) } } yield result @@ -79,14 +80,14 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo reason = Some(reason), bankAccountType = None ) - saveBankAccountDetails(bankAccount) + saveBankAccount(bankAccount) } def saveBankAccountType(bankAccountType: BankAccountType) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { - fetchBankAccountDetails.map { + getBankAccount.map { case Some(existing) => existing.copy(bankAccountType = Some(bankAccountType)) case None => BankAccount(isProvided = true, details = None, reason = None, bankAccountType = Some(bankAccountType)) - }.flatMap(saveBankAccountDetails) + }.flatMap(saveBankAccount) } } diff --git a/app/views/bankdetails/CheckBankDetailsView.scala.html b/app/views/bankdetails/CheckBankDetailsView.scala.html new file mode 100644 index 000000000..601d9c11b --- /dev/null +++ b/app/views/bankdetails/CheckBankDetailsView.scala.html @@ -0,0 +1,75 @@ +@* + * Copyright 2026 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. + *@ + +@import config.FrontendAppConfig +@import models.BankAccountDetails + +@this( + layout: layouts.layout, + h1: components.h1, + p: components.p, + button: components.button, + formWithCSRF: FormWithCSRF, + govukSummaryList: GovukSummaryList +) + +@(bankDetails: BankAccountDetails)(implicit request: Request[_], messages: Messages, appConfig: FrontendAppConfig) + +@layout(pageTitle = Some(messages("pages.checkBankDetails.heading"))) { + + @h1("pages.checkBankDetails.heading") + + @govukSummaryList(SummaryList( + card = Some(Card( + title = Some(CardTitle(content = Text(messages("pages.checkBankDetails.cardTitle")))), + actions = Some(Actions(items = Seq( + ActionItem( + href = controllers.bankdetails.routes.UkBankAccountDetailsController.show.url, + content = Text(messages("pages.checkBankDetails.change")), + visuallyHiddenText = Some(messages("pages.checkBankDetails.change.hidden")) + ) + ))) + )), + rows = Seq( + Some(SummaryListRow( + key = Key(content = Text(messages("pages.checkBankDetails.accountName"))), + value = Value(content = Text(bankDetails.name)) + )), + Some(SummaryListRow( + key = Key(content = Text(messages("pages.checkBankDetails.accountNumber"))), + value = Value(content = Text(bankDetails.number)) + )), + Some(SummaryListRow( + key = Key(content = Text(messages("pages.checkBankDetails.sortCode"))), + value = Value(content = Text(bankDetails.sortCode)) + )), + bankDetails.rollNumber.map { rollNumber => + SummaryListRow( + key = Key(content = Text(messages("pages.checkBankDetails.rollNumber"))), + value = Value(content = Text(rollNumber)) + ) + } + ).flatten + )) + + @p { + @messages("pages.checkBankDetails.p1") + } + + @formWithCSRF(action = controllers.bankdetails.routes.CheckBankDetailsController.submit) { + @button("app.common.confirmAndContinue") + } +} \ No newline at end of file diff --git a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html index f93cb3b06..934530566 100644 --- a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html +++ b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html @@ -36,7 +36,9 @@ @errorSummary(errors = bankDetailsForm.errors) @h1("pages.bankDetails.heading") - @p { @messages("pages.bankDetails.p1") } + @p { + @messages("pages.bankDetails.p1") + } @panelIndent { @messages("pages.bankDetails.info") @@ -44,28 +46,36 @@ @formWithCSRF(action = controllers.bankdetails.routes.UkBankAccountDetailsController.submit) { @inputText(form = bankDetailsForm, - id = "accountName", - name = "accountName", - label = messages("pages.bankDetails.accountName.label"), - classes = Some("govuk-input--width-10"), - isPageHeading = false, - hint = Some(Html(messages("")))) + id = "accountName", + name = "accountName", + label = messages("pages.bankDetails.accountName.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountName.hint")))) + + @inputText(form = bankDetailsForm, + id = "accountNumber", + name = "accountNumber", + label = messages("pages.bankDetails.accountNumber.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) @inputText(form = bankDetailsForm, - id = "accountNumber", - name = "accountNumber", - label = messages("pages.bankDetails.accountNumber.label"), - classes = Some("govuk-input--width-10"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) + id = "sortCode", + name = "sortCode", + label = messages("pages.bankDetails.sortCode.label"), + classes = Some("govuk-input--width-5"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) @inputText(form = bankDetailsForm, - id = "sortCode", - name = "sortCode", - label = messages("pages.bankDetails.sortCode.label"), - classes = Some("govuk-input--width-5"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) + id = "rollNumber", + name = "rollNumber", + label = messages("pages.bankDetails.rollNumber.label"), + classes = Some("govuk-input--width-20"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) @button("app.common.continue") } diff --git a/conf/app.routes b/conf/app.routes index 7254020a3..d61850bdb 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -130,10 +130,14 @@ POST /cannot-provide-bank-account-details controller GET /choose-account-type controllers.bankdetails.ChooseAccountTypeController.show POST /choose-account-type controllers.bankdetails.ChooseAccountTypeController.submit -## BANK DETAILS (ACCOUNT NAME, NUMBER, SORT CODE) +## ENTER BANK DETAILS GET /account-details controllers.bankdetails.UkBankAccountDetailsController.show POST /account-details controllers.bankdetails.UkBankAccountDetailsController.submit +## CHECK BANK DETAILS +GET /check-bank-details controllers.bankdetails.CheckBankDetailsController.show +POST /check-bank-details controllers.bankdetails.CheckBankDetailsController.submit + ## VAT CORRESPONDENCE GET /vat-correspondence-language controllers.business.VatCorrespondenceController.show POST /vat-correspondence-language controllers.business.VatCorrespondenceController.submit diff --git a/conf/application.conf b/conf/application.conf index 986e61536..bc1f88a86 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -259,6 +259,11 @@ Test { } } +json.encryption { + key = "MTIzNDU2Nzg5MDEyMzQ1Ng==" + previousKeys = [] +} + controllers.internal.RegistrationController = { needsLogging = true needsAuditing = false diff --git a/conf/messages b/conf/messages index dd8a986f1..c61cb2a7b 100644 --- a/conf/messages +++ b/conf/messages @@ -973,7 +973,6 @@ pages.hasCompanyBankAccount.bullet3 = in the UK pages.hasCompanyBankAccount.bullet4 = able to receive BACS payments validation.hasCompanyBankAccount.missing = Select yes if you are able to provide bank or building society details for the business - # Bank Account Type Page pages.chooseAccountType.heading = What kind of bank or building society account will you use for VAT repayments? pages.chooseAccountType.option.business = Business account @@ -991,14 +990,17 @@ pages.noUKBankAccount.dontWantToProvide = I do not want to provide pages.noUKBankAccount.error = Select why you cannot provide bank account details for the business # Bank Details page -pages.bankDetails.heading = What are the business’s bank or building society account details? -pages.bankDetails.p1 = HMRC VAT will only use this account to send VAT repayments. We will not take money from it. +pages.bankDetails.heading = What are the business’s account details? +pages.bankDetails.p1 = HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply. pages.bankDetails.info = You must tell us if your account details change. -pages.bankDetails.accountName.label = Account name +pages.bankDetails.accountName.label = Name on the account +pages.bankDetails.accountName.hint = Enter the name as it appears on bank statements and in your VAT account pages.bankDetails.accountNumber.label = Account number pages.bankDetails.accountNumber.hint = Must be between 6 and 8 digits long pages.bankDetails.sortCode.label = Sort code pages.bankDetails.sortCode.hint = Must be 6 digits long +pages.bankDetails.rollNumber.label = Building society roll number (if you have one) +pages.bankDetails.rollNumber.hint = You can find it on your card, statement or passbook validation.companyBankAccount.name.missing = Enter the account name validation.companyBankAccount.name.maxLength = The account name must be 60 characters or less validation.companyBankAccount.name.invalid = The account name must only include numbers, letters a to z, and special characters such as hyphens, apostrophes and brackets @@ -1007,6 +1009,20 @@ validation.companyBankAccount.number.invalid = Account number must be bet validation.companyBankAccount.sortCode.missing = Enter the account sort code validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code +validation.companyBankAccount.rollNumber.invalid = Enter a valid roll number + +# Check Bank Detials Page +pages.checkBankDetails.heading = Check your account details +pages.checkBankDetails.cardTitle = Bank account details +pages.checkBankDetails.change = Change +pages.checkBankDetails.accountName = Account name +pages.checkBankDetails.accountNumber = Account number +pages.checkBankDetails.sortCode = Sort code +pages.checkBankDetails.rollNumber = Building society roll number +pages.checkBankDetails.p1 = By confirming these account details, you agree the information you have provided is complete and correct. + + + # Main Business Activity Page pages.mainBusinessActivity.heading = Which activity is the business’s main source of income? diff --git a/test/fixtures/VatRegistrationFixture.scala b/test/fixtures/VatRegistrationFixture.scala index 93071d347..ea6467be0 100644 --- a/test/fixtures/VatRegistrationFixture.scala +++ b/test/fixtures/VatRegistrationFixture.scala @@ -34,6 +34,7 @@ trait BaseFixture { val testTradingName = "ACME INC" val testSortCode = "12-34-56" val testAccountNumber = "12345678" + val testRollNumber = Some("AB/121212") val validLabourSicCode: SicCode = SicCode("81221001", "BarFoo", "BarFoo") val validNoCompliance: SicCode = SicCode("12345678", "fooBar", "FooBar") val validExpectedOverTrue: Option[LocalDate] = Some(testDate) @@ -397,7 +398,7 @@ trait VatRegistrationFixture extends BaseFixture with FlatRateFixtures with Appl val testBankName = "testName" - val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, Some(ValidStatus)) + val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, None, Some(ValidStatus)) val bankAccount: BankAccount = BankAccount(isProvided = true, Some(testUkBankDetails), None) diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index fb2b8e877..4b7199d74 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -30,6 +30,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val validAccountName = s"${numStr}testAccountName" val validAccountNumber = "12345678" val validSortCode = "123456" + val validRollNumber = "AB/121212" "successfully bind data to the form with no errors and allow the return of a valid BankAccountDetails case class" in { val formData = Map( @@ -44,6 +45,19 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.get mustBe validBankAccountDetails } + "successfully bind data with a valid roll number" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> validRollNumber + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe Some("AB/121212") + } + "return a FormError when binding an empty account name to the form" in { val formData = Map( ACCOUNT_NAME -> "", @@ -58,6 +72,19 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe accountNameEmptyKey } + "successfully bind data when roll number is empty string" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> "" + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe None + } + "return a FormError when binding an account name which exceeds max length to the form" in { val exceedMaxLength = "AlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimited" @@ -74,6 +101,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe accountNameMaxLengthKey } + "return a FormError when binding an invalid account name to the form" in { val invalidAccountName = "123#@~" @@ -211,5 +239,60 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.key mustBe SORT_CODE boundForm.errors.head.message mustBe sortCodeEmptyKey } + + "successfully bind data with a valid roll number" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> validRollNumber + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe Some("AB/121212") + } + + "successfully bind data when roll number is empty string" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> "" + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe None + } + + "return a FormError when roll number exceeds max length" in { + val tooLongRollNumber = "A" * 26 + + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> tooLongRollNumber + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ROLL_NUMBER + boundForm.errors.head.message mustBe rollNumberInvalidKey + } + + "successfully bind roll number with spaces stripped" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> "AB 121 212" + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe Some("AB121212") + } } } diff --git a/test/models/BankAccountDetailsSpec.scala b/test/models/BankAccountDetailsSpec.scala index de6bc9bd5..3c3467c8f 100644 --- a/test/models/BankAccountDetailsSpec.scala +++ b/test/models/BankAccountDetailsSpec.scala @@ -16,8 +16,8 @@ package models -import play.api.libs.json.{Json, JsSuccess, JsError} import models.bars.BankAccountType +import play.api.libs.json.{JsError, Json, JsSuccess} import testHelpers.VatRegSpec class BankAccountDetailsSpec extends VatRegSpec { @@ -32,6 +32,11 @@ class BankAccountDetailsSpec extends VatRegSpec { Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) } + "parse successfully from Json when rollNumber is present in details" in { + val expected = validUkBankAccount.copy(details = Some(BankAccountDetails("testName", "12-34-56", "12345678", rollNumber = Some("AB/121212")))) + Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) + } + "parse successfully from Json when neither details or reason are present" in { val expected = validUkBankAccount.copy(details = None, reason = None) Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) @@ -85,5 +90,18 @@ class BankAccountDetailsSpec extends VatRegSpec { ) json.validate[BankAccount].map(_.bankAccountType) mustBe a[JsError] } + + "write rollNumber when present" in { + val json = Json.obj( + "isProvided" -> true, + "details" -> Json.obj( + "name" -> "testName", + "sortCode" -> "12-34-56", + "number" -> "12345678", + "rollNumber" -> "AB/121212" + ) + ) + json.validate[BankAccount].map(_.details.flatMap(_.rollNumber)) mustBe JsSuccess(Some("AB/121212")) + } } } diff --git a/test/models/bars/BankAccountDetailsSessionFormatSpec.scala b/test/models/bars/BankAccountDetailsSessionFormatSpec.scala new file mode 100644 index 000000000..a334cca21 --- /dev/null +++ b/test/models/bars/BankAccountDetailsSessionFormatSpec.scala @@ -0,0 +1,105 @@ +/* + * Copyright 2026 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.bars + +import models.BankAccountDetails +import models.api.ValidStatus +import org.scalatest.matchers.must.Matchers +import org.scalatest.wordspec.AnyWordSpec +import play.api.libs.json.{Format, Json} +import uk.gov.hmrc.crypto.SymmetricCryptoFactory + +class BankAccountDetailsSessionFormatSpec extends AnyWordSpec with Matchers { + + private val testEncryptionKey = "MTIzNDU2Nzg5MDEyMzQ1Ng==" + private val encrypter = SymmetricCryptoFactory.aesCryptoFromConfig( + baseConfigKey = "json.encryption", + config = com.typesafe.config.ConfigFactory.parseString( + s"""json.encryption.key="$testEncryptionKey"""" + ) + ) + + private implicit val format: Format[BankAccountDetails] = + BankAccountDetailsSessionFormat.format(encrypter) + + val details: BankAccountDetails = BankAccountDetails( + name = "Test Account", + sortCode = "123456", + number = "12345678", + rollNumber = None, + status = None + ) + + "BankAccountDetailsSessionFormat" should { + + "encrypt sortCode and number when writing to Json" in { + val json = Json.toJson(details) + (json \ "sortCode").as[String] mustNot equal("123456") + (json \ "number").as[String] mustNot equal("12345678") + } + + "not encrypt name when writing to Json" in { + val json = Json.toJson(details) + (json \ "name").as[String] mustBe "Test Account" + } + + "not encrypt rollNumber when writing to Json" in { + val withRollNumber = details.copy(rollNumber = Some("AB/121212")) + val json = Json.toJson(withRollNumber) + (json \ "rollNumber").as[String] mustBe "AB/121212" + } + + "rollNumber is not included from Json when None" in { + val json = Json.toJson(details) + (json \ "rollNumber").toOption mustBe None + } + + "status not included from Json when None" in { + val json = Json.toJson(details) + (json \ "status").toOption mustBe None + } + + "read and write BankAccountDetails with all fields" in { + val full = BankAccountDetails( + name = "Test Account", + sortCode = "123456", + number = "12345678", + rollNumber = Some("AB/121212"), + status = Some(ValidStatus) + ) + val json = Json.toJson(full) + val result = Json.fromJson[BankAccountDetails](json).get + result mustBe full + } + + "fail to decrypt with a different key" in { + val json = Json.toJson(details) + + val differentKey = "ZGlmZmVyZW50a2V5MTIzNA==" + val differentEncrypter = SymmetricCryptoFactory.aesCryptoFromConfig( + baseConfigKey = "json.encryption", + config = com.typesafe.config.ConfigFactory.parseString( + s"""json.encryption.key="$differentKey"""" + ) + ) + val differentFormat: Format[BankAccountDetails] = + BankAccountDetailsSessionFormat.format(differentEncrypter) + + Json.fromJson[BankAccountDetails](json)(differentFormat).isError mustBe true + } + } +} \ No newline at end of file diff --git a/test/services/BankAccountDetailsServiceSpec.scala b/test/services/BankAccountDetailsServiceSpec.scala index ff1bb4e63..d383b3e3e 100644 --- a/test/services/BankAccountDetailsServiceSpec.scala +++ b/test/services/BankAccountDetailsServiceSpec.scala @@ -47,7 +47,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn "return a BankAccount" in new Setup { mockGetSection[BankAccount](testRegId, Some(bankAccount)) - val result: Option[BankAccount] = await(service.fetchBankAccountDetails) + val result: Option[BankAccount] = await(service.getBankAccount) result mustBe Some(bankAccount) } @@ -55,7 +55,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn "return None if a BankAccount isn't found" in new Setup { mockGetSection[BankAccount](testRegId, None) - val result: Option[BankAccount] = await(service.fetchBankAccountDetails) + val result: Option[BankAccount] = await(service.getBankAccount) result mustBe None } @@ -67,7 +67,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn mockReplaceSection[BankAccount](testRegId, fullBankAccount) - val result: BankAccount = await(service.saveBankAccountDetails(fullBankAccount)) + val result: BankAccount = await(service.saveBankAccount(fullBankAccount)) result mustBe fullBankAccount verify(mockRegistrationApiConnector, times(1)) diff --git a/test/views/bankdetails/CheckBankDetailsViewSpec.scala b/test/views/bankdetails/CheckBankDetailsViewSpec.scala new file mode 100644 index 000000000..fe78be76e --- /dev/null +++ b/test/views/bankdetails/CheckBankDetailsViewSpec.scala @@ -0,0 +1,123 @@ +/* + * Copyright 2026 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 views.bankdetails + +import models.BankAccountDetails +import org.jsoup.Jsoup +import org.jsoup.nodes.Document +import views.VatRegViewSpec +import views.html.bankdetails.CheckBankDetailsView + +class CheckBankDetailsViewSpec extends VatRegViewSpec { + + val view: CheckBankDetailsView = app.injector.instanceOf[CheckBankDetailsView] + + val title = "Check your bank account details" + val heading = "Check your bank account details" + val cardTitle = "Bank account details" + val changeLink = "Change" + val accountNameLabel = "Account name" + val accountNumberLabel = "Account number" + val sortCodeLabel = "Sort code" + val rollNumberLabel = "Roll number" + val p1 = "By confirming, you are providing your consent for HMRC to verify your bank details with your bank." + val buttonText = "Confirm and continue" + + val bankDetails: BankAccountDetails = BankAccountDetails( + name = "Test Account", + sortCode = "123456", + number = "12345678", + rollNumber = None, + status = None + ) + + val bankDetailsWithRollNumber: BankAccountDetails = bankDetails.copy(rollNumber = Some("AB/121212")) + + "CheckBankDetailsView" should { + + implicit lazy val doc: Document = Jsoup.parse(view(bankDetails).body) + + "have the correct title" in new ViewSetup { + doc.title must include(title) + } + + "have the correct heading" in new ViewSetup { + doc.heading mustBe Some(heading) + } + + "have the correct card title" in new ViewSetup { + doc.select(".govuk-summary-card__title").text mustBe cardTitle + } + + "have a change link" in new ViewSetup { + doc.select(".govuk-summary-card__action a").text must include(changeLink) + } + + "have the correct account name label" in new ViewSetup { + doc.select(".govuk-summary-list__key").get(0).text mustBe accountNameLabel + } + + "have the correct account name value" in new ViewSetup { + doc.select(".govuk-summary-list__value").get(0).text mustBe "Test Account" + } + + "have the correct account number label" in new ViewSetup { + doc.select(".govuk-summary-list__key").get(1).text mustBe accountNumberLabel + } + + "have the correct account number value" in new ViewSetup { + doc.select(".govuk-summary-list__value").get(1).text mustBe "12345678" + } + + "have the correct sort code label" in new ViewSetup { + doc.select(".govuk-summary-list__key").get(2).text mustBe sortCodeLabel + } + + "have the correct sort code value" in new ViewSetup { + doc.select(".govuk-summary-list__value").get(2).text mustBe "123456" + } + + "not show roll number row when roll number is absent" in new ViewSetup { + doc.select(".govuk-summary-list__key").size mustBe 3 + } + + "have the correct p1" in new ViewSetup { + doc.para(1) mustBe Some(p1) + } + + "have the correct continue button" in new ViewSetup { + doc.submitButton mustBe Some(buttonText) + } + } + + "CheckBankDetailsView with roll number" should { + + implicit lazy val doc: Document = Jsoup.parse(view(bankDetailsWithRollNumber).body) + + "show the roll number row when roll number is present" in new ViewSetup { + doc.select(".govuk-summary-list__key").size mustBe 4 + } + + "have the correct roll number label" in new ViewSetup { + doc.select(".govuk-summary-list__key").get(3).text mustBe rollNumberLabel + } + + "have the correct roll number value" in new ViewSetup { + doc.select(".govuk-summary-list__value").get(3).text mustBe "AB/121212" + } + } +} \ No newline at end of file diff --git a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala index 02f2076a7..cd66103f0 100644 --- a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala +++ b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala @@ -26,15 +26,17 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { val view: EnterCompanyBankAccountDetails = app.injector.instanceOf[EnterCompanyBankAccountDetails] - val title = "What are the business’s bank or building society account details?" - val heading = "What are the business’s bank or building society account details?" - val p1 = "HMRC VAT will only use this account to send VAT repayments. We will not take money from it." + val title = "What are the business’s account details?" + val heading = "What are the business’s account details?" + val p1 = "HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply." val panelText = "You must tell us if your account details change." - val accountName = "Account name" + val accountName = "Name on the account" val accountNumber = "Account number" val accountNumberHint = "Must be between 6 and 8 digits long" val sortCode = "Sort code" val sortCodeHint = "Must be 6 digits long" + val rollNumber = "Building society roll number (if you have one)" + val rollNumberHint = "You can find it on your card, statement or passbook" val buttonText = "Save and continue" "Company Bank Details Page" should { @@ -76,6 +78,14 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { doc.hintWithMultiple(3) mustBe Some(sortCodeHint) } + "have the correct roll number label text" in { + doc.select(Selectors.label).get(3).text mustBe rollNumber + } + + "have the correct roll number Hint text" in new ViewSetup() { + doc.hintWithMultiple(4) mustBe Some(rollNumberHint) + } + "have the correct continue button" in new ViewSetup { doc.submitButton mustBe Some(buttonText) } From b2c032e852105e5ae8be31f41552f1ac649660b5 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Wed, 11 Mar 2026 06:28:50 +0000 Subject: [PATCH 02/11] feat: add rollnumber to view, form and controller. cache bank details. rename saving and get methods --- app/config/startup/VerifyCrypto.scala | 2 +- .../ChooseAccountTypeController.scala | 2 +- .../HasBankAccountController.scala | 2 +- .../NoUKBankAccountController.scala | 2 +- .../UkBankAccountDetailsController.scala | 81 ++++++++++++------- app/forms/BankAccountDetailsForms.scala | 21 +++-- app/models/BankAccountDetails.scala | 15 ++-- app/services/BankAccountDetailsService.scala | 19 ++--- .../EnterCompanyBankAccountDetails.scala.html | 48 ++++++----- conf/messages | 11 ++- test/fixtures/VatRegistrationFixture.scala | 3 +- test/forms/BankAccountDetailsFormSpec.scala | 56 +++++++++++++ test/models/BankAccountDetailsSpec.scala | 20 ++++- .../BankAccountDetailsServiceSpec.scala | 6 +- .../CompanyBankDetailsViewSpec.scala | 18 ++++- 15 files changed, 220 insertions(+), 86 deletions(-) diff --git a/app/config/startup/VerifyCrypto.scala b/app/config/startup/VerifyCrypto.scala index c074d4215..e0ee6cc0b 100644 --- a/app/config/startup/VerifyCrypto.scala +++ b/app/config/startup/VerifyCrypto.scala @@ -16,7 +16,7 @@ package config.startup -import uk.gov.hmrc.crypto.ApplicationCrypto +import uk.gov.hmrc.play.bootstrap.frontend.filters.crypto.ApplicationCrypto import javax.inject.{Inject, Singleton} diff --git a/app/controllers/bankdetails/ChooseAccountTypeController.scala b/app/controllers/bankdetails/ChooseAccountTypeController.scala index c0d76ead0..1de0d142d 100644 --- a/app/controllers/bankdetails/ChooseAccountTypeController.scala +++ b/app/controllers/bankdetails/ChooseAccountTypeController.scala @@ -39,7 +39,7 @@ class ChooseAccountTypeController @Inject() (val authConnector: AuthClientConnec def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => if (isEnabled(UseNewBarsVerify)) { - bankAccountDetailsService.fetchBankAccountDetails.map { bankDetails => + bankAccountDetailsService.getBankAccount.map { bankDetails => val filledForm = bankDetails .flatMap(_.bankAccountType) .fold(ChooseAccountTypeForm.form)(ChooseAccountTypeForm.form.fill) diff --git a/app/controllers/bankdetails/HasBankAccountController.scala b/app/controllers/bankdetails/HasBankAccountController.scala index d843e2615..704846c66 100644 --- a/app/controllers/bankdetails/HasBankAccountController.scala +++ b/app/controllers/bankdetails/HasBankAccountController.scala @@ -45,7 +45,7 @@ class HasBankAccountController @Inject()(val authConnector: AuthClientConnector, Future.successful(Redirect(controllers.flatratescheme.routes.JoinFlatRateSchemeController.show)) case _ => for { - bankDetails <- bankAccountDetailsService.fetchBankAccountDetails + bankDetails <- bankAccountDetailsService.getBankAccount filledForm = bankDetails.map(_.isProvided).fold(hasBankAccountForm)(hasBankAccountForm.fill) } yield Ok(view(filledForm)) } diff --git a/app/controllers/bankdetails/NoUKBankAccountController.scala b/app/controllers/bankdetails/NoUKBankAccountController.scala index 24d230e51..0a2639ee9 100644 --- a/app/controllers/bankdetails/NoUKBankAccountController.scala +++ b/app/controllers/bankdetails/NoUKBankAccountController.scala @@ -40,7 +40,7 @@ class NoUKBankAccountController @Inject()(noUKBankAccountView: NoUkBankAccount, implicit request => implicit profile => for { - optBankAccountDetails <- bankAccountDetailsService.fetchBankAccountDetails + optBankAccountDetails <- bankAccountDetailsService.getBankAccount form = optBankAccountDetails.flatMap(_.reason).fold(NoUKBankAccountForm.form)(NoUKBankAccountForm.form.fill) } yield Ok(noUKBankAccountView(form)) } diff --git a/app/controllers/bankdetails/UkBankAccountDetailsController.scala b/app/controllers/bankdetails/UkBankAccountDetailsController.scala index 4a7b2c48e..6596c59f6 100644 --- a/app/controllers/bankdetails/UkBankAccountDetailsController.scala +++ b/app/controllers/bankdetails/UkBankAccountDetailsController.scala @@ -18,49 +18,68 @@ package controllers.bankdetails import config.{AuthClientConnector, BaseControllerComponents, FrontendAppConfig} import controllers.BaseController +import featuretoggle.FeatureSwitch.UseNewBarsVerify +import featuretoggle.FeatureToggleSupport.isEnabled import forms.EnterBankAccountDetailsForm import forms.EnterBankAccountDetailsForm.{form => enterBankAccountDetailsForm} +import models.BankAccountDetails +import models.bars.BankAccountDetailsSessionFormat +import play.api.libs.json.Format import play.api.mvc.{Action, AnyContent} +import play.api.Configuration import services.{BankAccountDetailsService, SessionService} +import uk.gov.hmrc.crypto.SymmetricCryptoFactory import views.html.bankdetails.EnterCompanyBankAccountDetails import javax.inject.Inject import scala.concurrent.{ExecutionContext, Future} -class UkBankAccountDetailsController @Inject()(val authConnector: AuthClientConnector, - val bankAccountDetailsService: BankAccountDetailsService, - val sessionService: SessionService, - view: EnterCompanyBankAccountDetails) - (implicit appConfig: FrontendAppConfig, - val executionContext: ExecutionContext, - baseControllerComponents: BaseControllerComponents) extends BaseController { +class UkBankAccountDetailsController @Inject() ( + val authConnector: AuthClientConnector, + val bankAccountDetailsService: BankAccountDetailsService, + val sessionService: SessionService, + configuration: Configuration, + view: EnterCompanyBankAccountDetails +)(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) + extends BaseController { - def show: Action[AnyContent] = isAuthenticatedWithProfile { - implicit request => - implicit profile => - for { - bankDetails <- bankAccountDetailsService.fetchBankAccountDetails - filledForm = bankDetails.flatMap(_.details).fold(enterBankAccountDetailsForm)(enterBankAccountDetailsForm.fill) - } yield Ok(view(filledForm)) + private val encrypter = + SymmetricCryptoFactory.aesCryptoFromConfig("json.encryption", configuration.underlying) + + private implicit val encryptedFormat: Format[BankAccountDetails] = + BankAccountDetailsSessionFormat.format(encrypter) + + private val sessionKey = "bankAccountDetails" + + def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + if (isEnabled(UseNewBarsVerify)) { + sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { + case Some(details) => Ok(view(enterBankAccountDetailsForm.fill(details))) + case None => Ok(view(enterBankAccountDetailsForm)) + } + } else { + for { + bankDetails <- bankAccountDetailsService.getBankAccount + filledForm = bankDetails.flatMap(_.details).fold(enterBankAccountDetailsForm)(enterBankAccountDetailsForm.fill) + } yield Ok(view(filledForm)) + } } - def submit: Action[AnyContent] = isAuthenticatedWithProfile { - implicit request => - implicit profile => - enterBankAccountDetailsForm.bindFromRequest().fold( - formWithErrors => - Future.successful(BadRequest(view(formWithErrors))), - accountDetails => - bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { accountDetailsValid => - if (accountDetailsValid) { - Redirect(controllers.routes.TaskListController.show.url) - } - else { - val invalidDetails = EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails) - BadRequest(view(invalidDetails)) - } + def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => + enterBankAccountDetailsForm + .bindFromRequest() + .fold( + formWithErrors => Future.successful(BadRequest(view(formWithErrors))), + accountDetails => + if (isEnabled(UseNewBarsVerify)) + sessionService.cache[BankAccountDetails](sessionKey, accountDetails).map { _ => + Redirect(controllers.routes.TaskListController.show.url) } - ) + else + bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { + case true => Redirect(controllers.routes.TaskListController.show.url) + case false => BadRequest(view(EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) + } + ) } - } diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index bfe9a8f77..c2b1e1471 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -55,6 +55,7 @@ object EnterBankAccountDetailsForm { val ACCOUNT_NAME = "accountName" val ACCOUNT_NUMBER = "accountNumber" val SORT_CODE = "sortCode" + val ROLL_NUMBER = "rollNumber" val accountNameEmptyKey = "validation.companyBankAccount.name.missing" val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" @@ -63,6 +64,7 @@ object EnterBankAccountDetailsForm { val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" + val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" val invalidAccountReputationKey = "sortCodeAndAccountGroup" val invalidAccountReputationMessage = "validation.companyBankAccount.invalidCombination" @@ -71,6 +73,7 @@ object EnterBankAccountDetailsForm { private val accountNameMaxLength = 60 private val accountNumberRegex = """[0-9]{6,8}""".r private val sortCodeRegex = """[0-9]{6}""".r + private val rollNumberMaxLength = 25 val form = Form[BankAccountDetails]( mapping( @@ -93,11 +96,19 @@ object EnterBankAccountDetailsForm { stopOnFail( mandatory(sortCodeEmptyKey), matchesRegex(sortCodeRegex, sortCodeInvalidKey) - )) - )((accountName, accountNumber, sortCode) => BankAccountDetails.apply(accountName, accountNumber, sortCode, None))(bankAccountDetails => - BankAccountDetails.unapply(bankAccountDetails).map { case (accountName, accountNumber, sortCode, _) => - (accountName, accountNumber, sortCode) - }) + )), + ROLL_NUMBER -> optional( + text + .transform(removeSpaces, identity[String]) + .verifying( + maxLength(rollNumberMaxLength, rollNumberInvalidKey) + ) + ) + )((accountName, accountNumber, sortCode, rollNumber) => BankAccountDetails.apply(accountName, accountNumber, sortCode, rollNumber))( + bankAccountDetails => + BankAccountDetails.unapply(bankAccountDetails).map { case (accountName, accountNumber, sortCode, rollNumber, _) => + (accountName, accountNumber, sortCode, rollNumber) + }) ) val formWithInvalidAccountReputation: Form[BankAccountDetails] = diff --git a/app/models/BankAccountDetails.scala b/app/models/BankAccountDetails.scala index 45dae9ea0..2551f4c1d 100644 --- a/app/models/BankAccountDetails.scala +++ b/app/models/BankAccountDetails.scala @@ -26,7 +26,11 @@ case class BankAccount(isProvided: Boolean, reason: Option[NoUKBankAccount], bankAccountType: Option[BankAccountType] = None) -case class BankAccountDetails(name: String, number: String, sortCode: String, status: Option[BankAccountDetailsStatus] = None) +case class BankAccountDetails(name: String, + number: String, + sortCode: String, + rollNumber: Option[String] = None, + status: Option[BankAccountDetailsStatus] = None) object BankAccountDetails { implicit val accountReputationWrites: OWrites[BankAccountDetails] = new OWrites[BankAccountDetails] { @@ -42,10 +46,11 @@ object BankAccountDetails { def bankSeq(bankAccount: BankAccountDetails): Seq[String] = Seq( - bankAccount.name, - bankAccount.number, - bankAccount.sortCode - ) + Some(bankAccount.name), + Some(bankAccount.number), + Some(bankAccount.sortCode), + bankAccount.rollNumber + ).flatten } object BankAccount { diff --git a/app/services/BankAccountDetailsService.scala b/app/services/BankAccountDetailsService.scala index 4cf669d95..01812b2c4 100644 --- a/app/services/BankAccountDetailsService.scala +++ b/app/services/BankAccountDetailsService.scala @@ -20,6 +20,7 @@ import connectors.RegistrationApiConnector import models._ import models.bars.BankAccountType import models.api.{IndeterminateStatus, InvalidStatus, ValidStatus} +import play.api.libs.json.Json import uk.gov.hmrc.http.HeaderCarrier import javax.inject.{Inject, Singleton} @@ -30,18 +31,18 @@ import play.api.mvc.Request class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiConnector, val bankAccountRepService: BankAccountReputationService) { - def fetchBankAccountDetails(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = { + def getBankAccount(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = { regApiConnector.getSection[BankAccount](profile.registrationId) } - def saveBankAccountDetails(bankAccount: BankAccount) + def saveBankAccount(bankAccount: BankAccount) (implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = { regApiConnector.replaceSection[BankAccount](profile.registrationId, bankAccount) } def saveHasCompanyBankAccount(hasBankAccount: Boolean) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { - val bankAccount = fetchBankAccountDetails map { + val bankAccount = getBankAccount map { case Some(BankAccount(oldHasBankAccount, _, _, _)) if oldHasBankAccount != hasBankAccount => BankAccount(hasBankAccount, None, None, None) case Some(bankAccountDetails) => @@ -50,13 +51,13 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo BankAccount(hasBankAccount, None, None, None) } - bankAccount flatMap saveBankAccountDetails + bankAccount flatMap saveBankAccount } def saveEnteredBankAccountDetails(accountDetails: BankAccountDetails) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[Boolean] = { for { - existing <- fetchBankAccountDetails + existing <- getBankAccount result <- bankAccountRepService.validateBankDetails(accountDetails).flatMap { case status@(ValidStatus | IndeterminateStatus) => val bankAccount = BankAccount( @@ -65,7 +66,7 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo reason = None, bankAccountType = existing.flatMap(_.bankAccountType) ) - saveBankAccountDetails(bankAccount) map (_ => true) + saveBankAccount(bankAccount) map (_ => true) case InvalidStatus => Future.successful(false) } } yield result @@ -79,14 +80,14 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo reason = Some(reason), bankAccountType = None ) - saveBankAccountDetails(bankAccount) + saveBankAccount(bankAccount) } def saveBankAccountType(bankAccountType: BankAccountType) (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { - fetchBankAccountDetails.map { + getBankAccount.map { case Some(existing) => existing.copy(bankAccountType = Some(bankAccountType)) case None => BankAccount(isProvided = true, details = None, reason = None, bankAccountType = Some(bankAccountType)) - }.flatMap(saveBankAccountDetails) + }.flatMap(saveBankAccount) } } diff --git a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html index f93cb3b06..934530566 100644 --- a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html +++ b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html @@ -36,7 +36,9 @@ @errorSummary(errors = bankDetailsForm.errors) @h1("pages.bankDetails.heading") - @p { @messages("pages.bankDetails.p1") } + @p { + @messages("pages.bankDetails.p1") + } @panelIndent { @messages("pages.bankDetails.info") @@ -44,28 +46,36 @@ @formWithCSRF(action = controllers.bankdetails.routes.UkBankAccountDetailsController.submit) { @inputText(form = bankDetailsForm, - id = "accountName", - name = "accountName", - label = messages("pages.bankDetails.accountName.label"), - classes = Some("govuk-input--width-10"), - isPageHeading = false, - hint = Some(Html(messages("")))) + id = "accountName", + name = "accountName", + label = messages("pages.bankDetails.accountName.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountName.hint")))) + + @inputText(form = bankDetailsForm, + id = "accountNumber", + name = "accountNumber", + label = messages("pages.bankDetails.accountNumber.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) @inputText(form = bankDetailsForm, - id = "accountNumber", - name = "accountNumber", - label = messages("pages.bankDetails.accountNumber.label"), - classes = Some("govuk-input--width-10"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) + id = "sortCode", + name = "sortCode", + label = messages("pages.bankDetails.sortCode.label"), + classes = Some("govuk-input--width-5"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) @inputText(form = bankDetailsForm, - id = "sortCode", - name = "sortCode", - label = messages("pages.bankDetails.sortCode.label"), - classes = Some("govuk-input--width-5"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) + id = "rollNumber", + name = "rollNumber", + label = messages("pages.bankDetails.rollNumber.label"), + classes = Some("govuk-input--width-20"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) @button("app.common.continue") } diff --git a/conf/messages b/conf/messages index dd8a986f1..c8e0d2b6d 100644 --- a/conf/messages +++ b/conf/messages @@ -973,7 +973,6 @@ pages.hasCompanyBankAccount.bullet3 = in the UK pages.hasCompanyBankAccount.bullet4 = able to receive BACS payments validation.hasCompanyBankAccount.missing = Select yes if you are able to provide bank or building society details for the business - # Bank Account Type Page pages.chooseAccountType.heading = What kind of bank or building society account will you use for VAT repayments? pages.chooseAccountType.option.business = Business account @@ -991,14 +990,17 @@ pages.noUKBankAccount.dontWantToProvide = I do not want to provide pages.noUKBankAccount.error = Select why you cannot provide bank account details for the business # Bank Details page -pages.bankDetails.heading = What are the business’s bank or building society account details? -pages.bankDetails.p1 = HMRC VAT will only use this account to send VAT repayments. We will not take money from it. +pages.bankDetails.heading = What are the business’s account details? +pages.bankDetails.p1 = HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply. pages.bankDetails.info = You must tell us if your account details change. -pages.bankDetails.accountName.label = Account name +pages.bankDetails.accountName.label = Name on the account +pages.bankDetails.accountName.hint = Enter the name as it appears on bank statements and in your VAT account pages.bankDetails.accountNumber.label = Account number pages.bankDetails.accountNumber.hint = Must be between 6 and 8 digits long pages.bankDetails.sortCode.label = Sort code pages.bankDetails.sortCode.hint = Must be 6 digits long +pages.bankDetails.rollNumber.label = Building society roll number (if you have one) +pages.bankDetails.rollNumber.hint = You can find it on your card, statement or passbook validation.companyBankAccount.name.missing = Enter the account name validation.companyBankAccount.name.maxLength = The account name must be 60 characters or less validation.companyBankAccount.name.invalid = The account name must only include numbers, letters a to z, and special characters such as hyphens, apostrophes and brackets @@ -1007,6 +1009,7 @@ validation.companyBankAccount.number.invalid = Account number must be bet validation.companyBankAccount.sortCode.missing = Enter the account sort code validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code +validation.companyBankAccount.rollNumber.invalid = Enter a valid roll number # Main Business Activity Page pages.mainBusinessActivity.heading = Which activity is the business’s main source of income? diff --git a/test/fixtures/VatRegistrationFixture.scala b/test/fixtures/VatRegistrationFixture.scala index 93071d347..ea6467be0 100644 --- a/test/fixtures/VatRegistrationFixture.scala +++ b/test/fixtures/VatRegistrationFixture.scala @@ -34,6 +34,7 @@ trait BaseFixture { val testTradingName = "ACME INC" val testSortCode = "12-34-56" val testAccountNumber = "12345678" + val testRollNumber = Some("AB/121212") val validLabourSicCode: SicCode = SicCode("81221001", "BarFoo", "BarFoo") val validNoCompliance: SicCode = SicCode("12345678", "fooBar", "FooBar") val validExpectedOverTrue: Option[LocalDate] = Some(testDate) @@ -397,7 +398,7 @@ trait VatRegistrationFixture extends BaseFixture with FlatRateFixtures with Appl val testBankName = "testName" - val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, Some(ValidStatus)) + val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, None, Some(ValidStatus)) val bankAccount: BankAccount = BankAccount(isProvided = true, Some(testUkBankDetails), None) diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index fb2b8e877..b317ed4e3 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -30,6 +30,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val validAccountName = s"${numStr}testAccountName" val validAccountNumber = "12345678" val validSortCode = "123456" + val validRollNumber = "AB/121212" "successfully bind data to the form with no errors and allow the return of a valid BankAccountDetails case class" in { val formData = Map( @@ -211,5 +212,60 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.key mustBe SORT_CODE boundForm.errors.head.message mustBe sortCodeEmptyKey } + + "successfully bind data with a valid roll number" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> validRollNumber + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe Some("AB/121212") + } + + "successfully bind data when roll number is empty string" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> "" + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe None + } + + "return a FormError when roll number exceeds max length" in { + val longRollNumber = "A" * 26 + + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> longRollNumber + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ROLL_NUMBER + boundForm.errors.head.message mustBe rollNumberInvalidKey + } + + "successfully bind roll number with spaces stripped" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode, + ROLL_NUMBER -> "AB 121 212" + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 0 + boundForm.get.rollNumber mustBe Some("AB121212") + } } } diff --git a/test/models/BankAccountDetailsSpec.scala b/test/models/BankAccountDetailsSpec.scala index de6bc9bd5..3c3467c8f 100644 --- a/test/models/BankAccountDetailsSpec.scala +++ b/test/models/BankAccountDetailsSpec.scala @@ -16,8 +16,8 @@ package models -import play.api.libs.json.{Json, JsSuccess, JsError} import models.bars.BankAccountType +import play.api.libs.json.{JsError, Json, JsSuccess} import testHelpers.VatRegSpec class BankAccountDetailsSpec extends VatRegSpec { @@ -32,6 +32,11 @@ class BankAccountDetailsSpec extends VatRegSpec { Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) } + "parse successfully from Json when rollNumber is present in details" in { + val expected = validUkBankAccount.copy(details = Some(BankAccountDetails("testName", "12-34-56", "12345678", rollNumber = Some("AB/121212")))) + Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) + } + "parse successfully from Json when neither details or reason are present" in { val expected = validUkBankAccount.copy(details = None, reason = None) Json.toJson(expected).validate[BankAccount] mustBe JsSuccess(expected) @@ -85,5 +90,18 @@ class BankAccountDetailsSpec extends VatRegSpec { ) json.validate[BankAccount].map(_.bankAccountType) mustBe a[JsError] } + + "write rollNumber when present" in { + val json = Json.obj( + "isProvided" -> true, + "details" -> Json.obj( + "name" -> "testName", + "sortCode" -> "12-34-56", + "number" -> "12345678", + "rollNumber" -> "AB/121212" + ) + ) + json.validate[BankAccount].map(_.details.flatMap(_.rollNumber)) mustBe JsSuccess(Some("AB/121212")) + } } } diff --git a/test/services/BankAccountDetailsServiceSpec.scala b/test/services/BankAccountDetailsServiceSpec.scala index ff1bb4e63..d383b3e3e 100644 --- a/test/services/BankAccountDetailsServiceSpec.scala +++ b/test/services/BankAccountDetailsServiceSpec.scala @@ -47,7 +47,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn "return a BankAccount" in new Setup { mockGetSection[BankAccount](testRegId, Some(bankAccount)) - val result: Option[BankAccount] = await(service.fetchBankAccountDetails) + val result: Option[BankAccount] = await(service.getBankAccount) result mustBe Some(bankAccount) } @@ -55,7 +55,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn "return None if a BankAccount isn't found" in new Setup { mockGetSection[BankAccount](testRegId, None) - val result: Option[BankAccount] = await(service.fetchBankAccountDetails) + val result: Option[BankAccount] = await(service.getBankAccount) result mustBe None } @@ -67,7 +67,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn mockReplaceSection[BankAccount](testRegId, fullBankAccount) - val result: BankAccount = await(service.saveBankAccountDetails(fullBankAccount)) + val result: BankAccount = await(service.saveBankAccount(fullBankAccount)) result mustBe fullBankAccount verify(mockRegistrationApiConnector, times(1)) diff --git a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala index 02f2076a7..cd66103f0 100644 --- a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala +++ b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala @@ -26,15 +26,17 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { val view: EnterCompanyBankAccountDetails = app.injector.instanceOf[EnterCompanyBankAccountDetails] - val title = "What are the business’s bank or building society account details?" - val heading = "What are the business’s bank or building society account details?" - val p1 = "HMRC VAT will only use this account to send VAT repayments. We will not take money from it." + val title = "What are the business’s account details?" + val heading = "What are the business’s account details?" + val p1 = "HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply." val panelText = "You must tell us if your account details change." - val accountName = "Account name" + val accountName = "Name on the account" val accountNumber = "Account number" val accountNumberHint = "Must be between 6 and 8 digits long" val sortCode = "Sort code" val sortCodeHint = "Must be 6 digits long" + val rollNumber = "Building society roll number (if you have one)" + val rollNumberHint = "You can find it on your card, statement or passbook" val buttonText = "Save and continue" "Company Bank Details Page" should { @@ -76,6 +78,14 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { doc.hintWithMultiple(3) mustBe Some(sortCodeHint) } + "have the correct roll number label text" in { + doc.select(Selectors.label).get(3).text mustBe rollNumber + } + + "have the correct roll number Hint text" in new ViewSetup() { + doc.hintWithMultiple(4) mustBe Some(rollNumberHint) + } + "have the correct continue button" in new ViewSetup { doc.submitButton mustBe Some(buttonText) } From 4a659c872376614fb9f89952e6618aef37a7f6bc Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:23:09 +0000 Subject: [PATCH 03/11] update routes to add check Controller later and add ispec --- .../CheckBankDetailsController.scala | 78 ------ app/services/BankAccountDetailsService.scala | 56 +++-- .../CheckBankDetailsView.scala.html | 75 ------ conf/app.routes | 4 - .../UKBankAccountDetailsControllerISpec.scala | 227 +++++++++++++----- .../itFixtures/ITRegistrationFixtures.scala | 3 +- .../BankAccountDetailsSessionFormatSpec.scala | 4 +- .../CheckBankDetailsViewSpec.scala | 123 ---------- 8 files changed, 198 insertions(+), 372 deletions(-) delete mode 100644 app/controllers/bankdetails/CheckBankDetailsController.scala delete mode 100644 app/views/bankdetails/CheckBankDetailsView.scala.html delete mode 100644 test/views/bankdetails/CheckBankDetailsViewSpec.scala diff --git a/app/controllers/bankdetails/CheckBankDetailsController.scala b/app/controllers/bankdetails/CheckBankDetailsController.scala deleted file mode 100644 index efda04755..000000000 --- a/app/controllers/bankdetails/CheckBankDetailsController.scala +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2026 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.bankdetails - -import config.{AuthClientConnector, BaseControllerComponents, FrontendAppConfig} -import controllers.BaseController -import featuretoggle.FeatureSwitch.UseNewBarsVerify -import featuretoggle.FeatureToggleSupport.isEnabled -import models.BankAccountDetails -import models.bars.BankAccountDetailsSessionFormat -import play.api.Configuration -import play.api.libs.json.Format -import play.api.mvc.{Action, AnyContent} -import services.{BankAccountDetailsService, SessionService} -import uk.gov.hmrc.crypto.SymmetricCryptoFactory -import views.html.bankdetails.CheckBankDetailsView - -import javax.inject.Inject -import scala.concurrent.{ExecutionContext, Future} - -class CheckBankDetailsController @Inject() ( - val authConnector: AuthClientConnector, - val bankAccountDetailsService: BankAccountDetailsService, - val sessionService: SessionService, - configuration: Configuration, - view: CheckBankDetailsView -)(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) - extends BaseController { - - private val encrypter = - SymmetricCryptoFactory.aesCryptoFromConfig("json.encryption", configuration.underlying) - - private implicit val encryptedFormat: Format[BankAccountDetails] = - BankAccountDetailsSessionFormat.format(encrypter) - - private val sessionKey = "bankAccountDetails" - - def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => - if (isEnabled(UseNewBarsVerify)) { - sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { - case Some(details) => Ok(view(details)) - case None => Redirect(routes.UkBankAccountDetailsController.show) - } - } else { - Future.successful(Redirect(routes.HasBankAccountController.show)) - } - } - - def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => - if (isEnabled(UseNewBarsVerify)) { - sessionService.fetchAndGet[BankAccountDetails](sessionKey).flatMap { - case Some(details) => - bankAccountDetailsService.saveEnteredBankAccountDetails(details).flatMap { - case true => sessionService.remove.map(_ => Redirect(controllers.routes.TaskListController.show.url)) - case false => Future.successful(Redirect(routes.UkBankAccountDetailsController.show)) - - } - case None => Future.successful(Redirect(routes.UkBankAccountDetailsController.show)) - } - } else { - Future.successful(Redirect(routes.HasBankAccountController.show)) - } - } -} diff --git a/app/services/BankAccountDetailsService.scala b/app/services/BankAccountDetailsService.scala index 01812b2c4..63b0b736e 100644 --- a/app/services/BankAccountDetailsService.scala +++ b/app/services/BankAccountDetailsService.scala @@ -18,30 +18,28 @@ package services import connectors.RegistrationApiConnector import models._ -import models.bars.BankAccountType import models.api.{IndeterminateStatus, InvalidStatus, ValidStatus} -import play.api.libs.json.Json +import models.bars.BankAccountType +import play.api.mvc.Request import uk.gov.hmrc.http.HeaderCarrier import javax.inject.{Inject, Singleton} import scala.concurrent.{ExecutionContext, Future} -import play.api.mvc.Request @Singleton -class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiConnector, - val bankAccountRepService: BankAccountReputationService) { +class BankAccountDetailsService @Inject() (val regApiConnector: RegistrationApiConnector, val bankAccountRepService: BankAccountReputationService) { - def getBankAccount(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = { + def getBankAccount(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[Option[BankAccount]] = regApiConnector.getSection[BankAccount](profile.registrationId) - } - def saveBankAccount(bankAccount: BankAccount) - (implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = { + def saveBankAccount(bankAccount: BankAccount)(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = regApiConnector.replaceSection[BankAccount](profile.registrationId, bankAccount) - } - def saveHasCompanyBankAccount(hasBankAccount: Boolean) - (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { + def saveHasCompanyBankAccount(hasBankAccount: Boolean)(implicit + hc: HeaderCarrier, + profile: CurrentProfile, + ex: ExecutionContext, + request: Request[_]): Future[BankAccount] = { val bankAccount = getBankAccount map { case Some(BankAccount(oldHasBankAccount, _, _, _)) if oldHasBankAccount != hasBankAccount => BankAccount(hasBankAccount, None, None, None) @@ -54,12 +52,15 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo bankAccount flatMap saveBankAccount } - def saveEnteredBankAccountDetails(accountDetails: BankAccountDetails) - (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[Boolean] = { + def saveEnteredBankAccountDetails(accountDetails: BankAccountDetails)(implicit + hc: HeaderCarrier, + profile: CurrentProfile, + ex: ExecutionContext, + request: Request[_]): Future[Boolean] = for { existing <- getBankAccount - result <- bankAccountRepService.validateBankDetails(accountDetails).flatMap { - case status@(ValidStatus | IndeterminateStatus) => + result <- bankAccountRepService.validateBankDetails(accountDetails).flatMap { + case status @ (ValidStatus | IndeterminateStatus) => val bankAccount = BankAccount( isProvided = true, details = Some(accountDetails.copy(status = Some(status))), @@ -70,10 +71,9 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo case InvalidStatus => Future.successful(false) } } yield result - } - def saveNoUkBankAccountDetails(reason: NoUKBankAccount) - (implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = { + def saveNoUkBankAccountDetails( + reason: NoUKBankAccount)(implicit hc: HeaderCarrier, profile: CurrentProfile, request: Request[_]): Future[BankAccount] = { val bankAccount = BankAccount( isProvided = false, details = None, @@ -83,11 +83,15 @@ class BankAccountDetailsService @Inject()(val regApiConnector: RegistrationApiCo saveBankAccount(bankAccount) } - def saveBankAccountType(bankAccountType: BankAccountType) - (implicit hc: HeaderCarrier, profile: CurrentProfile, ex: ExecutionContext, request: Request[_]): Future[BankAccount] = { - getBankAccount.map { - case Some(existing) => existing.copy(bankAccountType = Some(bankAccountType)) - case None => BankAccount(isProvided = true, details = None, reason = None, bankAccountType = Some(bankAccountType)) - }.flatMap(saveBankAccount) - } + def saveBankAccountType(bankAccountType: BankAccountType)(implicit + hc: HeaderCarrier, + profile: CurrentProfile, + ex: ExecutionContext, + request: Request[_]): Future[BankAccount] = + getBankAccount + .map { + case Some(existing) => existing.copy(bankAccountType = Some(bankAccountType)) + case None => BankAccount(isProvided = true, details = None, reason = None, bankAccountType = Some(bankAccountType)) + } + .flatMap(saveBankAccount) } diff --git a/app/views/bankdetails/CheckBankDetailsView.scala.html b/app/views/bankdetails/CheckBankDetailsView.scala.html deleted file mode 100644 index 601d9c11b..000000000 --- a/app/views/bankdetails/CheckBankDetailsView.scala.html +++ /dev/null @@ -1,75 +0,0 @@ -@* - * Copyright 2026 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. - *@ - -@import config.FrontendAppConfig -@import models.BankAccountDetails - -@this( - layout: layouts.layout, - h1: components.h1, - p: components.p, - button: components.button, - formWithCSRF: FormWithCSRF, - govukSummaryList: GovukSummaryList -) - -@(bankDetails: BankAccountDetails)(implicit request: Request[_], messages: Messages, appConfig: FrontendAppConfig) - -@layout(pageTitle = Some(messages("pages.checkBankDetails.heading"))) { - - @h1("pages.checkBankDetails.heading") - - @govukSummaryList(SummaryList( - card = Some(Card( - title = Some(CardTitle(content = Text(messages("pages.checkBankDetails.cardTitle")))), - actions = Some(Actions(items = Seq( - ActionItem( - href = controllers.bankdetails.routes.UkBankAccountDetailsController.show.url, - content = Text(messages("pages.checkBankDetails.change")), - visuallyHiddenText = Some(messages("pages.checkBankDetails.change.hidden")) - ) - ))) - )), - rows = Seq( - Some(SummaryListRow( - key = Key(content = Text(messages("pages.checkBankDetails.accountName"))), - value = Value(content = Text(bankDetails.name)) - )), - Some(SummaryListRow( - key = Key(content = Text(messages("pages.checkBankDetails.accountNumber"))), - value = Value(content = Text(bankDetails.number)) - )), - Some(SummaryListRow( - key = Key(content = Text(messages("pages.checkBankDetails.sortCode"))), - value = Value(content = Text(bankDetails.sortCode)) - )), - bankDetails.rollNumber.map { rollNumber => - SummaryListRow( - key = Key(content = Text(messages("pages.checkBankDetails.rollNumber"))), - value = Value(content = Text(rollNumber)) - ) - } - ).flatten - )) - - @p { - @messages("pages.checkBankDetails.p1") - } - - @formWithCSRF(action = controllers.bankdetails.routes.CheckBankDetailsController.submit) { - @button("app.common.confirmAndContinue") - } -} \ No newline at end of file diff --git a/conf/app.routes b/conf/app.routes index d61850bdb..d66ea6c59 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -134,10 +134,6 @@ POST /choose-account-type controllers GET /account-details controllers.bankdetails.UkBankAccountDetailsController.show POST /account-details controllers.bankdetails.UkBankAccountDetailsController.submit -## CHECK BANK DETAILS -GET /check-bank-details controllers.bankdetails.CheckBankDetailsController.show -POST /check-bank-details controllers.bankdetails.CheckBankDetailsController.submit - ## VAT CORRESPONDENCE GET /vat-correspondence-language controllers.business.VatCorrespondenceController.show POST /vat-correspondence-language controllers.business.VatCorrespondenceController.submit diff --git a/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala b/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala index e90c8a62f..a51be2671 100644 --- a/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala +++ b/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala @@ -16,10 +16,11 @@ package controllers.bankdetails +import featuretoggle.FeatureSwitch.UseNewBarsVerify import itFixtures.ITRegistrationFixtures import itutil.ControllerISpec -import models.api.EligibilitySubmissionData import models.{BankAccount, TransferOfAGoingConcern} +import models.api.EligibilitySubmissionData import org.jsoup.Jsoup import org.jsoup.nodes.Document import play.api.libs.ws.WSResponse @@ -30,109 +31,207 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr val url = "/account-details" - "GET /account-details" must { - "return OK with a blank form if the VAT scheme doesn't contain bank details" in new Setup { - given() - .user.isAuthorised() - .registrationApi.getSection[BankAccount](None) + "GET /account-details" when { - insertCurrentProfileIntoDb(currentProfile, sessionString) + "UseNewBarsVerify is disabled" must { - val res: WSResponse = await(buildClient(url).get()) + "return OK with a blank form if the VAT scheme doesn't contain bank details" in new Setup { + disable(UseNewBarsVerify) + given().user + .isAuthorised() + .registrationApi + .getSection[BankAccount](None) - res.status mustBe OK - } - "return OK with a pre-populated form from backend" in new Setup { - given() - .user.isAuthorised() - .registrationApi.getSection[BankAccount](Some(bankAccount)) + insertCurrentProfileIntoDb(currentProfile, sessionString) - insertCurrentProfileIntoDb(currentProfile, sessionString) + val res: WSResponse = await(buildClient(url).get()) + res.status mustBe OK + } + + "return OK with a form pre-populated from the backend when bank details exist" in new Setup { + disable(UseNewBarsVerify) + given().user.isAuthorised().registrationApi.getSection[BankAccount](Some(bankAccount)) + + insertCurrentProfileIntoDb(currentProfile, sessionString) - val res: WSResponse = await(buildClient(url).get()) - val doc: Document = Jsoup.parse(res.body) + val res: WSResponse = await(buildClient(url).get()) + val doc: Document = Jsoup.parse(res.body) - res.status mustBe OK - doc.select("input[id=accountName]").`val`() mustBe testBankName - doc.select("input[id=accountNumber]").`val`() mustBe testAccountNumber - doc.select("input[id=sortCode]").`val`() mustBe testSortCode + res.status mustBe OK + doc.select("input[id=accountName]").`val`() mustBe testBankName + doc.select("input[id=accountNumber]").`val`() mustBe testAccountNumber + doc.select("input[id=sortCode]").`val`() mustBe testSortCode + } } - "return OK with a pre-populated form from the backend" in new Setup { - given() - .user.isAuthorised() - .registrationApi.getSection[BankAccount](Some(bankAccount)) - insertCurrentProfileIntoDb(currentProfile, sessionString) + "UseNewBarsVerify is enabled" must { + + "return OK with a blank form when session is empty" in new Setup { + enable(UseNewBarsVerify) + given().user.isAuthorised() - val res: WSResponse = await(buildClient(url).get()) - val doc: Document = Jsoup.parse(res.body) + insertCurrentProfileIntoDb(currentProfile, sessionString) - res.status mustBe OK - doc.select("input[id=accountName]").`val`() mustBe testBankName - doc.select("input[id=accountNumber]").`val`() mustBe testAccountNumber - doc.select("input[id=sortCode]").`val`() mustBe testSortCode + val res: WSResponse = await(buildClient(url).get()) + res.status mustBe OK + val doc: Document = Jsoup.parse(res.body) + doc.select("input[id=accountName]").`val`() mustBe "" + } + + "return OK with a form pre-populated from session when session contains bank details" in new Setup { + enable(UseNewBarsVerify) + given().user.isAuthorised() + + insertCurrentProfileIntoDb(currentProfile, sessionString) + + await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode, + "rollNumber" -> testRollNumber + ))) + + val res: WSResponse = await(buildClient(url).get()) + val doc: Document = Jsoup.parse(res.body) + + res.status mustBe OK + doc.select("input[id=accountName]").`val`() mustBe testBankName + doc.select("input[id=accountNumber]").`val`() mustBe testAccountNumber + doc.select("input[id=sortCode]").`val`() mustBe testSortCode + doc.select("input[id=rollNumber]").`val`() mustBe testRollNumber + } } } "POST /account-details" when { - "bank details and Bank Account Reputation states are invalid" must { - "return BAD_REQUEST" in new Setup { - given() - .user.isAuthorised() - .bankAccountReputation.fails + + "UseNewBarsVerify is disabled" must { + + "redirect to the Task List when valid bank details" in new Setup { + disable(UseNewBarsVerify) + given().user + .isAuthorised() + .bankAccountReputation + .passes .registrationApi.getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) + .registrationApi.replaceSection[BankAccount](bankAccount) .registrationApi.getSection[EligibilitySubmissionData](Some(testEligibilitySubmissionData.copy(registrationReason = TransferOfAGoingConcern))) insertCurrentProfileIntoDb(currentProfile, sessionString) + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode + ))) + + res.status mustBe SEE_OTHER + res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) + } + + "return BAD_REQUEST when valid bank details fail BARS verification" in new Setup { + disable(UseNewBarsVerify) + given().user + .isAuthorised() + .bankAccountReputation + .fails + .registrationApi + .getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) + .registrationApi + .getSection[EligibilitySubmissionData](Some(testEligibilitySubmissionData.copy(registrationReason = TransferOfAGoingConcern))) + + insertCurrentProfileIntoDb(currentProfile, sessionString) + + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode + ))) + + res.status mustBe BAD_REQUEST + } + + "return BAD_REQUEST when form fields are empty" in new Setup { + disable(UseNewBarsVerify) + given().user + .isAuthorised() + .bankAccountReputation + .fails + .registrationApi + .getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) + + insertCurrentProfileIntoDb(currentProfile, sessionString) + + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> "", + "accountNumber" -> "", + "sortCode" -> "" + ))) + + res.status mustBe BAD_REQUEST + } + } + + "UseNewBarsVerify is enabled" must { + + "save bank details to session and redirect to Task List when form is valid" in new Setup { + enable(UseNewBarsVerify) + given() + .user.isAuthorised() + + insertCurrentProfileIntoDb(currentProfile, sessionString) + val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> testBankName, + "accountName" -> testBankName, "accountNumber" -> testAccountNumber, - "sortCode" -> testSortCode + "sortCode" -> testSortCode ))) - res.status mustBe BAD_REQUEST + res.status mustBe SEE_OTHER + res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) } - } - "redirect to the Tasklist if the account details are valid" in new Setup { - given() - .user.isAuthorised() - .bankAccountReputation.passes - .registrationApi.getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) - .registrationApi.replaceSection[BankAccount](bankAccount) - .registrationApi.getSection[EligibilitySubmissionData](Some(testEligibilitySubmissionData.copy(registrationReason = TransferOfAGoingConcern))) + "save bank details including roll number to session and redirect to Task List" in new Setup { + enable(UseNewBarsVerify) + given() + .user.isAuthorised() - insertCurrentProfileIntoDb(currentProfile, sessionString) + insertCurrentProfileIntoDb(currentProfile, sessionString) - val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> testBankName, - "accountNumber" -> testAccountNumber, - "sortCode" -> testSortCode - ))) + val res: WSResponse = await(buildClient(url).post(Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode, + "rollNumber" -> testRollNumber + ))) - res.status mustBe SEE_OTHER - res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) - } + res.status mustBe SEE_OTHER + res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) + } - "bank details are incorrect" must { - "return BAD_REQUEST" in new Setup { + "return BAD_REQUEST without calling BARS when form fields are empty" in new Setup { + enable(UseNewBarsVerify) given() .user.isAuthorised() - .bankAccountReputation.fails - .registrationApi.getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) insertCurrentProfileIntoDb(currentProfile, sessionString) val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> "", + "accountName" -> "", "accountNumber" -> "", - "sortCode" -> "" + "sortCode" -> "" ))) res.status mustBe BAD_REQUEST } } } - } diff --git a/it/test/itFixtures/ITRegistrationFixtures.scala b/it/test/itFixtures/ITRegistrationFixtures.scala index 47accec61..6386ce51d 100644 --- a/it/test/itFixtures/ITRegistrationFixtures.scala +++ b/it/test/itFixtures/ITRegistrationFixtures.scala @@ -46,7 +46,8 @@ trait ITRegistrationFixtures extends ApplicantDetailsFixture { val testBankName = "testName" val testSortCode = "123456" val testAccountNumber = "12345678" - val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, Some(ValidStatus)) + val testRollNumber = "AB/121212" + val testUkBankDetails: BankAccountDetails = BankAccountDetails(testBankName, testAccountNumber, testSortCode, None, Some(ValidStatus)) val bankAccount: BankAccount = BankAccount(isProvided = true, Some(testUkBankDetails), None) val emptyBankAccount: BankAccount = BankAccount(isProvided = true, None, None) val bankAccountNotProvidedNoReason: BankAccount = BankAccount(isProvided = false, None, None) diff --git a/test/models/bars/BankAccountDetailsSessionFormatSpec.scala b/test/models/bars/BankAccountDetailsSessionFormatSpec.scala index a334cca21..aaa1c80e2 100644 --- a/test/models/bars/BankAccountDetailsSessionFormatSpec.scala +++ b/test/models/bars/BankAccountDetailsSessionFormatSpec.scala @@ -99,7 +99,9 @@ class BankAccountDetailsSessionFormatSpec extends AnyWordSpec with Matchers { val differentFormat: Format[BankAccountDetails] = BankAccountDetailsSessionFormat.format(differentEncrypter) - Json.fromJson[BankAccountDetails](json)(differentFormat).isError mustBe true + intercept[SecurityException] { + Json.fromJson[BankAccountDetails](json)(differentFormat).get + } } } } \ No newline at end of file diff --git a/test/views/bankdetails/CheckBankDetailsViewSpec.scala b/test/views/bankdetails/CheckBankDetailsViewSpec.scala deleted file mode 100644 index fe78be76e..000000000 --- a/test/views/bankdetails/CheckBankDetailsViewSpec.scala +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright 2026 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 views.bankdetails - -import models.BankAccountDetails -import org.jsoup.Jsoup -import org.jsoup.nodes.Document -import views.VatRegViewSpec -import views.html.bankdetails.CheckBankDetailsView - -class CheckBankDetailsViewSpec extends VatRegViewSpec { - - val view: CheckBankDetailsView = app.injector.instanceOf[CheckBankDetailsView] - - val title = "Check your bank account details" - val heading = "Check your bank account details" - val cardTitle = "Bank account details" - val changeLink = "Change" - val accountNameLabel = "Account name" - val accountNumberLabel = "Account number" - val sortCodeLabel = "Sort code" - val rollNumberLabel = "Roll number" - val p1 = "By confirming, you are providing your consent for HMRC to verify your bank details with your bank." - val buttonText = "Confirm and continue" - - val bankDetails: BankAccountDetails = BankAccountDetails( - name = "Test Account", - sortCode = "123456", - number = "12345678", - rollNumber = None, - status = None - ) - - val bankDetailsWithRollNumber: BankAccountDetails = bankDetails.copy(rollNumber = Some("AB/121212")) - - "CheckBankDetailsView" should { - - implicit lazy val doc: Document = Jsoup.parse(view(bankDetails).body) - - "have the correct title" in new ViewSetup { - doc.title must include(title) - } - - "have the correct heading" in new ViewSetup { - doc.heading mustBe Some(heading) - } - - "have the correct card title" in new ViewSetup { - doc.select(".govuk-summary-card__title").text mustBe cardTitle - } - - "have a change link" in new ViewSetup { - doc.select(".govuk-summary-card__action a").text must include(changeLink) - } - - "have the correct account name label" in new ViewSetup { - doc.select(".govuk-summary-list__key").get(0).text mustBe accountNameLabel - } - - "have the correct account name value" in new ViewSetup { - doc.select(".govuk-summary-list__value").get(0).text mustBe "Test Account" - } - - "have the correct account number label" in new ViewSetup { - doc.select(".govuk-summary-list__key").get(1).text mustBe accountNumberLabel - } - - "have the correct account number value" in new ViewSetup { - doc.select(".govuk-summary-list__value").get(1).text mustBe "12345678" - } - - "have the correct sort code label" in new ViewSetup { - doc.select(".govuk-summary-list__key").get(2).text mustBe sortCodeLabel - } - - "have the correct sort code value" in new ViewSetup { - doc.select(".govuk-summary-list__value").get(2).text mustBe "123456" - } - - "not show roll number row when roll number is absent" in new ViewSetup { - doc.select(".govuk-summary-list__key").size mustBe 3 - } - - "have the correct p1" in new ViewSetup { - doc.para(1) mustBe Some(p1) - } - - "have the correct continue button" in new ViewSetup { - doc.submitButton mustBe Some(buttonText) - } - } - - "CheckBankDetailsView with roll number" should { - - implicit lazy val doc: Document = Jsoup.parse(view(bankDetailsWithRollNumber).body) - - "show the roll number row when roll number is present" in new ViewSetup { - doc.select(".govuk-summary-list__key").size mustBe 4 - } - - "have the correct roll number label" in new ViewSetup { - doc.select(".govuk-summary-list__key").get(3).text mustBe rollNumberLabel - } - - "have the correct roll number value" in new ViewSetup { - doc.select(".govuk-summary-list__value").get(3).text mustBe "AB/121212" - } - } -} \ No newline at end of file From 00a3295964184e83b09fd6d0570dc6eeecd3762a Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Wed, 11 Mar 2026 15:20:24 +0000 Subject: [PATCH 04/11] update naming and feature toggle on view for roll number --- .../EnterCompanyBankAccountDetails.scala.html | 22 +++++++++------- conf/messages | 2 +- test/fixtures/VatRegistrationFixture.scala | 1 - .../BankAccountDetailsServiceSpec.scala | 4 +-- .../CompanyBankDetailsViewSpec.scala | 26 +++++++++++++------ 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html index 934530566..f135b3469 100644 --- a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html +++ b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html @@ -17,6 +17,8 @@ @import models.BankAccountDetails @import play.api.data.Form @import config.FrontendAppConfig +@import featuretoggle.FeatureSwitch._ +@import featuretoggle.FeatureToggleSupport._ @this( layout: layouts.layout, @@ -31,7 +33,7 @@ @(bankDetailsForm: Form[BankAccountDetails])(implicit request: Request[_], messages: Messages, appConfig: FrontendAppConfig) -@layout(pageTitle = Some(title(bankDetailsForm, messages("pages.bankDetails.heading"))), backLink = true) { +@layout(pageTitle = Some(title(bankDetailsForm, messages("pages.bankDetails.heading")))) { @errorSummary(errors = bankDetailsForm.errors) @@ -49,7 +51,7 @@ id = "accountName", name = "accountName", label = messages("pages.bankDetails.accountName.label"), - classes = Some("govuk-input--width-10"), + classes = Some("govuk-input--width-20"), isPageHeading = false, hint = Some(Html(messages("pages.bankDetails.accountName.hint")))) @@ -69,13 +71,15 @@ isPageHeading = false, hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) - @inputText(form = bankDetailsForm, - id = "rollNumber", - name = "rollNumber", - label = messages("pages.bankDetails.rollNumber.label"), - classes = Some("govuk-input--width-20"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) + @if(isEnabled(UseNewBarsVerify)) { + @inputText(form = bankDetailsForm, + id = "rollNumber", + name = "rollNumber", + label = messages("pages.bankDetails.rollNumber.label"), + classes = Some("govuk-input--width-20"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) + } @button("app.common.continue") } diff --git a/conf/messages b/conf/messages index c8e0d2b6d..1afc275b4 100644 --- a/conf/messages +++ b/conf/messages @@ -1009,7 +1009,7 @@ validation.companyBankAccount.number.invalid = Account number must be bet validation.companyBankAccount.sortCode.missing = Enter the account sort code validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code -validation.companyBankAccount.rollNumber.invalid = Enter a valid roll number +validation.companyBankAccount.rollNumber.invalid = Building society roll number must be shorter # Main Business Activity Page pages.mainBusinessActivity.heading = Which activity is the business’s main source of income? diff --git a/test/fixtures/VatRegistrationFixture.scala b/test/fixtures/VatRegistrationFixture.scala index ea6467be0..fa72960fe 100644 --- a/test/fixtures/VatRegistrationFixture.scala +++ b/test/fixtures/VatRegistrationFixture.scala @@ -34,7 +34,6 @@ trait BaseFixture { val testTradingName = "ACME INC" val testSortCode = "12-34-56" val testAccountNumber = "12345678" - val testRollNumber = Some("AB/121212") val validLabourSicCode: SicCode = SicCode("81221001", "BarFoo", "BarFoo") val validNoCompliance: SicCode = SicCode("12345678", "fooBar", "FooBar") val validExpectedOverTrue: Option[LocalDate] = Some(testDate) diff --git a/test/services/BankAccountDetailsServiceSpec.scala b/test/services/BankAccountDetailsServiceSpec.scala index d383b3e3e..58da16c15 100644 --- a/test/services/BankAccountDetailsServiceSpec.scala +++ b/test/services/BankAccountDetailsServiceSpec.scala @@ -40,7 +40,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn implicit val request: Request[_] = FakeRequest() - "fetchBankAccountDetails" should { + "getBankAccountDetails" should { val bankAccount = BankAccount(isProvided = true, Some(BankAccountDetails("testName", "testCode", "testAccNumber")), None) @@ -61,7 +61,7 @@ class BankAccountDetailsServiceSpec extends VatSpec with MockRegistrationApiConn } } - "saveBankAccountDetails" should { + "saveBankAccount" should { "return a BankAccount and save to the backend" in new Setup { val fullBankAccount: BankAccount = BankAccount(isProvided = true, Some(BankAccountDetails("testName", "testCode", "testAccNumber")), None) diff --git a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala index cd66103f0..03e4cbc7b 100644 --- a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala +++ b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala @@ -16,6 +16,8 @@ package views.bankdetails +import featuretoggle.FeatureSwitch.UseNewBarsVerify +import featuretoggle.FeatureToggleSupport._ import forms.EnterBankAccountDetailsForm import org.jsoup.Jsoup import org.jsoup.nodes.Document @@ -39,8 +41,10 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { val rollNumberHint = "You can find it on your card, statement or passbook" val buttonText = "Save and continue" - "Company Bank Details Page" should { - implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) + implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) + + "Company Bank Details Page common elements" should { + disable(UseNewBarsVerify) "have the correct title" in new ViewSetup { doc.title must include(title) @@ -78,18 +82,24 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { doc.hintWithMultiple(3) mustBe Some(sortCodeHint) } - "have the correct roll number label text" in { - doc.select(Selectors.label).get(3).text mustBe rollNumber - } - - "have the correct roll number Hint text" in new ViewSetup() { - doc.hintWithMultiple(4) mustBe Some(rollNumberHint) + "not show the roll number field when UseNewBarsVerify is disabled" in new ViewSetup { + doc.select(Selectors.label).size mustBe 3 } "have the correct continue button" in new ViewSetup { doc.submitButton mustBe Some(buttonText) } + } + + "Company Bank Details Page roll number field" should { + "be visible when UseNewBarsVerify is enabled" in new ViewSetup { + enable(UseNewBarsVerify) + val enabledDoc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) + enabledDoc.select(Selectors.label).get(3).text mustBe rollNumber + enabledDoc.hintWithMultiple(4) mustBe Some(rollNumberHint) + enabledDoc.select(Selectors.label).size mustBe 4 + } } } From d3c4fe3cc667cf49f75f33fa89ae77b779315df0 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Wed, 11 Mar 2026 15:30:27 +0000 Subject: [PATCH 05/11] add welsh --- conf/messages.cy | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/conf/messages.cy b/conf/messages.cy index 421a3e15e..271ae5b55 100644 --- a/conf/messages.cy +++ b/conf/messages.cy @@ -989,14 +989,17 @@ pages.chooseAccountType.option.business = Cyfrif busnes pages.chooseAccountType.option.personal = Cyfrif personol # Bank Details page -pages.bankDetails.heading = Beth yw manylion cyfrif banc neu gymdeithas adeiladu’r busnes? -pages.bankDetails.p1 = Dim ond i anfon ad-daliadau TAW y bydd tîm TAW CThEM yn defnyddio’r cyfrif hwn. Ni fyddwn yn cymryd arian ohono. +pages.bankDetails.heading = Beth yw manylion cyswllt y busnes? +pages.bankDetails.p1 = Dim ond i anfon ad-daliadau TAW y bydd tîm TAW CThEF yn defnyddio’r cyfrif hwn. Ni fydd arian yn cael ei gymryd o’r cyfrif rydych chi’n ei ddarparu. pages.bankDetails.info = Mae’n rhaid i chi roi gwybod i ni os bydd manylion eich cyfrif yn newid. -pages.bankDetails.accountName.label = Enw’r cyfrif +pages.bankDetails.accountName.label = Yr enw sydd ar y cyfrif +pages.bankDetails.accountName.hint = Nodwch yr enw fel y mae’n ymddangos ar gyfriflenni banc ac yn eich cyfrif TAW pages.bankDetails.accountNumber.label = Rhif y cyfrif pages.bankDetails.accountNumber.hint = Mae’n rhaid iddo fod rhwng 6 ac 8 digid o hyd pages.bankDetails.sortCode.label = Cod didoli pages.bankDetails.sortCode.hint = Mae’n rhaid iddo fod yn 6 digid o hyd +pages.bankDetails.rollNumber.label = Rhif rôl y gymdeithas adeiladu (os oes gennych un) +pages.bankDetails.rollNumber.hint = Bydd hwn i’w weld ar eich cerdyn, cyfriflen neu baslyfr validation.companyBankAccount.name.missing = Nodwch enw’r cyfrif validation.companyBankAccount.name.maxLength = Mae’n rhaid i enw’r cyfrif fod yn 60 o gymeriadau neu lai validation.companyBankAccount.name.invalid = Mae’n rhaid i enw’r cyfrif gynnwys rhifau, y llythrennau a i z, a chymeriadau arbennig megis cysylltnodau, collnodau a chromfachau yn unig @@ -1005,6 +1008,8 @@ validation.companyBankAccount.number.invalid = Mae’n rhaid i rif y cyfr validation.companyBankAccount.sortCode.missing = Nodwch god didoli’r cyfrif validation.companyBankAccount.sortCode.invalid = Nodwch god didoli dilys ar gyfer y cyfrif validation.companyBankAccount.invalidCombination = Nodwch rif cyfrif banc a chod didoli dilys +validation.companyBankAccount.rollNumber.invalid = Mae’n rhaid i rif rôl y gymdeithas adeiladu fod yn fyrrach + # Main Business Activity Page pages.mainBusinessActivity.heading = Pa weithgaredd yw prif ffynhonnell incwm y busnes? From 51df5ed51dce80ae55facbfc974dc20fa574d341 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Thu, 12 Mar 2026 16:59:49 +0000 Subject: [PATCH 06/11] feat: make new form and view and use new errors correctly --- .../BarsBankAccountDetailsA11ySpec.scala | 28 ++ .../UkBankAccountDetailsController.scala | 52 ++-- app/forms/BankAccountDetailsForms.scala | 84 ++++-- .../EnterBankAccountDetails.scala.html | 83 ++++++ .../EnterCompanyBankAccountDetails.scala.html | 56 ++-- conf/messages | 47 ++-- conf/messages.cy | 13 +- .../UKBankAccountDetailsControllerISpec.scala | 87 ++++--- test/forms/BankAccountDetailsFormSpec.scala | 243 ++++++++++++++---- .../CompanyBankDetailsViewSpec.scala | 32 +-- .../EnterBankDetailsViewSpec.scala | 97 +++++++ 11 files changed, 621 insertions(+), 201 deletions(-) create mode 100644 a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala create mode 100644 app/views/bankdetails/EnterBankAccountDetails.scala.html create mode 100644 test/views/bankdetails/EnterBankDetailsViewSpec.scala diff --git a/a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala b/a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala new file mode 100644 index 000000000..f59b50d8f --- /dev/null +++ b/a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala @@ -0,0 +1,28 @@ + +package pages.bankdetails + +import forms.EnterBankAccountDetailsForm +import helpers.A11ySpec +import models.BankAccountDetails +import play.api.data.Form +import views.html.bankdetails.EnterBankAccountDetails + +class BarsBankAccountDetailsA11ySpec extends A11ySpec { + + val view: EnterBankAccountDetails = app.injector.instanceOf[EnterBankAccountDetails] + val form: Form[BankAccountDetails] = EnterBankAccountDetailsForm.form + + "the Enter Company Bank Account Details page" when { + "there are no form errors" must { + "pass all a11y checks" in { + view(form).body must passAccessibilityChecks + } + } + "there are form errors" must { + "pass all a11y checks" in { + view(form.bind(Map("" -> ""))).body must passAccessibilityChecks + } + } + } + +} \ No newline at end of file diff --git a/app/controllers/bankdetails/UkBankAccountDetailsController.scala b/app/controllers/bankdetails/UkBankAccountDetailsController.scala index 6596c59f6..042c75138 100644 --- a/app/controllers/bankdetails/UkBankAccountDetailsController.scala +++ b/app/controllers/bankdetails/UkBankAccountDetailsController.scala @@ -22,6 +22,7 @@ import featuretoggle.FeatureSwitch.UseNewBarsVerify import featuretoggle.FeatureToggleSupport.isEnabled import forms.EnterBankAccountDetailsForm import forms.EnterBankAccountDetailsForm.{form => enterBankAccountDetailsForm} +import forms.EnterBankAccountDetailsNewBarsForm import models.BankAccountDetails import models.bars.BankAccountDetailsSessionFormat import play.api.libs.json.Format @@ -30,18 +31,20 @@ import play.api.Configuration import services.{BankAccountDetailsService, SessionService} import uk.gov.hmrc.crypto.SymmetricCryptoFactory import views.html.bankdetails.EnterCompanyBankAccountDetails +import views.html.bankdetails.EnterBankAccountDetails import javax.inject.Inject import scala.concurrent.{ExecutionContext, Future} class UkBankAccountDetailsController @Inject() ( - val authConnector: AuthClientConnector, - val bankAccountDetailsService: BankAccountDetailsService, - val sessionService: SessionService, - configuration: Configuration, - view: EnterCompanyBankAccountDetails -)(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) - extends BaseController { + val authConnector: AuthClientConnector, + val bankAccountDetailsService: BankAccountDetailsService, + val sessionService: SessionService, + configuration: Configuration, + newBarsView: EnterBankAccountDetails, + oldView: EnterCompanyBankAccountDetails + )(implicit appConfig: FrontendAppConfig, val executionContext: ExecutionContext, baseControllerComponents: BaseControllerComponents) + extends BaseController { private val encrypter = SymmetricCryptoFactory.aesCryptoFromConfig("json.encryption", configuration.underlying) @@ -53,33 +56,42 @@ class UkBankAccountDetailsController @Inject() ( def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => if (isEnabled(UseNewBarsVerify)) { + val newBarsForm = EnterBankAccountDetailsNewBarsForm.form sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { - case Some(details) => Ok(view(enterBankAccountDetailsForm.fill(details))) - case None => Ok(view(enterBankAccountDetailsForm)) + case Some(details) => Ok(newBarsView(newBarsForm.fill(details))) + case None => Ok(newBarsView(newBarsForm)) } } else { for { bankDetails <- bankAccountDetailsService.getBankAccount filledForm = bankDetails.flatMap(_.details).fold(enterBankAccountDetailsForm)(enterBankAccountDetailsForm.fill) - } yield Ok(view(filledForm)) + } yield Ok(oldView(filledForm)) } } def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => - enterBankAccountDetailsForm - .bindFromRequest() - .fold( - formWithErrors => Future.successful(BadRequest(view(formWithErrors))), - accountDetails => - if (isEnabled(UseNewBarsVerify)) + if (isEnabled(UseNewBarsVerify)) { + val newBarsForm = EnterBankAccountDetailsNewBarsForm.form + newBarsForm + .bindFromRequest() + .fold( + formWithErrors => Future.successful(BadRequest(newBarsView(formWithErrors))), + accountDetails => sessionService.cache[BankAccountDetails](sessionKey, accountDetails).map { _ => Redirect(controllers.routes.TaskListController.show.url) } - else + ) + } else { + enterBankAccountDetailsForm + .bindFromRequest() + .fold( + formWithErrors => Future.successful(BadRequest(oldView(formWithErrors))), + accountDetails => bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { case true => Redirect(controllers.routes.TaskListController.show.url) - case false => BadRequest(view(EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) + case false => BadRequest(oldView(EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) } - ) + ) + } } -} +} \ No newline at end of file diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index c2b1e1471..0a3650125 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -40,12 +40,15 @@ object ChooseAccountTypeForm { val form: Form[BankAccountType] = Form( single( - BUSINESS_OR_PERSONAL_ACCOUNT_RADIO -> default(text, "").verifying(stopOnFail( - mandatory(errorMsg) - )).transform[BankAccountType]( - s => BankAccountType.fromString(s).get, - _.asBars - ) + BUSINESS_OR_PERSONAL_ACCOUNT_RADIO -> default(text, "") + .verifying( + stopOnFail( + mandatory(errorMsg) + )) + .transform[BankAccountType]( + s => BankAccountType.fromString(s).get, + _.asBars + ) ) ) } @@ -55,7 +58,6 @@ object EnterBankAccountDetailsForm { val ACCOUNT_NAME = "accountName" val ACCOUNT_NUMBER = "accountNumber" val SORT_CODE = "sortCode" - val ROLL_NUMBER = "rollNumber" val accountNameEmptyKey = "validation.companyBankAccount.name.missing" val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" @@ -64,7 +66,6 @@ object EnterBankAccountDetailsForm { val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" - val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" val invalidAccountReputationKey = "sortCodeAndAccountGroup" val invalidAccountReputationMessage = "validation.companyBankAccount.invalidCombination" @@ -73,7 +74,6 @@ object EnterBankAccountDetailsForm { private val accountNameMaxLength = 60 private val accountNumberRegex = """[0-9]{6,8}""".r private val sortCodeRegex = """[0-9]{6}""".r - private val rollNumberMaxLength = 25 val form = Form[BankAccountDetails]( mapping( @@ -96,13 +96,69 @@ object EnterBankAccountDetailsForm { stopOnFail( mandatory(sortCodeEmptyKey), matchesRegex(sortCodeRegex, sortCodeInvalidKey) + )) + )((accountName, accountNumber, sortCode) => BankAccountDetails.apply(accountName, accountNumber, sortCode, None))(bankAccountDetails => + BankAccountDetails.unapply(bankAccountDetails).map { case (accountName, accountNumber, sortCode, _, _) => + (accountName, accountNumber, sortCode) + }) + ) + val formWithInvalidAccountReputation: Form[BankAccountDetails] = + form.withError(invalidAccountReputationKey, invalidAccountReputationMessage) +} + +object EnterBankAccountDetailsNewBarsForm { + + val ACCOUNT_NAME = "accountName" + val ACCOUNT_NUMBER = "accountNumber" + val SORT_CODE = "sortCode" + val ROLL_NUMBER = "rollNumber" + + val accountNameEmptyKey = "validation.companyBankAccount.name.missing.new" + val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" + val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.new" + val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" + val accountNumberDigitKey = "validation.companyBankAccount.number.digit.error" + val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" + val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" + val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" + val sortCodeLengthKey = "validation.companyBankAccount.sortCode.length" + val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" + + private val accountNameRegex = """^[A-Za-z0-9 '\-./]{1,60}$""".r + private val accountNameMaxLength = 60 + private val rollNumberMaxLength = 25 + private val accountNumberDigitRegex = """^[0-9]+$""".r + private val accountNumberLengthRegex = """^.{6,8}$""".r + private val sortCodeDigitRegex = """^[0-9]+$""".r + private val sortCodeLengthRegex = """^.{6}$""".r + + val form: Form[BankAccountDetails] = Form[BankAccountDetails]( + mapping( + ACCOUNT_NAME -> text.verifying( + stopOnFail( + mandatory(accountNameEmptyKey), + maxLength(accountNameMaxLength, accountNameMaxLengthKey), + matchesRegex(accountNameRegex, accountNameInvalidKey) + )), + ACCOUNT_NUMBER -> text + .transform(removeSpaces, identity[String]) + .verifying(stopOnFail( + mandatory(accountNumberEmptyKey), + matchesRegex(accountNumberDigitRegex, accountNumberInvalidKey), + matchesRegex(accountNumberLengthRegex, accountNumberDigitKey) + )), + SORT_CODE -> text + .transform(removeSpaces, identity[String]) + .verifying( + stopOnFail( + mandatory(sortCodeEmptyKey), + matchesRegex(sortCodeDigitRegex, sortCodeInvalidKey), + matchesRegex(sortCodeLengthRegex, sortCodeLengthKey) )), ROLL_NUMBER -> optional( text .transform(removeSpaces, identity[String]) - .verifying( - maxLength(rollNumberMaxLength, rollNumberInvalidKey) - ) + .verifying(maxLength(rollNumberMaxLength, rollNumberInvalidKey)) ) )((accountName, accountNumber, sortCode, rollNumber) => BankAccountDetails.apply(accountName, accountNumber, sortCode, rollNumber))( bankAccountDetails => @@ -110,8 +166,4 @@ object EnterBankAccountDetailsForm { (accountName, accountNumber, sortCode, rollNumber) }) ) - - val formWithInvalidAccountReputation: Form[BankAccountDetails] = - form.withError(invalidAccountReputationKey, invalidAccountReputationMessage) - } diff --git a/app/views/bankdetails/EnterBankAccountDetails.scala.html b/app/views/bankdetails/EnterBankAccountDetails.scala.html new file mode 100644 index 000000000..7dc220d1c --- /dev/null +++ b/app/views/bankdetails/EnterBankAccountDetails.scala.html @@ -0,0 +1,83 @@ +@* + * Copyright 2026 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. + *@ + +@import models.BankAccountDetails +@import play.api.data.Form +@import config.FrontendAppConfig + + +@this( + layout: layouts.layout, + h1: components.h1, + p: components.p, + button: components.button, + formWithCSRF: FormWithCSRF, + errorSummary: components.errorSummary, + panelIndent: components.panelIndent, + inputText: components.inputText +) + +@(bankDetailsForm: Form[BankAccountDetails])(implicit request: Request[_], messages: Messages, appConfig: FrontendAppConfig) + +@layout(pageTitle = Some(title(bankDetailsForm, messages("pages.bankDetails.heading.new")))) { + + @errorSummary(errors = bankDetailsForm.errors) + + @h1("pages.bankDetails.heading.new") + @p { + @messages("pages.bankDetails.p1.new") + } + + @panelIndent { + @messages("pages.bankDetails.info") + } + + @formWithCSRF(action = controllers.bankdetails.routes.UkBankAccountDetailsController.submit) { + @inputText(form = bankDetailsForm, + id = "accountName", + name = "accountName", + label = messages("pages.bankDetails.accountName.label.new"), + classes = Some("govuk-input--width-20"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountName.hint")))) + + @inputText(form = bankDetailsForm, + id = "accountNumber", + name = "accountNumber", + label = messages("pages.bankDetails.accountNumber.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) + + @inputText(form = bankDetailsForm, + id = "sortCode", + name = "sortCode", + label = messages("pages.bankDetails.sortCode.label"), + classes = Some("govuk-input--width-5"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) + + @inputText(form = bankDetailsForm, + id = "rollNumber", + name = "rollNumber", + label = messages("pages.bankDetails.rollNumber.label"), + classes = Some("govuk-input--width-20"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) + + @button("app.common.continue") + } +} \ No newline at end of file diff --git a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html index f135b3469..b7aedb067 100644 --- a/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html +++ b/app/views/bankdetails/EnterCompanyBankAccountDetails.scala.html @@ -17,8 +17,6 @@ @import models.BankAccountDetails @import play.api.data.Form @import config.FrontendAppConfig -@import featuretoggle.FeatureSwitch._ -@import featuretoggle.FeatureToggleSupport._ @this( layout: layouts.layout, @@ -33,14 +31,12 @@ @(bankDetailsForm: Form[BankAccountDetails])(implicit request: Request[_], messages: Messages, appConfig: FrontendAppConfig) -@layout(pageTitle = Some(title(bankDetailsForm, messages("pages.bankDetails.heading")))) { +@layout(pageTitle = Some(title(bankDetailsForm, messages("pages.bankDetails.heading.old")))) { @errorSummary(errors = bankDetailsForm.errors) - @h1("pages.bankDetails.heading") - @p { - @messages("pages.bankDetails.p1") - } + @h1("pages.bankDetails.heading.old") + @p { @messages("pages.bankDetails.p1.old") } @panelIndent { @messages("pages.bankDetails.info") @@ -48,38 +44,28 @@ @formWithCSRF(action = controllers.bankdetails.routes.UkBankAccountDetailsController.submit) { @inputText(form = bankDetailsForm, - id = "accountName", - name = "accountName", - label = messages("pages.bankDetails.accountName.label"), - classes = Some("govuk-input--width-20"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.accountName.hint")))) + id = "accountName", + name = "accountName", + label = messages("pages.bankDetails.accountName.label.old"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("")))) @inputText(form = bankDetailsForm, - id = "accountNumber", - name = "accountNumber", - label = messages("pages.bankDetails.accountNumber.label"), - classes = Some("govuk-input--width-10"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) + id = "accountNumber", + name = "accountNumber", + label = messages("pages.bankDetails.accountNumber.label"), + classes = Some("govuk-input--width-10"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.accountNumber.hint")))) @inputText(form = bankDetailsForm, - id = "sortCode", - name = "sortCode", - label = messages("pages.bankDetails.sortCode.label"), - classes = Some("govuk-input--width-5"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) - - @if(isEnabled(UseNewBarsVerify)) { - @inputText(form = bankDetailsForm, - id = "rollNumber", - name = "rollNumber", - label = messages("pages.bankDetails.rollNumber.label"), - classes = Some("govuk-input--width-20"), - isPageHeading = false, - hint = Some(Html(messages("pages.bankDetails.rollNumber.hint")))) - } + id = "sortCode", + name = "sortCode", + label = messages("pages.bankDetails.sortCode.label"), + classes = Some("govuk-input--width-5"), + isPageHeading = false, + hint = Some(Html(messages("pages.bankDetails.sortCode.hint")))) @button("app.common.continue") } diff --git a/conf/messages b/conf/messages index 1afc275b4..924b7f4ea 100644 --- a/conf/messages +++ b/conf/messages @@ -990,26 +990,33 @@ pages.noUKBankAccount.dontWantToProvide = I do not want to provide pages.noUKBankAccount.error = Select why you cannot provide bank account details for the business # Bank Details page -pages.bankDetails.heading = What are the business’s account details? -pages.bankDetails.p1 = HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply. -pages.bankDetails.info = You must tell us if your account details change. -pages.bankDetails.accountName.label = Name on the account -pages.bankDetails.accountName.hint = Enter the name as it appears on bank statements and in your VAT account -pages.bankDetails.accountNumber.label = Account number -pages.bankDetails.accountNumber.hint = Must be between 6 and 8 digits long -pages.bankDetails.sortCode.label = Sort code -pages.bankDetails.sortCode.hint = Must be 6 digits long -pages.bankDetails.rollNumber.label = Building society roll number (if you have one) -pages.bankDetails.rollNumber.hint = You can find it on your card, statement or passbook -validation.companyBankAccount.name.missing = Enter the account name -validation.companyBankAccount.name.maxLength = The account name must be 60 characters or less -validation.companyBankAccount.name.invalid = The account name must only include numbers, letters a to z, and special characters such as hyphens, apostrophes and brackets -validation.companyBankAccount.number.missing = Enter the account number -validation.companyBankAccount.number.invalid = Account number must be between 6 and 8 digits -validation.companyBankAccount.sortCode.missing = Enter the account sort code -validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code -validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code -validation.companyBankAccount.rollNumber.invalid = Building society roll number must be shorter +pages.bankDetails.heading.old = What are the business’s bank or building society account details? +pages.bankDetails.heading.new = What are the business’s account details? +pages.bankDetails.p1.old = HMRC VAT will only use this account to send VAT repayments. We will not take money from it. +pages.bankDetails.p1.new = HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply. +pages.bankDetails.info = You must tell us if your account details change. +pages.bankDetails.accountName.label.old = Account name +pages.bankDetails.accountName.label.new = Name on the account +pages.bankDetails.accountName.hint = Enter the name as it appears on bank statements and in your VAT account +pages.bankDetails.accountNumber.label = Account number +pages.bankDetails.accountNumber.hint = Must be between 6 and 8 digits long +pages.bankDetails.sortCode.label = Sort code +pages.bankDetails.sortCode.hint = Must be 6 digits long +pages.bankDetails.rollNumber.label = Building society roll number (if you have one) +pages.bankDetails.rollNumber.hint = You can find it on your card, statement or passbook +validation.companyBankAccount.name.missing.old = Enter the account name +validation.companyBankAccount.name.missing.new = Enter the name on the account +validation.companyBankAccount.name.maxLength = The account name must be 60 characters or less +validation.companyBankAccount.name.invalid.old = The account name must only include numbers, letters a to z, and special characters such as hyphens, apostrophes and brackets +validation.companyBankAccount.name.invalid.new = Name on the account must not use special characters +validation.companyBankAccount.number.missing = Enter the account number +validation.companyBankAccount.number.invalid = Account number must be between 6 and 8 digits +validation.companyBankAccount.number.digit.error = Account number must only contain numbers +validation.companyBankAccount.sortCode.missing = Enter the account sort code +validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code +validation.companyBankAccount.sortCode.digit.error = Sort code must only contain numbers +validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code +validation.companyBankAccount.rollNumber.invalid = Building society roll number must be shorter # Main Business Activity Page pages.mainBusinessActivity.heading = Which activity is the business’s main source of income? diff --git a/conf/messages.cy b/conf/messages.cy index 271ae5b55..4d9bdddab 100644 --- a/conf/messages.cy +++ b/conf/messages.cy @@ -1000,17 +1000,20 @@ pages.bankDetails.sortCode.label = Cod didoli pages.bankDetails.sortCode.hint = Mae’n rhaid iddo fod yn 6 digid o hyd pages.bankDetails.rollNumber.label = Rhif rôl y gymdeithas adeiladu (os oes gennych un) pages.bankDetails.rollNumber.hint = Bydd hwn i’w weld ar eich cerdyn, cyfriflen neu baslyfr -validation.companyBankAccount.name.missing = Nodwch enw’r cyfrif +validation.companyBankAccount.name.missing.old = Nodwch enw’r cyfrif +validation.companyBankAccount.name.missing.new = Nodwch yr enw sydd ar y cyfrif validation.companyBankAccount.name.maxLength = Mae’n rhaid i enw’r cyfrif fod yn 60 o gymeriadau neu lai -validation.companyBankAccount.name.invalid = Mae’n rhaid i enw’r cyfrif gynnwys rhifau, y llythrennau a i z, a chymeriadau arbennig megis cysylltnodau, collnodau a chromfachau yn unig +validation.companyBankAccount.name.invalid.old = Mae’n rhaid i enw’r cyfrif gynnwys rhifau, y llythrennau a i z, a chymeriadau arbennig megis cysylltnodau, collnodau a chromfachau yn unig +validation.companyBankAccount.name.invalid.new = Enw sydd ar y cyfrif: mae’n rhaid i hyn beidio cynnwys cymeriadau arbennig validation.companyBankAccount.number.missing = Nodwch rif y cyfrif -validation.companyBankAccount.number.invalid = Mae’n rhaid i rif y cyfrif fod rhwng 6 ac 8 digid +validation.companyBankAccount.number.invalid = Rhif y cyfrif: mae’n rhaid i hyn fod rhwng 6 ac 8 digid +validation.companyBankAccount.number.digit.error = Rhif y cyfrif: mae’n rhaid i hyn gynnwys rhifau yn unig validation.companyBankAccount.sortCode.missing = Nodwch god didoli’r cyfrif -validation.companyBankAccount.sortCode.invalid = Nodwch god didoli dilys ar gyfer y cyfrif +validation.companyBankAccount.sortCode.invalid = Cod didoli: mae’n rhaid i hyn fod yn 6 digid +validation.companyBankAccount.sortCode.digit.error = Cod didoli: mae’n rhaid i hyn gynnwys rhifau yn unig validation.companyBankAccount.invalidCombination = Nodwch rif cyfrif banc a chod didoli dilys validation.companyBankAccount.rollNumber.invalid = Mae’n rhaid i rif rôl y gymdeithas adeiladu fod yn fyrrach - # Main Business Activity Page pages.mainBusinessActivity.heading = Pa weithgaredd yw prif ffynhonnell incwm y busnes? validation.mainBusinessActivity.missing = Dewiswch brif ffynhonnell incwm y busnes diff --git a/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala b/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala index a51be2671..f4364727c 100644 --- a/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala +++ b/it/test/controllers/bankdetails/UKBankAccountDetailsControllerISpec.scala @@ -37,15 +37,18 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr "return OK with a blank form if the VAT scheme doesn't contain bank details" in new Setup { disable(UseNewBarsVerify) - given().user - .isAuthorised() - .registrationApi - .getSection[BankAccount](None) + given().user.isAuthorised().registrationApi.getSection[BankAccount](None) insertCurrentProfileIntoDb(currentProfile, sessionString) val res: WSResponse = await(buildClient(url).get()) + val doc: Document = Jsoup.parse(res.body) + res.status mustBe OK + doc.select("input[id=accountName]").size() mustBe 1 + doc.select("input[id=accountNumber]").size() mustBe 1 + doc.select("input[id=sortCode]").size() mustBe 1 + doc.select("input[id=rollNumber]").size() mustBe 0 } "return OK with a form pre-populated from the backend when bank details exist" in new Setup { @@ -61,6 +64,7 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr doc.select("input[id=accountName]").`val`() mustBe testBankName doc.select("input[id=accountNumber]").`val`() mustBe testAccountNumber doc.select("input[id=sortCode]").`val`() mustBe testSortCode + doc.select("input[id=rollNumber]").size() mustBe 0 } } @@ -115,9 +119,12 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr .isAuthorised() .bankAccountReputation .passes - .registrationApi.getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) - .registrationApi.replaceSection[BankAccount](bankAccount) - .registrationApi.getSection[EligibilitySubmissionData](Some(testEligibilitySubmissionData.copy(registrationReason = TransferOfAGoingConcern))) + .registrationApi + .getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) + .registrationApi + .replaceSection[BankAccount](bankAccount) + .registrationApi + .getSection[EligibilitySubmissionData](Some(testEligibilitySubmissionData.copy(registrationReason = TransferOfAGoingConcern))) insertCurrentProfileIntoDb(currentProfile, sessionString) @@ -161,8 +168,6 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr disable(UseNewBarsVerify) given().user .isAuthorised() - .bankAccountReputation - .fails .registrationApi .getSection[BankAccount](Some(BankAccount(isProvided = true, None, None))) @@ -184,16 +189,17 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr "save bank details to session and redirect to Task List when form is valid" in new Setup { enable(UseNewBarsVerify) - given() - .user.isAuthorised() + given().user.isAuthorised() insertCurrentProfileIntoDb(currentProfile, sessionString) - val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> testBankName, - "accountNumber" -> testAccountNumber, - "sortCode" -> testSortCode - ))) + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode + ))) res.status mustBe SEE_OTHER res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) @@ -201,17 +207,18 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr "save bank details including roll number to session and redirect to Task List" in new Setup { enable(UseNewBarsVerify) - given() - .user.isAuthorised() + given().user.isAuthorised() insertCurrentProfileIntoDb(currentProfile, sessionString) - val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> testBankName, - "accountNumber" -> testAccountNumber, - "sortCode" -> testSortCode, - "rollNumber" -> testRollNumber - ))) + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> testAccountNumber, + "sortCode" -> testSortCode, + "rollNumber" -> testRollNumber + ))) res.status mustBe SEE_OTHER res.header(HeaderNames.LOCATION) mustBe Some(controllers.routes.TaskListController.show.url) @@ -219,16 +226,34 @@ class UKBankAccountDetailsControllerISpec extends ControllerISpec with ITRegistr "return BAD_REQUEST without calling BARS when form fields are empty" in new Setup { enable(UseNewBarsVerify) - given() - .user.isAuthorised() + given().user.isAuthorised() + + insertCurrentProfileIntoDb(currentProfile, sessionString) + + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> "", + "accountNumber" -> "", + "sortCode" -> "" + ))) + + res.status mustBe BAD_REQUEST + } + + "return BAD_REQUEST without calling BARS when account number is invalid" in new Setup { + enable(UseNewBarsVerify) + given().user.isAuthorised() insertCurrentProfileIntoDb(currentProfile, sessionString) - val res: WSResponse = await(buildClient(url).post(Map( - "accountName" -> "", - "accountNumber" -> "", - "sortCode" -> "" - ))) + val res: WSResponse = await( + buildClient(url).post( + Map( + "accountName" -> testBankName, + "accountNumber" -> "invalid", + "sortCode" -> testSortCode + ))) res.status mustBe BAD_REQUEST } diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index b317ed4e3..2cf4acc9e 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -16,27 +16,27 @@ package forms -import forms.EnterBankAccountDetailsForm._ import models.BankAccountDetails import org.scalatestplus.play.PlaySpec class BankAccountDetailsFormSpec extends PlaySpec { - "EnterBankAccountDetailsForm" should { + val numStr = 60 + val validAccountName = s"${numStr}testAccountName" + val validAccountNumber = "12345678" + val validSortCode = "123456" + val validRollNumber = "AB/121212" - val form = EnterBankAccountDetailsForm.form + "EnterBankAccountDetailsForm (useBarsVerify OFF)" should { + import forms.EnterBankAccountDetailsForm._ - val numStr = 60 - val validAccountName = s"${numStr}testAccountName" - val validAccountNumber = "12345678" - val validSortCode = "123456" - val validRollNumber = "AB/121212" + val form = EnterBankAccountDetailsForm.form "successfully bind data to the form with no errors and allow the return of a valid BankAccountDetails case class" in { val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val validBankAccountDetails = BankAccountDetails(validAccountName, validAccountNumber, validSortCode) @@ -45,15 +45,23 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.get mustBe validBankAccountDetails } + "successfully bind an account name containing all valid characters" in { + val formData = Map( + ACCOUNT_NAME -> "O'Brien-Smith J. A/B", + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode + ) + form.bind(formData).errors mustBe empty + } + "return a FormError when binding an empty account name to the form" in { val formData = Map( - ACCOUNT_NAME -> "", + ACCOUNT_NAME -> "", ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) - boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NAME boundForm.errors.head.message mustBe accountNameEmptyKey @@ -63,9 +71,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val exceedMaxLength = "AlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimited" val formData = Map( - ACCOUNT_NAME -> exceedMaxLength, + ACCOUNT_NAME -> exceedMaxLength, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -79,9 +87,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val invalidAccountName = "123#@~" val formData = Map( - ACCOUNT_NAME -> invalidAccountName, + ACCOUNT_NAME -> invalidAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -93,9 +101,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { "return a FormError when binding an empty account number to the form" in { val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> "", - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -109,9 +117,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val invalidAccountNumber = "ABCDE" val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> invalidAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -125,9 +133,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val invalidAccountNumber = "12345" val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> invalidAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -141,9 +149,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val invalidAccountNumber = "123456789" val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> invalidAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -157,9 +165,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val validAccountNumber = "123 456" val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) @@ -167,14 +175,25 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.size mustBe 0 } + "return a FormError when sort code is empty" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> "" + ) + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe SORT_CODE + boundForm.errors.head.message mustBe sortCodeEmptyKey + } "return a FormError when binding an invalid sort code part to the form" in { val invalidSortCode = "ABCDEF" val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> invalidSortCode + SORT_CODE -> invalidSortCode ) val boundForm = form.bind(formData) @@ -183,37 +202,153 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.key mustBe SORT_CODE boundForm.errors.head.message mustBe sortCodeInvalidKey } - "return No FormError when binding a sort code with spaces ( ) part to the form" in { - val invalidSortCode = "02 03 06" + } + + "EnterBankAccountDetailsNewBarsForm (useBarsVerify ON)" should { + import forms.EnterBankAccountDetailsNewBarsForm._ + val form = EnterBankAccountDetailsNewBarsForm.form + "successfully bind valid data" in { val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> invalidSortCode + SORT_CODE -> validSortCode ) val boundForm = form.bind(formData) + boundForm.get mustBe BankAccountDetails(validAccountName, validAccountNumber, validSortCode) + } - boundForm.errors.size mustBe 0 + "return a FormError with the new missing account name error message when account name is empty" in { + val formData = Map( + ACCOUNT_NAME -> "", + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NAME + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameEmptyKey + } + + "return a FormError when account name exceeds 60 characters" in { + val exceedMaxLength = "AlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimitedAlPacinoLimited" + + val formData = Map( + ACCOUNT_NAME -> exceedMaxLength, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NAME + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameMaxLengthKey + } + + "return a FormError with the new invalid account name error message when account name contains invalid characters" in { + val invalidAccountName = "123#@~" + + val formData = Map( + ACCOUNT_NAME -> invalidAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NAME + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameInvalidKey + } + + "return a FormError when account number is empty" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> "", + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NUMBER + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberEmptyKey + } + + "return a FormError with the length error message when account number is fewer than 6 digits" in { + val invalidAccountNumber = "12345" + + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> invalidAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NUMBER + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberDigitKey } - "return a single FormError when the sort code is missing" in { - val emptySortCode = "" + "return a FormError with the invalid characters error message when account number is 8 characters but contains letters" in { + val invalidAccountNumber = "1234567A" + + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> invalidAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe ACCOUNT_NUMBER + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberInvalidKey + } + "return a FormError when sort code is empty" in { val formData = Map( - ACCOUNT_NAME -> validAccountName, + ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> emptySortCode + SORT_CODE -> "" ) val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe SORT_CODE + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeEmptyKey + } + + "return a FormError with the length error message when sort code is fewer than 6 digits" in { + val invalidSortCode = "12345" + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> invalidSortCode + ) + + val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe sortCodeEmptyKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeLengthKey } - "successfully bind data with a valid roll number" in { + "return a FormError with the invalid characters error message when sort code is 6 characters but contains letters" in { + val invalidSortCode = "ABCDEF" + + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> invalidSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors.size mustBe 1 + boundForm.errors.head.key mustBe SORT_CODE + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeInvalidKey + } + + "successfully bind with a valid roll number" in { val formData = Map( ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, @@ -222,11 +357,23 @@ class BankAccountDetailsFormSpec extends PlaySpec { ) val boundForm = form.bind(formData) - boundForm.errors.size mustBe 0 - boundForm.get.rollNumber mustBe Some("AB/121212") + boundForm.errors mustBe empty + boundForm.get.rollNumber mustBe Some(validRollNumber) } - "successfully bind data when roll number is empty string" in { + "successfully bind when roll number is not provided" in { + val formData = Map( + ACCOUNT_NAME -> validAccountName, + ACCOUNT_NUMBER -> validAccountNumber, + SORT_CODE -> validSortCode + ) + + val boundForm = form.bind(formData) + boundForm.errors mustBe empty + boundForm.get.rollNumber mustBe None + } + + "successfully bind when roll number is an empty string" in { val formData = Map( ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, @@ -235,11 +382,11 @@ class BankAccountDetailsFormSpec extends PlaySpec { ) val boundForm = form.bind(formData) - boundForm.errors.size mustBe 0 + boundForm.errors mustBe empty boundForm.get.rollNumber mustBe None } - "return a FormError when roll number exceeds max length" in { + "return a FormError when roll number exceeds 25 characters" in { val longRollNumber = "A" * 26 val formData = Map( @@ -252,10 +399,10 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ROLL_NUMBER - boundForm.errors.head.message mustBe rollNumberInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.rollNumberInvalidKey } - "successfully bind roll number with spaces stripped" in { + "successfully bind and strip spaces from roll number before storing" in { val formData = Map( ACCOUNT_NAME -> validAccountName, ACCOUNT_NUMBER -> validAccountNumber, @@ -264,7 +411,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { ) val boundForm = form.bind(formData) - boundForm.errors.size mustBe 0 + boundForm.errors mustBe empty boundForm.get.rollNumber mustBe Some("AB121212") } } diff --git a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala index 03e4cbc7b..02f2076a7 100644 --- a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala +++ b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala @@ -16,8 +16,6 @@ package views.bankdetails -import featuretoggle.FeatureSwitch.UseNewBarsVerify -import featuretoggle.FeatureToggleSupport._ import forms.EnterBankAccountDetailsForm import org.jsoup.Jsoup import org.jsoup.nodes.Document @@ -28,23 +26,19 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { val view: EnterCompanyBankAccountDetails = app.injector.instanceOf[EnterCompanyBankAccountDetails] - val title = "What are the business’s account details?" - val heading = "What are the business’s account details?" - val p1 = "HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply." + val title = "What are the business’s bank or building society account details?" + val heading = "What are the business’s bank or building society account details?" + val p1 = "HMRC VAT will only use this account to send VAT repayments. We will not take money from it." val panelText = "You must tell us if your account details change." - val accountName = "Name on the account" + val accountName = "Account name" val accountNumber = "Account number" val accountNumberHint = "Must be between 6 and 8 digits long" val sortCode = "Sort code" val sortCodeHint = "Must be 6 digits long" - val rollNumber = "Building society roll number (if you have one)" - val rollNumberHint = "You can find it on your card, statement or passbook" val buttonText = "Save and continue" - implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) - - "Company Bank Details Page common elements" should { - disable(UseNewBarsVerify) + "Company Bank Details Page" should { + implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) "have the correct title" in new ViewSetup { doc.title must include(title) @@ -82,24 +76,10 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { doc.hintWithMultiple(3) mustBe Some(sortCodeHint) } - "not show the roll number field when UseNewBarsVerify is disabled" in new ViewSetup { - doc.select(Selectors.label).size mustBe 3 - } - "have the correct continue button" in new ViewSetup { doc.submitButton mustBe Some(buttonText) } - } - - "Company Bank Details Page roll number field" should { - "be visible when UseNewBarsVerify is enabled" in new ViewSetup { - enable(UseNewBarsVerify) - val enabledDoc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) - enabledDoc.select(Selectors.label).get(3).text mustBe rollNumber - enabledDoc.hintWithMultiple(4) mustBe Some(rollNumberHint) - enabledDoc.select(Selectors.label).size mustBe 4 - } } } diff --git a/test/views/bankdetails/EnterBankDetailsViewSpec.scala b/test/views/bankdetails/EnterBankDetailsViewSpec.scala new file mode 100644 index 000000000..d85ae58ca --- /dev/null +++ b/test/views/bankdetails/EnterBankDetailsViewSpec.scala @@ -0,0 +1,97 @@ +/* + * 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 views.bankdetails + +import featuretoggle.FeatureSwitch.UseNewBarsVerify +import featuretoggle.FeatureToggleSupport._ +import forms.EnterBankAccountDetailsNewBarsForm +import org.jsoup.Jsoup +import org.jsoup.nodes.Document +import views.VatRegViewSpec +import views.html.bankdetails.EnterBankAccountDetails + +class EnterBankDetailsViewSpec extends VatRegViewSpec { + + val view: EnterBankAccountDetails = app.injector.instanceOf[EnterBankAccountDetails] + + val title = "What are the business’s account details?" + val heading = "What are the business’s account details?" + val p1 = "HMRC VAT will only use this information to send VAT repayments. Money will not be taken from the account you supply." + val panelText = "You must tell us if your account details change." + val accountName = "Name on the account" + val accountNumber = "Account number" + val accountNumberHint = "Must be between 6 and 8 digits long" + val sortCode = "Sort code" + val sortCodeHint = "Must be 6 digits long" + val rollNumber = "Building society roll number (if you have one)" + val rollNumberHint = "You can find it on your card, statement or passbook" + val buttonText = "Save and continue" + + implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsNewBarsForm.form).body) + + "Company Bank Details Page common elements" should { + disable(UseNewBarsVerify) + + "have the correct title" in new ViewSetup { + doc.title must include(title) + } + + "have the correct heading" in new ViewSetup { + doc.heading mustBe Some(heading) + } + + "have the correct p1" in new ViewSetup { + doc.para(1) mustBe Some(p1) + } + + "have the correct panel text" in new ViewSetup { + doc.panelIndent(1) mustBe Some(panelText) + } + + "have the correct Account Name label text" in { + doc.select(Selectors.label).get(0).text mustBe accountName + } + + "have the correct Account Number label text" in { + doc.select(Selectors.label).get(1).text mustBe accountNumber + } + + "have the correct Account Number Hint text" in new ViewSetup { + doc.hintWithMultiple(2) mustBe Some(accountNumberHint) + } + + "have the correct Sort Code label text" in { + doc.select(Selectors.label).get(2).text mustBe sortCode + } + + "have the correct Sort Code Hint text" in new ViewSetup { + doc.hintWithMultiple(3) mustBe Some(sortCodeHint) + } + + "have Roll Number label text" in new ViewSetup { + doc.select(Selectors.label).get(3).text mustBe rollNumber + } + + "have the correct Roll Number Hint text" in new ViewSetup { + doc.hintWithMultiple(4) mustBe Some(rollNumberHint) + } + + "have the correct continue button" in new ViewSetup { + doc.submitButton mustBe Some(buttonText) + } + } +} From f035b5edd149e07dab01b02c0a61ef2f235296b4 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:39:23 +0000 Subject: [PATCH 07/11] fix: error messages --- app/forms/BankAccountDetailsForms.scala | 28 ++++++++++----------- test/forms/BankAccountDetailsFormSpec.scala | 8 +++--- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index 0a3650125..35054cfaa 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -113,16 +113,16 @@ object EnterBankAccountDetailsNewBarsForm { val SORT_CODE = "sortCode" val ROLL_NUMBER = "rollNumber" - val accountNameEmptyKey = "validation.companyBankAccount.name.missing.new" - val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" - val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.new" - val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" - val accountNumberDigitKey = "validation.companyBankAccount.number.digit.error" - val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" - val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" - val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" - val sortCodeLengthKey = "validation.companyBankAccount.sortCode.length" - val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" + val accountNameEmptyKey = "validation.companyBankAccount.name.missing.new" + val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" + val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.new" + val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" + val accountNumberDigitErrorKey = "validation.companyBankAccount.number.digit.error" + val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" + val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" + val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" + val sortCodeDigitErrorKey = "validation.companyBankAccount.sortCode.digit.error" + val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" private val accountNameRegex = """^[A-Za-z0-9 '\-./]{1,60}$""".r private val accountNameMaxLength = 60 @@ -144,16 +144,16 @@ object EnterBankAccountDetailsNewBarsForm { .transform(removeSpaces, identity[String]) .verifying(stopOnFail( mandatory(accountNumberEmptyKey), - matchesRegex(accountNumberDigitRegex, accountNumberInvalidKey), - matchesRegex(accountNumberLengthRegex, accountNumberDigitKey) + matchesRegex(accountNumberDigitRegex, accountNumberDigitErrorKey), + matchesRegex(accountNumberLengthRegex, accountNumberInvalidKey) )), SORT_CODE -> text .transform(removeSpaces, identity[String]) .verifying( stopOnFail( mandatory(sortCodeEmptyKey), - matchesRegex(sortCodeDigitRegex, sortCodeInvalidKey), - matchesRegex(sortCodeLengthRegex, sortCodeLengthKey) + matchesRegex(sortCodeDigitRegex, sortCodeDigitErrorKey), + matchesRegex(sortCodeLengthRegex, sortCodeInvalidKey) )), ROLL_NUMBER -> optional( text diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index 2cf4acc9e..0f5a60140 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -287,7 +287,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberDigitKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberInvalidKey } "return a FormError with the invalid characters error message when account number is 8 characters but contains letters" in { @@ -302,7 +302,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberDigitErrorKey } "return a FormError when sort code is empty" in { @@ -330,7 +330,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeLengthKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeInvalidKey } "return a FormError with the invalid characters error message when sort code is 6 characters but contains letters" in { @@ -345,7 +345,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeDigitErrorKey } "successfully bind with a valid roll number" in { From 6508e7499c6125a6d70a709c6e0d0573474a7caf Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Fri, 13 Mar 2026 06:30:49 +0000 Subject: [PATCH 08/11] reformat: rename files for consistency --- ... => EnterBankAccountDetailsA11ySpec.scala} | 9 +++--- .../UkBankAccountDetailsA11ySpec.scala | 8 ++++-- .../UkBankAccountDetailsController.scala | 10 +++---- app/forms/BankAccountDetailsForms.scala | 4 +-- test/forms/BankAccountDetailsFormSpec.scala | 28 +++++++++---------- .../CompanyBankDetailsViewSpec.scala | 4 +-- .../EnterBankDetailsViewSpec.scala | 4 +-- 7 files changed, 34 insertions(+), 33 deletions(-) rename a11y/pages/bankdetails/{BarsBankAccountDetailsA11ySpec.scala => EnterBankAccountDetailsA11ySpec.scala} (74%) diff --git a/a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala b/a11y/pages/bankdetails/EnterBankAccountDetailsA11ySpec.scala similarity index 74% rename from a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala rename to a11y/pages/bankdetails/EnterBankAccountDetailsA11ySpec.scala index f59b50d8f..556edd984 100644 --- a/a11y/pages/bankdetails/BarsBankAccountDetailsA11ySpec.scala +++ b/a11y/pages/bankdetails/EnterBankAccountDetailsA11ySpec.scala @@ -1,4 +1,3 @@ - package pages.bankdetails import forms.EnterBankAccountDetailsForm @@ -7,12 +6,12 @@ import models.BankAccountDetails import play.api.data.Form import views.html.bankdetails.EnterBankAccountDetails -class BarsBankAccountDetailsA11ySpec extends A11ySpec { +class EnterBankAccountDetailsA11ySpec extends A11ySpec { - val view: EnterBankAccountDetails = app.injector.instanceOf[EnterBankAccountDetails] + val view: EnterBankAccountDetails = app.injector.instanceOf[EnterBankAccountDetails] val form: Form[BankAccountDetails] = EnterBankAccountDetailsForm.form - "the Enter Company Bank Account Details page" when { + "the Enter Bank Account Details page" when { "there are no form errors" must { "pass all a11y checks" in { view(form).body must passAccessibilityChecks @@ -25,4 +24,4 @@ class BarsBankAccountDetailsA11ySpec extends A11ySpec { } } -} \ No newline at end of file +} diff --git a/a11y/pages/bankdetails/UkBankAccountDetailsA11ySpec.scala b/a11y/pages/bankdetails/UkBankAccountDetailsA11ySpec.scala index 1914f236d..2b5c4f0bd 100644 --- a/a11y/pages/bankdetails/UkBankAccountDetailsA11ySpec.scala +++ b/a11y/pages/bankdetails/UkBankAccountDetailsA11ySpec.scala @@ -3,12 +3,14 @@ package pages.bankdetails import helpers.A11ySpec import views.html.bankdetails.EnterCompanyBankAccountDetails -import forms.EnterBankAccountDetailsForm +import forms.EnterCompanyBankAccountDetailsForm +import models.BankAccountDetails +import play.api.data.Form class UkBankAccountDetailsA11ySpec extends A11ySpec { - val view = app.injector.instanceOf[EnterCompanyBankAccountDetails] - val form = EnterBankAccountDetailsForm.form + val view: EnterCompanyBankAccountDetails = app.injector.instanceOf[EnterCompanyBankAccountDetails] + val form: Form[BankAccountDetails] = EnterCompanyBankAccountDetailsForm.form "the Enter Company Bank Account Details page" when { "there are no form errors" must { diff --git a/app/controllers/bankdetails/UkBankAccountDetailsController.scala b/app/controllers/bankdetails/UkBankAccountDetailsController.scala index 042c75138..58bff9740 100644 --- a/app/controllers/bankdetails/UkBankAccountDetailsController.scala +++ b/app/controllers/bankdetails/UkBankAccountDetailsController.scala @@ -20,9 +20,9 @@ import config.{AuthClientConnector, BaseControllerComponents, FrontendAppConfig} import controllers.BaseController import featuretoggle.FeatureSwitch.UseNewBarsVerify import featuretoggle.FeatureToggleSupport.isEnabled +import forms.EnterCompanyBankAccountDetailsForm +import forms.EnterCompanyBankAccountDetailsForm.{form => enterBankAccountDetailsForm} import forms.EnterBankAccountDetailsForm -import forms.EnterBankAccountDetailsForm.{form => enterBankAccountDetailsForm} -import forms.EnterBankAccountDetailsNewBarsForm import models.BankAccountDetails import models.bars.BankAccountDetailsSessionFormat import play.api.libs.json.Format @@ -56,7 +56,7 @@ class UkBankAccountDetailsController @Inject() ( def show: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => if (isEnabled(UseNewBarsVerify)) { - val newBarsForm = EnterBankAccountDetailsNewBarsForm.form + val newBarsForm = EnterBankAccountDetailsForm.form sessionService.fetchAndGet[BankAccountDetails](sessionKey).map { case Some(details) => Ok(newBarsView(newBarsForm.fill(details))) case None => Ok(newBarsView(newBarsForm)) @@ -71,7 +71,7 @@ class UkBankAccountDetailsController @Inject() ( def submit: Action[AnyContent] = isAuthenticatedWithProfile { implicit request => implicit profile => if (isEnabled(UseNewBarsVerify)) { - val newBarsForm = EnterBankAccountDetailsNewBarsForm.form + val newBarsForm = EnterBankAccountDetailsForm.form newBarsForm .bindFromRequest() .fold( @@ -89,7 +89,7 @@ class UkBankAccountDetailsController @Inject() ( accountDetails => bankAccountDetailsService.saveEnteredBankAccountDetails(accountDetails).map { case true => Redirect(controllers.routes.TaskListController.show.url) - case false => BadRequest(oldView(EnterBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) + case false => BadRequest(oldView(EnterCompanyBankAccountDetailsForm.formWithInvalidAccountReputation.fill(accountDetails))) } ) } diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index 35054cfaa..7a2b68094 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -53,7 +53,7 @@ object ChooseAccountTypeForm { ) } -object EnterBankAccountDetailsForm { +object EnterCompanyBankAccountDetailsForm { val ACCOUNT_NAME = "accountName" val ACCOUNT_NUMBER = "accountNumber" @@ -106,7 +106,7 @@ object EnterBankAccountDetailsForm { form.withError(invalidAccountReputationKey, invalidAccountReputationMessage) } -object EnterBankAccountDetailsNewBarsForm { +object EnterBankAccountDetailsForm { val ACCOUNT_NAME = "accountName" val ACCOUNT_NUMBER = "accountNumber" diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index 0f5a60140..e2a52c890 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -28,9 +28,9 @@ class BankAccountDetailsFormSpec extends PlaySpec { val validRollNumber = "AB/121212" "EnterBankAccountDetailsForm (useBarsVerify OFF)" should { - import forms.EnterBankAccountDetailsForm._ + import forms.EnterCompanyBankAccountDetailsForm._ - val form = EnterBankAccountDetailsForm.form + val form = EnterCompanyBankAccountDetailsForm.form "successfully bind data to the form with no errors and allow the return of a valid BankAccountDetails case class" in { val formData = Map( @@ -205,8 +205,8 @@ class BankAccountDetailsFormSpec extends PlaySpec { } "EnterBankAccountDetailsNewBarsForm (useBarsVerify ON)" should { - import forms.EnterBankAccountDetailsNewBarsForm._ - val form = EnterBankAccountDetailsNewBarsForm.form + import forms.EnterBankAccountDetailsForm._ + val form = EnterBankAccountDetailsForm.form "successfully bind valid data" in { val formData = Map( @@ -229,7 +229,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NAME - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameEmptyKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNameEmptyKey } "return a FormError when account name exceeds 60 characters" in { @@ -244,7 +244,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NAME - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameMaxLengthKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNameMaxLengthKey } "return a FormError with the new invalid account name error message when account name contains invalid characters" in { @@ -259,7 +259,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NAME - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNameInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNameInvalidKey } "return a FormError when account number is empty" in { @@ -272,7 +272,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberEmptyKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberEmptyKey } "return a FormError with the length error message when account number is fewer than 6 digits" in { @@ -287,7 +287,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberInvalidKey } "return a FormError with the invalid characters error message when account number is 8 characters but contains letters" in { @@ -302,7 +302,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.accountNumberDigitErrorKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberDigitErrorKey } "return a FormError when sort code is empty" in { @@ -315,7 +315,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeEmptyKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeEmptyKey } "return a FormError with the length error message when sort code is fewer than 6 digits" in { @@ -330,7 +330,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeInvalidKey } "return a FormError with the invalid characters error message when sort code is 6 characters but contains letters" in { @@ -345,7 +345,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.sortCodeDigitErrorKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeDigitErrorKey } "successfully bind with a valid roll number" in { @@ -399,7 +399,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ROLL_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsNewBarsForm.rollNumberInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.rollNumberInvalidKey } "successfully bind and strip spaces from roll number before storing" in { diff --git a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala index 02f2076a7..0d0cf2dec 100644 --- a/test/views/bankdetails/CompanyBankDetailsViewSpec.scala +++ b/test/views/bankdetails/CompanyBankDetailsViewSpec.scala @@ -16,7 +16,7 @@ package views.bankdetails -import forms.EnterBankAccountDetailsForm +import forms.EnterCompanyBankAccountDetailsForm import org.jsoup.Jsoup import org.jsoup.nodes.Document import views.VatRegViewSpec @@ -38,7 +38,7 @@ class CompanyBankDetailsViewSpec extends VatRegViewSpec { val buttonText = "Save and continue" "Company Bank Details Page" should { - implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) + implicit lazy val doc: Document = Jsoup.parse(view(EnterCompanyBankAccountDetailsForm.form).body) "have the correct title" in new ViewSetup { doc.title must include(title) diff --git a/test/views/bankdetails/EnterBankDetailsViewSpec.scala b/test/views/bankdetails/EnterBankDetailsViewSpec.scala index d85ae58ca..8ed0e493e 100644 --- a/test/views/bankdetails/EnterBankDetailsViewSpec.scala +++ b/test/views/bankdetails/EnterBankDetailsViewSpec.scala @@ -18,7 +18,7 @@ package views.bankdetails import featuretoggle.FeatureSwitch.UseNewBarsVerify import featuretoggle.FeatureToggleSupport._ -import forms.EnterBankAccountDetailsNewBarsForm +import forms.EnterBankAccountDetailsForm import org.jsoup.Jsoup import org.jsoup.nodes.Document import views.VatRegViewSpec @@ -41,7 +41,7 @@ class EnterBankDetailsViewSpec extends VatRegViewSpec { val rollNumberHint = "You can find it on your card, statement or passbook" val buttonText = "Save and continue" - implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsNewBarsForm.form).body) + implicit lazy val doc: Document = Jsoup.parse(view(EnterBankAccountDetailsForm.form).body) "Company Bank Details Page common elements" should { disable(UseNewBarsVerify) From bc178573a5ab9e396c135b086c5c6b5799e3fee5 Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Fri, 13 Mar 2026 07:17:32 +0000 Subject: [PATCH 09/11] feat: apply correct error messages --- app/forms/BankAccountDetailsForms.scala | 30 ++++++++++----------- conf/messages | 5 ++-- conf/messages.cy | 7 ++--- test/forms/BankAccountDetailsFormSpec.scala | 27 +++++++------------ 4 files changed, 31 insertions(+), 38 deletions(-) diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index 7a2b68094..dfb1ea433 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -59,9 +59,9 @@ object EnterCompanyBankAccountDetailsForm { val ACCOUNT_NUMBER = "accountNumber" val SORT_CODE = "sortCode" - val accountNameEmptyKey = "validation.companyBankAccount.name.missing" + val accountNameEmptyKey = "validation.companyBankAccount.name.missing.old" val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" - val accountNameInvalidKey = "validation.companyBankAccount.name.invalid" + val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.old" val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" @@ -113,16 +113,16 @@ object EnterBankAccountDetailsForm { val SORT_CODE = "sortCode" val ROLL_NUMBER = "rollNumber" - val accountNameEmptyKey = "validation.companyBankAccount.name.missing.new" - val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" - val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.new" - val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" - val accountNumberDigitErrorKey = "validation.companyBankAccount.number.digit.error" - val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" - val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" - val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" - val sortCodeDigitErrorKey = "validation.companyBankAccount.sortCode.digit.error" - val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" + val accountNameEmptyKey = "validation.companyBankAccount.name.missing.new" + val accountNameMaxLengthKey = "validation.companyBankAccount.name.maxLength" + val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.new" + val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" + val accountNumberFormatKey = "validation.companyBankAccount.number.format" + val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" + val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" + val sortCodeLengthKey = "validation.companyBankAccount.sortCode.length" + val sortCodeFormatKey = "validation.companyBankAccount.sortCode.format" + val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" private val accountNameRegex = """^[A-Za-z0-9 '\-./]{1,60}$""".r private val accountNameMaxLength = 60 @@ -144,7 +144,7 @@ object EnterBankAccountDetailsForm { .transform(removeSpaces, identity[String]) .verifying(stopOnFail( mandatory(accountNumberEmptyKey), - matchesRegex(accountNumberDigitRegex, accountNumberDigitErrorKey), + matchesRegex(accountNumberDigitRegex, accountNumberFormatKey), matchesRegex(accountNumberLengthRegex, accountNumberInvalidKey) )), SORT_CODE -> text @@ -152,8 +152,8 @@ object EnterBankAccountDetailsForm { .verifying( stopOnFail( mandatory(sortCodeEmptyKey), - matchesRegex(sortCodeDigitRegex, sortCodeDigitErrorKey), - matchesRegex(sortCodeLengthRegex, sortCodeInvalidKey) + matchesRegex(sortCodeDigitRegex, sortCodeFormatKey), + matchesRegex(sortCodeLengthRegex, sortCodeLengthKey) )), ROLL_NUMBER -> optional( text diff --git a/conf/messages b/conf/messages index 924b7f4ea..8564b0b2a 100644 --- a/conf/messages +++ b/conf/messages @@ -1011,10 +1011,11 @@ validation.companyBankAccount.name.invalid.old = The account name must validation.companyBankAccount.name.invalid.new = Name on the account must not use special characters validation.companyBankAccount.number.missing = Enter the account number validation.companyBankAccount.number.invalid = Account number must be between 6 and 8 digits -validation.companyBankAccount.number.digit.error = Account number must only contain numbers +validation.companyBankAccount.number.format = Account number must only contain numbers validation.companyBankAccount.sortCode.missing = Enter the account sort code validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code -validation.companyBankAccount.sortCode.digit.error = Sort code must only contain numbers +validation.companyBankAccount.sortCode.format = Sort code must only contain numbers +validation.companyBankAccount.sortCode.length = Sort code must be 6 digits long validation.companyBankAccount.invalidCombination = Enter a valid bank account number and sort code validation.companyBankAccount.rollNumber.invalid = Building society roll number must be shorter diff --git a/conf/messages.cy b/conf/messages.cy index 4d9bdddab..fa170ba1a 100644 --- a/conf/messages.cy +++ b/conf/messages.cy @@ -1007,10 +1007,11 @@ validation.companyBankAccount.name.invalid.old = Mae’n rhaid i enw’r cy validation.companyBankAccount.name.invalid.new = Enw sydd ar y cyfrif: mae’n rhaid i hyn beidio cynnwys cymeriadau arbennig validation.companyBankAccount.number.missing = Nodwch rif y cyfrif validation.companyBankAccount.number.invalid = Rhif y cyfrif: mae’n rhaid i hyn fod rhwng 6 ac 8 digid -validation.companyBankAccount.number.digit.error = Rhif y cyfrif: mae’n rhaid i hyn gynnwys rhifau yn unig +validation.companyBankAccount.number.format = Rhif y cyfrif: mae’n rhaid i hyn gynnwys rhifau yn unig validation.companyBankAccount.sortCode.missing = Nodwch god didoli’r cyfrif -validation.companyBankAccount.sortCode.invalid = Cod didoli: mae’n rhaid i hyn fod yn 6 digid -validation.companyBankAccount.sortCode.digit.error = Cod didoli: mae’n rhaid i hyn gynnwys rhifau yn unig +validation.companyBankAccount.sortCode.invalid = Nodwch god didoli dilys ar gyfer y cyfrif +validation.companyBankAccount.sortCode.length = Cod didoli: mae’n rhaid i hyn fod yn 6 digid +validation.companyBankAccount.sortCode.format = Cod didoli: mae’n rhaid i hyn gynnwys rhifau yn unig validation.companyBankAccount.invalidCombination = Nodwch rif cyfrif banc a chod didoli dilys validation.companyBankAccount.rollNumber.invalid = Mae’n rhaid i rif rôl y gymdeithas adeiladu fod yn fyrrach diff --git a/test/forms/BankAccountDetailsFormSpec.scala b/test/forms/BankAccountDetailsFormSpec.scala index e2a52c890..e4bf67750 100644 --- a/test/forms/BankAccountDetailsFormSpec.scala +++ b/test/forms/BankAccountDetailsFormSpec.scala @@ -45,15 +45,6 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.get mustBe validBankAccountDetails } - "successfully bind an account name containing all valid characters" in { - val formData = Map( - ACCOUNT_NAME -> "O'Brien-Smith J. A/B", - ACCOUNT_NUMBER -> validAccountNumber, - SORT_CODE -> validSortCode - ) - form.bind(formData).errors mustBe empty - } - "return a FormError when binding an empty account name to the form" in { val formData = Map( ACCOUNT_NAME -> "", @@ -219,7 +210,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.get mustBe BankAccountDetails(validAccountName, validAccountNumber, validSortCode) } - "return a FormError with the new missing account name error message when account name is empty" in { + "return a FormError when account name is empty" in { val formData = Map( ACCOUNT_NAME -> "", ACCOUNT_NUMBER -> validAccountNumber, @@ -247,7 +238,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNameMaxLengthKey } - "return a FormError with the new invalid account name error message when account name contains invalid characters" in { + "return a FormError when account name contains invalid characters" in { val invalidAccountName = "123#@~" val formData = Map( @@ -275,7 +266,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberEmptyKey } - "return a FormError with the length error message when account number is fewer than 6 digits" in { + "return a FormError when account number is fewer than 6 digits" in { val invalidAccountNumber = "12345" val formData = Map( @@ -290,7 +281,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberInvalidKey } - "return a FormError with the invalid characters error message when account number is 8 characters but contains letters" in { + "return a FormError when account number is 8 characters but contains letters" in { val invalidAccountNumber = "1234567A" val formData = Map( @@ -302,7 +293,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe ACCOUNT_NUMBER - boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberDigitErrorKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.accountNumberFormatKey } "return a FormError when sort code is empty" in { @@ -318,7 +309,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeEmptyKey } - "return a FormError with the length error message when sort code is fewer than 6 digits" in { + "return a FormError when sort code is fewer than 6 digits" in { val invalidSortCode = "12345" val formData = Map( @@ -330,10 +321,10 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeInvalidKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeLengthKey } - "return a FormError with the invalid characters error message when sort code is 6 characters but contains letters" in { + "return a FormError when sort code is 6 characters but contains letters" in { val invalidSortCode = "ABCDEF" val formData = Map( @@ -345,7 +336,7 @@ class BankAccountDetailsFormSpec extends PlaySpec { val boundForm = form.bind(formData) boundForm.errors.size mustBe 1 boundForm.errors.head.key mustBe SORT_CODE - boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeDigitErrorKey + boundForm.errors.head.message mustBe EnterBankAccountDetailsForm.sortCodeFormatKey } "successfully bind with a valid roll number" in { From 93a6a606d6ccc907f8be6febe417d8e11662ea4a Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Fri, 13 Mar 2026 07:34:03 +0000 Subject: [PATCH 10/11] feat: fix welsh labels --- conf/messages.cy | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/conf/messages.cy b/conf/messages.cy index fa170ba1a..dacc37e78 100644 --- a/conf/messages.cy +++ b/conf/messages.cy @@ -989,10 +989,13 @@ pages.chooseAccountType.option.business = Cyfrif busnes pages.chooseAccountType.option.personal = Cyfrif personol # Bank Details page -pages.bankDetails.heading = Beth yw manylion cyswllt y busnes? -pages.bankDetails.p1 = Dim ond i anfon ad-daliadau TAW y bydd tîm TAW CThEF yn defnyddio’r cyfrif hwn. Ni fydd arian yn cael ei gymryd o’r cyfrif rydych chi’n ei ddarparu. +pages.bankDetails.heading.old = Beth yw manylion cyfrif banc neu gymdeithas adeiladu’r busnes? +pages.bankDetails.heading.new = Beth yw manylion cyswllt y busnes? +pages.bankDetails.p1.old = Dim ond i anfon ad-daliadau TAW y bydd tîm TAW CThEM yn defnyddio’r cyfrif hwn. Ni fyddwn yn cymryd arian ohono. +pages.bankDetails.p1.new = Dim ond i anfon ad-daliadau TAW y bydd tîm TAW CThEF yn defnyddio’r cyfrif hwn. Ni fydd arian yn cael ei gymryd o’r cyfrif rydych chi’n ei ddarparu. pages.bankDetails.info = Mae’n rhaid i chi roi gwybod i ni os bydd manylion eich cyfrif yn newid. -pages.bankDetails.accountName.label = Yr enw sydd ar y cyfrif +pages.bankDetails.accountName.label.old = Enw’r cyfrif +pages.bankDetails.accountName.label.new = Yr enw sydd ar y cyfrif pages.bankDetails.accountName.hint = Nodwch yr enw fel y mae’n ymddangos ar gyfriflenni banc ac yn eich cyfrif TAW pages.bankDetails.accountNumber.label = Rhif y cyfrif pages.bankDetails.accountNumber.hint = Mae’n rhaid iddo fod rhwng 6 ac 8 digid o hyd From 8edda80c5cb027cf1df45c54f5f7c049131965fb Mon Sep 17 00:00:00 2001 From: mi-aspectratio <254715997+mi-aspectratio@users.noreply.github.com> Date: Fri, 13 Mar 2026 10:47:05 +0000 Subject: [PATCH 11/11] feat:fix sort code missing error --- app/forms/BankAccountDetailsForms.scala | 4 ++-- conf/messages | 3 ++- conf/messages.cy | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/forms/BankAccountDetailsForms.scala b/app/forms/BankAccountDetailsForms.scala index dfb1ea433..8dd7cb038 100644 --- a/app/forms/BankAccountDetailsForms.scala +++ b/app/forms/BankAccountDetailsForms.scala @@ -64,7 +64,7 @@ object EnterCompanyBankAccountDetailsForm { val accountNameInvalidKey = "validation.companyBankAccount.name.invalid.old" val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" - val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" + val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing.old" val sortCodeInvalidKey = "validation.companyBankAccount.sortCode.invalid" val invalidAccountReputationKey = "sortCodeAndAccountGroup" @@ -119,7 +119,7 @@ object EnterBankAccountDetailsForm { val accountNumberEmptyKey = "validation.companyBankAccount.number.missing" val accountNumberFormatKey = "validation.companyBankAccount.number.format" val accountNumberInvalidKey = "validation.companyBankAccount.number.invalid" - val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing" + val sortCodeEmptyKey = "validation.companyBankAccount.sortCode.missing.new" val sortCodeLengthKey = "validation.companyBankAccount.sortCode.length" val sortCodeFormatKey = "validation.companyBankAccount.sortCode.format" val rollNumberInvalidKey = "validation.companyBankAccount.rollNumber.invalid" diff --git a/conf/messages b/conf/messages index 8564b0b2a..42b962bfe 100644 --- a/conf/messages +++ b/conf/messages @@ -1012,7 +1012,8 @@ validation.companyBankAccount.name.invalid.new = Name on the account mu validation.companyBankAccount.number.missing = Enter the account number validation.companyBankAccount.number.invalid = Account number must be between 6 and 8 digits validation.companyBankAccount.number.format = Account number must only contain numbers -validation.companyBankAccount.sortCode.missing = Enter the account sort code +validation.companyBankAccount.sortCode.missing.old = Enter the account sort code +validation.companyBankAccount.sortCode.missing.new = Enter the sort code validation.companyBankAccount.sortCode.invalid = Enter a valid account sort code validation.companyBankAccount.sortCode.format = Sort code must only contain numbers validation.companyBankAccount.sortCode.length = Sort code must be 6 digits long diff --git a/conf/messages.cy b/conf/messages.cy index dacc37e78..7dfc2d624 100644 --- a/conf/messages.cy +++ b/conf/messages.cy @@ -1011,7 +1011,8 @@ validation.companyBankAccount.name.invalid.new = Enw sydd ar y cyfrif: mae validation.companyBankAccount.number.missing = Nodwch rif y cyfrif validation.companyBankAccount.number.invalid = Rhif y cyfrif: mae’n rhaid i hyn fod rhwng 6 ac 8 digid validation.companyBankAccount.number.format = Rhif y cyfrif: mae’n rhaid i hyn gynnwys rhifau yn unig -validation.companyBankAccount.sortCode.missing = Nodwch god didoli’r cyfrif +validation.companyBankAccount.sortCode.missing.old = Nodwch god didoli’r cyfrif +validation.companyBankAccount.sortCode.missing.new = Nodwch y cod didoli validation.companyBankAccount.sortCode.invalid = Nodwch god didoli dilys ar gyfer y cyfrif validation.companyBankAccount.sortCode.length = Cod didoli: mae’n rhaid i hyn fod yn 6 digid validation.companyBankAccount.sortCode.format = Cod didoli: mae’n rhaid i hyn gynnwys rhifau yn unig