[NU-1772] Refactor ScenarioActivity state handling (#7007)

designer/deployment-manager-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivityManager.scala

@@ -1,41 +1,19 @@
 package pl.touk.nussknacker.engine.api.deployment
 
-import pl.touk.nussknacker.engine.api.deployment.ScenarioActivityManager.ModificationResult
-
 import scala.concurrent.Future
 
 trait ScenarioActivityManager {
 
   def saveActivity(
-      scenarioActivity: ScenarioActivity
+      activity: DeploymentRelatedActivity
   ): Future[Unit]
 
-  def modifyActivity(
-      scenarioActivityId: ScenarioActivityId,
-      modify: ScenarioActivity => ScenarioActivity,
-  ): Future[ModificationResult]
-
-}
-
-object ScenarioActivityManager {
-  sealed trait ModificationResult
-
-  object ModificationResult {
-    case object Success extends ModificationResult
-    case object Failure extends ModificationResult
-  }
-
 }
 
 object NoOpScenarioActivityManager extends ScenarioActivityManager {
 
   def saveActivity(
-      scenarioActivity: ScenarioActivity
+      activity: DeploymentRelatedActivity
   ): Future[Unit] = Future.unit
 
-  def modifyActivity(
-      scenarioActivityId: ScenarioActivityId,
-      modify: ScenarioActivity => ScenarioActivity,
-  ): Future[ModificationResult] = Future.successful(ModificationResult.Success)
-
 }

designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala

@@ -6,12 +6,7 @@ import pl.touk.nussknacker.engine.api.deployment.ScenarioActivityHandling.{
   AllScenarioActivitiesStoredByNussknacker,
   ManagerSpecificScenarioActivitiesStoredByManager
 }
-import pl.touk.nussknacker.engine.api.deployment.{
-  ScenarioActivity,
-  ScenarioActivityId,
-  ScenarioAttachment,
-  ScenarioComment
-}
+import pl.touk.nussknacker.engine.api.deployment.{ScenarioActivity, _}
 import pl.touk.nussknacker.engine.api.process.{ProcessId, ProcessIdWithName, ProcessName}
 import pl.touk.nussknacker.security.Permission
 import pl.touk.nussknacker.security.Permission.Permission
@@ -233,9 +228,22 @@ class ScenarioActivityApiHttpService(
           case None =>
             Future.successful(List.empty)
         }
-        combinedActivities       = (generalActivities ++ deploymentManagerSpecificActivities).map(toDto)
-        sortedCombinedActivities = combinedActivities.toList.sortBy(_.date)
-      } yield sortedCombinedActivities
+        combinedActivities = generalActivities ++ deploymentManagerSpecificActivities
+        //  The API endpoint returning scenario activities does not yet have support for filtering. We made a decision to:
+        //  - for activities not related to deployments:        always display them on FE
+        //  - for activities related to batch deployments:      always display them on FE
+        //  - for activities related to non-batch deployments:  display on FE only those, that represent successful operations
+        combinedSuccessfulActivities = combinedActivities.filter {
+          case _: BatchDeploymentRelatedActivity => true
+          case activity: DeploymentRelatedActivity =>
+            activity.result match {
+              case _: DeploymentResult.Success => true
+              case _: DeploymentResult.Failure => false
+            }
+          case _ => true
+        }
+        sortedResult = combinedSuccessfulActivities.map(toDto).toList.sortBy(_.date)
+      } yield sortedResult
     }
 
   private def toDto(scenarioComment: ScenarioComment): Dtos.ScenarioActivityComment = {
@@ -297,36 +305,37 @@ class ScenarioActivityApiHttpService(
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value)
         )
-      case ScenarioActivity.ScenarioDeployed(_, scenarioActivityId, user, date, scenarioVersionId, comment) =>
+      case ScenarioActivity.ScenarioDeployed(_, scenarioActivityId, user, date, scenarioVersionId, comment, _) =>
         Dtos.ScenarioActivity.forScenarioDeployed(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value),
           comment = toDto(comment),
         )
-      case ScenarioActivity.ScenarioPaused(_, scenarioActivityId, user, date, scenarioVersionId, comment) =>
+      case ScenarioActivity.ScenarioPaused(_, scenarioActivityId, user, date, scenarioVersionId, comment, _) =>
         Dtos.ScenarioActivity.forScenarioPaused(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value),
           comment = toDto(comment),
         )
-      case ScenarioActivity.ScenarioCanceled(_, scenarioActivityId, user, date, scenarioVersionId, comment) =>
+      case ScenarioActivity.ScenarioCanceled(_, scenarioActivityId, user, date, scenarioVersionId, comment, _) =>
         Dtos.ScenarioActivity.forScenarioCanceled(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value),
           comment = toDto(comment),
         )
-      case ScenarioActivity.ScenarioModified(_, scenarioActivityId, user, date, scenarioVersionId, comment) =>
+      case ScenarioActivity.ScenarioModified(_, scenarioActivityId, user, date, oldVersionId, newVersionId, comment) =>
         Dtos.ScenarioActivity.forScenarioModified(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
-          scenarioVersionId = scenarioVersionId.map(_.value),
+          previousScenarioVersionId = oldVersionId.map(_.value),
+          scenarioVersionId = newVersionId.map(_.value),
           comment = toDto(comment),
         )
       case ScenarioActivity.ScenarioNameChanged(_, id, user, date, version, oldName, newName) =>
@@ -406,29 +415,29 @@ class ScenarioActivityApiHttpService(
             date,
             scenarioVersionId,
             comment,
-            dateFinished,
-            status,
-            errorMessage,
+            result,
           ) =>
         Dtos.ScenarioActivity.forPerformedSingleExecution(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value),
           comment = toDto(comment),
-          dateFinished = dateFinished,
-          status = status,
-          errorMessage = errorMessage,
+          dateFinished = result.dateFinished,
+          errorMessage = result match {
+            case DeploymentResult.Success(_)               => None
+            case DeploymentResult.Failure(_, errorMessage) => errorMessage
+          },
         )
       case ScenarioActivity.PerformedScheduledExecution(
             _,
             scenarioActivityId,
             user,
             date,
             scenarioVersionId,
+            scheduledExecutionStatus,
             dateFinished,
             scheduleName,
-            status,
             createdAt,
             nextRetryAt,
             retriesLeft,
@@ -440,7 +449,7 @@ class ScenarioActivityApiHttpService(
           scenarioVersionId = scenarioVersionId.map(_.value),
           dateFinished = dateFinished,
           scheduleName = scheduleName,
-          status = status,
+          scheduledExecutionStatus = scheduledExecutionStatus,
           createdAt = createdAt,
           retriesLeft = retriesLeft,
           nextRetryAt = nextRetryAt,
@@ -452,17 +461,24 @@ class ScenarioActivityApiHttpService(
             date,
             scenarioVersionId,
             changes,
-            errorMessage,
           ) =>
         Dtos.ScenarioActivity.forAutomaticUpdate(
           id = scenarioActivityId.value,
           user = user.name.value,
           date = date,
           scenarioVersionId = scenarioVersionId.map(_.value),
           changes = changes,
-          errorMessage = errorMessage,
         )
-      case ScenarioActivity.CustomAction(_, scenarioActivityId, user, date, scenarioVersionId, actionName, comment) =>
+      case ScenarioActivity.CustomAction(
+            _,
+            scenarioActivityId,
+            user,
+            date,
+            scenarioVersionId,
+            actionName,
+            comment,
+            result,
+          ) =>
         Dtos.ScenarioActivity.forCustomAction(
           id = scenarioActivityId.value,
           user = user.name.value,
@@ -471,6 +487,10 @@ class ScenarioActivityApiHttpService(
           actionName = actionName,
           comment = toDto(comment),
           customIcon = None,
+          errorMessage = result match {
+            case DeploymentResult.Success(_)               => None
+            case DeploymentResult.Failure(_, errorMessage) => errorMessage
+          },
         )
     }
   }

designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/scenarioActivity/Dtos.scala

@@ -143,7 +143,7 @@ object Dtos {
     }
 
     case object ScenarioModified extends ScenarioActivityType {
-      override def displayableName: String        = "New version saved"
+      override def displayableName: String        = "Scenario modified"
       override def icon: String                   = "/assets/activities/scenarioModified.svg"
       override def supportedActions: List[String] = commentRelatedActions ::: "compare" :: Nil
     }
@@ -442,6 +442,7 @@ object Dtos {
         id: UUID,
         user: String,
         date: Instant,
+        previousScenarioVersionId: Option[Long],
         scenarioVersionId: Option[Long],
         comment: ScenarioActivityComment,
     ): ScenarioActivity = ScenarioActivity(
@@ -453,9 +454,19 @@ object Dtos {
       comment = Some(comment),
       attachment = None,
       additionalFields = List.empty,
-      overrideDisplayableName = scenarioVersionId.map(version => s"Version $version saved"),
+      overrideDisplayableName = updatedVersionId(previousScenarioVersionId, scenarioVersionId).map(updatedVersion =>
+        s"Version $updatedVersion saved"
+      )
     )
 
+    private def updatedVersionId(oldVersionIdOpt: Option[Long], newVersionIdOpt: Option[Long]) = {
+      for {
+        newVersionId <- newVersionIdOpt
+        oldVersionIdOrZero = oldVersionIdOpt.getOrElse(0L)
+        updatedVersionId <- if (newVersionId > oldVersionIdOrZero) Some(newVersionId) else None
+      } yield updatedVersionId
+    }
+
     def forScenarioNameChanged(
         id: UUID,
         user: String,
@@ -586,8 +597,7 @@ object Dtos {
         date: Instant,
         scenarioVersionId: Option[Long],
         comment: ScenarioActivityComment,
-        dateFinished: Option[Instant],
-        status: Option[String],
+        dateFinished: Instant,
         errorMessage: Option[String],
     ): ScenarioActivity = ScenarioActivity(
       id = id,
@@ -598,8 +608,7 @@ object Dtos {
       comment = Some(comment),
       attachment = None,
       additionalFields = List(
-        status.map(AdditionalField("status", _)),
-        dateFinished.map(date => AdditionalField("dateFinished", date.toString)),
+        Some(AdditionalField("dateFinished", dateFinished.toString)),
         errorMessage.map(e => AdditionalField("errorMessage", e)),
       ).flatten
     )
@@ -609,16 +618,14 @@ object Dtos {
         user: String,
         date: Instant,
         scenarioVersionId: Option[Long],
-        dateFinished: Option[Instant],
+        dateFinished: Instant,
         scheduleName: String,
-        status: ScheduledExecutionStatus,
+        scheduledExecutionStatus: ScheduledExecutionStatus,
         createdAt: Instant,
         nextRetryAt: Option[Instant],
         retriesLeft: Option[Int],
     ): ScenarioActivity = {
-      val humanReadableStatus = status match {
-        case ScheduledExecutionStatus.Scheduled               => "Scheduled"
-        case ScheduledExecutionStatus.Deployed                => "Deployed"
+      val humanReadableStatus = scheduledExecutionStatus match {
         case ScheduledExecutionStatus.Finished                => "Execution finished"
         case ScheduledExecutionStatus.Failed                  => "Execution failed"
         case ScheduledExecutionStatus.DeploymentWillBeRetried => "Deployment will be retried"
@@ -635,7 +642,7 @@ object Dtos {
         additionalFields = List(
           Some(AdditionalField("status", humanReadableStatus)),
           Some(AdditionalField("createdAt", createdAt.toString)),
-          dateFinished.map(date => AdditionalField("dateFinished", date.toString)),
+          Some(AdditionalField("dateFinished", dateFinished.toString)),
           Some(AdditionalField("scheduleName", scheduleName)),
           Some(AdditionalField("retriesLeft", retriesLeft.toString)),
           nextRetryAt.map(nra => AdditionalField("nextRetryAt", nra.toString)),
@@ -651,7 +658,6 @@ object Dtos {
         date: Instant,
         scenarioVersionId: Option[Long],
         changes: String,
-        errorMessage: Option[String],
     ): ScenarioActivity = ScenarioActivity(
       id = id,
       `type` = ScenarioActivityType.AutomaticUpdate,
@@ -661,9 +667,8 @@ object Dtos {
       comment = None,
       attachment = None,
       additionalFields = List(
-        Some(AdditionalField("changes", changes)),
-        errorMessage.map(e => AdditionalField("errorMessage", e)),
-      ).flatten
+        AdditionalField("changes", changes),
+      ),
     )
 
     def forCustomAction(
@@ -674,6 +679,7 @@ object Dtos {
         comment: ScenarioActivityComment,
         actionName: String,
         customIcon: Option[String],
+        errorMessage: Option[String],
     ): ScenarioActivity = ScenarioActivity(
       id = id,
       `type` = ScenarioActivityType.CustomAction,
@@ -683,8 +689,9 @@ object Dtos {
       comment = Some(comment),
       attachment = None,
       additionalFields = List(
-        AdditionalField("actionName", actionName),
-      ),
+        Some(AdditionalField("actionName", actionName)),
+        errorMessage.map(e => AdditionalField("errorMessage", e)),
+      ).flatten,
       overrideIcon = customIcon,
     )