From 0d8faf7c830975145a21bdf2591bcec8bacd07e5 Mon Sep 17 00:00:00 2001
From: PaulCDurham <23658502+PaulCDurham@users.noreply.github.com>
Date: Tue, 8 Oct 2024 15:15:38 +0100
Subject: [PATCH 1/4] Modify environment names

---
 .../application/AddCredentialController.scala |   6 +-
 .../EnvironmentAndCredentialsController.scala |   6 +-
 .../helpers/ApplicationApiBuilder.scala       |   4 +-
 .../application/ApplicationLenses.scala       |   5 +-
 app/models/application/EnvironmentName.scala  |   8 +-
 .../application/ApplicationApi.scala          |   7 +-
 .../EnvironmentAndCredentialsView.scala.html  |   6 +-
 environment-parity.md                         | 160 ++++++++++++++++++
 .../ApplicationsConnectorSpec.scala           |  16 +-
 .../AddCredentialControllerSpec.scala         |   6 +-
 ...ironmentAndCredentialsControllerSpec.scala |  10 +-
 ...equestProductionAccessControllerSpec.scala |   2 +-
 test/services/ApiHubServiceSpec.scala         |   8 +-
 .../application/ApplicationApiSpec.scala      |  14 +-
 14 files changed, 211 insertions(+), 47 deletions(-)
 create mode 100644 environment-parity.md

diff --git a/app/controllers/application/AddCredentialController.scala b/app/controllers/application/AddCredentialController.scala
index c9102940..3b2bce7d 100644
--- a/app/controllers/application/AddCredentialController.scala
+++ b/app/controllers/application/AddCredentialController.scala
@@ -19,7 +19,7 @@ package controllers.application
 import controllers.actions._
 import controllers.helpers.ErrorResultBuilder
 import forms.AddCredentialChecklistFormProvider
-import models.application.{Application, Credential, Primary, Secondary}
+import models.application.{Application, Credential, Production, Test}
 import models.exception.ApplicationCredentialLimitException
 import models.user.UserModel
 import play.api.i18n.{I18nSupport, Messages, MessagesApi}
@@ -59,7 +59,7 @@ class AddCredentialController @Inject()(
         formWithErrors =>
           Future.successful(BadRequest(view(formWithErrors, applicationId))),
         _ =>
-          apiHubService.addCredential(request.application.id, Primary) flatMap {
+          apiHubService.addCredential(request.application.id, Production) flatMap {
             case Right(Some(credential)) =>
               fetchApiNames(request.application).map(
                 apiNames =>
@@ -74,7 +74,7 @@ class AddCredentialController @Inject()(
 
   def addDevelopmentCredential(applicationId: String): Action[AnyContent] = (identify andThen applicationAuth(applicationId)).async {
     implicit request =>
-      apiHubService.addCredential(request.application.id, Secondary) flatMap {
+      apiHubService.addCredential(request.application.id, Test) flatMap {
         case Right(Some(_)) =>
           Future.successful(SeeOther(controllers.application.routes.EnvironmentAndCredentialsController.onPageLoad(request.application.id).url))
         case Right(None) => applicationNotFound(request.application)
diff --git a/app/controllers/application/EnvironmentAndCredentialsController.scala b/app/controllers/application/EnvironmentAndCredentialsController.scala
index 1ef37f66..be3c347a 100644
--- a/app/controllers/application/EnvironmentAndCredentialsController.scala
+++ b/app/controllers/application/EnvironmentAndCredentialsController.scala
@@ -20,7 +20,7 @@ import com.google.inject.Inject
 import config.FrontendAppConfig
 import controllers.actions.{ApplicationAuthActionProvider, IdentifierAction}
 import controllers.helpers.ErrorResultBuilder
-import models.application.{EnvironmentName, Primary, Secondary}
+import models.application.{EnvironmentName, Production, Test}
 import models.exception.ApplicationCredentialLimitException
 import models.user.Permissions
 import play.api.i18n.{I18nSupport, Messages}
@@ -51,7 +51,7 @@ class EnvironmentAndCredentialsController @Inject()(
       request.identifierRequest.user.permissions match {
         case Permissions(_, true, _) | Permissions(_, _, true) =>
           val url = s"${controllers.application.routes.EnvironmentAndCredentialsController.onPageLoad(id).url}#hip-production"
-          deleteCredential(id, clientId, Primary, url)
+          deleteCredential(id, clientId, Production, url)
         case _ =>
           Future.successful(Redirect(controllers.routes.UnauthorisedController.onPageLoad))
       }
@@ -60,7 +60,7 @@ class EnvironmentAndCredentialsController @Inject()(
   def deleteSecondaryCredential(id: String, clientId: String): Action[AnyContent] = (identify andThen applicationAuth(id, enrich = true)).async {
     implicit request =>
       val url = s"${controllers.application.routes.EnvironmentAndCredentialsController.onPageLoad(id).url}#hip-development"
-      deleteCredential(id, clientId, Secondary, url)
+      deleteCredential(id, clientId, Test, url)
   }
 
   private def deleteCredential(id: String, clientId: String, environmentName: EnvironmentName, url: String)(implicit request: Request[?]) = {
diff --git a/app/controllers/helpers/ApplicationApiBuilder.scala b/app/controllers/helpers/ApplicationApiBuilder.scala
index 36098ad2..c4f66ac9 100644
--- a/app/controllers/helpers/ApplicationApiBuilder.scala
+++ b/app/controllers/helpers/ApplicationApiBuilder.scala
@@ -54,8 +54,8 @@ class ApplicationApiBuilder @Inject()(
                     endpointMethod.summary,
                     endpointMethod.description,
                     endpointMethod.scopes,
-                    ApplicationEndpointAccess(application, hasPendingAccessRequest(apiDetail.id, pendingAccessRequests), endpointMethod, Primary),
-                    ApplicationEndpointAccess(application, hasPendingAccessRequest(apiDetail.id, pendingAccessRequests), endpointMethod, Secondary)
+                    ApplicationEndpointAccess(application, hasPendingAccessRequest(apiDetail.id, pendingAccessRequests), endpointMethod, Production),
+                    ApplicationEndpointAccess(application, hasPendingAccessRequest(apiDetail.id, pendingAccessRequests), endpointMethod, Test)
                   )
               )
         }
diff --git a/app/models/application/ApplicationLenses.scala b/app/models/application/ApplicationLenses.scala
index bf7a6f76..746d2875 100644
--- a/app/models/application/ApplicationLenses.scala
+++ b/app/models/application/ApplicationLenses.scala
@@ -149,8 +149,9 @@ object ApplicationLenses {
 
     def getCredentialsFor(environmentName: EnvironmentName): Seq[Credential] = {
       environmentName match {
-        case Primary => application.getPrimaryCredentials
-        case Secondary => application.getSecondaryCredentials
+        case Production => application.getPrimaryCredentials
+        case Test => application.getSecondaryCredentials
+        case _ => throw new IllegalArgumentException(s"Unsupported environment: $environmentName")  // TODO
       }
     }
 
diff --git a/app/models/application/EnvironmentName.scala b/app/models/application/EnvironmentName.scala
index f33ee726..1bfe5a91 100644
--- a/app/models/application/EnvironmentName.scala
+++ b/app/models/application/EnvironmentName.scala
@@ -20,12 +20,14 @@ import models.{Enumerable, WithName}
 
 sealed trait EnvironmentName
 
-case object Primary extends WithName("primary") with EnvironmentName
-case object Secondary extends WithName("secondary") with EnvironmentName
+case object Production extends WithName("primary") with EnvironmentName
+case object PreProduction extends WithName("pre-production") with EnvironmentName
+case object Test extends WithName("secondary") with EnvironmentName
+case object Development extends WithName("development") with EnvironmentName
 
 object EnvironmentName extends Enumerable.Implicits {
 
-  val values: Seq[EnvironmentName] = Seq(Primary, Secondary)
+  val values: Seq[EnvironmentName] = Seq(Production, PreProduction, Test, Development)
 
   implicit val enumerable: Enumerable[EnvironmentName] =
     Enumerable(values.map(value => value.toString -> value)*)
diff --git a/app/viewmodels/application/ApplicationApi.scala b/app/viewmodels/application/ApplicationApi.scala
index 2f396622..b7761196 100644
--- a/app/viewmodels/application/ApplicationApi.scala
+++ b/app/viewmodels/application/ApplicationApi.scala
@@ -18,7 +18,7 @@ package viewmodels.application
 
 import models.{Enumerable, WithName}
 import models.api.{ApiDetail, EndpointMethod}
-import models.application.{Api, Application, EnvironmentName, Primary, Secondary, SelectedEndpoint}
+import models.application.{Api, Application, EnvironmentName, Production, Test, SelectedEndpoint}
 import models.application.ApplicationLenses.ApplicationLensOps
 import play.api.libs.json.{Format, Json, Writes}
 
@@ -52,8 +52,9 @@ object ApplicationEndpointAccess extends Enumerable.Implicits{
   ): ApplicationEndpointAccess = {
 
     val scopes = environmentName match {
-      case Primary => application.getPrimaryScopes
-      case Secondary => application.getSecondaryScopes
+      case Production => application.getPrimaryScopes
+      case Test => application.getSecondaryScopes
+      case _ => throw new IllegalArgumentException(s"Unsupported environment: $environmentName")  // TODO
     }
 
     if (endpointMethod.scopes.toSet.subsetOf(scopes.map(_.name).toSet)) {
diff --git a/app/views/application/EnvironmentAndCredentialsView.scala.html b/app/views/application/EnvironmentAndCredentialsView.scala.html
index 8f3cc40f..15952d68 100644
--- a/app/views/application/EnvironmentAndCredentialsView.scala.html
+++ b/app/views/application/EnvironmentAndCredentialsView.scala.html
@@ -38,7 +38,7 @@
 @canDeleteCredentials(environmentName: EnvironmentName) = @{
     application.getCredentialsFor(environmentName).size > 1 &&
         (user.permissions.canSupport ||
-            (application.hasTeamMember(user) && (environmentName == Secondary || user.permissions.isPrivileged)))
+            (application.hasTeamMember(user) && (environmentName == Test || user.permissions.isPrivileged)))
 }
 
 @rolesGuidanceLink() = {
@@ -90,7 +90,7 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.test.heading")<
                 <strong>@messages("environmentAndCredentials.created"):</strong>
                 <span class="utcDateTime">@ViewUtils.formatLocalDateTimeContainingUtc(credential.created)</span>
             </p>
-            @if(canDeleteCredentials(Secondary)) {
+            @if(canDeleteCredentials(Test)) {
                 <div class="govuk-button-group govuk-!-margin-bottom-3">
                     <a class="govuk-link govuk-!-margin-bottom-0" href="@controllers.application.routes.EnvironmentAndCredentialsController.deleteSecondaryCredential(application.id, credential.clientId).url">
                         @messages("environmentAndCredentials.deleteCredential")
@@ -164,7 +164,7 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.production.head
                     <strong>@messages("environmentAndCredentials.created"):</strong>
                     <span class="utcDateTime">@ViewUtils.formatLocalDateTimeContainingUtc(credential.created)</span>
                 </p>
-                @if(canDeleteCredentials(Primary)) {
+                @if(canDeleteCredentials(Production)) {
                     <div class="govuk-button-group govuk-!-margin-bottom-3">
                         <a class="govuk-link govuk-!-margin-bottom-0" href="@controllers.application.routes.EnvironmentAndCredentialsController.deletePrimaryCredential(application.id, credential.clientId).url">
                             @messages("environmentAndCredentials.deleteCredential")
diff --git a/environment-parity.md b/environment-parity.md
new file mode 100644
index 00000000..ed616820
--- /dev/null
+++ b/environment-parity.md
@@ -0,0 +1,160 @@
+# Investigate how to support a configurable number of environments in The Hub
+[Jira ticket](https://jira.tools.tax.service.gov.uk/browse/HIPP-1577)
+
+## Environment names
+We are using these at the moment:
+* Primary
+* Secondary
+
+We need to stop that as it would get confusing. Simply use the environment names.
+
+New environment names:
+* Production
+* PreProduction
+* Test
+* Development
+
+Old to new environment name mapping:
+* Primary -> Production
+* Secondary -> Test
+
+## Data model
+Should we have fixed or variable structure?
+* Bad normalisation
+* Fixed means four environments everywhere
+
+Can we abstract code from structure via lenses?
+* Done in some places but maybe not all?
+
+We will at some point need a data migration for production to add environments.
+* Would this include credentials and scopes?
+
+## Configuration
+What does our configuration look like?
+* We can go simple and configure environments on or off
+  * More code (eg one tab per environment) but simpler 
+* We can go complicated and fully-data driven
+  * Less code (eg one tab repeated) but more complex
+
+This spike is going to use the first option. There will probably be some configuration other than simply on/off.
+
+This spike will have configuration for four environments with two turned off (as we're working locally). This will
+use the current environments of Production and Test.
+
+```
+Environments {
+  production {
+    on: true
+  },
+  preProduction {
+    on: false
+  },
+  test {
+    on: true
+  },
+  development {
+    on: false
+  },
+  deployTo: test
+}
+```
+
+## Live service
+This is a live service so how can we evolve to environment parity while maintaining a path to production?
+
+## Backend endpoints
+Analysis of backend endpoints called from the frontend and whether any change might be necessary.
+
+### registerApplication
+When an application is registered we create credentials in Test and Production. The Production credential is hidden from 
+the user.
+
+Should we add credentials for all "on" environments?
+
+### getApplication
+This call has the option to enrich with information from Test and Production IDMS.
+
+Should the backend fetch IDMS information for each "on" environment?
+* Note this may impact performance in production, especially if two calls have to go via eBridge 
+
+### deleteApplication
+When we delete an application we also call IDMS to delete credentials from Test and Production.
+* Deleting a credential implicitly removes scopes granted to it. 
+
+The backend should delete credentials in all "on" environments.
+
+### addApi
+When we add/remove endpoints for an API we grant scopes in Test and revoke scopes in both Test and Production. 
+
+Should we grant in all non-production "on" environments?
+
+Should we revoke in all "on" environments?
+
+### removeApi
+When we remove an API we revoke scopes from Test and Production.
+
+Should we revoke scopes from all "on" environments?
+
+### addCredential
+This specifies the environment to add the credential to.
+
+Make sure this works for all four environments.
+
+### deleteCredential
+This specifies the environment to delete the credential from.
+
+Make sure this works for all four environments.
+
+### approveAccessRequest
+This grants scopes in Production.
+
+Presumably no change if scopes are granted automatically in lower environments.
+
+### generateDeployment
+This creates a new API in Test.
+
+We will need some configuration telling us which environment to create APIs in.
+
+### updateDeployment
+This updates an API in Test.
+
+We will need some configuration telling us which environment to update APIs in.
+
+### promoteToProduction
+Promotes an API from Test to Production.
+
+We need the API lifecycle to know what changes to make. We will then probably need configuration to specify which 
+environments can promote and which environments they can promote to.
+
+### getApiDeploymentStatuses
+Gets an API's deployment status from Test and Production.
+
+Should this fetch the status in each "on" environment?
+
+### apisInProduction
+Calls APIM Production to get the list of APIs deployed there.
+
+Presumably no change.
+
+### No call to APIM
+The following endpoints do not result in any call to APIM and are presumably environment agnostic.
+* getApplications
+* getApplicationsUsingApi
+* getApplicationsByTeam
+* updateApplicationTeam
+* removeApplicationTeam
+* createAccessRequest
+* getAccessRequests
+* getAccessRequest
+* rejectAccessRequest
+* addTeamMember
+* findTeamById
+* findTeamByName
+* findTeams
+* createTeam
+* addTeamMemberToTeam
+* removeTeamMemberFromTeam
+* changeTeamName
+* getUserContactDetails
+* updateApiTeam
+* removeApiTeam
diff --git a/it/test/connectors/ApplicationsConnectorSpec.scala b/it/test/connectors/ApplicationsConnectorSpec.scala
index 010c7e1c..bbc84560 100644
--- a/it/test/connectors/ApplicationsConnectorSpec.scala
+++ b/it/test/connectors/ApplicationsConnectorSpec.scala
@@ -319,7 +319,7 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).addCredential(FakeApplication.id, Primary)(HeaderCarrier()) map {
+      buildConnector(this).addCredential(FakeApplication.id, Production)(HeaderCarrier()) map {
         actual =>
           actual mustBe Right(Some(expected))
       }
@@ -336,7 +336,7 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).addCredential(FakeApplication.id, Primary)(HeaderCarrier()) map {
+      buildConnector(this).addCredential(FakeApplication.id, Production)(HeaderCarrier()) map {
         actual =>
           actual mustBe Right(None)
       }
@@ -353,9 +353,9 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).addCredential(FakeApplication.id, Primary)(HeaderCarrier()) map {
+      buildConnector(this).addCredential(FakeApplication.id, Production)(HeaderCarrier()) map {
         actual =>
-          actual mustBe Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Primary))
+          actual mustBe Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Production))
       }
     }
   }
@@ -373,7 +373,7 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).deleteCredential(FakeApplication.id, Primary, clientId)(HeaderCarrier()) map {
+      buildConnector(this).deleteCredential(FakeApplication.id, Production, clientId)(HeaderCarrier()) map {
         actual =>
           actual mustBe Right(Some(()))
       }
@@ -390,7 +390,7 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).deleteCredential(FakeApplication.id, Primary, clientId)(HeaderCarrier()) map {
+      buildConnector(this).deleteCredential(FakeApplication.id, Production, clientId)(HeaderCarrier()) map {
         actual =>
           actual mustBe Right(None)
       }
@@ -407,9 +407,9 @@ class ApplicationsConnectorSpec
           )
       )
 
-      buildConnector(this).deleteCredential(FakeApplication.id, Primary, clientId)(HeaderCarrier()) map {
+      buildConnector(this).deleteCredential(FakeApplication.id, Production, clientId)(HeaderCarrier()) map {
         actual =>
-          actual mustBe Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Primary))
+          actual mustBe Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Production))
       }
     }
   }
diff --git a/test/controllers/application/AddCredentialControllerSpec.scala b/test/controllers/application/AddCredentialControllerSpec.scala
index bab1ad0b..78ba86f1 100644
--- a/test/controllers/application/AddCredentialControllerSpec.scala
+++ b/test/controllers/application/AddCredentialControllerSpec.scala
@@ -21,7 +21,7 @@ import controllers.actions.{FakeApplication, FakePrivilegedUser, FakeUser, FakeU
 import controllers.routes
 import forms.AddCredentialChecklistFormProvider
 import models.api.{ApiDetail, Live, Maintainer}
-import models.application.{Api, Application, Credential, Primary, Secondary}
+import models.application.{Api, Application, Credential, Production, Test}
 import models.exception.ApplicationCredentialLimitException
 import models.user.UserModel
 import org.mockito.ArgumentMatchers.{any, eq => eqTo}
@@ -112,7 +112,7 @@ class AddCredentialControllerSpec extends SpecBase with MockitoSugar with TestHe
           val fixture = buildFixture(user, Some(application))
           val credential = Credential("test-client-id", LocalDateTime.now(clock), Some("test-secret"), Some("test-fragment"))
 
-          when(fixture.apiHubService.addCredential(eqTo(application.id), eqTo(Primary))(any()))
+          when(fixture.apiHubService.addCredential(eqTo(application.id), eqTo(Production))(any()))
             .thenReturn(Future.successful(Right(Some(credential))))
 
           when(fixture.apiHubService.getApiDetail(eqTo(api1.id))(any()))
@@ -198,7 +198,7 @@ class AddCredentialControllerSpec extends SpecBase with MockitoSugar with TestHe
 
       val credential = Credential("test-client-id", LocalDateTime.now(clock), Some("test-secret"), Some("test-fragment"))
 
-      when(fixture.apiHubService.addCredential(eqTo(FakeApplication.id), eqTo(Secondary))(any()))
+      when(fixture.apiHubService.addCredential(eqTo(FakeApplication.id), eqTo(Test))(any()))
         .thenReturn(Future.successful(Right(Some(credential))))
 
       running(fixture.playApplication) {
diff --git a/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala b/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
index 69b422c9..bffc3365 100644
--- a/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
+++ b/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
@@ -20,7 +20,7 @@ import base.SpecBase
 import config.FrontendAppConfig
 import controllers.actions.{FakeApplication, FakePrivilegedUser, FakeUser, FakeUserNotTeamMember}
 import controllers.routes
-import models.application.{Primary, Secondary}
+import models.application.{Production, Test}
 import models.application.ApplicationLenses._
 import models.exception.ApplicationCredentialLimitException
 import models.user.UserModel
@@ -125,7 +125,7 @@ class EnvironmentAndCredentialsControllerSpec extends SpecBase with MockitoSugar
           redirectLocation(result) mustBe Some(expectedUrl)
           verify(fixture.apiHubService).deleteCredential(
             eqTo(application.id),
-            eqTo(Primary),
+            eqTo(Production),
             eqTo(clientId))(any()
           )
         }
@@ -191,7 +191,7 @@ class EnvironmentAndCredentialsControllerSpec extends SpecBase with MockitoSugar
         .thenReturn(Future.successful(Some(application)))
 
       when(fixture.apiHubService.deleteCredential(any(), any(), any())(any()))
-        .thenReturn(Future.successful(Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Primary))))
+        .thenReturn(Future.successful(Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Production))))
 
       running(fixture.playApplication) {
         val url = controllers.application.routes.EnvironmentAndCredentialsController.deletePrimaryCredential(application.id, clientId) .url
@@ -237,7 +237,7 @@ class EnvironmentAndCredentialsControllerSpec extends SpecBase with MockitoSugar
           redirectLocation(result) mustBe Some(expectedUrl)
           verify(fixture.apiHubService).deleteCredential(
             eqTo(application.id),
-            eqTo(Secondary),
+            eqTo(Test),
             eqTo(clientId))(any()
           )
         }
@@ -302,7 +302,7 @@ class EnvironmentAndCredentialsControllerSpec extends SpecBase with MockitoSugar
         .thenReturn(Future.successful(Some(application)))
 
       when(fixture.apiHubService.deleteCredential(any(), any(), any())(any()))
-        .thenReturn(Future.successful(Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Primary))))
+        .thenReturn(Future.successful(Left(ApplicationCredentialLimitException.forId(FakeApplication.id, Production))))
 
       running(fixture.playApplication) {
         val url = controllers.application.routes.EnvironmentAndCredentialsController.deleteSecondaryCredential(application.id, clientId) .url
diff --git a/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala b/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
index 4f4dc0e7..5dfa9719 100644
--- a/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
+++ b/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
@@ -174,7 +174,7 @@ class RequestProductionAccessControllerSpec extends SpecBase with MockitoSugar w
                 summary = method.summary,
                 description = method.description,
                 scopes = method.scopes,
-                primaryAccess = ApplicationEndpointAccess(application, false, method, Primary),
+                primaryAccess = ApplicationEndpointAccess(application, false, method, Production),
                 secondaryAccess = Inaccessible
               )
           )
diff --git a/test/services/ApiHubServiceSpec.scala b/test/services/ApiHubServiceSpec.scala
index 8b3a62b7..233c46b9 100644
--- a/test/services/ApiHubServiceSpec.scala
+++ b/test/services/ApiHubServiceSpec.scala
@@ -417,10 +417,10 @@ class ApiHubServiceSpec
       val fixture = buildFixture()
       val expected = Credential("test-client-id", LocalDateTime.now(), Some("test-secret"), Some("test-fragment"))
 
-      when(fixture.applicationsConnector.addCredential(eqTo(FakeApplication.id), eqTo(Primary))(any()))
+      when(fixture.applicationsConnector.addCredential(eqTo(FakeApplication.id), eqTo(Production))(any()))
         .thenReturn(Future.successful(Right(Some(expected))))
 
-      fixture.service.addCredential(FakeApplication.id, Primary)(HeaderCarrier()).map {
+      fixture.service.addCredential(FakeApplication.id, Production)(HeaderCarrier()).map {
         actual =>
           actual mustBe Right(Some(expected))
       }
@@ -435,11 +435,11 @@ class ApiHubServiceSpec
       when(fixture.applicationsConnector.deleteCredential(any(), any(), any())(any()))
         .thenReturn(Future.successful(Right(Some(()))))
 
-      fixture.service.deleteCredential(FakeApplication.id, Primary, clientId)(HeaderCarrier()).map {
+      fixture.service.deleteCredential(FakeApplication.id, Production, clientId)(HeaderCarrier()).map {
         actual =>
           verify(fixture.applicationsConnector).deleteCredential(
             eqTo(FakeApplication.id),
-            eqTo(Primary),
+            eqTo(Production),
             eqTo(clientId))(any()
           )
 
diff --git a/test/viewmodels/application/ApplicationApiSpec.scala b/test/viewmodels/application/ApplicationApiSpec.scala
index d8906a2a..71244a91 100644
--- a/test/viewmodels/application/ApplicationApiSpec.scala
+++ b/test/viewmodels/application/ApplicationApiSpec.scala
@@ -19,7 +19,7 @@ package viewmodels.application
 import controllers.actions.FakeApplication
 import models.api.{ApiDetail, Endpoint, EndpointMethod, Live, Maintainer}
 import models.application.ApplicationLenses.ApplicationLensOps
-import models.application.{Api, Primary, Scope, Secondary, SelectedEndpoint}
+import models.application.{Api, Production, Scope, Test, SelectedEndpoint}
 import org.scalatest.freespec.AnyFreeSpec
 import org.scalatest.matchers.must.Matchers
 import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks
@@ -34,7 +34,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Accessible when an application has the scopes required by the endpoint" in {
       val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2, testScope3))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope1, testScope3))
-      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Primary)
+      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
       actual mustBe Accessible
     }
@@ -42,7 +42,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Inaccessible when an application does not have the scopes required by the endpoint" in {
       val application = FakeApplication.setPrimaryScopes(scopes(testScope1))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
-      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Primary)
+      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
       actual mustBe Inaccessible
     }
@@ -50,7 +50,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Inaccessible when an application only has a subset of the scopes required by the endpoint" in {
       val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
-      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Primary)
+      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
       actual mustBe Inaccessible
     }
@@ -58,7 +58,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Inaccessible when an application has the scopes required but in the wrong environment" in {
       val application = FakeApplication.setPrimaryScopes(scopes("test-scope-1", "test-scope-2", "test-scope-3"))
       val endpointMethod = EndpointMethod("GET", None, None, Seq("test-scope-1", "test-scope-3"))
-      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Secondary)
+      val actual = ApplicationEndpointAccess(application, false, endpointMethod, Test)
 
       actual mustBe Inaccessible
     }
@@ -66,7 +66,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Requested for a primary endpoint without required scopes when there is a pending production access request" in {
       val application = FakeApplication.setPrimaryScopes(scopes(testScope1))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
-      val actual = ApplicationEndpointAccess(application, true, endpointMethod, Primary)
+      val actual = ApplicationEndpointAccess(application, true, endpointMethod, Production)
 
       actual mustBe Requested
     }
@@ -74,7 +74,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     "must return Accessible for a primary endpoint with required scopes when there is a pending production access request" in {
       val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2, testScope3))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope1, testScope3))
-      val actual = ApplicationEndpointAccess(application, true, endpointMethod, Primary)
+      val actual = ApplicationEndpointAccess(application, true, endpointMethod, Production)
 
       actual mustBe Accessible
     }

From 82f498cea2441900df44d37317adcc91dbbb203e Mon Sep 17 00:00:00 2001
From: PaulCDurham <23658502+PaulCDurham@users.noreply.github.com>
Date: Tue, 8 Oct 2024 17:27:15 +0100
Subject: [PATCH 2/4] Data model and lenses

---
 app/models/application/Application.scala      |   8 +-
 .../application/ApplicationLenses.scala       | 123 ++++++++--------
 app/models/application/Environments.scala     |   8 +-
 app/services/CurlCommandService.scala         |   2 +-
 .../application/ApplicationApi.scala          |   4 +-
 .../admin/ManageApplicationsView.scala.html   |   3 +-
 .../EnvironmentAndCredentialsView.scala.html  |  10 +-
 ...AnApiSelectApplicationControllerSpec.scala |   4 +-
 .../ApplicationApisControllerSpec.scala       |   2 +-
 .../ApplicationDetailsControllerSpec.scala    |   4 +-
 ...eSupportingInformationControllerSpec.scala |   2 +-
 ...equestProductionAccessControllerSpec.scala |   2 +-
 ...uctionAccessEndJourneyControllerSpec.scala |   2 +-
 ...tProductionAccessStartControllerSpec.scala |   2 +-
 .../helpers/ApplicationApiBuilderSpec.scala   |   4 +-
 .../application/ApplicationLensesSpec.scala   | 136 +++++++++---------
 .../application/ApplicationApiSpec.scala      |  12 +-
 17 files changed, 171 insertions(+), 157 deletions(-)

diff --git a/app/models/application/Application.scala b/app/models/application/Application.scala
index 298e35a7..2128996a 100644
--- a/app/models/application/Application.scala
+++ b/app/models/application/Application.scala
@@ -33,7 +33,13 @@ case class Application (
   issues: Seq[String] = Seq.empty,
   deleted: Option[Deleted] = None,
   teamName: Option[String] = None,
-)
+) {
+
+  def newEnvironments: Map[EnvironmentName, Environment] = {
+    environments.toNewEnvironments
+  }
+
+}
 
 object Application {
 
diff --git a/app/models/application/ApplicationLenses.scala b/app/models/application/ApplicationLenses.scala
index 746d2875..f0fe01fa 100644
--- a/app/models/application/ApplicationLenses.scala
+++ b/app/models/application/ApplicationLenses.scala
@@ -21,11 +21,18 @@ import models.user.UserModel
 
 object ApplicationLenses {
 
-  val applicationEnvironments: Lens[Application, Environments] =
-    Lens[Application, Environments](
-      get = _.environments,
-      set = (application, environments) => application.copy(environments = environments)
+  // TODO: replace these hacks with something better
+  private def setProductionEnvironment(application: Application, environment: Environment): Application = {
+    application.copy(
+      environments = application.environments.copy(primary = environment)
     )
+  }
+
+  private def setTestEnvironment(application: Application, environment: Environment): Application = {
+    application.copy(
+      environments = application.environments.copy(secondary = environment)
+    )
+  }
 
   val environmentScopes: Lens[Environment, Seq[Scope]] =
     Lens[Environment, Seq[Scope]](
@@ -39,35 +46,29 @@ object ApplicationLenses {
       set = (environment, credentials) => environment.copy(credentials = credentials)
     )
 
-  val environmentPrimary: Lens[Environments, Environment] =
-    Lens[Environments, Environment](
-      get = _.primary,
-      set = (environments, primary) => environments.copy(primary = primary)
+  val applicationProduction: Lens[Application, Environment] =
+    Lens[Application, Environment](
+      get = _.newEnvironments.getOrElse(Production, Environment()),
+      set = (application, environment) => setProductionEnvironment(application, environment)
     )
 
-  val applicationPrimary: Lens[Application, Environment] =
-    Lens.compose(applicationEnvironments, environmentPrimary)
-
-  val applicationPrimaryScopes: Lens[Application, Seq[Scope]] =
-    Lens.compose(applicationPrimary, environmentScopes)
+  val applicationProductionScopes: Lens[Application, Seq[Scope]] =
+    Lens.compose(applicationProduction, environmentScopes)
 
-  val applicationPrimaryCredentials: Lens[Application, Seq[Credential]] =
-    Lens.compose(applicationPrimary, environmentCredentials)
+  val applicationProductionCredentials: Lens[Application, Seq[Credential]] =
+    Lens.compose(applicationProduction, environmentCredentials)
 
-  val environmentSecondary: Lens[Environments, Environment] =
-    Lens[Environments, Environment](
-      get = _.secondary,
-      set = (environments, secondary) => environments.copy(secondary = secondary)
+  val applicationTest: Lens[Application, Environment] =
+    Lens[Application, Environment](
+      get = _.newEnvironments.getOrElse(Test, Environment()),
+      set = (application, environment) => setTestEnvironment(application, environment)
     )
 
-  val applicationSecondary: Lens[Application, Environment] =
-    Lens.compose(applicationEnvironments, environmentSecondary)
-
-  val applicationSecondaryScopes: Lens[Application, Seq[Scope]] =
-    Lens.compose(applicationSecondary, environmentScopes)
+  val applicationTestScopes: Lens[Application, Seq[Scope]] =
+    Lens.compose(applicationTest, environmentScopes)
 
-  val applicationSecondaryCredentials: Lens[Application, Seq[Credential]] =
-    Lens.compose(applicationSecondary, environmentCredentials)
+  val applicationTestCredentials: Lens[Application, Seq[Credential]] =
+    Lens.compose(applicationTest, environmentCredentials)
 
   val applicationTeamMembers: Lens[Application, Seq[TeamMember]] =
     Lens[Application, Seq[TeamMember]](
@@ -89,68 +90,68 @@ object ApplicationLenses {
 
   implicit class ApplicationLensOps(application: Application) {
 
-    def getPrimaryScopes: Seq[Scope] =
-      applicationPrimaryScopes.get(application)
+    def getProductionScopes: Seq[Scope] =
+      applicationProductionScopes.get(application)
 
-    def setPrimaryScopes(scopes: Seq[Scope]): Application =
-      applicationPrimaryScopes.set(application, scopes)
+    def setProductionScopes(scopes: Seq[Scope]): Application =
+      applicationProductionScopes.set(application, scopes)
 
-    def addPrimaryScope(scope: Scope): Application =
-      applicationPrimaryScopes.set(
+    def addProductionScope(scope: Scope): Application =
+      applicationProductionScopes.set(
         application,
-        applicationPrimaryScopes.get(application) :+ scope
+        applicationProductionScopes.get(application) :+ scope
       )
 
-    def getPrimaryMasterCredential: Option[Credential] =
-      applicationPrimaryCredentials.get(application)
+    def getProductionMasterCredential: Option[Credential] =
+      applicationProductionCredentials.get(application)
         .sortWith((a, b) => a.created.isAfter(b.created))
         .headOption
 
-    def getPrimaryCredentials: Seq[Credential] =
-      applicationPrimaryCredentials.get(application)
+    def getProductionCredentials: Seq[Credential] =
+      applicationProductionCredentials.get(application)
 
-    def setPrimaryCredentials(credentials: Seq[Credential]): Application =
-      applicationPrimaryCredentials.set(application, credentials)
+    def setProductionCredentials(credentials: Seq[Credential]): Application =
+      applicationProductionCredentials.set(application, credentials)
 
-    def addPrimaryCredential(credential: Credential): Application =
-      applicationPrimaryCredentials.set(
+    def addProductionCredential(credential: Credential): Application =
+      applicationProductionCredentials.set(
         application,
-        applicationPrimaryCredentials.get(application) :+ credential
+        applicationProductionCredentials.get(application) :+ credential
       )
 
-    def getSecondaryScopes: Seq[Scope] =
-      applicationSecondaryScopes.get(application)
+    def getTestScopes: Seq[Scope] =
+      applicationTestScopes.get(application)
 
-    def setSecondaryScopes(scopes: Seq[Scope]): Application =
-      applicationSecondaryScopes.set(application, scopes)
+    def setTestScopes(scopes: Seq[Scope]): Application =
+      applicationTestScopes.set(application, scopes)
 
-    def addSecondaryScope(scope: Scope): Application =
-      applicationSecondaryScopes.set(
+    def addTestScope(scope: Scope): Application =
+      applicationTestScopes.set(
         application,
-        applicationSecondaryScopes.get(application) :+ scope
+        applicationTestScopes.get(application) :+ scope
       )
 
-    def getSecondaryMasterCredential: Option[Credential] =
-      applicationSecondaryCredentials.get(application)
+    def getTestMasterCredential: Option[Credential] =
+      applicationTestCredentials.get(application)
         .sortWith((a, b) => a.created.isAfter(b.created))
         .headOption
 
-    def getSecondaryCredentials: Seq[Credential] =
-      applicationSecondaryCredentials.get(application)
+    def getTestCredentials: Seq[Credential] =
+      applicationTestCredentials.get(application)
 
-    def setSecondaryCredentials(credentials: Seq[Credential]): Application =
-      applicationSecondaryCredentials.set(application, credentials)
+    def setTestCredentials(credentials: Seq[Credential]): Application =
+      applicationTestCredentials.set(application, credentials)
 
-    def addSecondaryCredential(credential: Credential): Application =
-      applicationSecondaryCredentials.set(
+    def addTestCredential(credential: Credential): Application =
+      applicationTestCredentials.set(
         application,
-        applicationSecondaryCredentials.get(application) :+ credential
+        applicationTestCredentials.get(application) :+ credential
       )
 
     def getCredentialsFor(environmentName: EnvironmentName): Seq[Credential] = {
       environmentName match {
-        case Production => application.getPrimaryCredentials
-        case Test => application.getSecondaryCredentials
+        case Production => application.getProductionCredentials
+        case Test => application.getTestCredentials
         case _ => throw new IllegalArgumentException(s"Unsupported environment: $environmentName")  // TODO
       }
     }
@@ -210,7 +211,7 @@ object ApplicationLenses {
 
     def getRequiredScopeNames: Set[String] = {
       application
-        .getSecondaryScopes
+        .getTestScopes
         .map(_.name)
         .toSet
     }
diff --git a/app/models/application/Environments.scala b/app/models/application/Environments.scala
index 7fb2d559..7895daf7 100644
--- a/app/models/application/Environments.scala
+++ b/app/models/application/Environments.scala
@@ -21,7 +21,13 @@ import play.api.libs.json.{Format, Json}
 case class Environments(
   primary:Environment,
   secondary:Environment
-)
+) {
+
+  def toNewEnvironments: Map[EnvironmentName, Environment] = {
+    Map(Production -> primary, Test -> secondary)
+  }
+
+}
 
 object Environments {
 
diff --git a/app/services/CurlCommandService.scala b/app/services/CurlCommandService.scala
index 4830fc0d..bdfb2ddf 100644
--- a/app/services/CurlCommandService.scala
+++ b/app/services/CurlCommandService.scala
@@ -54,7 +54,7 @@ class CurlCommandService extends Logging {
 
   private def getCommonHeaders(application: Application): Map[String,String] = {
     val maybeAuthHeader = for {
-      credential <- application.getSecondaryMasterCredential
+      credential <- application.getTestMasterCredential
       secret <- credential.clientSecret
       credentials = s"${credential.clientId}:$secret"
       encodedCredentials = getEncoder.encodeToString(credentials.getBytes)
diff --git a/app/viewmodels/application/ApplicationApi.scala b/app/viewmodels/application/ApplicationApi.scala
index b7761196..d3c69ada 100644
--- a/app/viewmodels/application/ApplicationApi.scala
+++ b/app/viewmodels/application/ApplicationApi.scala
@@ -52,8 +52,8 @@ object ApplicationEndpointAccess extends Enumerable.Implicits{
   ): ApplicationEndpointAccess = {
 
     val scopes = environmentName match {
-      case Production => application.getPrimaryScopes
-      case Test => application.getSecondaryScopes
+      case Production => application.getProductionScopes
+      case Test => application.getTestScopes
       case _ => throw new IllegalArgumentException(s"Unsupported environment: $environmentName")  // TODO
     }
 
diff --git a/app/views/admin/ManageApplicationsView.scala.html b/app/views/admin/ManageApplicationsView.scala.html
index 151c126f..c50bef09 100644
--- a/app/views/admin/ManageApplicationsView.scala.html
+++ b/app/views/admin/ManageApplicationsView.scala.html
@@ -21,6 +21,7 @@
 @import views.html.components.SearchBox
 @import views.html.components.IconsLink
 @import models.application.Application
+@import models.application.ApplicationLenses._
 @import views.html.helper.CSPNonce
 @import views.html.components.Paginator
 
@@ -41,7 +42,7 @@
 }
 
 @clientIds(application: Application) = @{
-    Seq(application.environments.primary, application.environments.secondary).flatMap(_.credentials.map(_.clientId)).mkString(",")
+    Seq(application.getProductionCredentials, application.getTestCredentials).flatten.map(_.clientId).mkString(",")
 }
 
 @layout(
diff --git a/app/views/application/EnvironmentAndCredentialsView.scala.html b/app/views/application/EnvironmentAndCredentialsView.scala.html
index 15952d68..8f0a176b 100644
--- a/app/views/application/EnvironmentAndCredentialsView.scala.html
+++ b/app/views/application/EnvironmentAndCredentialsView.scala.html
@@ -56,7 +56,7 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.test.heading")<
                     Button(
                         classes = "govuk-button--secondary",
                         content = Text(messages("environmentAndCredentials.addNewCredential"))
-                    ).disabled(application.getSecondaryCredentials.size >= 5).withId("addTestCredentialButton")
+                    ).disabled(application.getTestCredentials.size >= 5).withId("addTestCredentialButton")
                 )
             }
         </div>
@@ -76,7 +76,7 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.test.heading")<
         </div>
     }
 
-    @for(credential <- application.getSecondaryCredentials) {
+    @for(credential <- application.getTestCredentials) {
         <div class="api-panel api-panel--small">
             <p data-secondary-credential-client-id="@credential.clientId" class="govuk-body govuk-!-margin-bottom-3">
                 <strong>@messages("environmentAndCredentials.clientId"):</strong>
@@ -108,7 +108,7 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.production.head
         </div>
         <div class="right govuk-!-text-align-right">
             @if(user.permissions.isPrivileged) {
-                @if(application.getPrimaryCredentials.size < 5) {
+                @if(application.getProductionCredentials.size < 5) {
                     @govukButton(
                         Button(
                             classes = "govuk-button--secondary",
@@ -145,12 +145,12 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.production.head
         </div>
     }
 
-    @if(application.getPrimaryCredentials.isEmpty) {
+    @if(application.getProductionCredentials.isEmpty) {
         <p class="govuk-body">
             @messages("environmentAndCredentials.noCredentials")
         </p>
     } else {
-        @for(credential <- application.getPrimaryCredentials) {
+        @for(credential <- application.getProductionCredentials) {
             <div class="api-panel api-panel--small">
                 <p class="govuk-body govuk-!-margin-bottom-3">
                     <strong>@messages("environmentAndCredentials.clientId"):</strong>
diff --git a/test/controllers/AddAnApiSelectApplicationControllerSpec.scala b/test/controllers/AddAnApiSelectApplicationControllerSpec.scala
index de203494..1ab688ed 100644
--- a/test/controllers/AddAnApiSelectApplicationControllerSpec.scala
+++ b/test/controllers/AddAnApiSelectApplicationControllerSpec.scala
@@ -360,14 +360,14 @@ class AddAnApiSelectApplicationControllerSpec extends SpecBase with MockitoSugar
   private def buildApplicationWithoutAccess(): Application = {
     FakeApplication
       .copy(id = "test-application-id-2", name = "test-application-name-2")
-      .setSecondaryScopes(Seq.empty)
+      .setTestScopes(Seq.empty)
   }
 
   private def buildApplicationsWithoutAccess(count: Int): Seq[Application] = {
     Seq.tabulate(count) { i =>
       FakeApplication
         .copy(id = s"test-application-id-$i", name = s"test-application-name-$i")
-        .setSecondaryScopes(Seq.empty)
+        .setTestScopes(Seq.empty)
     }
   }
 
diff --git a/test/controllers/application/ApplicationApisControllerSpec.scala b/test/controllers/application/ApplicationApisControllerSpec.scala
index a5415861..73c6cd40 100644
--- a/test/controllers/application/ApplicationApisControllerSpec.scala
+++ b/test/controllers/application/ApplicationApisControllerSpec.scala
@@ -86,7 +86,7 @@ class ApplicationApisControllerSpec extends SpecBase with MockitoSugar with Test
 
       val application = FakeApplication
         .addApi(Api(apiDetail.id, apiDetail.title, Seq(SelectedEndpoint("GET", "/test"))))
-        .setSecondaryScopes(Seq(Scope("test-scope")))
+        .setTestScopes(Seq(Scope("test-scope")))
 
       val applicationApis = Seq(
         ApplicationApi(apiDetail, Seq(ApplicationEndpoint("GET", "/test", None, None, Seq("test-scope"), Inaccessible, Accessible)), false)
diff --git a/test/controllers/application/ApplicationDetailsControllerSpec.scala b/test/controllers/application/ApplicationDetailsControllerSpec.scala
index 5235a8a4..ef532fdf 100644
--- a/test/controllers/application/ApplicationDetailsControllerSpec.scala
+++ b/test/controllers/application/ApplicationDetailsControllerSpec.scala
@@ -154,7 +154,7 @@ class ApplicationDetailsControllerSpec extends SpecBase with MockitoSugar with T
 
       val application = FakeApplication
         .addApi(Api(apiDetail.id, apiDetail.title, Seq(SelectedEndpoint("GET", "/test"))))
-        .setSecondaryScopes(Seq(Scope("test-scope")))
+        .setTestScopes(Seq(Scope("test-scope")))
 
       val applicationApis = Seq(
         ApplicationApi(apiDetail, Seq(ApplicationEndpoint("GET", "/test", None, None, Seq("test-scope"), Inaccessible, Accessible)), false)
@@ -189,7 +189,7 @@ class ApplicationDetailsControllerSpec extends SpecBase with MockitoSugar with T
 
       val application = FakeApplication
         .addApi(api)
-        .setSecondaryScopes(Seq(Scope("test-scope")))
+        .setTestScopes(Seq(Scope("test-scope")))
 
       val applicationApis = Seq(
         ApplicationApi(api, false)
diff --git a/test/controllers/application/accessrequest/ProvideSupportingInformationControllerSpec.scala b/test/controllers/application/accessrequest/ProvideSupportingInformationControllerSpec.scala
index 9d265404..1c52ec57 100644
--- a/test/controllers/application/accessrequest/ProvideSupportingInformationControllerSpec.scala
+++ b/test/controllers/application/accessrequest/ProvideSupportingInformationControllerSpec.scala
@@ -150,7 +150,7 @@ class ProvideSupportingInformationControllerSpec extends SpecBase with MockitoSu
 
     val application = FakeApplication
       .addApi(Api(apiDetail.id, apiDetail.title, Seq(SelectedEndpoint("GET", "/test"))))
-      .setSecondaryScopes(Seq(Scope("test-scope")))
+      .setTestScopes(Seq(Scope("test-scope")))
 
     application
   }
diff --git a/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala b/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
index 5dfa9719..fe265e04 100644
--- a/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
+++ b/test/controllers/application/accessrequest/RequestProductionAccessControllerSpec.scala
@@ -137,7 +137,7 @@ class RequestProductionAccessControllerSpec extends SpecBase with MockitoSugar w
 
     val application = FakeApplication
       .addApi(Api(apiDetail.id, apiDetail.title, Seq(SelectedEndpoint("GET", "/test"))))
-      .setSecondaryScopes(Seq(Scope("test-scope")))
+      .setTestScopes(Seq(Scope("test-scope")))
 
     application
   }
diff --git a/test/controllers/application/accessrequest/RequestProductionAccessEndJourneyControllerSpec.scala b/test/controllers/application/accessrequest/RequestProductionAccessEndJourneyControllerSpec.scala
index dd51db1c..dc0669b3 100644
--- a/test/controllers/application/accessrequest/RequestProductionAccessEndJourneyControllerSpec.scala
+++ b/test/controllers/application/accessrequest/RequestProductionAccessEndJourneyControllerSpec.scala
@@ -258,7 +258,7 @@ object RequestProductionAccessEndJourneyControllerSpec extends OptionValues{
   private val anApplication =
     FakeApplication
       .addApi(Api(applicationApi.apiId, applicationApi.apiTitle, Seq(SelectedEndpoint("GET", "/test"), SelectedEndpoint("POST", "/anothertest"))))
-      .setSecondaryScopes(Seq(Scope("test-scope")))
+      .setTestScopes(Seq(Scope("test-scope")))
 
   private def buildApplicationApi(apiId: Int, endpointAccesses: Seq[ApplicationEndpointAccess]): ApplicationApi = {
     ApplicationApi(
diff --git a/test/controllers/application/accessrequest/RequestProductionAccessStartControllerSpec.scala b/test/controllers/application/accessrequest/RequestProductionAccessStartControllerSpec.scala
index ec09a55e..32e89acc 100644
--- a/test/controllers/application/accessrequest/RequestProductionAccessStartControllerSpec.scala
+++ b/test/controllers/application/accessrequest/RequestProductionAccessStartControllerSpec.scala
@@ -161,7 +161,7 @@ object RequestProductionAccessStartControllerSpec {
   private val application =
     FakeApplication
       .addApi(Api(apiDetail.id, apiDetail.title, Seq(SelectedEndpoint("GET", "/test"))))
-      .setSecondaryScopes(Seq(Scope("test-scope")))
+      .setTestScopes(Seq(Scope("test-scope")))
 
   private val applicationApi = ApplicationApi(
     apiDetail = apiDetail,
diff --git a/test/controllers/helpers/ApplicationApiBuilderSpec.scala b/test/controllers/helpers/ApplicationApiBuilderSpec.scala
index 68452e41..4df0c70b 100644
--- a/test/controllers/helpers/ApplicationApiBuilderSpec.scala
+++ b/test/controllers/helpers/ApplicationApiBuilderSpec.scala
@@ -47,8 +47,8 @@ class ApplicationApiBuilderSpec extends SpecBase with MockitoSugar {
         .addApi(Api(apiId1, apiTitle1, Seq(SelectedEndpoint("GET", "/test1/1"), SelectedEndpoint("POST", "/test1/1"), SelectedEndpoint("GET", "/test1/2"))))
         .addApi(Api(apiId2, apiTitle2, Seq(SelectedEndpoint("GET", "/test2/1"))))
         .addApi(Api(apiId3, apiTitle3, Seq(SelectedEndpoint("GET", "/test3/1"))))
-        .setPrimaryScopes(scopes("all:test-scope-1", "get:test-scope-1-1", "get:test-scope-1-2"))
-        .setSecondaryScopes(scopes("all:test-scope-1", "get:test-scope-1-1", "post:test-scope-1-1", "get:test-scope-1-2", "get:test-scope-3-1"))
+        .setProductionScopes(scopes("all:test-scope-1", "get:test-scope-1-1", "get:test-scope-1-2"))
+        .setTestScopes(scopes("all:test-scope-1", "get:test-scope-1-1", "post:test-scope-1-1", "get:test-scope-1-2", "get:test-scope-3-1"))
 
       when(fixture.apiHubService.getApiDetail(eqTo(apiId1))(any()))
         .thenReturn(Future.successful(Some(apiDetail1)))
diff --git a/test/models/application/ApplicationLensesSpec.scala b/test/models/application/ApplicationLensesSpec.scala
index f15fa088..381b7d55 100644
--- a/test/models/application/ApplicationLensesSpec.scala
+++ b/test/models/application/ApplicationLensesSpec.scala
@@ -27,22 +27,22 @@ import scala.util.Random
 
 class ApplicationLensesSpec extends LensBehaviours {
 
-  "applicationEnvironments" - {
-    "must get the correct Environments" in {
-      val expected = randomEnvironments()
-      val application = testApplication.copy(environments = expected)
-
-      val actual = applicationEnvironments.get(application)
-      actual mustBe expected
-    }
-
-    "must set the Environments correctly" in {
-      val expected = randomEnvironments()
-      val application = applicationEnvironments.set(testApplication, expected)
-
-      application.environments mustBe expected
-    }
-  }
+//  "applicationEnvironments" - {
+//    "must get the correct Environments" in {
+//      val expected = randomEnvironments()
+//      val application = testApplication.copy(environments = expected)
+//
+//      val actual = applicationEnvironments.get(application)
+//      actual mustBe expected
+//    }
+//
+//    "must set the Environments correctly" in {
+//      val expected = randomEnvironments()
+//      val application = applicationEnvironments.set(testApplication, expected)
+//
+//      application.environments mustBe expected
+//    }
+//  }
 
   "environmentScopes" - {
     "must get the correct Scopes" in {
@@ -80,19 +80,19 @@ class ApplicationLensesSpec extends LensBehaviours {
     }
   }
 
-  "environmentPrimary" - {
-    "must" - {
-      behave like environmentsToEnvironmentLens(
-        environmentPrimary,
-        _.primary
-      )
-    }
-  }
+//  "environmentPrimary" - {
+//    "must" - {
+//      behave like environmentsToEnvironmentLens(
+//        environmentProduction,
+//        _.primary
+//      )
+//    }
+//  }
 
   "applicationPrimary" - {
     "must" - {
       behave like applicationToEnvironmentLens(
-        applicationPrimary,
+        applicationProduction,
         _.primary
       )
     }
@@ -101,7 +101,7 @@ class ApplicationLensesSpec extends LensBehaviours {
   "applicationPrimaryScopes" - {
     "must" - {
       behave like applicationToScopesLens(
-        applicationPrimaryScopes,
+        applicationProductionScopes,
         _.primary
       )
     }
@@ -110,25 +110,25 @@ class ApplicationLensesSpec extends LensBehaviours {
   "applicationPrimaryCredentials" - {
     "must" - {
       behave like applicationToCredentialsLens(
-        applicationPrimaryCredentials,
+        applicationProductionCredentials,
         _.primary
       )
     }
   }
 
-  "environmentSecondary" - {
-    "must" - {
-      behave like environmentsToEnvironmentLens(
-        environmentSecondary,
-        _.secondary
-      )
-    }
-  }
+//  "environmentSecondary" - {
+//    "must" - {
+//      behave like environmentsToEnvironmentLens(
+//        environmentTest,
+//        _.secondary
+//      )
+//    }
+//  }
 
   "applicationSecondary" - {
     "must" - {
       behave like applicationToEnvironmentLens(
-        applicationSecondary,
+        applicationTest,
         _.secondary
       )
     }
@@ -137,7 +137,7 @@ class ApplicationLensesSpec extends LensBehaviours {
   "applicationSecondaryScopes" - {
     "must" - {
       behave like applicationToScopesLens(
-        applicationSecondaryScopes,
+        applicationTestScopes,
         _.secondary
       )
     }
@@ -146,7 +146,7 @@ class ApplicationLensesSpec extends LensBehaviours {
   "applicationSecondaryCredentials" - {
     "must" - {
       behave like applicationToCredentialsLens(
-        applicationSecondaryCredentials,
+        applicationTestCredentials,
         _.secondary
       )
     }
@@ -186,8 +186,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "getPrimaryScopes" - {
       "must" - {
         behave like applicationScopesGetterFunction(
-          applicationPrimaryScopes,
-          application => ApplicationLensOps(application).getPrimaryScopes
+          applicationProductionScopes,
+          application => ApplicationLensOps(application).getProductionScopes
         )
       }
     }
@@ -195,8 +195,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "setPrimaryScopes" - {
       "must" - {
         behave like applicationScopesSetterFunction(
-          applicationPrimaryScopes,
-          (application, scopes) => ApplicationLensOps(application).setPrimaryScopes(scopes)
+          applicationProductionScopes,
+          (application, scopes) => ApplicationLensOps(application).setProductionScopes(scopes)
         )
       }
     }
@@ -204,8 +204,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "addPrimaryScope" - {
       "must" - {
         behave like applicationAddScopeFunction(
-          applicationPrimaryScopes,
-          (application, scope) => ApplicationLensOps(application).addPrimaryScope(scope)
+          applicationProductionScopes,
+          (application, scope) => ApplicationLensOps(application).addProductionScope(scope)
         )
       }
     }
@@ -217,17 +217,17 @@ class ApplicationLensesSpec extends LensBehaviours {
         val credential2 = randomCredential().copy(created = LocalDateTime.now().minusDays(2))
 
         val application = testApplication
-          .setPrimaryCredentials(Seq(credential1, master, credential2))
+          .setProductionCredentials(Seq(credential1, master, credential2))
 
-        application.getPrimaryMasterCredential mustBe Some(master)
+        application.getProductionMasterCredential mustBe Some(master)
       }
     }
 
     "getPrimaryCredentials" - {
       "must" - {
         behave like applicationCredentialsGetterFunction(
-          applicationPrimaryCredentials,
-          application => ApplicationLensOps(application).getPrimaryCredentials
+          applicationProductionCredentials,
+          application => ApplicationLensOps(application).getProductionCredentials
         )
       }
     }
@@ -235,8 +235,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "setPrimaryCredentials" - {
       "must" - {
         behave like applicationCredentialsSetterFunction(
-          applicationPrimaryCredentials,
-          (application, credentials) => ApplicationLensOps(application).setPrimaryCredentials(credentials)
+          applicationProductionCredentials,
+          (application, credentials) => ApplicationLensOps(application).setProductionCredentials(credentials)
         )
       }
     }
@@ -244,8 +244,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "addPrimaryCredential" - {
       "must" - {
         behave like applicationAddCredentialFunction(
-          applicationPrimaryCredentials,
-          (application, credential) => ApplicationLensOps(application).addPrimaryCredential(credential)
+          applicationProductionCredentials,
+          (application, credential) => ApplicationLensOps(application).addProductionCredential(credential)
         )
       }
     }
@@ -253,8 +253,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "getSecondaryScopes" - {
       "must" - {
         behave like applicationScopesGetterFunction(
-          applicationSecondaryScopes,
-          application => ApplicationLensOps(application).getSecondaryScopes
+          applicationTestScopes,
+          application => ApplicationLensOps(application).getTestScopes
         )
       }
     }
@@ -262,8 +262,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "setSecondaryScopes" - {
       "must" - {
         behave like applicationScopesSetterFunction(
-          applicationSecondaryScopes,
-          (application, scopes) => ApplicationLensOps(application).setSecondaryScopes(scopes)
+          applicationTestScopes,
+          (application, scopes) => ApplicationLensOps(application).setTestScopes(scopes)
         )
       }
     }
@@ -271,8 +271,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "addSecondaryScope" - {
       "must" - {
         behave like applicationAddScopeFunction(
-          applicationSecondaryScopes,
-          (application, scope) => ApplicationLensOps(application).addSecondaryScope(scope)
+          applicationTestScopes,
+          (application, scope) => ApplicationLensOps(application).addTestScope(scope)
         )
       }
     }
@@ -284,17 +284,17 @@ class ApplicationLensesSpec extends LensBehaviours {
         val credential2 = randomCredential().copy(created = LocalDateTime.now().minusDays(2))
 
         val application = testApplication
-          .setSecondaryCredentials(Seq(credential1, master, credential2))
+          .setTestCredentials(Seq(credential1, master, credential2))
 
-        application.getSecondaryMasterCredential mustBe Some(master)
+        application.getTestMasterCredential mustBe Some(master)
       }
     }
 
     "getSecondaryCredentials" - {
       "must" - {
         behave like applicationCredentialsGetterFunction(
-          applicationSecondaryCredentials,
-          application => ApplicationLensOps(application).getSecondaryCredentials
+          applicationTestCredentials,
+          application => ApplicationLensOps(application).getTestCredentials
         )
       }
     }
@@ -302,8 +302,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "setSecondaryCredentials" - {
       "must" - {
         behave like applicationCredentialsSetterFunction(
-          applicationSecondaryCredentials,
-          (application, credentials) => ApplicationLensOps(application).setSecondaryCredentials(credentials)
+          applicationTestCredentials,
+          (application, credentials) => ApplicationLensOps(application).setTestCredentials(credentials)
         )
       }
     }
@@ -311,8 +311,8 @@ class ApplicationLensesSpec extends LensBehaviours {
     "addSecondaryCredential" - {
       "must" - {
         behave like applicationAddCredentialFunction(
-          applicationSecondaryCredentials,
-          (application, credential) => ApplicationLensOps(application).addSecondaryCredential(credential)
+          applicationTestCredentials,
+          (application, credential) => ApplicationLensOps(application).addTestCredential(credential)
         )
       }
     }
@@ -364,14 +364,14 @@ class ApplicationLensesSpec extends LensBehaviours {
           Scope("test-scope-2")
         )
 
-        val application = testApplication.setSecondaryScopes(scopes)
+        val application = testApplication.setTestScopes(scopes)
 
         val actual = application.getRequiredScopeNames
         actual must contain theSameElementsAs Set("test-scope-1", "test-scope-2")
       }
 
       "must return an empty set when there are no secondary scopes" in {
-        val application = testApplication.setSecondaryScopes(Seq.empty)
+        val application = testApplication.setTestScopes(Seq.empty)
 
         val actual = application.getRequiredScopeNames
         actual mustBe empty
diff --git a/test/viewmodels/application/ApplicationApiSpec.scala b/test/viewmodels/application/ApplicationApiSpec.scala
index 71244a91..54fd7e48 100644
--- a/test/viewmodels/application/ApplicationApiSpec.scala
+++ b/test/viewmodels/application/ApplicationApiSpec.scala
@@ -32,7 +32,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
 
   "ApplicationEndpointAccess" - {
     "must return Accessible when an application has the scopes required by the endpoint" in {
-      val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2, testScope3))
+      val application = FakeApplication.setProductionScopes(scopes(testScope1, testScope2, testScope3))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope1, testScope3))
       val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
@@ -40,7 +40,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     }
 
     "must return Inaccessible when an application does not have the scopes required by the endpoint" in {
-      val application = FakeApplication.setPrimaryScopes(scopes(testScope1))
+      val application = FakeApplication.setProductionScopes(scopes(testScope1))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
       val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
@@ -48,7 +48,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     }
 
     "must return Inaccessible when an application only has a subset of the scopes required by the endpoint" in {
-      val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2))
+      val application = FakeApplication.setProductionScopes(scopes(testScope1, testScope2))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
       val actual = ApplicationEndpointAccess(application, false, endpointMethod, Production)
 
@@ -56,7 +56,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     }
 
     "must return Inaccessible when an application has the scopes required but in the wrong environment" in {
-      val application = FakeApplication.setPrimaryScopes(scopes("test-scope-1", "test-scope-2", "test-scope-3"))
+      val application = FakeApplication.setProductionScopes(scopes("test-scope-1", "test-scope-2", "test-scope-3"))
       val endpointMethod = EndpointMethod("GET", None, None, Seq("test-scope-1", "test-scope-3"))
       val actual = ApplicationEndpointAccess(application, false, endpointMethod, Test)
 
@@ -64,7 +64,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     }
 
     "must return Requested for a primary endpoint without required scopes when there is a pending production access request" in {
-      val application = FakeApplication.setPrimaryScopes(scopes(testScope1))
+      val application = FakeApplication.setProductionScopes(scopes(testScope1))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope2, testScope3))
       val actual = ApplicationEndpointAccess(application, true, endpointMethod, Production)
 
@@ -72,7 +72,7 @@ class ApplicationApiSpec extends AnyFreeSpec with Matchers with ScalaCheckProper
     }
 
     "must return Accessible for a primary endpoint with required scopes when there is a pending production access request" in {
-      val application = FakeApplication.setPrimaryScopes(scopes(testScope1, testScope2, testScope3))
+      val application = FakeApplication.setProductionScopes(scopes(testScope1, testScope2, testScope3))
       val endpointMethod = EndpointMethod("GET", None, None, Seq(testScope1, testScope3))
       val actual = ApplicationEndpointAccess(application, true, endpointMethod, Production)
 

From e410bd164fd6071a94bdc25c8678766488702870 Mon Sep 17 00:00:00 2001
From: PaulCDurham <23658502+PaulCDurham@users.noreply.github.com>
Date: Wed, 9 Oct 2024 17:01:57 +0100
Subject: [PATCH 3/4] Environment config

---
 app/config/Environments.scala                 |  65 +++++
 app/config/Module.scala                       |   3 +-
 .../EnvironmentAndCredentialsController.scala |   9 +-
 .../EnvironmentAndCredentialsView.scala.html  |  61 ++++-
 environment-parity.md                         | 222 +++++++++++++-----
 ...ironmentAndCredentialsControllerSpec.scala |  13 +-
 6 files changed, 291 insertions(+), 82 deletions(-)
 create mode 100644 app/config/Environments.scala

diff --git a/app/config/Environments.scala b/app/config/Environments.scala
new file mode 100644
index 00000000..9af43dfb
--- /dev/null
+++ b/app/config/Environments.scala
@@ -0,0 +1,65 @@
+/*
+ * 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 config
+
+import com.google.inject.{Inject, Singleton}
+import models.application.{Development, EnvironmentName, PreProduction, Production, Test}
+import play.api.Logging
+import play.api.libs.json.{Format, Json}
+
+case class EnvironmentConfig(environmentName: EnvironmentName, on: Boolean)
+
+object EnvironmentConfig {
+
+  implicit val formatEnvironmentConfig: Format[EnvironmentConfig] = Json.format[EnvironmentConfig]
+
+}
+
+case class EnvironmentsConfig(
+  production: EnvironmentConfig,
+  preProduction: EnvironmentConfig,
+  test: EnvironmentConfig,
+  development: EnvironmentConfig
+)
+
+object EnvironmentsConfig {
+
+  implicit val formatEnvironmentsConfig: Format[EnvironmentsConfig] = Json.format[EnvironmentsConfig]
+
+}
+
+trait Environments {
+
+  def config: EnvironmentsConfig
+
+}
+
+@Singleton
+class EnvironmentsImpl @Inject() extends Environments with Logging {
+
+  private val environmentsConfig = EnvironmentsConfig(
+    production = EnvironmentConfig(Production, on = true),
+    preProduction = EnvironmentConfig(PreProduction, on = true),
+    test = EnvironmentConfig(Test, on = true),
+    development = EnvironmentConfig(Development, on = true)
+  )
+
+  logger.info(s"Environment configuration${System.lineSeparator()}${Json.prettyPrint(Json.toJson(config))}")
+
+  override def config: EnvironmentsConfig = environmentsConfig
+
+}
diff --git a/app/config/Module.scala b/app/config/Module.scala
index ce88c807..1c4458f0 100644
--- a/app/config/Module.scala
+++ b/app/config/Module.scala
@@ -47,7 +47,8 @@ class Module extends play.api.inject.Module {
       bindz[Domains].to(classOf[DomainsImpl]).eagerly(),
       bindz[Hods].to(classOf[HodsImpl]).eagerly(),
       bindz[Platforms].to(classOf[PlatformsImpl]).eagerly(),
-      bindz[EmailDomains].to(classOf[EmailDomainsImpl]).eagerly()
+      bindz[EmailDomains].to(classOf[EmailDomainsImpl]).eagerly(),
+      bindz[Environments].to(classOf[EnvironmentsImpl]).eagerly()
     )
 
     val authTokenInitialiserBindings: Seq[Binding[?]] = if (configuration.get[Boolean]("create-internal-auth-token-on-start")) {
diff --git a/app/controllers/application/EnvironmentAndCredentialsController.scala b/app/controllers/application/EnvironmentAndCredentialsController.scala
index be3c347a..46442895 100644
--- a/app/controllers/application/EnvironmentAndCredentialsController.scala
+++ b/app/controllers/application/EnvironmentAndCredentialsController.scala
@@ -17,14 +17,14 @@
 package controllers.application
 
 import com.google.inject.Inject
-import config.FrontendAppConfig
+import config.{Environments, FrontendAppConfig}
 import controllers.actions.{ApplicationAuthActionProvider, IdentifierAction}
 import controllers.helpers.ErrorResultBuilder
 import models.application.{EnvironmentName, Production, Test}
 import models.exception.ApplicationCredentialLimitException
 import models.user.Permissions
 import play.api.i18n.{I18nSupport, Messages}
-import play.api.mvc._
+import play.api.mvc.*
 import services.ApiHubService
 import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendBaseController
 import views.html.application.EnvironmentAndCredentialsView
@@ -38,12 +38,13 @@ class EnvironmentAndCredentialsController @Inject()(
   view: EnvironmentAndCredentialsView,
   apiHubService: ApiHubService,
   errorResultBuilder: ErrorResultBuilder,
-  config: FrontendAppConfig
+  config: FrontendAppConfig,
+  environments: Environments
 )(implicit ec: ExecutionContext) extends FrontendBaseController with I18nSupport {
 
   def onPageLoad(id: String): Action[AnyContent] = (identify andThen applicationAuth(id, enrich = true)) {
     implicit request =>
-      Ok(view(request.application, request.identifierRequest.user, config.helpDocsPath))
+      Ok(view(request.application, request.identifierRequest.user, config.helpDocsPath, environments))
   }
 
   def deletePrimaryCredential(id: String, clientId: String): Action[AnyContent] = (identify andThen applicationAuth(id, enrich = true)).async {
diff --git a/app/views/application/EnvironmentAndCredentialsView.scala.html b/app/views/application/EnvironmentAndCredentialsView.scala.html
index 8f0a176b..f86d3524 100644
--- a/app/views/application/EnvironmentAndCredentialsView.scala.html
+++ b/app/views/application/EnvironmentAndCredentialsView.scala.html
@@ -14,6 +14,7 @@
  * limitations under the License.
  *@
 
+@import config.Environments
 @import models.application._
 @import models.application.ApplicationLenses._
 @import models.user.UserModel
@@ -29,7 +30,7 @@
     iconsLink: IconsLink
 )
 
-@(application: Application, user: UserModel, apiHubGuideUrl: String)(implicit request: Request[?], messages: Messages)
+@(application: Application, user: UserModel, apiHubGuideUrl: String, environments: Environments)(implicit request: Request[?], messages: Messages)
 
 @sortCredentials(credentials: Seq[Credential]) = @{
     credentials.sortWith((a, b) => a.created.isBefore(b.created))
@@ -45,7 +46,7 @@
     <a class="govuk-link" href="@apiHubGuideUrl/documentation/what-access-do-i-need.html#what-access-do-i-need">@messages("environmentAndCredentials.notPrivileged.additional3")</a>
 }
 
-@developmentTab() = {
+@testTab() = {
     <div class="environment-block-header">
         <div class="left">
             <h2 class="govuk-heading-m">@messages("environmentAndCredentials.test.heading")</h2>
@@ -176,6 +177,26 @@ <h2 class="govuk-heading-m">@messages("environmentAndCredentials.production.head
     }
 }
 
+@preProductionTab() = {
+    @if(environments.config.preProduction.on) {
+        <div class="environment-block-header">
+            <div class="left">
+                <h2 class="govuk-heading-m">@messages("Pre-production environment")</h2>
+            </div>
+        </div>
+    }
+}
+
+@developmentTab() = {
+    @if(environments.config.development.on) {
+        <div class="environment-block-header">
+            <div class="left">
+                <h2 class="govuk-heading-m">@messages("Development environment")</h2>
+            </div>
+        </div>
+    }
+}
+
 @problem() = {
     <div class="environment-block-header">
         <div class="left">
@@ -189,7 +210,7 @@ <h2 class="govuk-heading-m">@messages("applicationDetails.environment.credential
 }
 @developmentTabOrProblem(application: Application) = {
     @if(application.issues.isEmpty) {
-        @developmentTab()
+        @testTab()
     } else {
         @problem()
     }
@@ -211,21 +232,45 @@ <h1 class="govuk-heading-l">@messages("environmentAndCredentials.heading")</h1>
 
         @govukTabs(Tabs(
             items = Seq(
-                TabItem(
+                if (environments.config.development.on) {
+                    Some(TabItem(
+                        id = Some("hip-pre-dev"),
+                        label = messages("HIP Dev"),
+                        panel = TabPanel(
+                            content = HtmlContent(developmentTab())
+                        )
+                    ))
+                }
+                else {
+                    None
+                },
+                Some(TabItem(
                     id = Some("hip-development"),
                     label = messages("environmentAndCredentials.test.label"),
                     panel = TabPanel(
                         content = HtmlContent(developmentTabOrProblem(application))
                     )
-                ),
-                TabItem(
+                )),
+                if (environments.config.preProduction.on) {
+                    Some(TabItem(
+                        id = Some("hip-pre-production"),
+                        label = messages("HIP Pre-Prod"),
+                        panel = TabPanel(
+                            content = HtmlContent(preProductionTab())
+                        )
+                    ))
+                }
+                else {
+                    None
+                },
+                Some(TabItem(
                     id = Some("hip-production"),
                     label = messages("environmentAndCredentials.production.label"),
                     panel = TabPanel(
                         content = HtmlContent(productionTabOrProblem(application))
                     )
-                )
-            )
+                ))
+            ).flatten
         ))
 
     }
diff --git a/environment-parity.md b/environment-parity.md
index ed616820..1bc478bc 100644
--- a/environment-parity.md
+++ b/environment-parity.md
@@ -18,16 +18,45 @@ Old to new environment name mapping:
 * Primary -> Production
 * Secondary -> Test
 
+The spike has done some limited renaming. Completely changing "primary" to "production" etc is a purely mechanical 
+process we know we can perform. There is a little gain in actually doing that in a spike branch.
+
+Full renaming should probably happen almost as a first step, even if just to remove the "development" naming of what
+is the Test environment. We'll have confusion adding the actual Development environment if we don't do that. 
+
 ## Data model
 Should we have fixed or variable structure?
-* Bad normalisation
-* Fixed means four environments everywhere
+* We have bad normalisation at present with a repeating data structure
+* Fixed structure means four environments in all environments
+* Fixed might require less data migration
+
+Fixed structure is this:
+
+```json
+"environments": {
+  "primary": {"scopes": [], "credentials": []},
+  "secondary": {"scopes": [], "credentials": []}
+}
+```
 
-Can we abstract code from structure via lenses?
-* Done in some places but maybe not all?
+Variable (normalised) structure is this:
+
+```json
+"environments": [
+  {"environmentName": "production", "scopes": [], "credentials": []},
+  {"environmentName": "test", "scopes": [], "credentials": []}
+]
+```
+
+The spike has assumed a normalised data model, just to see if that can be done and whether this causes issues. We know
+a static structure works already. This does not represent a decision/recommendation. 
+
+Can we abstract code from structure via lens methods?
+* This is done already in most places but not all
+* The spike attempts to do more of this and appears to work well
 
 We will at some point need a data migration for production to add environments.
-* Would this include credentials and scopes?
+* Would this include adding credentials and scopes to new environments?
 
 ## Configuration
 What does our configuration look like?
@@ -41,6 +70,9 @@ This spike is going to use the first option. There will probably be some configu
 This spike will have configuration for four environments with two turned off (as we're working locally). This will
 use the current environments of Production and Test.
 
+See `config.Environments` for the implementation of config. This is used on the Environments and Credentials page to
+control which environment tabs are displayed. 
+
 ```
 Environments {
   production {
@@ -60,101 +92,165 @@ Environments {
 ```
 
 ## Live service
-This is a live service so how can we evolve to environment parity while maintaining a path to production?
+This is a live service so how can we evolve toward environment parity while maintaining a path to production for other 
+work?
 
-## Backend endpoints
-Analysis of backend endpoints called from the frontend and whether any change might be necessary.
+## Widgets
+We have a few widgets that are re-used within the service. They will need updating to cover four environments.
 
-### registerApplication
-When an application is registered we create credentials in Test and Production. The Production credential is hidden from 
-the user.
+A couple of examples follow.
 
-Should we add credentials for all "on" environments?
+### ApplicationApiBuilder
+This is a frontend component that stitches together data from:
+* An enriched application
+* Its selected APIs
+* Its access requests
 
-### getApplication
-This call has the option to enrich with information from Test and Production IDMS.
+It then tells us useful things such as:
+* Endpoint availability per environment
+* Whether the application has pending access requests
+* Whether a selected API is missing 
 
-Should the backend fetch IDMS information for each "on" environment?
-* Note this may impact performance in production, especially if two calls have to go via eBridge 
+This drives UI behaviour on multiple pages.
 
-### deleteApplication
-When we delete an application we also call IDMS to delete credentials from Test and Production.
-* Deleting a credential implicitly removes scopes granted to it. 
+Questions:
+* Does this component still stitch together all the information we require?
+* Is this information needed for all four environments?
+
+### ApplicationEnrichers
+A container of backend helper functions that typically:
+1. Call IDMS
+2. Manipulate an application's model with the results
 
-The backend should delete credentials in all "on" environments.
+We'll need to make sure this works for four environments. There are some environment-specific rules.
+
+### ScopeFixer
+This Backend component is called when adding/removing endpoints to grant and revoke scopes.
+* Scopes can only be added to Test
+* Scopes can be revoked from Test and Production
+
+Questions:
+* Should we automatically grant scopes in all "on" non-production environments?
+* Should we revoke scopes from all "on" environments?
+
+## Journeys
+Analysis of al user journeys based on backend service methods.
+
+### approveAccessRequest
+This grants scopes in Production.
+
+Presumably no change if scopes are granted automatically in lower environments.
 
 ### addApi
-When we add/remove endpoints for an API we grant scopes in Test and revoke scopes in both Test and Production. 
+This adds either a new selected API or adds/removes endpoints for an already selected API.
 
-Should we grant in all non-production "on" environments?
+When we add/remove endpoints for an API we grant scopes in Test and revoke scopes in both Test and Production.
 
-Should we revoke in all "on" environments?
+Questions:
+* Should we grant scopes in all non-production "on" environments?
+* Should we revoke scopes in all "on" environments?
 
 ### removeApi
 When we remove an API we revoke scopes from Test and Production.
 
-Should we revoke scopes from all "on" environments?
+Questions:
+* Should we revoke scopes from all "on" environments?
 
 ### addCredential
-This specifies the environment to add the credential to.
+This adds a credential to a specified environment and copies all scopes from the "master" credential for that
+environment.
 
-Make sure this works for all four environments.
+Questions:
+* This should work as is but in all four environments?
 
 ### deleteCredential
-This specifies the environment to delete the credential from.
+Deletes a specified credential from a specified environment. All scopes granted to the credential are implicitly
+revoked within APIM.
 
-Make sure this works for all four environments.
+Questions:
+* This should work as is but in all four environments?
 
-### approveAccessRequest
-This grants scopes in Production.
+### registerApplication
+When an application is registered we create credentials in Test and Production. The Production credential is hidden from
+the user. This hidden credential is used to grant scopes in the production environment before the user creates a known
+production credential.
 
-Presumably no change if scopes are granted automatically in lower environments.
+Questions:
+* We should create credentials in all "on" environments?
+* We retain the hidden production credential?
+
+### deleteApplication
+When we delete an application we also call IDMS to delete credentials from Test and Production.
+* Deleting a credential implicitly removes scopes granted to it.
 
-### generateDeployment
-This creates a new API in Test.
+There is a "soft delete" rule that only soft deletes that have production access requests.
 
-We will need some configuration telling us which environment to create APIs in.
+Questions:
+* Should we delete credentials in all "on" environments?
 
-### updateDeployment
-This updates an API in Test.
+### findById (application)
+This call has the option to enrich with information from Test and Production IDMS.
 
-We will need some configuration telling us which environment to update APIs in.
+Questions:
+* Should the backend fetch IDMS information for each "on" environment?
+  * Note this may impact performance in production, especially if two calls have to go via eBridge
 
-### promoteToProduction
-Promotes an API from Test to Production.
+### deployToSecondary
+This creates a new API in Test from the "generate" page.
+
+Questions:
+* Which environment do we create an API in?
+
+### redeployToSecondary
+This updates an API in test from the "update" page.
+
+Questions:
+* Which environment do we update an API in?
 
-We need the API lifecycle to know what changes to make. We will then probably need configuration to specify which 
-environments can promote and which environments they can promote to.
+### getDeployment
+This fetches the deployed status of an API in a specified environment.
 
-### getApiDeploymentStatuses
-Gets an API's deployment status from Test and Production.
+Questions:
+* This should work as is but in all four environments?
 
-Should this fetch the status in each "on" environment?
+### getDeploymentDetails
+This fetches an API's meta-data from Test for use on the "update" page.
+
+Questions:
+* Which environment do we update an API in?
+
+### promoteToProduction
+This promotes the version of an API in Test to Production.
+
+Questions:
+* What are the valid combinations or "from" and "to" environments when promoting an API?
 
 ### apisInProduction
-Calls APIM Production to get the list of APIs deployed there.
+This is used on the new HUB Stats page.
 
-Presumably no change.
+Questions:
+* Are we still only interested in production?
 
-### No call to APIM
-The following endpoints do not result in any call to APIM and are presumably environment agnostic.
-* getApplications
-* getApplicationsUsingApi
-* getApplicationsByTeam
-* updateApplicationTeam
-* removeApplicationTeam
+### No change
+The following journeys have no change:
 * createAccessRequest
 * getAccessRequests
 * getAccessRequest
 * rejectAccessRequest
-* addTeamMember
-* findTeamById
-* findTeamByName
-* findTeams
-* createTeam
-* addTeamMemberToTeam
-* removeTeamMemberFromTeam
-* changeTeamName
-* getUserContactDetails
+* rejectAccessRequest
+* changeOwningTeam (application)
+* removeOwningTeamFromApplication
+* addTeamMember (application)
+* findAll (applications)
+* findAllUsingApi (applications)
+* findByTeamId (applications)
 * updateApiTeam
-* removeApiTeam
+* removeOwningTeamFromApi
+* create (team)
+* findAll (teams)
+* findById (team)
+* findByName (team)
+* addTeamMember (team)
+* removeTeamMember (team)
+* renameTeam
+* getUniqueEmails
diff --git a/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala b/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
index bffc3365..b30ae00b 100644
--- a/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
+++ b/test/controllers/application/EnvironmentAndCredentialsControllerSpec.scala
@@ -17,20 +17,20 @@
 package controllers.application
 
 import base.SpecBase
-import config.FrontendAppConfig
+import config.{EnvironmentsImpl, FrontendAppConfig}
 import controllers.actions.{FakeApplication, FakePrivilegedUser, FakeUser, FakeUserNotTeamMember}
 import controllers.routes
 import models.application.{Production, Test}
-import models.application.ApplicationLenses._
+import models.application.ApplicationLenses.*
 import models.exception.ApplicationCredentialLimitException
 import models.user.UserModel
-import org.mockito.ArgumentMatchers.{any, eq => eqTo}
+import org.mockito.ArgumentMatchers.{any, eq as eqTo}
 import org.mockito.Mockito.{verify, when}
 import org.scalatestplus.mockito.MockitoSugar
 import play.api.inject.bind
 import play.api.test.FakeRequest
-import play.api.test.Helpers._
-import play.api.{Application => PlayApplication}
+import play.api.test.Helpers.*
+import play.api.Application as PlayApplication
 import services.ApiHubService
 import utils.{HtmlValidation, TestHelpers}
 import views.html.ErrorTemplate
@@ -54,9 +54,10 @@ class EnvironmentAndCredentialsControllerSpec extends SpecBase with MockitoSugar
             val result = route(fixture.playApplication, request).value
             val view = fixture.playApplication.injector.instanceOf[EnvironmentAndCredentialsView]
             val config = fixture.playApplication.injector.instanceOf[FrontendAppConfig]
+            val environments = new EnvironmentsImpl()
 
             status(result) mustEqual OK
-            contentAsString(result) mustBe view(FakeApplication, user, config.helpDocsPath)(request, messages(fixture.playApplication)).toString
+            contentAsString(result) mustBe view(FakeApplication, user, config.helpDocsPath, environments)(request, messages(fixture.playApplication)).toString
             contentAsString(result) must validateAsHtml
           }
       }

From 115fa4c8ea4e03441b2b0faaafc71a5e3a7d4fb2 Mon Sep 17 00:00:00 2001
From: PaulCDurham <23658502+PaulCDurham@users.noreply.github.com>
Date: Thu, 10 Oct 2024 15:49:55 +0100
Subject: [PATCH 4/4] Connectors

---
 environment-parity.md | 61 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/environment-parity.md b/environment-parity.md
index 1bc478bc..e75f0d97 100644
--- a/environment-parity.md
+++ b/environment-parity.md
@@ -33,19 +33,37 @@ Should we have fixed or variable structure?
 Fixed structure is this:
 
 ```json
-"environments": {
-  "primary": {"scopes": [], "credentials": []},
-  "secondary": {"scopes": [], "credentials": []}
+{
+  "environments": {
+    "primary": {
+      "scopes": [],
+      "credentials": []
+    },
+    "secondary": {
+      "scopes": [],
+      "credentials": []
+    }
+  }
 }
 ```
 
 Variable (normalised) structure is this:
 
 ```json
-"environments": [
-  {"environmentName": "production", "scopes": [], "credentials": []},
-  {"environmentName": "test", "scopes": [], "credentials": []}
-]
+{
+  "environments": [
+    {
+      "name": "production",
+      "scopes": [],
+      "credentials": []
+    },
+    {
+      "name": "test",
+      "scopes": [],
+      "credentials": []
+    }
+  ]
+}
 ```
 
 The spike has assumed a normalised data model, just to see if that can be done and whether this causes issues. We know
@@ -133,8 +151,35 @@ Questions:
 * Should we automatically grant scopes in all "on" non-production environments?
 * Should we revoke scopes from all "on" environments?
 
+## APIM Connectors
+In the backend we have several connectors that speak to APIM. Most of them accept an environment parameter and should 
+need little to no change.
+
+The configuration should change to match the new environment names and add the additional environments too. We might
+need some thought around configuration for unused environments. Hopefully just configuring an environment off means
+its remaining configuration is ignored.
+
+There are some methods that specify environments in their names, for example:
+* validateInPrimary
+* deployToSecondary
+
+These are on the whole V2 journeys that we expect to work on further. We should try and make them configuration driven
+through. So `validateInPrimary` should be something like `validateOas` and take an environment parameter. The value
+of that parameter should come from configuration.
+
+```
+Environments {
+  production {
+    on: true
+  },
+  ...,
+  deployTo: test,
+  validateOasIn: production
+}
+```
+
 ## Journeys
-Analysis of al user journeys based on backend service methods.
+Analysis of all user journeys based on backend service methods.
 
 ### approveAccessRequest
 This grants scopes in Production.