From 70ab48bd8c1bcbd141d1614f4f4d14455cd8ac4e Mon Sep 17 00:00:00 2001 From: Carl Thompson Date: Fri, 24 May 2024 00:15:11 +0100 Subject: [PATCH 1/2] Navigation fixes and placeholder text resolved --- .../ContactHavePhoneController.scala | 20 +++++++++------- .../FirstContactEmailController.scala | 21 ++++++++++------- .../SecondContactEmailController.scala | 23 +++++++++++-------- app/navigation/Navigator.scala | 11 ++++----- .../ContactHavePhoneControllerSpec.scala | 20 ++++++++-------- .../FirstContactEmailControllerSpec.scala | 7 ++++-- .../SecondContactEmailControllerSpec.scala | 9 +++++--- test/navigation/NavigatorSpec.scala | 6 +++++ 8 files changed, 69 insertions(+), 48 deletions(-) diff --git a/app/controllers/addFinancialInstitution/ContactHavePhoneController.scala b/app/controllers/addFinancialInstitution/ContactHavePhoneController.scala index f7012106..b45390f3 100644 --- a/app/controllers/addFinancialInstitution/ContactHavePhoneController.scala +++ b/app/controllers/addFinancialInstitution/ContactHavePhoneController.scala @@ -21,6 +21,7 @@ import forms.addFinancialInstitution.ContactHavePhoneFormProvider import models.Mode import navigation.Navigator import pages.addFinancialInstitution.{ContactHavePhonePage, ContactNamePage} +import play.api.data.Form import play.api.i18n.{I18nSupport, MessagesApi} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import repositories.SessionRepository @@ -46,19 +47,20 @@ class ContactHavePhoneController @Inject() ( with I18nSupport with ContactHelper { - val form = formProvider() - val fi = "Placeholder Financial Institution" // todo: pull in this when available + val form: Form[Boolean] = formProvider() def onPageLoad(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData) { implicit request => - val ua = request.userAnswers - val preparedForm = request.userAnswers.get(ContactHavePhonePage) match { + val ua = request.userAnswers + val contactName = ua.get(ContactNamePage) + val fi = getFinancialInstitutionName(ua) + + val preparedForm = ua.get(ContactHavePhonePage) match { case None => form case Some(value) => form.fill(value) } - val contactName = ua.get(ContactNamePage) contactName match { - case None => Redirect(controllers.routes.IndexController.onPageLoad) + case None => Redirect(controllers.routes.IndexController.onPageLoad()) case Some(contactName) => Ok(view(preparedForm, mode, fi, contactName)) } @@ -66,13 +68,15 @@ class ContactHavePhoneController @Inject() ( def onSubmit(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData).async { implicit request => + val ua = request.userAnswers + val fi = getFinancialInstitutionName(ua) form .bindFromRequest() .fold( - formWithErrors => Future.successful(BadRequest(view(formWithErrors, mode, fi, getFirstContactName(request.userAnswers)))), + formWithErrors => Future.successful(BadRequest(view(formWithErrors, mode, fi, getFirstContactName(ua)))), value => for { - updatedAnswers <- Future.fromTry(request.userAnswers.set(ContactHavePhonePage, value)) + updatedAnswers <- Future.fromTry(ua.set(ContactHavePhonePage, value)) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(ContactHavePhonePage, mode, updatedAnswers)) ) diff --git a/app/controllers/addFinancialInstitution/FirstContactEmailController.scala b/app/controllers/addFinancialInstitution/FirstContactEmailController.scala index f544bce1..69b076e0 100644 --- a/app/controllers/addFinancialInstitution/FirstContactEmailController.scala +++ b/app/controllers/addFinancialInstitution/FirstContactEmailController.scala @@ -21,10 +21,12 @@ import forms.addFinancialInstitution.FirstContactEmailFormProvider import models.Mode import navigation.Navigator import pages.addFinancialInstitution.{ContactNamePage, FirstContactEmailPage} +import play.api.data.Form import play.api.i18n.{I18nSupport, MessagesApi} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import repositories.SessionRepository import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController +import utils.ContactHelper import views.html.addFinancialInstitution.FirstContactEmailView import javax.inject.Inject @@ -42,14 +44,15 @@ class FirstContactEmailController @Inject() ( view: FirstContactEmailView )(implicit ec: ExecutionContext) extends FrontendBaseController - with I18nSupport { + with I18nSupport + with ContactHelper { - val form = formProvider() - val fi = "Placeholder Financial Institution" // todo: pull in this when available + val form: Form[String] = formProvider() def onPageLoad(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData) { implicit request => val ua = request.userAnswers + val fi = getFinancialInstitutionName(ua) val preparedForm = ua.get(FirstContactEmailPage) match { case None => form @@ -57,17 +60,19 @@ class FirstContactEmailController @Inject() ( } val contactName = ua.get(ContactNamePage) contactName match { - case None => Redirect(controllers.routes.IndexController.onPageLoad) + case None => Redirect(controllers.routes.IndexController.onPageLoad()) case Some(name) => Ok(view(preparedForm, mode, fi, name)) } } def onSubmit(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData).async { implicit request => - request.userAnswers - .get(ContactNamePage) + val ua = request.userAnswers + val fi = getFinancialInstitutionName(ua) + + ua.get(ContactNamePage) .fold { - Future.successful(Redirect(controllers.routes.IndexController.onPageLoad)) + Future.successful(Redirect(controllers.routes.IndexController.onPageLoad())) } { name => form @@ -76,7 +81,7 @@ class FirstContactEmailController @Inject() ( formWithErrors => Future.successful(BadRequest(view(formWithErrors, mode, fi, name))), value => for { - updatedAnswers <- Future.fromTry(request.userAnswers.set(FirstContactEmailPage, value)) + updatedAnswers <- Future.fromTry(ua.set(FirstContactEmailPage, value)) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(FirstContactEmailPage, mode, updatedAnswers)) ) diff --git a/app/controllers/addFinancialInstitution/SecondContactEmailController.scala b/app/controllers/addFinancialInstitution/SecondContactEmailController.scala index 71d1680c..e3fa8776 100644 --- a/app/controllers/addFinancialInstitution/SecondContactEmailController.scala +++ b/app/controllers/addFinancialInstitution/SecondContactEmailController.scala @@ -21,10 +21,12 @@ import forms.addFinancialInstitution.SecondContactEmailFormProvider import models.Mode import navigation.Navigator import pages.addFinancialInstitution.{SecondContactEmailPage, SecondContactNamePage} +import play.api.data.Form import play.api.i18n.{I18nSupport, MessagesApi} import play.api.mvc.{Action, AnyContent, MessagesControllerComponents} import repositories.SessionRepository import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController +import utils.ContactHelper import views.html.addFinancialInstitution.SecondContactEmailView import javax.inject.Inject @@ -42,32 +44,35 @@ class SecondContactEmailController @Inject() ( view: SecondContactEmailView )(implicit ec: ExecutionContext) extends FrontendBaseController - with I18nSupport { + with I18nSupport + with ContactHelper { - val form = formProvider() - val fi = "Placeholder Financial Institution" // todo: pull in this when available + val form: Form[String] = formProvider() def onPageLoad(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData) { implicit request => val ua = request.userAnswers + val fi = getFinancialInstitutionName(ua) + val preparedForm = ua.get(SecondContactEmailPage) match { case None => form case Some(value) => form.fill(value) } - val secondContactName = ua.get(SecondContactNamePage) secondContactName match { - case None => Redirect(controllers.routes.IndexController.onPageLoad) + case None => Redirect(controllers.routes.IndexController.onPageLoad()) case Some(name) => Ok(view(preparedForm, mode, fi, name)) } } def onSubmit(mode: Mode): Action[AnyContent] = (identify andThen getData andThen requireData).async { implicit request => - request.userAnswers - .get(SecondContactNamePage) + val ua = request.userAnswers + val fi = getFinancialInstitutionName(ua) + + ua.get(SecondContactNamePage) .fold { - Future.successful(Redirect(controllers.routes.IndexController.onPageLoad)) + Future.successful(Redirect(controllers.routes.IndexController.onPageLoad())) } { name => form @@ -76,7 +81,7 @@ class SecondContactEmailController @Inject() ( formWithErrors => Future.successful(BadRequest(view(formWithErrors, mode, fi, name))), value => for { - updatedAnswers <- Future.fromTry(request.userAnswers.set(SecondContactEmailPage, value)) + updatedAnswers <- Future.fromTry(ua.set(SecondContactEmailPage, value)) _ <- sessionRepository.set(updatedAnswers) } yield Redirect(navigator.nextPage(SecondContactEmailPage, mode, updatedAnswers)) ) diff --git a/app/navigation/Navigator.scala b/app/navigation/Navigator.scala index 884c6502..234241a8 100644 --- a/app/navigation/Navigator.scala +++ b/app/navigation/Navigator.scala @@ -107,18 +107,15 @@ class Navigator @Inject() () { userAnswers, HaveGIINPage, routes.WhatIsGIINController.onPageLoad(NormalMode), - controllers.routes.IndexController.onPageLoad + routes.WhereIsFIBasedController.onPageLoad(NormalMode) ) case UkAddressPage => _ => routes.ContactNameController.onPageLoad(NormalMode) case NonUkAddressPage => _ => routes.ContactNameController.onPageLoad(NormalMode) case _ => - _ => controllers.routes.IndexController.onPageLoad + _ => controllers.routes.IndexController.onPageLoad() } - private val checkRouteMap: Page => UserAnswers => Call = { - case _ => - _ => routes.CheckYourAnswersController.onPageLoad - } + private val checkRouteMap: Page => UserAnswers => Call = _ => _ => routes.CheckYourAnswersController.onPageLoad private def addressLookupNavigation(mode: Mode)(ua: UserAnswers): Call = ua.get(AddressLookupPage) match { @@ -129,7 +126,7 @@ class Navigator @Inject() () { private def yesNoPage(ua: UserAnswers, fromPage: QuestionPage[Boolean], yesCall: => Call, noCall: => Call): Call = ua.get(fromPage) .map(if (_) yesCall else noCall) - .getOrElse(controllers.routes.JourneyRecoveryController.onPageLoad) + .getOrElse(controllers.routes.JourneyRecoveryController.onPageLoad()) def nextPage(page: Page, mode: Mode, userAnswers: UserAnswers): Call = mode match { case NormalMode => diff --git a/test/controllers/addFinancialInstitution/ContactHavePhoneControllerSpec.scala b/test/controllers/addFinancialInstitution/ContactHavePhoneControllerSpec.scala index f6ba1917..05dea70e 100644 --- a/test/controllers/addFinancialInstitution/ContactHavePhoneControllerSpec.scala +++ b/test/controllers/addFinancialInstitution/ContactHavePhoneControllerSpec.scala @@ -23,7 +23,7 @@ import navigation.{FakeNavigator, Navigator} import org.mockito.ArgumentMatchers.any import org.mockito.Mockito.when import org.scalatestplus.mockito.MockitoSugar -import pages.addFinancialInstitution.{ContactHavePhonePage, ContactNamePage} +import pages.addFinancialInstitution._ import play.api.inject.bind import play.api.mvc.Call import play.api.test.FakeRequest @@ -43,13 +43,17 @@ class ContactHavePhoneControllerSpec extends SpecBase with MockitoSugar { lazy val contactHavePhoneRoute = routes.ContactHavePhoneController.onPageLoad(NormalMode).url val contactName = "Mr Test" val financialInstitution = "Placeholder Financial Institution" - private val userAnswers = emptyUserAnswers.set(ContactNamePage, contactName).get + + private val ua = + emptyUserAnswers + .withPage(ContactNamePage, contactName) + .withPage(NameOfFinancialInstitutionPage, financialInstitution) "ContactHavePhone Controller" - { "must return OK and the correct view for a GET" in { - val application = applicationBuilder(userAnswers = Some(userAnswers)).build() + val application = applicationBuilder(userAnswers = Some(ua)).build() running(application) { val request = FakeRequest(GET, contactHavePhoneRoute) @@ -64,13 +68,7 @@ class ContactHavePhoneControllerSpec extends SpecBase with MockitoSugar { } "must populate the view correctly on a GET when the question has previously been answered" in { - val userAnswers = emptyUserAnswers - .set(ContactNamePage, contactName) - .success - .value - .set(ContactHavePhonePage, true) - .success - .value + val userAnswers = ua.withPage(ContactHavePhonePage, true) val application = applicationBuilder(userAnswers = Some(userAnswers)).build() @@ -114,7 +112,7 @@ class ContactHavePhoneControllerSpec extends SpecBase with MockitoSugar { "must return a Bad Request and errors when invalid data is submitted" in { - val application = applicationBuilder(userAnswers = Some(userAnswers)).build() + val application = applicationBuilder(userAnswers = Some(ua)).build() running(application) { val request = diff --git a/test/controllers/addFinancialInstitution/FirstContactEmailControllerSpec.scala b/test/controllers/addFinancialInstitution/FirstContactEmailControllerSpec.scala index b13c37b9..bc55d941 100644 --- a/test/controllers/addFinancialInstitution/FirstContactEmailControllerSpec.scala +++ b/test/controllers/addFinancialInstitution/FirstContactEmailControllerSpec.scala @@ -23,7 +23,7 @@ import navigation.{FakeNavigator, Navigator} import org.mockito.ArgumentMatchers.any import org.mockito.Mockito.when import org.scalatestplus.mockito.MockitoSugar -import pages.addFinancialInstitution.{ContactNamePage, FirstContactEmailPage} +import pages.addFinancialInstitution.{ContactNamePage, FirstContactEmailPage, NameOfFinancialInstitutionPage} import play.api.inject.bind import play.api.mvc.Call import play.api.test.FakeRequest @@ -44,7 +44,10 @@ class FirstContactEmailControllerSpec extends SpecBase with MockitoSugar { val contactName = "Mr Test" val financialInstitution = "Placeholder Financial Institution" - private val ua = emptyUserAnswers.set(ContactNamePage, contactName).get + + private val ua = emptyUserAnswers + .withPage(ContactNamePage, contactName) + .withPage(NameOfFinancialInstitutionPage, financialInstitution) "FirstContactEmail Controller" - { diff --git a/test/controllers/addFinancialInstitution/SecondContactEmailControllerSpec.scala b/test/controllers/addFinancialInstitution/SecondContactEmailControllerSpec.scala index 93c64766..11143bbf 100644 --- a/test/controllers/addFinancialInstitution/SecondContactEmailControllerSpec.scala +++ b/test/controllers/addFinancialInstitution/SecondContactEmailControllerSpec.scala @@ -23,7 +23,7 @@ import navigation.{FakeNavigator, Navigator} import org.mockito.ArgumentMatchers.any import org.mockito.Mockito.when import org.scalatestplus.mockito.MockitoSugar -import pages.addFinancialInstitution.{SecondContactEmailPage, SecondContactNamePage} +import pages.addFinancialInstitution._ import play.api.inject.bind import play.api.mvc.Call import play.api.test.FakeRequest @@ -41,9 +41,12 @@ class SecondContactEmailControllerSpec extends SpecBase with MockitoSugar { val form = formProvider() val contactName = "Mr Test" val financialInstitution = "Placeholder Financial Institution" - private val ua = emptyUserAnswers.set(SecondContactNamePage, contactName).get - lazy val secondContactEmailRoute = routes.SecondContactEmailController.onPageLoad(NormalMode).url + private val ua = emptyUserAnswers + .withPage(SecondContactNamePage, contactName) + .withPage(NameOfFinancialInstitutionPage, financialInstitution) + + private lazy val secondContactEmailRoute = routes.SecondContactEmailController.onPageLoad(NormalMode).url "SecondContactEmail Controller" - { diff --git a/test/navigation/NavigatorSpec.scala b/test/navigation/NavigatorSpec.scala index 71baf7ae..c7fae536 100644 --- a/test/navigation/NavigatorSpec.scala +++ b/test/navigation/NavigatorSpec.scala @@ -133,6 +133,12 @@ class NavigatorSpec extends SpecBase { routes.WhatIsGIINController.onPageLoad(NormalMode) } + "must go from HaveGIIN to WhatIsGIIN when user answer is no" in { + val userAnswers = emptyUserAnswers.withPage(HaveGIINPage, false) + navigator.nextPage(HaveGIINPage, NormalMode, userAnswers) mustBe + routes.WhereIsFIBasedController.onPageLoad(NormalMode) + } + "must go from WhatIsGIIN to WhereIsFIBased when user answers yes" in { val userAnswers = emptyUserAnswers.withPage(WhatIsGIINPage, "answer") navigator.nextPage(WhatIsGIINPage, NormalMode, userAnswers) mustBe From 747cfcf3666c5ab5b2cf06d1b05cd1053d340e03 Mon Sep 17 00:00:00 2001 From: Carl Thompson Date: Fri, 24 May 2024 11:24:34 +0100 Subject: [PATCH 2/2] test description corrected, prbot comments address, sbt update --- conf/application-json-logger.xml | 13 ------------- project/build.properties | 2 +- test/navigation/NavigatorSpec.scala | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 conf/application-json-logger.xml diff --git a/conf/application-json-logger.xml b/conf/application-json-logger.xml deleted file mode 100644 index 6628cd69..00000000 --- a/conf/application-json-logger.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - - - - - - diff --git a/project/build.properties b/project/build.properties index ea2722e7..864c396e 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1,2 +1,2 @@ -sbt.version=1.9.7 +sbt.version=1.9.9 hmrc-frontend-scaffold.version=0.30.0 diff --git a/test/navigation/NavigatorSpec.scala b/test/navigation/NavigatorSpec.scala index c7fae536..5fc89d8c 100644 --- a/test/navigation/NavigatorSpec.scala +++ b/test/navigation/NavigatorSpec.scala @@ -133,7 +133,7 @@ class NavigatorSpec extends SpecBase { routes.WhatIsGIINController.onPageLoad(NormalMode) } - "must go from HaveGIIN to WhatIsGIIN when user answer is no" in { + "must go from HaveGIIN to WhereIsFIBased when user answer is no" in { val userAnswers = emptyUserAnswers.withPage(HaveGIINPage, false) navigator.nextPage(HaveGIINPage, NormalMode, userAnswers) mustBe routes.WhereIsFIBasedController.onPageLoad(NormalMode)