GFORMS-3005 - Refactor SectionNumber to be more predictable (#2331)

app/uk/gov/hmrc/gform/binders/ValueClassBinder.scala

@@ -61,8 +61,11 @@ object ValueClassBinder {
   implicit val sectionTitle4GaBinder: PathBindable[SectionTitle4Ga] = valueClassBinder(_.value)
   implicit val sectionNumberBinder: PathBindable[SectionNumber] = new PathBindable[SectionNumber] {
     override def bind(key: String, value: String): Either[String, SectionNumber] = {
-      val sectionNumber: Try[SectionNumber] = SectionNumber.parse(value)
-      sectionNumber.map(_.asRight).getOrElse(s"No valid value in path $key: $value".asLeft)
+      val sectionNumber: Option[SectionNumber] = SectionNumber.parse(value)
+      sectionNumber
+        .map(_.asRight)
+        .getOrElse(SectionNumber.Legacy(value).asRight)
+      //.getOrElse(s"No valid value in path $key: $value".asLeft)
     }
     override def unbind(key: String, sectionNumber: SectionNumber): String = sectionNumber.value
   }
@@ -173,7 +176,7 @@ object ValueClassBinder {
         case "-" => Right(None)
         case a =>
           Try(value.toInt) match {
-            case Success(a)     => Right(Some(SectionNumber.Classic(a)))
+            case Success(a)     => Right(Some(SectionNumber.Classic.NormalPage(TemplateSectionIndex(a))))
             case Failure(error) => Left(("No valid value in url binding: " + value + ". Error: " + error))
           }
       }
@@ -191,7 +194,8 @@ object ValueClassBinder {
         SectionNumber
           .parse(value)
           .map(_.asRight)
-          .getOrElse(s"No valid value in path $key: $value".asLeft)
+          .getOrElse(SectionNumber.Legacy(value).asRight)
+      //.getOrElse(s"No valid value in path $key: $value".asLeft)
       }
 
     override def unbind(key: String, sectionNumber: SectionNumber): String =
@@ -232,21 +236,20 @@ object ValueClassBinder {
       private def toSectionNumber(key: String, value: String): Either[String, SectionNumber] =
         SectionNumber
           .parse(value)
-          .toOption
-          .fold[Either[String, SectionNumber]](s"No valid value in path $key: $value".asLeft)(sn => sn.asRight)
+          .fold[Either[String, SectionNumber]](SectionNumber.Legacy(value).asRight)(sn => sn.asRight)
+      //.fold[Either[String, SectionNumber]](s"No valid value in path $key: $value".asLeft)(sn => sn.asRight)
 
-      private val cyaPat = raw"cya([\d,]+)".r
       override def bind(key: String, params: Map[String, Seq[String]]): Option[Either[String, FastForward]] =
         params.get(key).flatMap(_.headOption).map {
           case FastForward.ffYes            => FastForward.Yes.asRight
           case FastForward.ffCYAFormSummary => FastForward.CYA(SectionOrSummary.FormSummary).asRight
           case FastForward.ffCYATaskSummary => FastForward.CYA(SectionOrSummary.TaskSummary).asRight
-          case cyaPat(from) =>
+          case value if value.startsWith("cya:") =>
             for {
-              sn2 <- toSectionNumber(key, from)
+              sn2 <- toSectionNumber(key, value.drop(4))
             } yield FastForward.CYA(SectionOrSummary.Section(sn2))
-          case value if value.startsWith("back") =>
-            toSectionNumber(key, value.replace("back", "")).map(FastForward.BackUntil)
+          case value if value.startsWith("back:") =>
+            toSectionNumber(key, value.replace("back:", "")).map(FastForward.BackUntil)
           case value => toSectionNumber(key, value).map(FastForward.StopAt)
         }
 

app/uk/gov/hmrc/gform/builder/BuilderController.scala

@@ -67,9 +67,6 @@ class BuilderController(
     final def apply(html: Html): Json = Json.fromString(html.toString)
   }
 
-  private val compatibilityVersion =
-    21 // This is managed manually. Increase it any time API used by builder extension is changed.
-
   // Returns section from raw json which correspond to runtime sectionNumber parameter.
   def originalSection(
     formTemplateId: FormTemplateId,
@@ -89,16 +86,17 @@ class BuilderController(
             val bracket = formModel.bracket(sectionNumber)
 
             bracket.fold { nonRepeatingPage =>
-              val pageModel = nonRepeatingPage.singleton
+              val pageModel = nonRepeatingPage.singleton.singleton
 
               pageModel.allFormComponentIds match {
                 case Nil => None
                 case head :: _ =>
                   val compName = head.baseComponentId
 
                   sectionNumber match {
-                    case SectionNumber.Classic(value) =>
+                    case SectionNumber.Classic.NormalPage(value) =>
                       originalNonRepeatingPageJson(json.hcursor, compName, Nil)
+
                     case SectionNumber.TaskList(coordinates, sn) =>
                       val acursor: ACursor = json.hcursor
                         .downField("sections")
@@ -113,11 +111,18 @@ class BuilderController(
                           List(DownArray, DownField("sections"))
 
                       originalNonRepeatingPageJson(acursor, compName, historySuffix)
+                    case _ => Option.empty[Json]
                   }
               }
             }(repeatingPage => Option.empty[Json]) { addToList =>
               sectionNumber match {
-                case SectionNumber.Classic(value) =>
+                case SectionNumber.Legacy(_) => Option.empty[Json]
+                case SectionNumber.Classic.NormalPage(_) | SectionNumber.Classic.RepeatedPage(_, _) =>
+                  Option.empty[Json]
+                case SectionNumber.Classic.AddToListPage.DefaultPage(_) |
+                    SectionNumber.Classic.AddToListPage.Page(_, _, _) |
+                    SectionNumber.Classic.AddToListPage.CyaPage(_, _) |
+                    SectionNumber.Classic.AddToListPage.RepeaterPage(_, _) =>
                   originalSectionInAddToList(json, addToList, formModel, sectionNumber, Nil)
                 case SectionNumber.TaskList(coordinates, sn) =>
                   json.hcursor
@@ -152,8 +157,7 @@ class BuilderController(
               _.deepMerge(
                 Json.obj(
                   "hiddenComponentIds" := hiddenComponentIds,
-                  "hiddenChoicesLookup" := hiddenChoiceIndexes(formModelOptics, sectionNumber),
-                  "version" := compatibilityVersion
+                  "hiddenChoicesLookup" := hiddenChoiceIndexes(formModelOptics, sectionNumber)
                 )
               )
             )
@@ -202,8 +206,7 @@ class BuilderController(
           json
             .map(json =>
               Json.obj(
-                "formTemplate" := json,
-                "version" := compatibilityVersion
+                "formTemplate" := json
               )
             )
             .fold(BadRequest(s"Form template not found for $formTemplateId"))(json => Ok(json))
@@ -1064,11 +1067,11 @@ class BuilderController(
         val bracket: Bracket[DataExpanded] = formModel.bracket(sectionNumber)
 
         bracket match {
-          case Bracket.NonRepeatingPage(_, _, _) =>
+          case Bracket.NonRepeatingPage(_, _) =>
             badRequest("Expected AddToList bracket, but got NonRepeatingPage").pure[Future]
           case Bracket.RepeatingPage(_, _) =>
             badRequest("Expected AddToList bracket, but got RepeatingPage").pure[Future]
-          case bracket @ Bracket.AddToList(_, _) =>
+          case bracket @ Bracket.AddToList(_, _, _) =>
             pageModel
               .fold(singleton => badRequest("Invalid page model. Expected Repeater got Singleton"))(checkYourAnswers =>
                 badRequest("Invalid page model. Expected Repeater got CheckYourAnswers")

app/uk/gov/hmrc/gform/controllers/Navigation.scala

@@ -17,7 +17,7 @@
 package uk.gov.hmrc.gform.controllers
 
 import cats.syntax.eq._
-import uk.gov.hmrc.gform.models.{ Bracket, Visibility }
+import uk.gov.hmrc.gform.models.Visibility
 import uk.gov.hmrc.gform.sharedmodel.formtemplate._
 import uk.gov.hmrc.gform.models.FormModel
 
@@ -28,23 +28,24 @@ trait Navigation {
   val availableSectionNumbers: List[SectionNumber] =
     formModel.availableSectionNumbers
 
-  val minSectionNumber: SectionNumber = availableSectionNumbers.min(Ordering.by((_: SectionNumber).numberValue))
+  val minSectionNumber: SectionNumber = availableSectionNumbers.min
 
-  val addToListBrackets: List[Bracket.AddToList[Visibility]] =
-    formModel.brackets.addToListBrackets
+  val addToListSectionNumbers: List[SectionNumber] = availableSectionNumbers.filter(_.isAddToList)
 
-  val addToListSectionNumbers: List[SectionNumber] =
-    addToListBrackets.flatMap(_.toPageModelWithNumber.toList).map(_._2)
-
-  val addToListRepeaterSectionNumbers: List[SectionNumber] =
-    addToListBrackets.flatMap(_.iterations.toList).map(_.repeater.sectionNumber)
+  val addToListRepeaterSectionNumbers: List[SectionNumber] = availableSectionNumbers.filter(_.isAddToListRepeaterPage)
 
   val addToListNonRepeaterSectionNumbers: List[SectionNumber] =
     addToListSectionNumbers.filterNot(addToListRepeaterSectionNumbers.toSet)
 
   val samePageRepeatersSectionNumbers: List[List[SectionNumber]] =
-    formModel.brackets.addToListBrackets
-      .map(_.iterations.toList.map(_.repeater.sectionNumber))
+    addToListRepeaterSectionNumbers
+      .groupBy(
+        _.fold(a => (Coordinates(TaskSectionNumber(0), TaskNumber(0)), a.sectionIndex))(a =>
+          (a.coordinates, a.sectionNumber.sectionIndex)
+        )
+      )
+      .values
+      .toList
 
   val filteredSectionNumbers: SectionNumber => List[SectionNumber] = sectionNumber => {
     val availableSn = availableSectionNumbers.filter(
@@ -70,32 +71,21 @@ case class Navigator(
   sectionNumber: SectionNumber,
   formModel: FormModel[Visibility]
 ) extends Navigation {
-  private val maxSectionNumber: SectionNumber =
-    availableSectionNumbers.max(Ordering.by((_: SectionNumber).numberValue))
 
   val previousSectionNumber: Option[SectionNumber] =
     filteredSectionNumbers(sectionNumber).findLast(_ < sectionNumber)
 
   val nextSectionNumber: SectionNumber = {
-    val sn = sectionNumber.increment
+    val sn = sectionNumber.increment(formModel)
     if (addToListSectionNumbers.contains(sectionNumber)) {
       sn
     } else {
-      addToListRepeaterSectionNumbers
+      val repeaters = addToListRepeaterSectionNumbers
         .filter(_.fold(_ => true)(taskList => taskList.coordinates === sn.toCoordinatesUnsafe))
         .find(_ >= sn)
-        .fold(sn) { nrsn =>
-          filteredSectionNumbers(nrsn).filter(_ < nrsn).find(_ >= sn).getOrElse(nrsn)
-        }
+      repeaters.fold(sn) { nrsn =>
+        filteredSectionNumbers(nrsn).filter(_ < nrsn).find(_ >= sn).getOrElse(nrsn)
+      }
     }
   }
-
-  require(
-    sectionNumber >= minSectionNumber,
-    s"section number is too low: $sectionNumber is not >= $minSectionNumber"
-  )
-  require(
-    sectionNumber <= maxSectionNumber,
-    s"section number is too big: $sectionNumber is not <= $maxSectionNumber"
-  )
 }

app/uk/gov/hmrc/gform/eval/AllPageModelExpressions.scala

@@ -16,18 +16,18 @@
 
 package uk.gov.hmrc.gform.eval
 
-import uk.gov.hmrc.gform.models.{ BracketPlain, PageMode, Repeater, Singleton }
+import uk.gov.hmrc.gform.models.{ Bracket, PageMode, Repeater, SingletonWithNumber }
 import uk.gov.hmrc.gform.sharedmodel.formtemplate.Expr
 
 /*
  * Extracts metadata for all expressions of a Page.
  * This doesn't include expressions in fields
  */
 object AllPageModelExpressions extends ExprExtractorHelpers {
-  def unapply[A <: PageMode](bracket: BracketPlain[A]): Option[List[ExprMetadata]] = {
+  def unapply[A <: PageMode](bracket: Bracket[A]): Option[List[ExprMetadata]] = {
 
-    def fromSingleton(singleton: Singleton[_]): List[Expr] = {
-      val page = singleton.page
+    def fromSingleton(singleton: SingletonWithNumber[_]): List[Expr] = {
+      val page = singleton.singleton.page
       val pageExprs = page.title.allInterpolations ++ fromOption(
         page.description,
         page.shortName,
@@ -51,13 +51,13 @@ object AllPageModelExpressions extends ExprExtractorHelpers {
         repeater.expandedSummaryName
       )
 
-    def fromNonRepeatingBracket(bracket: BracketPlain.NonRepeatingPage[A]): List[Expr] =
+    def fromNonRepeatingBracket(bracket: Bracket.NonRepeatingPage[A]): List[Expr] =
       fromSingleton(bracket.singleton)
 
-    def fromRepeatedBracket(bracket: BracketPlain.RepeatingPage[A]): List[Expr] =
+    def fromRepeatedBracket(bracket: Bracket.RepeatingPage[A]): List[Expr] =
       bracket.source.repeats :: bracket.singletons.toList.flatMap(fromSingleton)
 
-    def fromAddToListBracket(bracket: BracketPlain.AddToList[A]): List[Expr] =
+    def fromAddToListBracket(bracket: Bracket.AddToList[A]): List[Expr] =
       fromSmartStrings(
         bracket.source.summaryName,
         bracket.source.addAnotherQuestion.label,
@@ -66,7 +66,7 @@ object AllPageModelExpressions extends ExprExtractorHelpers {
         bracket.source.infoMessage,
         bracket.source.addAnotherQuestion.helpText
       ) ++ bracket.iterations.toList.flatMap { iteration =>
-        iteration.singletons.toList.flatMap(fromSingleton) ::: fromRepeater(iteration.repeater)
+        iteration.singletons.toList.flatMap(fromSingleton) ::: fromRepeater(iteration.repeater.repeater)
       }
 
     val pageExprs: List[Expr] = bracket.fold(fromNonRepeatingBracket)(fromRepeatedBracket)(fromAddToListBracket)

app/uk/gov/hmrc/gform/eval/AllPageModelExpressionsGetter.scala

@@ -16,21 +16,21 @@
 
 package uk.gov.hmrc.gform.eval
 
-import uk.gov.hmrc.gform.models.{ BracketPlain, DataExpanded, FormModel, PageMode, Singleton }
+import uk.gov.hmrc.gform.models.{ Bracket, DataExpanded, FormModel, PageMode, Singleton }
 import uk.gov.hmrc.gform.sharedmodel.formtemplate.{ BooleanExpr, Expr }
 import uk.gov.hmrc.gform.sharedmodel.formtemplate.CheckYourAnswersPage
 
 object AllPageModelExpressionsGetter extends ExprExtractorHelpers {
   /*
    * Returns list of every single expression in a bracket
    */
-  def allExprs[A <: PageMode](formModel: FormModel[DataExpanded])(bracketPlain: BracketPlain[A]): List[Expr] = {
+  def allExprs[A <: PageMode](formModel: FormModel[DataExpanded])(bracket: Bracket[A]): List[Expr] = {
     val bracketExprs =
-      bracketPlain match {
+      bracket match {
         case AllPageModelExpressions(exprMetadatas) => exprMetadatas.map(_.expr)
         case otherwise                              => List.empty[Expr]
       }
-    bracketExprs ++ formComponentsExprs(formModel)(bracketPlain)
+    bracketExprs ++ formComponentsExprs(formModel)(bracket)
 
   }
 
@@ -78,16 +78,17 @@ object AllPageModelExpressionsGetter extends ExprExtractorHelpers {
 
   private def formComponentsExprs[A <: PageMode](
     formModel: FormModel[DataExpanded]
-  )(bracketPlain: BracketPlain[A]): List[Expr] =
-    bracketPlain.fold { nonRepeatingPage =>
-      fromSingleton(formModel)(nonRepeatingPage.singleton)
+  )(bracket: Bracket[A]): List[Expr] =
+    bracket.fold { nonRepeatingPage =>
+      fromSingleton(formModel)(nonRepeatingPage.singleton.singleton)
     } { repeatingPage =>
-      repeatingPage.singletons.toList.flatMap(fromSingleton(formModel))
+      repeatingPage.singletons.map(_.singleton).toList.flatMap(fromSingleton(formModel))
     } { addToList =>
-      addToList.iterations.toList.flatMap { iteration =>
-        iteration.singletons.toList.flatMap(fromSingleton(formModel)) ++
-          addToList.source.cyaPage.map(fromCheckYourAnswerPage).getOrElse(Nil)
-      }
+      addToList.defaultPage.map(_.singleton).toList.flatMap(fromSingleton(formModel)) ++
+        addToList.iterations.toList.flatMap { iteration =>
+          iteration.singletons.map(_.singleton).toList.flatMap(fromSingleton(formModel)) ++
+            addToList.source.cyaPage.map(fromCheckYourAnswerPage).getOrElse(Nil)
+        }
     }
 
 }

app/uk/gov/hmrc/gform/eval/AllPageModelSums.scala

@@ -16,11 +16,11 @@
 
 package uk.gov.hmrc.gform.eval
 
-import uk.gov.hmrc.gform.models.{ BracketPlain, PageMode }
+import uk.gov.hmrc.gform.models.{ Bracket, PageMode }
 import uk.gov.hmrc.gform.sharedmodel.formtemplate.Sum
 
 object AllPageModelSums {
-  def unapply[A <: PageMode](bracket: BracketPlain[A]): Option[Set[Sum]] = bracket match {
+  def unapply[A <: PageMode](bracket: Bracket[A]): Option[Set[Sum]] = bracket match {
     case AllPageModelExpressions(exprMetadatas) => Some(exprMetadatas.flatMap(_.expr.sums).toSet).filter(_.nonEmpty)
     case _                                      => None
   }

app/uk/gov/hmrc/gform/eval/StandaloneSumInfo.scala

@@ -17,7 +17,7 @@
 package uk.gov.hmrc.gform.eval
 
 import cats.syntax.eq._
-import uk.gov.hmrc.gform.models.{ BracketPlainCoordinated, PageMode, TaskModelCoordinated }
+import uk.gov.hmrc.gform.models.{ BracketPlainCoordinated, PageMode, TaskModel }
 import uk.gov.hmrc.gform.sharedmodel.formtemplate.{ FormComponentId, FormCtx, Sum }
 
 /** This represents \${abc.sum} expression, where this expression
@@ -41,13 +41,13 @@ object StandaloneSumInfo {
 
     val allSums: List[Set[Sum]] =
       brackets.fold { classic =>
-        classic.bracketPlains.collect { case AllPageModelSums(sums) =>
+        classic.brackets.collect { case AllPageModelSums(sums) =>
           sums
         }
       } { taskList =>
-        taskList.bracketPlains
+        taskList.coordinatedBrackets
           .map(_._2)
-          .collect { case TaskModelCoordinated.Editable(brackets) => brackets.toList }
+          .collect { case TaskModel.Editable(brackets) => brackets.toList }
           .flatten
           .collect { case AllPageModelSums(sums) => sums }
       }

app/uk/gov/hmrc/gform/gform/DownloadController.scala

@@ -52,7 +52,7 @@ class DownloadController(
       OperationWithForm.DownloadFileByInternalLink
     ) { _ => _ => _ => _ => formModelOptics =>
       val formModel = formModelOptics.formModelRenderPageOptics.formModel
-      val allExprs = formModel.brackets.toBracketsPlains.toList.flatMap(_.allExprs(formModel))
+      val allExprs = formModel.brackets.toBrackets.toList.flatMap(_.allExprs(formModel))
       val extension = fileName.substring(fileName.lastIndexOf('.') + 1)
 
       if (!allExprs.contains(LinkCtx(InternalLink.Download(fileName)))) {

app/uk/gov/hmrc/gform/gform/FastForwardService.scala

@@ -160,7 +160,7 @@ class FastForwardService(
             )
         )
       case _ =>
-        val maybeCoordinates = maybeSectionNumber.flatMap(_.toCoordinates)
+        val maybeCoordinates = maybeSectionNumber.flatMap(_.maybeCoordinates)
         Redirect(
           routes.SummaryController
             .summaryById(cache.formTemplate._id, maybeAccessCode, maybeCoordinates, None, ff = fastForward.headOption)

app/uk/gov/hmrc/gform/gform/FormAddToListController.scala

@@ -87,7 +87,7 @@ class FormAddToListController(
             .confirmRemoval(formTemplateId, maybeAccessCode, sectionNumber, index, addToListId)
         val maybeBracket = formModel.bracket(sectionNumber)
         maybeBracket match {
-          case bracket @ Bracket.AddToList(iterations, source) =>
+          case bracket @ Bracket.AddToList(defaultPage, iterations, source) =>
             val (pageError, fieldErrors) = {
               val errorMessage =
                 source.errorMessage.fold(request.messages.messages("addToList.error.selectOption"))(error =>