Skip to content

Commit

Permalink
Merge pull request #1532 from betagouv/master
Browse files Browse the repository at this point in the history
MEP
  • Loading branch information
ssedoudbgouv authored Jan 25, 2024
2 parents c1ad772 + 780de69 commit 8d17d24
Show file tree
Hide file tree
Showing 19 changed files with 82 additions and 62 deletions.
2 changes: 1 addition & 1 deletion app/controllers/AccountController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class AccountController(
def validateEmail() = Action.async(parse.json) { implicit request =>
for {
token <- request.parseBody[String](JsPath \ "token")
user <- accessesOrchestrator.validateDGCCRFEmail(token)
user <- accessesOrchestrator.validateAgentEmail(token)
cookie <- authenticator.init(user.email) match {
case Right(value) => Future.successful(value)
case Left(error) => Future.failed(error)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/ReportController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class ReportController(
reportWithDataOrchestrator
.getReportFull(reportId, request.identity)
.flatMap(_.liftTo[Future](AppError.ReportNotFound(reportId)))
.map(reportData => massImportService.reportSummaryWithAttachmentsZip(reportData))
.flatMap(reportData => massImportService.reportSummaryWithAttachmentsZip(reportData))
.map(pdfSource =>
Ok.chunked(
content = pdfSource,
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/error/AppError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ object AppError {
override val titleForLogs: String = "server_error"
}

final case class DGCCRFActivationTokenNotFound(token: String) extends NotFoundError {
final case class AgentActivationTokenNotFound(token: String) extends NotFoundError {
override val `type`: String = "SC-0002"
override val title: String = s"DGCCRF user token $token not found"
override val title: String = s"Agent user token $token not found"
override val details: String = s"Le lien d'activation n'est pas valide ($token). Merci de contacter le support"
override val titleForLogs: String = "dgccrf_activation_token_not_found"
override val titleForLogs: String = "agent_activation_token_not_found"
}

final case class CompanyActivationTokenNotFound(token: String, siret: SIRET) extends NotFoundError {
Expand Down
12 changes: 6 additions & 6 deletions app/orchestrators/AccessesOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class AccessesOrchestrator(
.findToken(token)
accessToken <- maybeAccessToken
.map(Future.successful(_))
.getOrElse(Future.failed[AccessToken](DGCCRFActivationTokenNotFound(token)))
.getOrElse(Future.failed[AccessToken](AgentActivationTokenNotFound(token)))
_ = logger.debug("Validating email")
emailTo <-
accessToken.emailedTo
Expand Down Expand Up @@ -325,12 +325,12 @@ class AccessesOrchestrator(
)
} yield logger.debug(s"Sent email validation to ${user.email}")

def validateDGCCRFEmail(token: String): Future[User] =
def validateAgentEmail(token: String): Future[User] =
for {
accessToken <- accessTokenRepository.findToken(token)
emailValidationToken <- accessToken
.filter(_.kind == ValidateEmail)
.liftTo[Future](DGCCRFActivationTokenNotFound(token))
.liftTo[Future](AgentActivationTokenNotFound(token))

emailTo <- emailValidationToken.emailedTo.liftTo[Future](
ServerError("ValidateEmailToken should have valid email associated")
Expand All @@ -350,10 +350,10 @@ class AccessesOrchestrator(
user <- userOrchestrator
.findOrError(email)
.ensure {
logger.error("Cannot revalidate user with role different from DGCCRF")
logger.error("Cannot revalidate user with role different from DGCCRF or DGAL")
CantPerformAction
}(_.userRole == UserRole.DGCCRF)
_ = logger.debug(s"Validating DGCCRF user email")
}(user => user.userRole == UserRole.DGCCRF || user.userRole == UserRole.DGAL)
_ = logger.debug(s"Validating agent user email")
_ <-
if (emailValidationToken.nonEmpty) {
emailValidationToken.map(accessTokenRepository.validateEmail(_, user)).sequence
Expand Down
6 changes: 3 additions & 3 deletions app/orchestrators/AuthOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class AuthOrchestrator(
_ = logger.debug(s"Found user (maybe deleted)")
_ <- handleDeletedUser(user, userLogin)
_ = logger.debug(s"Check last validation email for DGCCRF users")
_ <- validateDGCCRFAccountLastEmailValidation(user)
_ <- validateAgentAccountLastEmailValidation(user)
_ = logger.debug(s"Successful login for user")
cookie <- getCookie(userLogin)(request)
_ = logger.debug(s"Successful generated token for user")
Expand Down Expand Up @@ -180,8 +180,8 @@ class AuthOrchestrator(
}
} yield cookie

private def validateDGCCRFAccountLastEmailValidation(user: User): Future[User] = user.userRole match {
case UserRole.DGCCRF if needsEmailRevalidation(user) =>
private def validateAgentAccountLastEmailValidation(user: User): Future[User] = user.userRole match {
case UserRole.DGCCRF | UserRole.DGAL if needsEmailRevalidation(user) =>
accessesOrchestrator
.sendEmailValidation(user)
.flatMap(_ => throw DGCCRFUserEmailValidationExpired(user.email.value))
Expand Down
6 changes: 3 additions & 3 deletions app/orchestrators/ReportFileOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,11 @@ class ReportFileOrchestrator(
for {
reportFiles <- reportFileRepository
.retrieveReportFiles(report.id)

filteredFilesByOrigin = reportFiles.filter { f =>
origin.contains(f.origin) || origin.isEmpty
}
_ <- Future.successful(filteredFilesByOrigin).ensure(AppError.NoReportFiles)(_.nonEmpty)
} yield reportZipExportService.reportAttachmentsZip(report.creationDate, filteredFilesByOrigin)
_ <- Future.successful(filteredFilesByOrigin).ensure(AppError.NoReportFiles)(_.nonEmpty)
res <- reportZipExportService.reportAttachmentsZip(report.creationDate, filteredFilesByOrigin)
} yield res

}
43 changes: 27 additions & 16 deletions app/orchestrators/ReportZipExportService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import akka.stream.IOResult
import akka.stream.Materializer
import akka.stream.scaladsl.Source
import akka.util.ByteString
import cats.implicits.toTraverseOps
import controllers.HtmlFromTemplateGenerator
import models.report.ReportFile
import play.api.Logger
Expand Down Expand Up @@ -39,29 +40,39 @@ class ReportZipExportService(

def reportSummaryWithAttachmentsZip(
reportWithData: ReportWithData
): Source[ByteString, Future[IOResult]] = {
): Future[Source[ByteString, Future[IOResult]]] = for {
reportAttachmentSources <- buildReportAttachmentsSources(reportWithData.report.creationDate, reportWithData.files)
reportPdfSummarySource = buildReportPdfSummarySource(reportWithData)
fileSourcesFutures = reportAttachmentSources :+ reportPdfSummarySource
} yield ZipBuilder.buildZip(fileSourcesFutures)

val reportAttachmentSources = reportWithData.files.zipWithIndex.map { case (file, i) =>
buildReportAttachmentSource(reportWithData.report.creationDate, file, i)
private def buildReportAttachmentsSources(
creationDate: OffsetDateTime,
reportFiles: Seq[ReportFile]
) = for {
existingFiles <- reportFiles.traverse(f =>
s3Service.exists(f.storageFilename).map(exists => (f, exists))
) map (_.collect { case (file, true) =>
file
})
reportAttachmentSources = existingFiles.zipWithIndex.map { case (file, i) =>
buildReportAttachmentSource(creationDate, file, i + 1)
}
val reportPdfSummarySource = buildReportPdfSummarySource(reportWithData)

val fileSourcesFutures = reportAttachmentSources :+ reportPdfSummarySource

ZipBuilder.buildZip(fileSourcesFutures)
}
} yield reportAttachmentSources

def reportAttachmentsZip(
creationDate: OffsetDateTime,
reports: Seq[ReportFile]
): Source[ByteString, Future[IOResult]] = {

val reportAttachmentSources = reports.zipWithIndex.map { case (file, i) =>
reportFiles: Seq[ReportFile]
): Future[Source[ByteString, Future[IOResult]]] = for {
existingFiles <- reportFiles.traverse(f =>
s3Service.exists(f.storageFilename).map(exists => (f, exists))
) map (_.collect { case (file, true) =>
file
})
reportAttachmentSources = existingFiles.zipWithIndex.map { case (file, i) =>
buildReportAttachmentSource(creationDate, file, i + 1)
}

ZipBuilder.buildZip(reportAttachmentSources)
}
} yield ZipBuilder.buildZip(reportAttachmentSources)

private def buildReportPdfSummarySource(
reportWithData: ReportWithData
Expand Down
13 changes: 7 additions & 6 deletions app/repositories/user/UserRepository.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package repositories.user

import authentication.PasswordHasherRegistry
import controllers.error.AppError.EmailAlreadyExist
import models.UserRole.DGAL
import models.UserRole.DGCCRF
import models._
import play.api.Logger
Expand Down Expand Up @@ -36,29 +37,29 @@ class UserRepository(

import dbConfig._

override def listExpiredDGCCRF(expirationDate: OffsetDateTime): Future[List[User]] =
override def listExpiredAgents(expirationDate: OffsetDateTime): Future[List[User]] =
db
.run(
table
.filter(_.role === DGCCRF.entryName)
.filter(user => user.role === DGCCRF.entryName || user.role === DGAL.entryName)
.filter(_.lastEmailValidation <= expirationDate)
.to[List]
.result
)

override def listInactiveDGCCRFWithSentEmailCount(
override def listInactiveAgentsWithSentEmailCount(
reminderDate: OffsetDateTime,
expirationDate: OffsetDateTime
): Future[List[(User, Option[Int])]] =
db.run(
UserTable.table
.filter(_.role === DGCCRF.entryName)
.filter(user => user.role === DGCCRF.entryName || user.role === DGAL.entryName)
.filter(_.lastEmailValidation <= reminderDate)
.filter(_.lastEmailValidation > expirationDate)
.joinLeft(
EventTable.table
.filter(_.action === ActionEvent.EMAIL_INACTIVE_DGCCRF_ACCOUNT.value)
.filter(_.eventType === EventType.DGCCRF.value)
.filter(_.action === ActionEvent.EMAIL_INACTIVE_AGENT_ACCOUNT.value)
.filter(user => user.eventType === EventType.DGCCRF.value || user.eventType === EventType.DGAL.value)
.filter(_.userId.isDefined)
.groupBy(_.userId)
.map { case (userId, results) => userId -> results.length }
Expand Down
4 changes: 2 additions & 2 deletions app/repositories/user/UserRepositoryInterface.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import scala.concurrent.Future

trait UserRepositoryInterface extends CRUDRepositoryInterface[User] {

def listExpiredDGCCRF(expirationDate: OffsetDateTime): Future[List[User]]
def listExpiredAgents(expirationDate: OffsetDateTime): Future[List[User]]

def listInactiveDGCCRFWithSentEmailCount(
def listInactiveAgentsWithSentEmailCount(
reminderDate: OffsetDateTime,
expirationDate: OffsetDateTime
): Future[List[(User, Option[Int])]]
Expand Down
3 changes: 3 additions & 0 deletions app/services/S3Service.scala
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class S3Service(implicit
alpakkaS3Client
.getObject(bucketName, bucketKey)

def exists(bucketKey: String): Future[Boolean] =
S3.getObjectMetadata(bucketName, bucketKey).runWith(Sink.headOption).map(_.isDefined)

override def delete(bucketKey: String): Future[Done] =
alpakkaS3Client.deleteObject(bucketName, bucketKey).runWith(Sink.head)

Expand Down
1 change: 1 addition & 0 deletions app/services/S3ServiceInterface.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ trait S3ServiceInterface {

def getSignedUrl(bucketKey: String, method: HttpMethod = HttpMethod.GET): String
def downloadFromBucket(bucketKey: String): Source[ByteString, Future[ObjectMetadata]]
def exists(bucketKey: String): Future[Boolean]
}
9 changes: 5 additions & 4 deletions app/tasks/account/InactiveDgccrfAccountReminderTask.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tasks.account

import models.User
import models.UserRole
import models.event.Event
import play.api.Logger
import play.api.libs.json.Json
Expand Down Expand Up @@ -35,11 +36,11 @@ class InactiveDgccrfAccountReminderTask(
inactivePeriod: Period
): Future[Unit] =
for {
firstReminderEvents <- userRepository.listInactiveDGCCRFWithSentEmailCount(
firstReminderEvents <- userRepository.listInactiveAgentsWithSentEmailCount(
firstReminderThreshold,
expirationDateThreshold
)
secondReminderEvents <- userRepository.listInactiveDGCCRFWithSentEmailCount(
secondReminderEvents <- userRepository.listInactiveAgentsWithSentEmailCount(
secondReminderThreshold,
expirationDateThreshold
)
Expand All @@ -64,8 +65,8 @@ class InactiveDgccrfAccountReminderTask(
None,
Some(user.id),
now,
EventType.DGCCRF,
ActionEvent.EMAIL_INACTIVE_DGCCRF_ACCOUNT,
if (user.userRole == UserRole.DGAL) EventType.DGAL else EventType.DGCCRF,
ActionEvent.EMAIL_INACTIVE_AGENT_ACCOUNT,
Json.obj(
"lastEmailValidation" -> user.lastEmailValidation,
"expirationDate" -> expirationDate
Expand Down
2 changes: 1 addition & 1 deletion app/tasks/account/InactiveDgccrfAccountRemoveTask.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class InactiveDgccrfAccountRemoveTask(

logger.info(s"Soft delete inactive DGCCRF accounts with last validation below $expirationDateThreshold")
for {
inactiveDGCCRFAccounts <- userRepository.listExpiredDGCCRF(expirationDateThreshold)
inactiveDGCCRFAccounts <- userRepository.listExpiredAgents(expirationDateThreshold)
results <- inactiveDGCCRFAccounts.map(removeWithSubscriptions).sequence
} yield results.sequence
}
Expand Down
2 changes: 1 addition & 1 deletion app/utils/Constants.scala
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ object Constants {

object USER_DELETION extends ActionEventValue("Suppression d'un utilisateur")

object EMAIL_INACTIVE_DGCCRF_ACCOUNT extends ActionEventValue("Email «compte inactif» envoyé à l'agent")
object EMAIL_INACTIVE_AGENT_ACCOUNT extends ActionEventValue("Email «compte inactif» envoyé à l'agent")

object CONSUMER_THREATEN_BY_PRO extends ActionEventValue("ConsumerThreatenByProReportDeletion")
object REFUND_BLACKMAIL extends ActionEventValue("RefundBlackMailReportDeletion")
Expand Down
1 change: 1 addition & 0 deletions test/models/UserRoleTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class UserRoleTest extends Specification {
"get value from string name" in {

UserRole.withName("DGCCRF") shouldEqual UserRole.DGCCRF
UserRole.withName("DGAL") shouldEqual UserRole.DGAL
UserRole.withName("Admin") shouldEqual UserRole.Admin
UserRole.withName("Professionnel") shouldEqual UserRole.Professionnel
Try(UserRole.withName("XXXXXXXXXX")).isFailure shouldEqual true
Expand Down
12 changes: 6 additions & 6 deletions test/orchestrators/AccessesOrchestratorTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class AccessesOrchestratorTest extends Specification with AppSpec {
"email validation should fail when token not found" >> {
val unknownToken = ""
components.accessesOrchestrator
.validateDGCCRFEmail(unknownToken) must throwA[DGCCRFActivationTokenNotFound].await
.validateAgentEmail(unknownToken) must throwA[AgentActivationTokenNotFound].await
}

"email validation should fail when no validation email token found" >> {
Expand All @@ -109,11 +109,11 @@ class AccessesOrchestratorTest extends Specification with AppSpec {
)
val result = for {
_ <- components.accessTokenRepository.create(nonRelevantToken)
res <- components.accessesOrchestrator.validateDGCCRFEmail(nonRelevantToken.token)
res <- components.accessesOrchestrator.validateAgentEmail(nonRelevantToken.token)

} yield res

result must throwA[DGCCRFActivationTokenNotFound].await
result must throwA[AgentActivationTokenNotFound].await
}

"email validation should fail when validation email token found not link to any email" >> {
Expand All @@ -128,7 +128,7 @@ class AccessesOrchestratorTest extends Specification with AppSpec {

val result = for {
_ <- components.accessTokenRepository.create(invalidValidationEmailToken)
res <- components.accessesOrchestrator.validateDGCCRFEmail(invalidValidationEmailToken.token)
res <- components.accessesOrchestrator.validateAgentEmail(invalidValidationEmailToken.token)

} yield res

Expand All @@ -147,7 +147,7 @@ class AccessesOrchestratorTest extends Specification with AppSpec {

val result = for {
_ <- components.accessTokenRepository.create(validationEmailToken)
res <- components.accessesOrchestrator.validateDGCCRFEmail(validationEmailToken.token)
res <- components.accessesOrchestrator.validateAgentEmail(validationEmailToken.token)

} yield res

Expand All @@ -170,7 +170,7 @@ class AccessesOrchestratorTest extends Specification with AppSpec {
val result = for {
_ <- components.accessTokenRepository.create(validationEmailToken)
_ <- components.userRepository.create(dgccrfUser)
res <- components.accessesOrchestrator.validateDGCCRFEmail(validationEmailToken.token)
res <- components.accessesOrchestrator.validateAgentEmail(validationEmailToken.token)

} yield res

Expand Down
8 changes: 4 additions & 4 deletions test/repositories/UserRepositorySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class UserRepositorySpec(implicit ee: ExecutionEnv) extends Specification with A
userId = Some(inactiveDgccrfUserWithEmails.id),
creationDate = now,
eventType = EventType.DGCCRF,
action = ActionEvent.EMAIL_INACTIVE_DGCCRF_ACCOUNT,
action = ActionEvent.EMAIL_INACTIVE_AGENT_ACCOUNT,
details = Json.obj()
)
),
Expand All @@ -79,7 +79,7 @@ class UserRepositorySpec(implicit ee: ExecutionEnv) extends Specification with A
userId = Some(inactiveDgccrfUserWithEmails.id),
creationDate = now,
eventType = EventType.DGCCRF,
action = ActionEvent.EMAIL_INACTIVE_DGCCRF_ACCOUNT,
action = ActionEvent.EMAIL_INACTIVE_AGENT_ACCOUNT,
details = Json.obj()
)
),
Expand Down Expand Up @@ -113,12 +113,12 @@ class UserRepositorySpec(implicit ee: ExecutionEnv) extends Specification with A
def e4 = userRepository.get(userToto.id).map(_.isDefined) must beFalse.await

def e6 = userRepository
.listInactiveDGCCRFWithSentEmailCount(now.minusMonths(1), now.minusYears(1))
.listInactiveAgentsWithSentEmailCount(now.minusMonths(1), now.minusYears(1))
.map(_.map { case (user, count) => (user.id, count) }) must beEqualTo(
List(inactiveDgccrfUser.id -> None, inactiveDgccrfUserWithEmails.id -> Some(2))
).await
def e7 = userRepository
.listInactiveDGCCRFWithSentEmailCount(now.minusMonths(1), now.minusMonths(2)) must beEmpty[List[
.listInactiveAgentsWithSentEmailCount(now.minusMonths(1), now.minusMonths(2)) must beEmpty[List[
(User, Option[Int])
]].await

Expand Down
Loading

0 comments on commit 8d17d24

Please sign in to comment.