Do not specify migrations which did not change process in process mig…

designer/server/src/main/scala/pl/touk/nussknacker/ui/db/entity/CommentEntityFactory.scala

@@ -55,22 +55,21 @@ trait CommentActions {
     Sequence[Long]("process_comments_id_sequence").next.result
   }
 
-  def newCommentAction(processId: ProcessId, processVersionId: VersionId, comment: Comment)
+  def newCommentAction(processId: ProcessId, processVersionId: VersionId, comment: Option[Comment])
                       (implicit ec: ExecutionContext, loggedUser: LoggedUser): DB[Option[Long]] = {
-    if (comment.value.nonEmpty) {
-      for {
+    comment match {
+      case Some(c) if c.value.nonEmpty => for {
         newId <- nextIdAction
         _ <- commentsTable += CommentEntityData(
           id = newId,
           processId = processId,
           processVersionId = processVersionId,
-          content = comment.value,
+          content = c.value,
           user = loggedUser.username,
           createDate = Timestamp.from(Instant.now())
         )
       } yield Some(newId)
-    } else {
-      DBIO.successful(None)
+      case _ => DBIO.successful(None)
     }
   }
 

designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala

@@ -243,7 +243,7 @@ class DBProcessService(managerActor: ActorRef,
         }
         processUpdated <- EitherT(repositoryManager
           .runInTransaction(processRepository
-            .updateProcess(UpdateProcessAction(processIdWithName.id, substituted, action.comment, increaseVersionWhenJsonNotChanged = false))
+            .updateProcess(UpdateProcessAction(processIdWithName.id, substituted, Option(action.comment), increaseVersionWhenJsonNotChanged = false))
           ))
       } yield UpdateProcessResponse(
         processUpdated

designer/server/src/main/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigrator.scala

@@ -16,7 +16,7 @@ case class MigrationResult(process: CanonicalProcess, migrationsApplied: List[Pr
   def toUpdateAction(processId: ProcessId): UpdateProcessAction = UpdateProcessAction(
     id = processId,
     canonicalProcess = process,
-    comment = MigrationComment(migrationsApplied),
+    comment = Option(migrationsApplied).filter(_.nonEmpty).map(MigrationComment),
     increaseVersionWhenJsonNotChanged = true
   )
 
@@ -39,10 +39,13 @@ class ProcessModelMigrator(migrations: ProcessingTypeDataProvider[ProcessMigrati
   }
 
   private def migrateWithMigrations(process: CanonicalProcess, migrationsToApply: List[ProcessMigration]): MigrationResult = {
-    val resultProcess = migrationsToApply.foldLeft(process) {
-      case (processToConvert, migration) => migration.migrateProcess(processToConvert)
+    val (resultProcess, migrationsApplied) = migrationsToApply.foldLeft((process, Nil: List[ProcessMigration])) {
+      case ((processToConvert, migrationsAppliedAcc), migration) =>
+        val migrated = migration.migrateProcess(processToConvert)
+        val migrationsApplied = if (migrated != processToConvert) migration :: migrationsAppliedAcc else migrationsAppliedAcc
+        (migrated, migrationsApplied)
     }
-    MigrationResult(resultProcess, migrationsToApply)
+    MigrationResult(resultProcess, migrationsApplied.reverse)
   }
 
 }

designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/DbProcessActivityRepository.scala

@@ -30,7 +30,7 @@ case class DbProcessActivityRepository(dbConfig: DbConfig)
 
   override def addComment(processId: ProcessId, processVersionId: VersionId, comment: CommentValue)
                 (implicit ec: ExecutionContext, loggedUser: LoggedUser): Future[Unit] = {
-    run(newCommentAction(processId, processVersionId, comment)).map(_ => ())
+    run(newCommentAction(processId, processVersionId, Option(comment))).map(_ => ())
   }
 
   override def deleteComment(commentId: Long)(implicit ec: ExecutionContext): Future[Unit] = {

designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessActionRepository.scala

@@ -56,7 +56,7 @@ extends BasicRepository with EspTables with CommentActions with ProcessActionRep
   //FIXME: Use ProcessVersionId instead of Long at processVersion
   private def action(processId: ProcessId, processVersion: VersionId, comment: Option[Comment], action: ProcessActionType, buildInfo: Option[String])(implicit user: LoggedUser) =
     for {
-      commentId <- withComment(processId, processVersion, comment)
+      commentId <- newCommentAction(processId, processVersion, comment)
       processActionData = ProcessActionEntityData(
         processId = processId,
         processVersionId = processVersion,
@@ -68,9 +68,4 @@ extends BasicRepository with EspTables with CommentActions with ProcessActionRep
       )
       _ <- processActionsTable += processActionData
     } yield processActionData
-
-  private def withComment(processId: ProcessId, processVersion: VersionId, comment: Option[Comment])(implicit ec: ExecutionContext, user: LoggedUser): DB[Option[Long]] = comment match {
-    case None => DBIOAction.successful(None)
-    case Some(comm) => newCommentAction(processId, processVersion, comm)
-  }
 }

designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessRepository.scala

@@ -31,7 +31,7 @@ object ProcessRepository {
   def create(dbConfig: DbConfig, modelData: ProcessingTypeDataProvider[ModelData]): DBProcessRepository =
     new DBProcessRepository(dbConfig, modelData.mapValues(_.migrations.version))
 
-  case class UpdateProcessAction(id: ProcessId, canonicalProcess: CanonicalProcess, comment: Comment, increaseVersionWhenJsonNotChanged: Boolean)
+  case class UpdateProcessAction(id: ProcessId, canonicalProcess: CanonicalProcess, comment: Option[Comment], increaseVersionWhenJsonNotChanged: Boolean)
 
   case class CreateProcessAction(processName: ProcessName, category: String, canonicalProcess: CanonicalProcess, processingType: ProcessingType, isSubprocess: Boolean)
 
@@ -191,7 +191,7 @@ class DBProcessRepository(val dbConfig: DbConfig, val modelVersion: ProcessingTy
       .filter(_.processId === process.id)
       .sortBy(_.id.desc)
       .result.headOption.flatMap {
-      case Some(version) => newCommentAction(process.id, version.id, UpdateProcessComment(s"Rename: [${process.name.value}] -> [$newName]"))
+      case Some(version) => newCommentAction(process.id, version.id, Some(UpdateProcessComment(s"Rename: [${process.name.value}] -> [$newName]")))
       case None =>  DBIO.successful(())
     }
 

designer/server/src/test/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigratorSpec.scala

@@ -24,13 +24,20 @@ class ProcessModelMigratorSpec extends AnyFlatSpec with BeforeAndAfterEach with
 
   private implicit val user = TestFactory.adminUser("test1")
 
+  it should "return only migrations that changed process in migrationsApplied in MigrationResult" in {
+
+    val migrationResult: MigrationResult = migrateByVersions(None, 1, 2, 3, 4, 6, 7, 8, 9)
+
+    migrationResult.migrationsApplied.map(_.description) should contain theSameElementsAs List("testMigration1", "testMigration2", "testMigration8")
+  }
+
   it should "migrate processes to new versions when not migrated" in {
 
     val migrationResult: MigrationResult = migrateByVersions(None, 1, 2)
 
     extractParallelism(migrationResult) shouldBe 11
 
-    migrationResult.toUpdateAction(ProcessId(1L)).comment shouldBe MigrationComment(migrationResult.migrationsApplied)
+    migrationResult.toUpdateAction(ProcessId(1L)).comment shouldBe Some(MigrationComment(migrationResult.migrationsApplied))
 
     val processor = extractProcessor(migrationResult)
     processor shouldBe ServiceRef(ProcessTestData.otherExistingServiceId, List())
@@ -51,7 +58,7 @@ class ProcessModelMigratorSpec extends AnyFlatSpec with BeforeAndAfterEach with
     extractParallelism(migrationResult) shouldBe 11
 
     val processor = extractProcessor(migrationResult)
-    migrationResult.toUpdateAction(ProcessId(1L)).comment shouldBe MigrationComment(migrationResult.migrationsApplied)
+    migrationResult.toUpdateAction(ProcessId(1L)).comment shouldBe Some(MigrationComment(migrationResult.migrationsApplied))
     processor shouldBe ServiceRef(ProcessTestData.existingServiceId, List())
   }
 

designer/server/src/test/scala/pl/touk/nussknacker/ui/process/repository/DBFetchingProcessRepositorySpec.scala

@@ -214,7 +214,7 @@ class DBFetchingProcessRepositorySpec
     fetching.fetchProcessId(processName).futureValue.nonEmpty
 
   private def updateProcess(processId: ProcessId, canonicalProcess: CanonicalProcess, increaseVersionWhenJsonNotChanged: Boolean): ProcessUpdated = {
-    val action = UpdateProcessAction(processId, canonicalProcess, UpdateProcessComment(""), increaseVersionWhenJsonNotChanged)
+    val action = UpdateProcessAction(processId, canonicalProcess, None, increaseVersionWhenJsonNotChanged)
 
     val processUpdated = repositoryManager.runInTransaction(writingRepo.updateProcess(action)).futureValue
     processUpdated shouldBe 'right

docs/Changelog.md

@@ -46,6 +46,7 @@
 * [#3682](https://github.com/TouK/nussknacker/pull/3682) Extract generic `BaseSharedKafkaProducer`, rename `SharedKafkaProducerHolder` to `DefaultSharedKafkaProducerHolder`. 
 * [#3701](https://github.com/TouK/nussknacker/pull/3701) `TypedMap` allows access to non-existing keys in SpEL (returning `null`)
 * [#3733](https://github.com/TouK/nussknacker/pull/3733) Fix for: some validation (e.g. Flink scenario name validation) were causing error message blinking in scenario properties.
+* [#3752](https://github.com/TouK/nussknacker/pull/3752) Do not specify migrations which did not change process in process migration comment. If no migrations, do not add comment.
 
 1.6.1 (8 Nov 2022)
 ------------------------