review changes

TouK · Nov 13, 2024 · a435bd4 · a435bd4
1 parent 071be85
commit a435bd4
Show file tree

Hide file tree

Showing 12 changed files with 45 additions and 56 deletions.
diff --git a/common-api/src/main/scala/pl/touk/nussknacker/engine/api/Comment.scala b/common-api/src/main/scala/pl/touk/nussknacker/engine/api/Comment.scala
@@ -7,7 +7,8 @@ class Comment private (val content: String) extends AnyVal {
 object Comment {
 
   def from(content: String): Option[Comment] = {
-    if (content.trim.nonEmpty) Some(new Comment(content)) else None
+    val trimmedContent = content.trim
+    if (trimmedContent.nonEmpty) Some(new Comment(trimmedContent)) else None
   }
 
   def unsafeFrom(content: String): Comment = {

diff --git a/...ner/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala b/...ner/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala
@@ -272,7 +272,7 @@ class ScenarioActivityApiHttpService(
   private def toDto(scenarioComment: ScenarioComment): Dtos.ScenarioActivityComment = scenarioComment match {
     case ScenarioComment.WithContent(comment, _, _) =>
       Dtos.ScenarioActivityComment(
-        content = Dtos.ScenarioActivityCommentContent.Available(comment),
+        content = Dtos.ScenarioActivityCommentContent.Available(comment.content),
         lastModifiedBy = scenarioComment.lastModifiedByUserName.value,
         lastModifiedAt = scenarioComment.lastModifiedAt,
       )

diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioActivityAuditLog.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ScenarioActivityAuditLog.scala
@@ -182,7 +182,7 @@ object ScenarioActivityAuditLog {
   }
 
   private def stringify(comment: ScenarioComment): String = comment match {
-    case ScenarioComment.WithContent(comment, _, _) => comment
+    case ScenarioComment.WithContent(comment, _, _) => comment.content
     case ScenarioComment.WithoutContent(_, _)       => "none"
   }
 

diff --git a/...er/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityService.scala b/...er/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityService.scala
@@ -75,7 +75,7 @@ class ActivityService(
               user = loggedUser.scenarioUser,
               date = now,
               scenarioVersionId = Some(ScenarioVersionId.from(scenarioGraphVersionId)),
-              comment = ScenarioComment.from(commentOpt.map(_.content), UserName(loggedUser.username), now),
+              comment = ScenarioComment.from(commentOpt, UserName(loggedUser.username), now),
               result = DeploymentResult.Success(clock.instant()),
             )
           )

diff --git a/...r/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala b/...r/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala
@@ -212,7 +212,7 @@ class DBProcessRepository(
           previousScenarioVersionId = oldVersionId.map(ScenarioVersionId.from),
           scenarioVersionId = versionId.map(ScenarioVersionId.from),
           comment = ScenarioComment.from(
-            content = updateProcessAction.comment.map(_.content),
+            content = updateProcessAction.comment,
             lastModifiedByUserName = UserName(loggedUser.username),
             lastModifiedAt = clock.instant(),
           )

diff --git a/...a/pl/touk/nussknacker/ui/process/repository/activities/DbScenarioActivityRepository.scala b/...a/pl/touk/nussknacker/ui/process/repository/activities/DbScenarioActivityRepository.scala
@@ -3,6 +3,7 @@ package pl.touk.nussknacker.ui.process.repository.activities
 import cats.implicits.catsSyntaxEitherId
 import com.typesafe.scalalogging.LazyLogging
 import db.util.DBIOActionInstances.DB
+import pl.touk.nussknacker.engine.api.Comment
 import pl.touk.nussknacker.engine.api.component.ProcessingMode
 import pl.touk.nussknacker.engine.api.deployment.ScenarioAttachment.{AttachmentFilename, AttachmentId}
 import pl.touk.nussknacker.engine.api.deployment._
@@ -292,8 +293,7 @@ class DbScenarioActivityRepository private (override protected val dbRef: DbRef,
         toComment(
           id,
           activity,
-          ScenarioComment
-            .from(Some(s"Rename: [${activity.oldName}] -> [${activity.newName}]"), UserName(""), activity.date),
+          ScenarioComment.from(s"Rename: [${activity.oldName}] -> [${activity.newName}]", UserName(""), activity.date),
           None
         )
       case activity: ScenarioActivity.CommentAdded =>
@@ -307,8 +307,7 @@ class DbScenarioActivityRepository private (override protected val dbRef: DbRef,
           id,
           activity,
           ScenarioComment.from(
-            content =
-              Some(s"Scenario migrated from ${activity.sourceEnvironment.name} by ${activity.sourceUser.value}"),
+            content = s"Scenario migrated from ${activity.sourceEnvironment.name} by ${activity.sourceUser.value}",
             lastModifiedByUserName = activity.user.name,
             lastModifiedAt = activity.date
           ),
@@ -325,7 +324,7 @@ class DbScenarioActivityRepository private (override protected val dbRef: DbRef,
           id,
           activity,
           ScenarioComment.from(
-            content = Some(s"Migrations applied: ${activity.changes}"),
+            content = s"Migrations applied: ${activity.changes}",
             lastModifiedByUserName = activity.user.name,
             lastModifiedAt = activity.date
           ),
@@ -506,8 +505,8 @@ class DbScenarioActivityRepository private (override protected val dbRef: DbRef,
 
   private def comment(scenarioComment: ScenarioComment): Option[String] = {
     scenarioComment match {
-      case ScenarioComment.WithContent(comment, _, _) => Some(comment)
-      case _                                          => None
+      case ScenarioComment.WithContent(comment, _, _) => Some(comment.content)
+      case ScenarioComment.WithoutContent(_, _)       => None
     }
   }
 
@@ -678,7 +677,7 @@ class DbScenarioActivityRepository private (override protected val dbRef: DbRef,
       lastModifiedByUserName <- entity.lastModifiedByUserName.toRight("Missing lastModifiedByUserName field")
       lastModifiedAt         <- entity.lastModifiedAt.toRight("Missing lastModifiedAt field")
     } yield ScenarioComment.from(
-      content = entity.comment,
+      content = entity.comment.flatMap(Comment.from),
       lastModifiedByUserName = UserName(lastModifiedByUserName),
       lastModifiedAt = lastModifiedAt.toInstant
     )

diff --git a/...ala/pl/touk/nussknacker/ui/process/repository/activities/ScenarioActivityRepository.scala b/...ala/pl/touk/nussknacker/ui/process/repository/activities/ScenarioActivityRepository.scala
@@ -1,6 +1,7 @@
 package pl.touk.nussknacker.ui.process.repository.activities
 
 import db.util.DBIOActionInstances.DB
+import pl.touk.nussknacker.engine.api.Comment
 import pl.touk.nussknacker.engine.api.deployment._
 import pl.touk.nussknacker.engine.api.process.{ProcessId, VersionId}
 import pl.touk.nussknacker.ui.api.description.scenarioActivity.Dtos.Legacy
@@ -42,7 +43,7 @@ trait ScenarioActivityRepository {
         date = now,
         scenarioVersionId = Some(ScenarioVersionId.from(processVersionId)),
         comment = ScenarioComment.from(
-          content = Some(comment),
+          content = comment,
           lastModifiedByUserName = UserName(user.username),
           lastModifiedAt = now,
         )

diff --git a/...r/src/test/scala/db/migration/V1_057__MigrateActionsAndCommentsToScenarioActivities.scala b/...r/src/test/scala/db/migration/V1_057__MigrateActionsAndCommentsToScenarioActivities.scala
@@ -123,7 +123,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
             user = user,
             date = date,
             scenarioVersionId = sv,
-            comment = ScenarioComment.from(Some("Deployment with scenario fix"), user.name, date),
+            comment = ScenarioComment.from("Deployment with scenario fix", user.name, date),
             result = DeploymentResult.Success(date),
           )
       )
@@ -139,7 +139,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
             user = user,
             date = date,
             scenarioVersionId = sv,
-            comment = ScenarioComment.from(Some("I'm canceling this scenario, it causes problems"), user.name, date),
+            comment = ScenarioComment.from("I'm canceling this scenario, it causes problems", user.name, date),
             result = DeploymentResult.Success(date),
           )
       )
@@ -183,8 +183,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
             user = user,
             date = date,
             scenarioVersionId = sv,
-            comment =
-              ScenarioComment.from(Some("Paused because marketing campaign is paused for now"), user.name, date),
+            comment = ScenarioComment.from("Paused because marketing campaign is paused for now", user.name, date),
             result = DeploymentResult.Success(date),
           )
       )
@@ -240,7 +239,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
             user = user,
             date = date,
             scenarioVersionId = sv,
-            comment = ScenarioComment.from(Some("Deployed at the request of business"), user.name, date),
+            comment = ScenarioComment.from("Deployed at the request of business", user.name, date),
             result = DeploymentResult.Success(date),
           )
       )
@@ -257,7 +256,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
             date = date,
             scenarioVersionId = sv,
             actionName = "special action",
-            comment = ScenarioComment.from(Some("Special action needed to be executed"), user.name, date),
+            comment = ScenarioComment.from("Special action needed to be executed", user.name, date),
             result = DeploymentResult.Success(date),
           )
       )
@@ -282,15 +281,15 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities
           user = ScenarioUser(None, UserName("John Doe"), None, None),
           date = now.toInstant,
           scenarioVersionId = Some(ScenarioVersionId(processVersionId)),
-          comment = ScenarioComment.from(Some("ABC1"), UserName(user), now.toInstant)
+          comment = ScenarioComment.from("ABC1", UserName(user), now.toInstant)
         ),
         ScenarioActivity.CommentAdded(
           scenarioId = ScenarioId(process.id.value),
           scenarioActivityId = activities(1).scenarioActivityId,
           user = ScenarioUser(None, UserName("John Doe"), None, None),
           date = now.toInstant,
           scenarioVersionId = Some(ScenarioVersionId(processVersionId)),
-          comment = ScenarioComment.from(Some("ABC2"), UserName(user), now.toInstant)
+          comment = ScenarioComment.from("ABC2", UserName(user), now.toInstant)
         )
       )
     }

diff --git a/...erver/src/test/scala/db/migration/V1_058__UpdateAndAddMissingScenarioActivitiesSpec.scala b/...erver/src/test/scala/db/migration/V1_058__UpdateAndAddMissingScenarioActivitiesSpec.scala
@@ -70,7 +70,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
             date = Instant.now,
             scenarioVersionId = None,
             comment = ScenarioComment.from(
-              content = Some("Scenario migrated from TEST_ENV by test env user"),
+              content = "Scenario migrated from TEST_ENV by test env user",
               lastModifiedByUserName = UserName(loggedUser.username),
               lastModifiedAt = Instant.now
             )
@@ -87,7 +87,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
           date = activities(0).date,
           previousScenarioVersionId = None,
           scenarioVersionId = Some(ScenarioVersionId(2)),
-          comment = ScenarioComment.from(None, UserName("Test User"), activities(0).date)
+          comment = ScenarioComment.WithoutContent(UserName("Test User"), activities(0).date)
         ),
         ScenarioActivity.ScenarioCreated(
           scenarioId = ScenarioId(process.id.value),
@@ -129,7 +129,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
             previousScenarioVersionId = None,
             scenarioVersionId = None,
             comment = ScenarioComment.from(
-              content = Some("Scenario migrated from TEST_ENV by test env user"),
+              content = "Scenario migrated from TEST_ENV by test env user",
               lastModifiedByUserName = UserName(loggedUser.username),
               lastModifiedAt = Instant.now
             )
@@ -178,7 +178,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
             date = Instant.now,
             scenarioVersionId = None,
             comment = ScenarioComment.from(
-              content = Some("Migrations applied: feature A\\nfeature B\\nfeature C"),
+              content = "Migrations applied: feature A\\nfeature B\\nfeature C",
               lastModifiedByUserName = UserName(loggedUser.username),
               lastModifiedAt = Instant.now
             )
@@ -227,7 +227,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
             previousScenarioVersionId = None,
             scenarioVersionId = None,
             comment = ScenarioComment.from(
-              content = Some("Migrations applied: feature A\\nfeature B\\nfeature C"),
+              content = "Migrations applied: feature A\\nfeature B\\nfeature C",
               lastModifiedByUserName = UserName(loggedUser.username),
               lastModifiedAt = Instant.now
             )
@@ -275,7 +275,7 @@ class V1_058__UpdateAndAddMissingScenarioActivitiesSpec
             date = Instant.now,
             scenarioVersionId = None,
             comment = ScenarioComment.from(
-              content = Some("Rename: [oldUglyName] -> [newPrettyName]"),
+              content = "Rename: [oldUglyName] -> [newPrettyName]",
               lastModifiedByUserName = UserName(loggedUser.username),
               lastModifiedAt = Instant.now
             )

diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/test/mock/MockDeploymentManager.scala b/designer/server/src/test/scala/pl/touk/nussknacker/test/mock/MockDeploymentManager.scala
@@ -103,7 +103,7 @@ class MockDeploymentManager(
           scenarioVersionId = Some(ScenarioVersionId.from(processVersion.versionId)),
           actionName = "Custom action of MockDeploymentManager just before deployment",
           comment = ScenarioComment.from(
-            content = Some("With comment from DeploymentManager"),
+            content = "With comment from DeploymentManager",
             lastModifiedByUserName = ScenarioUser.internalNuUser.name,
             lastModifiedAt = Instant.now()
           ),

diff --git a/...rc/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala b/...rc/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala
@@ -404,7 +404,7 @@ class ScenarioActivityApiHttpServiceBusinessSpec
                 scenarioVersionId = None,
                 actionName = "Custom action handled by deployment manager",
                 comment = ScenarioComment.from(
-                  content = Some("Executed on custom deployment manager"),
+                  content = "Executed on custom deployment manager",
                   lastModifiedByUserName = UserName("custom-user"),
                   lastModifiedAt = clock.instant()
                 ),
@@ -418,7 +418,7 @@ class ScenarioActivityApiHttpServiceBusinessSpec
                 scenarioVersionId = None,
                 actionName = "Custom action handled by deployment manager",
                 comment = ScenarioComment.from(
-                  content = Some("Executed on custom deployment manager"),
+                  content = "Executed on custom deployment manager",
                   lastModifiedByUserName = UserName("custom-user"),
                   lastModifiedAt = clock.instant()
                 ),
@@ -482,7 +482,7 @@ class ScenarioActivityApiHttpServiceBusinessSpec
                 date = clock.instant(),
                 scenarioVersionId = None,
                 comment = ScenarioComment.from(
-                  content = Some("Immediate execution"),
+                  content = "Immediate execution",
                   lastModifiedByUserName = UserName("custom-user"),
                   lastModifiedAt = clock.instant()
                 ),

diff --git a/...sions-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivity.scala b/...sions-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivity.scala
@@ -2,6 +2,7 @@ package pl.touk.nussknacker.engine.api.deployment
 
 import enumeratum.EnumEntry.UpperSnakecase
 import enumeratum.{Enum, EnumEntry}
+import pl.touk.nussknacker.engine.api.Comment
 import pl.touk.nussknacker.engine.api.component.ProcessingMode
 import pl.touk.nussknacker.engine.api.process.VersionId
 
@@ -44,49 +45,37 @@ sealed trait ScenarioComment {
 
 object ScenarioComment {
 
-  final case class WithContent private (
-      comment: String,
+  final case class WithContent(
+      comment: Comment,
       lastModifiedByUserName: UserName,
       lastModifiedAt: Instant,
   ) extends ScenarioComment
 
-  object WithContent {
-
-    private[ScenarioComment] def apply(
-        comment: String,
-        lastModifiedByUserName: UserName,
-        lastModifiedAt: Instant
-    ): WithContent = new WithContent(comment, lastModifiedByUserName, lastModifiedAt)
-
-  }
-
-  final case class WithoutContent private (
+  final case class WithoutContent(
       lastModifiedByUserName: UserName,
       lastModifiedAt: Instant,
   ) extends ScenarioComment
 
-  object WithoutContent {
-
-    private[ScenarioComment] def apply(
-        lastModifiedByUserName: UserName,
-        lastModifiedAt: Instant
-    ): WithoutContent = new WithoutContent(lastModifiedByUserName, lastModifiedAt)
-
-  }
+  def from(
+      content: String,
+      lastModifiedByUserName: UserName,
+      lastModifiedAt: Instant,
+  ): ScenarioComment =
+    from(Comment.from(content), lastModifiedByUserName, lastModifiedAt)
 
   def from(
-      content: Option[String],
+      content: Option[Comment],
       lastModifiedByUserName: UserName,
       lastModifiedAt: Instant,
   ): ScenarioComment = {
     content match {
-      case Some(content) if content.trim.nonEmpty =>
+      case Some(content) =>
         WithContent(
           comment = content,
           lastModifiedByUserName = lastModifiedByUserName,
           lastModifiedAt = lastModifiedAt,
         )
-      case Some(_) | None =>
+      case None =>
         WithoutContent(
           lastModifiedByUserName = lastModifiedByUserName,
           lastModifiedAt = lastModifiedAt,