[NGR-2496] - Create Navigator and User Answers #32

william-hatzar · 2025-09-02T23:49:53Z

No description provided.

platops-pr-bot · 2025-09-02T23:53:15Z

uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.11.0 in project/AppDependencies.scala.

platops-pr-bot · 2025-09-11T12:58:14Z

~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.11.0 in project/AppDependencies.scala.~~
uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.12.0 in project/AppDependencies.scala.
uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.10.0 in project/AppDependencies.scala.
Remove println(Console.MAGENTA + value + Console.RESET) from app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala

platops-pr-bot · 2025-09-18T08:52:46Z

~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.11.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.10.0 in project/AppDependencies.scala.~~
uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.12.0 in project/AppDependencies.scala.
uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.11.0 in project/AppDependencies.scala.
Remove println(Console.MAGENTA + value + Console.RESET) from app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala

platops-pr-bot · 2025-09-19T18:01:26Z

~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.11.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.10.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.12.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.11.0 in project/AppDependencies.scala.~~
uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.12.0 in project/AppDependencies.scala.
Remove println(Console.MAGENTA + value + Console.RESET) from app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala

platops-pr-bot · 2025-09-23T16:11:38Z

~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.11.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.10.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:centralised-authorisation-resource-client-play-30 can be updated to 1.12.0 in project/AppDependencies.scala.~~
~~uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.11.0 in project/AppDependencies.scala.~~
~~Remove println(Console.MAGENTA + value + Console.RESET) from app/uk/gov/hmrc/ngrraldfrontend/navigation/Navigator.scala~~
uk.gov.hmrc:play-frontend-hmrc-play-30 can be updated to 12.12.0 in project/AppDependencies.scala.

anna-shen-hmrc · 2025-09-24T13:05:18Z

...hmrc/ngrraldfrontend/controllers/HowManyParkingSpacesOrGaragesIncludedInRentController.scala

+              mode = mode
+            )))
        },
        rentAmount =>


I know it wasn't you. Can you please rename it?

anna-shen-hmrc · 2025-09-24T13:18:38Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/RentDatesAgreeController.scala

-  def show: Action[AnyContent] = {
-    (authenticate andThen hasLinkedProperties).async { implicit request =>
-      request.propertyLinking.map(property =>
+  //TODO Add in preparedForm


Does this still need?

anna-shen-hmrc · 2025-09-24T13:26:49Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/RentDatesAgreeController.scala

            (formError.key, formError.messages) match
-              case (key, messages) if messages.head.contains("rentDatesAgree.date") =>
-                setCorrectKey(formError, "rentDatesAgree", "date")
+              case (key, messages) if messages.contains("rentDatesAgree.date.month.required.error") =>


Can you please change to the same main branch? As setCorrectKey method is doing all what you are trying to do. It's common method that can be use for setting correct field to be highlighted.

Suggested change

case (key, messages) if messages.contains("rentDatesAgree.date.month.required.error") =>

case (key, messages) if messages.head.contains("rentDatesAgree.date") =>

setCorrectKey(formError, "rentDatesAgree", "date")

case _ =>

formError

anna-shen-hmrc · 2025-09-24T13:37:41Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/RentPeriodsController.scala

  extends FrontendController(mcc) with I18nSupport with CurrencyHelper {

-  def firstTable(userAnswers: RaldUserAnswers)(implicit messages: Messages): Table =
+  def firstTable(userAnswers: ProvideDetailsOfFirstSecondRentPeriod)(implicit messages: Messages): Table =


Can you please change userAnswers to something else? It's a bit confusing. Maybe rentPeriods?

anna-shen-hmrc · 2025-09-24T13:38:29Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/RentPeriodsController.scala

    )

-  def secondTable(userAnswers: RaldUserAnswers)(implicit messages: Messages): Table = Table(
+  def secondTable(userAnswers: ProvideDetailsOfFirstSecondRentPeriod)(implicit messages: Messages): Table = Table(


Maybe change userAnswers to something else.

anna-shen-hmrc · 2025-09-24T13:54:49Z

app/uk/gov/hmrc/ngrraldfrontend/models/NGRMonthYear.scala

+    val parts = dateString.split("-").map(_.toInt)
+    val year = parts(0).toString
+    val month = f"${parts(1)}%02d"
+    NGRMonthYear(month, year)


Since makeString has already padding the missing 0. Maybe you can just get the value and set back into NGRMonthYear. Line 29, Can you please change getOrElse to default 1 or throw Exception. There isn't 0 month. I know it wasn't you. Sorry.

Suggested change

NGRMonthYear(month, year)

NGRMonthYear(parts(1), parts(0))

anna-shen-hmrc · 2025-09-24T13:56:34Z

app/uk/gov/hmrc/ngrraldfrontend/models/forms/RentPeriodsForm.scala

-  private val yes: NGRRadioButtons = NGRRadioButtons(radioContent = "rentPeriods.yes", radioValue = No)
-  private val no: NGRRadioButtons = NGRRadioButtons(radioContent = "rentPeriods.no", radioValue = Yes)
+  private val yes: NGRRadioButtons = NGRRadioButtons(radioContent = "rentPeriods.yes", radioValue = Yes)
+  private val no: NGRRadioButtons = NGRRadioButtons(radioContent = "rentPeriods.no", radioValue = No)


Well spotted!

anna-shen-hmrc · 2025-09-24T14:01:25Z

app/uk/gov/hmrc/ngrraldfrontend/models/package.scala

+
+import play.api.libs.json.*
+
+package object models {


Is it correct that the file name is package.scala?

Yes, as discussed yesterday

anna-shen-hmrc · 2025-09-24T14:05:33Z

app/uk/gov/hmrc/ngrraldfrontend/repo/RaldRepo.scala

@@ -1,283 +0,0 @@
-/*


Can you please remove the empty file?

anna-shen-hmrc · 2025-09-24T14:15:38Z

app/uk/gov/hmrc/ngrraldfrontend/pages/TellUsAboutRentPage.scala

+package uk.gov.hmrc.ngrraldfrontend.pages
+
+import play.api.libs.json.JsPath
+import uk.gov.hmrc.ngrraldfrontend.models.{AgreementType, RaldUserAnswers}


Don't forget to remove RaldUserAnswers

JakeReid2020 · 2025-09-25T08:25:00Z

app/uk/gov/hmrc/ngrraldfrontend/config/AppConfig.scala

  val ngrLoginRegistrationHost: String
  val ngrDashboardUrl: String
  val ngrLogoutUrl: String
  val timeToLive: String


I don't think we need this timeToLive as it was used for RadlUserAnswers

JakeReid2020 · 2025-09-25T08:26:40Z

app/uk/gov/hmrc/ngrraldfrontend/config/AppConfig.scala

  override val features = new Features()(config)
  override val nextGenerationRatesHost: String = servicesConfig.baseUrl("next-generation-rates")
  override val ngrLoginRegistrationHost: String = servicesConfig.baseUrl("ngr-login-register-frontend")
  override val timeToLive: String = servicesConfig.getString("time-to-live.time")


Same for this, this was being used for RaldUserAnswers.

JakeReid2020 · 2025-09-25T08:28:53Z

conf/application.conf

 }

 time-to-live.time = "3"



time-to-live.time = "3" can be removed this was for RaldUserAnswers

JakeReid2020 · 2025-09-25T08:32:51Z

app/uk/gov/hmrc/ngrraldfrontend/connectors/NGRConnector.scala

      .map {
        case Some(propertyLinkingUserAnswers) => Some(propertyLinkingUserAnswers.vmvProperty)
-        case None => throw new NotFoundException("failed to find propertyLinkingUserAnswers from backend mongo")
+        case None => None


Just thinking if we deal with the exception here we don't have to do it anywhere else this is called. And maybe in the future we should show that page was talking about to not show kibana errors.

Its been changed to None, as it was firing the exception in this file and not hitting the data retrieval action to hit the exception there. I had to do this in order to get the tests to work if a property wasn't found

JakeReid2020 · 2025-09-25T08:33:27Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/AgreedRentChangeController.scala

+      val preparedForm = request.userAnswers.getOrElse(UserAnswers(request.credId)).get(AgreedRentChangePage) match {
+        case None => form
+        case Some(value) => form.fill(AgreedRentChangeForm(value))
+      }


Thanks for this man! big improvement!

JakeReid2020 · 2025-09-25T08:35:45Z

app/uk/gov/hmrc/ngrraldfrontend/controllers/AgreementController.scala

+        }, value.openEndedDate match {
+          case Some(value) => Some(NGRDate.fromString(value))
+          case None => None
+        }, if (value.haveBreakClause) {


I'm thinking storing things as boolean is making our code more annoying then good, we should raise this.

Yeah I think in the future we should have some kind of standard. A lot of the radio buttons are strings which are just simple Yes or No answers. So I think we should be storing them as booleans as it makes it easier to prepopulate the form rather than having to put in the name of the radio value. If you get me. For example what your rent includes page really should just be booleans for each value not the whole value that you have to pass back

william-hatzar force-pushed the NGR-2496 branch from 450ad84 to 8235a93 Compare September 19, 2025 17:57

william-hatzar added 12 commits September 23, 2025 08:43

Navigator WIP

32faf75

WIP

568d7c2

WIP

720a7be

Navigator complete working on check mode and normal mode

44c74f6

Got check mode working

b4df0aa

Tests in progress

437ecf0

tests WIP

91c6554

WIP

e33356e

Tests in progress

1a97940

Tests passing WIP

ace614b

Prepop data tests added

abde70c

Navigator tests and journey working

0dbd7c7

william-hatzar force-pushed the NGR-2496 branch from 84a17fb to 0dbd7c7 Compare September 23, 2025 16:08

Fixed tests from rebase

0c6f88b

william-hatzar marked this pull request as ready for review September 24, 2025 08:52

anna-shen-hmrc reviewed Sep 24, 2025

View reviewed changes

JakeReid2020 reviewed Sep 25, 2025

View reviewed changes

william-hatzar added 2 commits September 25, 2025 12:37

Responding to PR comments

efe7231

Addressing PR comments

5fdcd8f

JakeReid2020 approved these changes Sep 25, 2025

View reviewed changes

anna-shen-hmrc approved these changes Sep 25, 2025

View reviewed changes

william-hatzar merged commit 8e5cba8 into main Sep 25, 2025
1 check passed

william-hatzar deleted the NGR-2496 branch September 25, 2025 13:16

[NGR-2496] - Create Navigator and User Answers #32

[NGR-2496] - Create Navigator and User Answers #32

Uh oh!

Conversation

william-hatzar commented Sep 2, 2025

Uh oh!

platops-pr-bot commented Sep 2, 2025

Uh oh!

platops-pr-bot commented Sep 11, 2025

Uh oh!

platops-pr-bot commented Sep 18, 2025

Uh oh!

platops-pr-bot commented Sep 19, 2025

Uh oh!

platops-pr-bot commented Sep 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-hatzar Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

william-hatzar Sep 25, 2025 •

edited

Loading