From a435bd4d23ebfb84f3fc5befebd005f576ad9ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Goworko?= Date: Wed, 13 Nov 2024 19:13:24 +0100 Subject: [PATCH] review changes --- .../touk/nussknacker/engine/api/Comment.scala | 3 +- .../api/ScenarioActivityApiHttpService.scala | 2 +- .../ui/process/ScenarioActivityAuditLog.scala | 2 +- .../process/newactivity/ActivityService.scala | 2 +- .../repository/ProcessRepository.scala | 2 +- .../DbScenarioActivityRepository.scala | 15 ++++---- .../ScenarioActivityRepository.scala | 3 +- ...tionsAndCommentsToScenarioActivities.scala | 15 ++++---- ...eAndAddMissingScenarioActivitiesSpec.scala | 12 +++--- .../test/mock/MockDeploymentManager.scala | 2 +- ...ioActivityApiHttpServiceBusinessSpec.scala | 6 +-- .../api/deployment/ScenarioActivity.scala | 37 +++++++------------ 12 files changed, 45 insertions(+), 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 index 42b6a2e7e3d..635f8c8d898 100644 --- 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/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala index ff2e663f036..a669b48acbb 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpService.scala +++ b/designer/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 index 12c1dadd1f9..1ae4b861530 100644 --- 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/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityService.scala index 21ce9aef6af..aeff6e772ad 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityService.scala +++ b/designer/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/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala index 23c7d1c7c5a..505ec16d56f 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala +++ b/designer/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/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/DbScenarioActivityRepository.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/DbScenarioActivityRepository.scala index 031963fb0bb..69623e63bb7 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/DbScenarioActivityRepository.scala +++ b/designer/server/src/main/scala/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/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/ScenarioActivityRepository.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/ScenarioActivityRepository.scala index ba3c9b33f75..563a89c9fad 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/activities/ScenarioActivityRepository.scala +++ b/designer/server/src/main/scala/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/designer/server/src/test/scala/db/migration/V1_057__MigrateActionsAndCommentsToScenarioActivities.scala b/designer/server/src/test/scala/db/migration/V1_057__MigrateActionsAndCommentsToScenarioActivities.scala index 1bf2b06f168..bc9c38f453f 100644 --- a/designer/server/src/test/scala/db/migration/V1_057__MigrateActionsAndCommentsToScenarioActivities.scala +++ b/designer/server/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,7 +281,7 @@ 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), @@ -290,7 +289,7 @@ class V1_057__MigrateActionsAndCommentsToScenarioActivities 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/designer/server/src/test/scala/db/migration/V1_058__UpdateAndAddMissingScenarioActivitiesSpec.scala b/designer/server/src/test/scala/db/migration/V1_058__UpdateAndAddMissingScenarioActivitiesSpec.scala index 6757effefbf..7c4fa9204c8 100644 --- a/designer/server/src/test/scala/db/migration/V1_058__UpdateAndAddMissingScenarioActivitiesSpec.scala +++ b/designer/server/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 index 6a449bdc6f7..dcd25483e6d 100644 --- 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/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala index 3831ff0ed77..dd99b751867 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala +++ b/designer/server/src/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/extensions-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivity.scala b/extensions-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivity.scala index 19efb3b7680..a6b11e130ae 100644 --- a/extensions-api/src/main/scala/pl/touk/nussknacker/engine/api/deployment/ScenarioActivity.scala +++ b/extensions-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,