Merge pull request #1894 from betagouv/master

MEP

app/orchestrators/EngagementOrchestrator.scala

@@ -1,7 +1,6 @@
 package orchestrators
 
 import cats.implicits.catsSyntaxOption
-import cats.implicits.catsSyntaxOptionId
 import controllers.error.AppError.CannotReviewReportResponse
 import controllers.error.AppError.EngagementNotFound
 import controllers.error.AppError.ReportNotFound
@@ -11,8 +10,8 @@ import models.engagement.EngagementId
 import models.event.Event
 import models.report.ExistingReportResponse
 import models.report.ReportStatus.hasResponse
-import models.report.review.EngagementReview
 import models.report.review.ConsumerReviewApi
+import models.report.review.EngagementReview
 import models.report.review.ResponseConsumerReviewId
 import play.api.Logger
 import play.api.libs.json.Json
@@ -108,7 +107,7 @@ class EngagementOrchestrator(
 
   def removeEngagement(reportId: UUID): Future[Unit] =
     for {
-      _ <- getEngagementReview(reportId).flatMap {
+      _ <- reportEngagementReviewRepository.findByReportId(reportId).flatMap {
         case Some(engagementReview) =>
           reportEngagementReviewRepository.delete(engagementReview.id).map(_ => ())
         case None => Future.unit
@@ -119,24 +118,9 @@ class EngagementOrchestrator(
   def getVisibleEngagementReview(reportId: UUID, user: User): Future[Option[EngagementReview]] =
     for {
       _           <- visibleReportOrchestrator.checkReportIsVisible(reportId, user)
-      maybeReview <- getEngagementReview(reportId)
+      maybeReview <- reportEngagementReviewRepository.findByReportId(reportId)
     } yield maybeReview
 
-  private def getEngagementReview(reportId: UUID): Future[Option[EngagementReview]] =
-    reportEngagementReviewRepository.findByReportId(reportId) map {
-      case Nil =>
-        logger.info(s"No engagement review found for report $reportId")
-        None
-      case review :: Nil => Some(review)
-      case engagementReviews =>
-        io.sentry.Sentry.captureException(
-          new Exception(
-            s"More than one engagement review for report id $reportId, this is not and expected behavior it should be investigated"
-          )
-        )
-        engagementReviews.maxBy(_.creationDate).some
-    }
-
   def getEngagementReviews(reportIds: Seq[UUID]): Future[Map[UUID, Option[EngagementReview]]] =
     reportEngagementReviewRepository.findByReportIds(reportIds)
 
@@ -160,7 +144,7 @@ class EngagementOrchestrator(
       }
       _ = logger.debug(s"Report validated")
       reviews <- reportEngagementReviewRepository.findByReportId(reportId)
-      _ <- reviews.headOption match {
+      _ <- reviews match {
         case Some(review) =>
           updateEngagementReview(review.copy(evaluation = reviewApi.evaluation, details = reviewApi.details))
         case None =>
@@ -172,14 +156,14 @@ class EngagementOrchestrator(
   def deleteDetails(reportId: UUID): Future[Unit] = for {
     reviews <- reportEngagementReviewRepository.findByReportId(reportId)
     _ <- reviews match {
-      case review :: _ => reportEngagementReviewRepository.update(review.id, review.copy(details = Some("")))
-      case _           => Future.unit
+      case Some(review) => reportEngagementReviewRepository.update(review.id, review.copy(details = Some("")))
+      case None         => Future.unit
     }
   } yield ()
 
   def doesEngagementReviewExists(reportId: UUID): Future[Boolean] =
     for {
-      maybeReview <- getEngagementReview(reportId)
+      maybeReview <- reportEngagementReviewRepository.findByReportId(reportId)
       hasNonEmptyReview = maybeReview.exists(_.details.nonEmpty)
     } yield hasNonEmptyReview
 
@@ -191,6 +175,7 @@ class EngagementOrchestrator(
         details = review.details
       )
     )
+
   private def createEngagementReview(reportId: UUID, review: ConsumerReviewApi): Future[Event] =
     reportEngagementReviewRepository
       .createOrUpdate(

app/orchestrators/ReportConsumerReviewOrchestrator.scala

@@ -2,7 +2,6 @@ package orchestrators
 
 import org.apache.pekko.Done
 import controllers.error.AppError.CannotReviewReportResponse
-import controllers.error.AppError.ServerError
 import models.User
 import models.report.ReportStatus.hasResponse
 import models.report.review.ConsumerReviewApi
@@ -32,7 +31,7 @@ class ReportConsumerReviewOrchestrator(
   val logger = Logger(this.getClass)
 
   def remove(reportId: UUID): Future[Done] =
-    getReview(reportId).flatMap {
+    responseConsumerReviewRepository.findByReportId(reportId).flatMap {
       case Some(responseConsumerReview) =>
         responseConsumerReviewRepository.delete(responseConsumerReview.id).map(_ => Done)
       case None => Future.successful(Done)
@@ -41,27 +40,15 @@ class ReportConsumerReviewOrchestrator(
   def getVisibleReview(reportId: UUID, user: User): Future[Option[ResponseConsumerReview]] =
     for {
       _           <- visibleReportOrchestrator.checkReportIsVisible(reportId, user)
-      maybeReview <- getReview(reportId)
+      maybeReview <- responseConsumerReviewRepository.findByReportId(reportId)
     } yield maybeReview
 
   def doesReviewExists(reportId: UUID): Future[Boolean] =
     for {
-      maybeReview <- getReview(reportId)
+      maybeReview <- responseConsumerReviewRepository.findByReportId(reportId)
       hasNonEmptyReview = maybeReview.exists(_.details.nonEmpty)
     } yield hasNonEmptyReview
 
-  private def getReview(reportId: UUID): Future[Option[ResponseConsumerReview]] =
-    for {
-      reviews <- responseConsumerReviewRepository.findByReportId(reportId)
-      maybeReview = reviews match {
-        case Nil =>
-          logger.info(s"No review found for report $reportId")
-          None
-        case review :: Nil => Some(review)
-        case _             => throw ServerError(s"More than one consumer review for report id $reportId")
-      }
-    } yield maybeReview
-
   def getReviews(reportIds: List[UUID]): Future[Map[UUID, Option[ResponseConsumerReview]]] =
     responseConsumerReviewRepository.findByReportIds(reportIds)
 
@@ -87,7 +74,7 @@ class ReportConsumerReviewOrchestrator(
       }
       _ = logger.debug(s"Report validated")
       reviews <- responseConsumerReviewRepository.findByReportId(reportId)
-      _ <- reviews.headOption match {
+      _ <- reviews match {
         case Some(review) =>
           updateReview(review.copy(evaluation = reviewApi.evaluation, details = reviewApi.details))
         case None =>
@@ -99,8 +86,8 @@ class ReportConsumerReviewOrchestrator(
   def deleteDetails(reportId: UUID): Future[Unit] = for {
     reviews <- responseConsumerReviewRepository.findByReportId(reportId)
     _ <- reviews match {
-      case review :: _ => responseConsumerReviewRepository.update(review.id, review.copy(details = Some("")))
-      case _           => Future.unit
+      case Some(review) => responseConsumerReviewRepository.update(review.id, review.copy(details = Some("")))
+      case None         => Future.unit
     }
   } yield ()
 

app/orchestrators/UserOrchestrator.scala

@@ -19,6 +19,7 @@ import java.time.OffsetDateTime
 import cats.syntax.option._
 import models.AuthProvider.ProConnect
 import models.AuthProvider.SignalConso
+import models.UserRole.DGCCRF
 import models.event.Event
 import models.event.Event.stringToDetailsJsValue
 import models.proconnect.ProConnectClaim
@@ -88,11 +89,13 @@ class UserOrchestrator(userRepository: UserRepositoryInterface, eventRepository:
   override def getProConnectUser(claim: ProConnectClaim, role: UserRole): Future[User] =
     OptionT(userRepository.findByAuthProviderId(claim.sub))
       .orElseF(userRepository.findByEmail(claim.email))
+      .filter(_.userRole == DGCCRF)
       .semiflatMap { user =>
         val updated = user.copy(
           email = EmailAddress(claim.email),
           firstName = claim.givenName,
           lastName = claim.usualName,
+          userRole = role,
           authProvider = ProConnect,
           authProviderId = claim.sub.some,
           lastEmailValidation = Some(OffsetDateTime.now())

app/repositories/reportconsumerreview/ResponseConsumerReviewRepository.scala

@@ -25,8 +25,8 @@ class ResponseConsumerReviewRepository(
 
   import dbConfig._
 
-  override def findByReportId(reportId: UUID): Future[List[ResponseConsumerReview]] =
-    db.run(table.filter(_.reportId === reportId).to[List].result)
+  override def findByReportId(reportId: UUID): Future[Option[ResponseConsumerReview]] =
+    db.run(table.filter(_.reportId === reportId).to[List].result.headOption)
 
   override def findByReportIds(reportIds: List[UUID]): Future[Map[UUID, Option[ResponseConsumerReview]]] =
     db.run(

app/repositories/reportconsumerreview/ResponseConsumerReviewRepositoryInterface.scala

@@ -10,7 +10,7 @@ import scala.concurrent.Future
 trait ResponseConsumerReviewRepositoryInterface
     extends TypedCRUDRepositoryInterface[ResponseConsumerReview, ResponseConsumerReviewId] {
 
-  def findByReportId(reportId: UUID): Future[List[ResponseConsumerReview]]
+  def findByReportId(reportId: UUID): Future[Option[ResponseConsumerReview]]
 
   def findByReportIds(reportIds: List[UUID]): Future[Map[UUID, Option[ResponseConsumerReview]]]
 

app/repositories/reportengagementreview/ReportEngagementReviewRepository.scala

@@ -23,8 +23,9 @@ class ReportEngagementReviewRepository(
 
   import dbConfig._
 
-  override def findByReportId(reportId: UUID): Future[List[EngagementReview]] =
-    db.run(table.filter(_.reportId === reportId).to[List].result)
+  override def findByReportId(reportId: UUID): Future[Option[EngagementReview]] =
+    db.run(table.filter(_.reportId === reportId).to[List].result.headOption)
+
   override def findByReportIds(reportIds: Seq[UUID]): Future[Map[UUID, Option[EngagementReview]]] =
     db.run(
       table

app/repositories/reportengagementreview/ReportEngagementReviewRepositoryInterface.scala

@@ -9,7 +9,7 @@ import scala.concurrent.Future
 
 trait ReportEngagementReviewRepositoryInterface
     extends TypedCRUDRepositoryInterface[EngagementReview, ResponseConsumerReviewId] {
-  def findByReportId(reportId: UUID): Future[List[EngagementReview]]
+  def findByReportId(reportId: UUID): Future[Option[EngagementReview]]
   def findByReportIds(reportIds: Seq[UUID]): Future[Map[UUID, Option[EngagementReview]]]
 
   def findByCompany(companyId: Option[UUID]): Future[List[EngagementReview]]

build.sbt

@@ -58,6 +58,11 @@ Test / tpolecatExcludeOptions += ScalacOptions.lintMissingInterpolator
 
 resolvers += "Atlassian Releases" at "https://packages.atlassian.com/maven-public/"
 
+def doLint(cmd: String) = s"scalafmt;$cmd;"
+
+addCommandAlias("lint", doLint("scalafmt;compile;scalafix"))
+addCommandAlias("lintTest", doLint("scalafmtAll;Test / compile;scalafixAll"))
+
 Universal / mappings ++=
   (baseDirectory.value / "appfiles" * "*" get) map
     (x => x -> ("appfiles/" + x.getName))

conf/db/migration/default/V55__unique_engagement_constraint.sql

@@ -0,0 +1,12 @@
+-- Removing duplicates before applying constraint
+DELETE
+FROM engagement_reviews r
+    USING (SELECT rr.report_id, MAX(rr.creation_date) AS max_creation
+           FROM engagement_reviews rr
+           GROUP BY rr.report_id
+           HAVING COUNT(1) > 1) agg
+WHERE agg.report_id = r.report_id
+  AND agg.max_creation <> r.creation_date;
+
+ALTER TABLE engagement_reviews
+    ADD CONSTRAINT unique_engagement_report_id UNIQUE (report_id);