Merge pull request #950 from hmrc/BDOG-3260

BDOG-3260: Refactor CreateRepositoryController to filter out non github teams
hmrc · Sep 25, 2024 · 250052c · 250052c
2 parents 4cfa61b + 21a414d
commit 250052c
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 39 deletions.
diff --git a/app/uk/gov/hmrc/cataloguefrontend/createrepository/CreateRepositoryController.scala b/app/uk/gov/hmrc/cataloguefrontend/createrepository/CreateRepositoryController.scala
@@ -25,10 +25,9 @@ import play.api.mvc.{Action, AnyContent, MessagesControllerComponents, Request,
 import play.api.libs.json.{Format, Json, Reads, Writes}
 import play.twirl.api.Html
 import uk.gov.hmrc.cataloguefrontend.auth.CatalogueAuthBuilders
-import uk.gov.hmrc.cataloguefrontend.connector.{BuildDeployApiConnector, TeamsAndRepositoriesConnector}
+import uk.gov.hmrc.cataloguefrontend.connector.{BuildDeployApiConnector, GitHubTeam, TeamsAndRepositoriesConnector}
 import uk.gov.hmrc.cataloguefrontend.createrepository.view.html.{CreatePrototypePage, CreateRepositoryConfirmationPage, CreateServicePage, CreateTestPage, SelectRepoTypePage}
 import uk.gov.hmrc.cataloguefrontend.model.TeamName
-import uk.gov.hmrc.http.HeaderCarrier
 import uk.gov.hmrc.internalauth.client.*
 import uk.gov.hmrc.mongo.{MongoComponent, TimestampSupport}
 import uk.gov.hmrc.mongo.cache.{DataKey, SessionCacheRepository}
@@ -114,12 +113,13 @@ class CreateRepositoryController @Inject()(
     ).async: request =>
       given Request[AnyContent] = request
       (for
-        userTeams   <- EitherT.pure[Future, Result](cleanseUserTeams(request.retrieval))
-        repoTypeOut <- getRepoTypeOut()
-        result      =  repoTypeOut.repoType match
-                         case RepoType.Prototype => Ok(createPrototypePage(CreatePrototype.form, userTeams.filterNot(_ == TeamName("Designers"))))
-                         case RepoType.Test      => Ok(createTestPage(CreateTest.form, userTeams))
-                         case RepoType.Service   => Ok(createServicePage(CreateService.form, userTeams))
+        githubTeams       <- EitherT.liftF(teamsAndReposConnector.allTeams())
+        userGithubTeams   <- EitherT.pure[Future, Result](cleanseUserTeams(request.retrieval, githubTeams))
+        repoTypeOut       <- getRepoTypeOut()
+        result            =  repoTypeOut.repoType match
+                               case RepoType.Prototype => Ok(createPrototypePage(CreatePrototype.form, userGithubTeams.filterNot(_ == TeamName("Designers"))))
+                               case RepoType.Test      => Ok(createTestPage(CreateTest.form, userGithubTeams))
+                               case RepoType.Service   => Ok(createServicePage(CreateService.form, userGithubTeams))
        yield result
       ).merge
 
@@ -129,22 +129,20 @@ class CreateRepositoryController @Inject()(
     bndApiCreateRepo: T => Future[Either[String, BuildDeployApiConnector.AsyncRequestId]],
   )(using request: AuthenticatedRequest[_, Set[Resource]]): Future[Result] =
     (for
-      repoTypeOut   <- getRepoTypeOut()
-      userTeams     <- EitherT.pure[Future, Result](cleanseUserTeams(request.retrieval))
-      submittedForm =  bindForm.bindFromRequest()
-      validForm     <- submittedForm.fold[EitherT[Future, Result, T]](
-                         formWithErrors => EitherT.leftT(BadRequest(createPage(formWithErrors, userTeams))),
-                         validForm      => EitherT.pure(validForm)
-                       )
-      _             <- EitherT.liftF(auth.authorised(Some(createRepositoryPermission(validForm.teamName))))
-      _             <- EitherT(verifyGithubTeamExists(validForm.teamName))
-                         .leftMap: error =>
-                           BadRequest(createPage(submittedForm.withError("teamName", error), userTeams))
-      id            <- EitherT(bndApiCreateRepo(validForm))
-                         .leftMap: error =>
-                           logger.info(s"CreateRepository request for ${validForm.repositoryName} failed with message: $error")
-                           BadRequest(createPage(submittedForm.withGlobalError(s"Repository creation failed! Error: $error"), userTeams))
-      _             =  logger.info(s"CreateRepository request for ${validForm.repositoryName} successfully sent. Bnd api request id: $id:")
+      repoTypeOut       <- getRepoTypeOut()
+      githubTeams       <- EitherT.liftF(teamsAndReposConnector.allTeams())
+      userGithubTeams   <- EitherT.pure[Future, Result](cleanseUserTeams(request.retrieval, githubTeams))
+      submittedForm     =  bindForm.bindFromRequest()
+      validForm         <- submittedForm.fold[EitherT[Future, Result, T]](
+                             formWithErrors => EitherT.leftT(BadRequest(createPage(formWithErrors, userGithubTeams))),
+                             validForm      => EitherT.pure(validForm)
+                           )
+      _                 <- EitherT.liftF(auth.authorised(Some(createRepositoryPermission(validForm.teamName))))
+      id                <- EitherT(bndApiCreateRepo(validForm))
+                             .leftMap: error =>
+                               logger.info(s"CreateRepository request for ${validForm.repositoryName} failed with message: $error")
+                               BadRequest(createPage(submittedForm.withGlobalError(s"Repository creation failed! Error: $error"), userGithubTeams))
+      _                 =  logger.info(s"CreateRepository request for ${validForm.repositoryName} successfully sent. Bnd api request id: $id:")
      yield Redirect(routes.CreateRepositoryController.createRepoConfirmation(repoTypeOut.repoType, validForm.repositoryName))
     ).merge
 
@@ -203,18 +201,14 @@ class CreateRepositoryController @Inject()(
       getFromSession(repoTypeKey),
       Redirect(routes.CreateRepositoryController.createRepoLandingGet())
     )
-
-  private def verifyGithubTeamExists(selectedTeam: TeamName)(using HeaderCarrier): Future[Either[String, Unit]] =
-    teamsAndReposConnector.allTeams().map: gitTeams =>
-      if gitTeams.map(_.name).contains(selectedTeam) then Right(())
-      else Left(s"'${selectedTeam.asString}' does not exist as a team on Github.")
 
-  private def cleanseUserTeams(resources: Set[Resource]): Seq[TeamName] =
+  private def cleanseUserTeams(resources: Set[Resource], githubTeams: Seq[GitHubTeam]): Seq[TeamName] =
     resources
       .map(_.resourceLocation.value.stripPrefix("teams/"))
       .filterNot(_.contains("app_group_"))
       .map(TeamName.apply)
       .toSeq
+      .filter(ut => githubTeams.exists(_.name == ut))
       .sorted
 end CreateRepositoryController
 

diff --git a/test/uk/gov/hmrc/cataloguefrontend/createrepository/CreateRepositoryControllerSpec.scala b/test/uk/gov/hmrc/cataloguefrontend/createrepository/CreateRepositoryControllerSpec.scala
@@ -115,9 +115,9 @@ class CreateRepositoryControllerSpec
       status(result) shouldBe OK
       contentAsString(result) should include("Creating service repository: test-repo")
 
-    "return a BadRequest for repository creation when the team doesn't exist on Github" in new Setup:
+    "render the repository creation page with correctly filtered teams on GET request" in new Setup:
       when(authStubBehaviour.stubAuth(any[Option[Predicate.Permission]], any[Retrieval[Set[Resource]]]))
-        .thenReturn(Future.successful(Set(Resource(ResourceType("catalogue-frontend"), ResourceLocation("teams/TestTeam")))))
+        .thenReturn(Future.successful(Set(Resource(ResourceType("catalogue-frontend"), ResourceLocation("teams/TestTeam")), Resource(ResourceType("catalogue-frontend"), ResourceLocation("teams/TestTeam2")))))
 
       when(authStubBehaviour.stubAuth(any[Option[Predicate.Permission]], eqTo(Retrieval.EmptyRetrieval)))
         .thenReturn(Future.unit)
@@ -129,17 +129,17 @@ class CreateRepositoryControllerSpec
         .thenReturn(Future.successful(()))
 
       when(mockTeamsAndReposConnector.allTeams()(using any[HeaderCarrier]))
-        .thenReturn(Future.successful(Seq.empty))
+        .thenReturn(Future.successful(Seq(GitHubTeam(TeamName("TestTeam"), None, Seq.empty))))
 
-      val fakeRequest: FakeRequest[AnyContent] = FakeRequest(POST, "/create-repo/2")
-        .withFormUrlEncodedBody("teamName" -> "TestTeam", "repositoryName" -> "repo-test", "makePrivate" -> "false", "testType" -> "UI Journey Test")
-        .withSession(SessionKeys.authToken -> "Token token")
+      val fakeRequest: FakeRequest[AnyContent] =
+        FakeRequest().withSession(SessionKeys.authToken -> "Token token")
 
       val result: Future[Result] =
-        controller.createRepoPost()(fakeRequest)
+        controller.createRepoGet()(fakeRequest)
 
-      status(result) shouldBe BAD_REQUEST
-      contentAsString(result) should include("does not exist as a team on Github.")
+      status(result) shouldBe OK
+      contentAsString(result) should include("""option value="TestTeam"""")
+      contentAsString(result) should not include("""option value="TestTeam2"""")
 
     "return a BadRequest when repository creation fails" in new Setup:
       when(authStubBehaviour.stubAuth(any[Option[Predicate.Permission]], any[Retrieval[Set[Resource]]]))