review changes

TouK · Oct 22, 2024 · 47ed819 · 47ed819
1 parent 06d5d69
commit 47ed819
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 26 deletions.
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
@@ -23,6 +23,7 @@ import pl.touk.nussknacker.ui.process.ProcessService.GetScenarioWithDetailsOptio
 import pl.touk.nussknacker.ui.process.deployment.DeploymentManagerDispatcher
 import pl.touk.nussknacker.ui.process.repository.DBIOActionRunner
 import pl.touk.nussknacker.ui.process.repository.activities.ScenarioActivityRepository
+import pl.touk.nussknacker.ui.process.repository.activities.ScenarioActivityRepository.DeleteAttachmentError
 import pl.touk.nussknacker.ui.process.{ProcessService, ScenarioAttachmentService}
 import pl.touk.nussknacker.ui.security.api.{AuthManager, LoggedUser}
 import pl.touk.nussknacker.ui.server.HeadersSupport.ContentDisposition
@@ -267,18 +268,19 @@ class ScenarioActivityApiHttpService(
       } yield sortedResult
     }
 
-  private def toDto(scenarioComment: ScenarioComment): Dtos.ScenarioActivityComment = {
-    val content = scenarioComment match {
-      case ScenarioComment.WithContent(comment, _, _) =>
-        Dtos.ScenarioActivityCommentContent.Available(comment)
-      case ScenarioComment.WithoutContent(_, _) | ScenarioComment.WithContent(_, _, _) =>
-        Dtos.ScenarioActivityCommentContent.NotAvailable
-    }
-    Dtos.ScenarioActivityComment(
-      content = content,
-      lastModifiedBy = scenarioComment.lastModifiedByUserName.value,
-      lastModifiedAt = scenarioComment.lastModifiedAt,
-    )
+  private def toDto(scenarioComment: ScenarioComment): Dtos.ScenarioActivityComment = scenarioComment match {
+    case ScenarioComment.WithContent(comment, _, _) =>
+      Dtos.ScenarioActivityComment(
+        content = Dtos.ScenarioActivityCommentContent.Available(comment),
+        lastModifiedBy = scenarioComment.lastModifiedByUserName.value,
+        lastModifiedAt = scenarioComment.lastModifiedAt,
+      )
+    case ScenarioComment.WithoutContent(_, _) =>
+      Dtos.ScenarioActivityComment(
+        content = Dtos.ScenarioActivityCommentContent.NotAvailable,
+        lastModifiedBy = scenarioComment.lastModifiedByUserName.value,
+        lastModifiedAt = scenarioComment.lastModifiedAt,
+      )
   }
 
   private def toDto(attachment: ScenarioAttachment): Dtos.ScenarioActivityAttachment = {
@@ -583,7 +585,7 @@ class ScenarioActivityApiHttpService(
       dbioActionRunner.run(
         scenarioActivityRepository.markAttachmentAsDeleted(scenarioId, request.attachmentId)
       )
-    ).leftMap(_ => NoAttachment(request.attachmentId))
+    ).leftMap { case DeleteAttachmentError.CouldNotDeleteAttachment => NoAttachment(request.attachmentId) }
 
   private def buildResponse(maybeAttachment: Option[(String, Array[Byte])]): GetAttachmentResponse =
     maybeAttachment match {

diff --git a/...r/src/test/scala/pl/touk/nussknacker/test/processes/WithScenarioActivitySpecAsserts.scala b/...r/src/test/scala/pl/touk/nussknacker/test/processes/WithScenarioActivitySpecAsserts.scala
@@ -2,12 +2,12 @@ package pl.touk.nussknacker.test.processes
 
 import io.restassured.RestAssured.`given`
 import io.restassured.module.scala.RestAssuredSupport.AddThenToResponse
+import io.restassured.response.ValidatableResponse
 import org.scalatest.freespec.AnyFreeSpecLike
 import pl.touk.nussknacker.test.NuRestAssureMatchers
 import pl.touk.nussknacker.test.base.it.NuItTest
 import pl.touk.nussknacker.test.config.{WithBusinessCaseRestAssuredUsersExtensions, WithDesignerConfig}
-
-import scala.util.Try
+import pl.touk.nussknacker.test.processes.WithScenarioActivitySpecAsserts.ScenarioActivitiesResponseWrapper
 
 trait WithScenarioActivitySpecAsserts
     extends AnyFreeSpecLike
@@ -96,14 +96,14 @@ trait WithScenarioActivitySpecAsserts
       )
   }
 
-  def verifyAttachmentAddedActivityExistsAndReturnFileId(
+  def verifyAttachmentAddedActivityExists(
       user: String,
       scenarioName: String,
       fileIdPresent: Boolean,
       filename: String,
       fileStatus: String,
       overrideDisplayableName: String,
-  ): Option[Long] = {
+  ): ScenarioActivitiesResponseWrapper = {
     val fileJson = if (fileIdPresent) {
       s"""
         |"file": {
@@ -157,12 +157,7 @@ trait WithScenarioActivitySpecAsserts
              |""".stripMargin
         )
       )
-    if (fileIdPresent) {
-      Try(response.extractString("activities[1].attachment.file.id").toLong).toOption
-    } else {
-      None
-    }
-
+    ScenarioActivitiesResponseWrapper(response)
   }
 
   def verifyEmptyCommentsAndAttachments(scenarioName: String): Unit = {
@@ -208,3 +203,12 @@ trait WithScenarioActivitySpecAsserts
   }
 
 }
+
+object WithScenarioActivitySpecAsserts extends WithBusinessCaseRestAssuredUsersExtensions {
+
+  final case class ScenarioActivitiesResponseWrapper(validatableResponse: ValidatableResponse) {
+    def extractString(activityIndex: Int, path: String): String =
+      validatableResponse.extractString(s"activities[$activityIndex].$path")
+  }
+
+}
diff --git a/...rc/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala b/...rc/test/scala/pl/touk/nussknacker/ui/api/ScenarioActivityApiHttpServiceBusinessSpec.scala
@@ -241,14 +241,14 @@ class ScenarioActivityApiHttpServiceBusinessSpec
         .Then()
         .statusCode(200)
 
-      val attachmentId = verifyAttachmentAddedActivityExistsAndReturnFileId(
+      val attachmentId = verifyAttachmentAddedActivityExists(
         user = "allpermuser",
         scenarioName = exampleScenarioName,
         fileIdPresent = true,
         filename = fileName,
         fileStatus = "AVAILABLE",
         overrideDisplayableName = fileName
-      ).getOrElse(throw new Exception("Missing attachmentId"))
+      ).extractString(1, "attachment.file.id")
 
       given()
         .when()
@@ -257,7 +257,7 @@ class ScenarioActivityApiHttpServiceBusinessSpec
         .Then()
         .statusCode(200)
         .verifyApplicationState {
-          verifyAttachmentAddedActivityExistsAndReturnFileId(
+          verifyAttachmentAddedActivityExists(
             user = "allpermuser",
             scenarioName = exampleScenarioName,
             fileIdPresent = false,