diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/db/entity/CommentEntityFactory.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/db/entity/CommentEntityFactory.scala index 0f29af35291..80da9b3cb47 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/db/entity/CommentEntityFactory.scala +++ b/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 None => DBIO.successful(None) } } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala index 2f95c7974e2..9b02b85e3e9 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/ProcessService.scala +++ b/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 diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigrator.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigrator.scala index 5b4f93086fc..d2f13f84afa 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigrator.scala +++ b/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) } } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/DbProcessActivityRepository.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/DbProcessActivityRepository.scala index be2ca4fc391..e052e6e71b4 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/DbProcessActivityRepository.scala +++ b/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] = { diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessActionRepository.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessActionRepository.scala index 7d9c30a18db..3852e3d140e 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/process/repository/ProcessActionRepository.scala +++ b/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) - } } 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 3f6d6c73f27..6a03986ea4d 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 @@ -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(()) } diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigratorSpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigratorSpec.scala index 3761fe1e6ea..33926ee9928 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigratorSpec.scala +++ b/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/migrate/ProcessModelMigratorSpec.scala @@ -24,6 +24,13 @@ class ProcessModelMigratorSpec extends AnyFlatSpec with BeforeAndAfterEach with private implicit val user = TestFactory.adminUser("test1") + it should "return migrations that actually 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) diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/repository/DBFetchingProcessRepositorySpec.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/repository/DBFetchingProcessRepositorySpec.scala index 2bc1d0b5b72..2ed8ec83268 100644 --- a/designer/server/src/test/scala/pl/touk/nussknacker/ui/process/repository/DBFetchingProcessRepositorySpec.scala +++ b/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 diff --git a/docs/Changelog.md b/docs/Changelog.md index 6f2fcb0d2cd..fa35e119c04 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -1,6 +1,3 @@ - -# Changelog - 1.7.0 (Not released yet) ------------------------ * [#3560](https://github.com/TouK/nussknacker/pull/3560), [#3560](https://github.com/TouK/nussknacker/pull/3560), [#3595](https://github.com/TouK/nussknacker/pull/3595) Migrate from Flink Scala API to Java API @@ -46,6 +43,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) ------------------------