From 4bf3249239efdd3f39c3e8249fdce0da7e222669 Mon Sep 17 00:00:00 2001 From: Filip Michalski Date: Tue, 19 Nov 2024 20:03:58 +0100 Subject: [PATCH] Add stickyNotes length and count validation, add stickyNotes configuration, fix error method (or did i?) --- .../src/actions/notificationActions.tsx | 5 +- .../src/components/StickyNotePreview.tsx | 1 - .../components/graph/EspNode/stickyNote.ts | 6 ++- .../stickyNotes/StickyNotesPanel.tsx | 2 +- .../main/resources/defaultDesignerConfig.conf | 5 ++ .../ui/api/StickyNotesApiHttpService.scala | 49 ++++++++++++++++++- .../description/StickyNotesApiEndpoints.scala | 49 +++++++++++++++++-- .../ui/api/description/stickynotes/Dtos.scala | 16 +++++- .../ui/config/FeatureTogglesConfig.scala | 6 ++- .../server/AkkaHttpBasedRouteProvider.scala | 3 +- .../resources/config/common-designer.conf | 5 ++ 11 files changed, 131 insertions(+), 16 deletions(-) diff --git a/designer/client/src/actions/notificationActions.tsx b/designer/client/src/actions/notificationActions.tsx index bd3250bfef9..5bf7f259403 100644 --- a/designer/client/src/actions/notificationActions.tsx +++ b/designer/client/src/actions/notificationActions.tsx @@ -12,10 +12,11 @@ export function success(message: string): Action { }); } -export function error(message: string): Action { +//TODO please take a look at this method and my changes, am I wrong or was it incomplete (without `error` and `showErrorText`) and had incomplete logic +export function error(message: string, error?: string, showErrorText?: boolean): Action { return Notifications.error({ autoDismiss: 10, - children: } message={message} />, + children: } message={showErrorText ? error : message} />, }); } diff --git a/designer/client/src/components/StickyNotePreview.tsx b/designer/client/src/components/StickyNotePreview.tsx index dcf7d0946eb..926a7cd0053 100644 --- a/designer/client/src/components/StickyNotePreview.tsx +++ b/designer/client/src/components/StickyNotePreview.tsx @@ -58,7 +58,6 @@ export function StickyNotePreview({ node, isActive, isOver }: { node: NodeType; }); const imageColors = css({ - background: theme.palette.custom.getNodeStyles(node)?.fill, color: theme.palette.common.black, }); diff --git a/designer/client/src/components/graph/EspNode/stickyNote.ts b/designer/client/src/components/graph/EspNode/stickyNote.ts index 84bec7d21cf..d43b332e527 100644 --- a/designer/client/src/components/graph/EspNode/stickyNote.ts +++ b/designer/client/src/components/graph/EspNode/stickyNote.ts @@ -46,7 +46,11 @@ const body: dia.MarkupNodeJSON = { const renderer = new marked.Renderer(); renderer.link = function (href, title, text) { - return `${text}` + ""; + return `${text}`; +}; +renderer.image = function (href, title, text) { + // SVG don't support HTML img inside foreignObject + return `${text} (attached img)`; }; const foreignObject = (stickyNote: StickyNote): MarkupNodeJSON => { diff --git a/designer/client/src/components/stickyNotes/StickyNotesPanel.tsx b/designer/client/src/components/stickyNotes/StickyNotesPanel.tsx index 4813a3241b4..efd20d657ff 100644 --- a/designer/client/src/components/stickyNotes/StickyNotesPanel.tsx +++ b/designer/client/src/components/stickyNotes/StickyNotesPanel.tsx @@ -21,7 +21,7 @@ export function StickyNotesPanel(props: ToolbarPanelProps): JSX.Element { diff --git a/designer/server/src/main/resources/defaultDesignerConfig.conf b/designer/server/src/main/resources/defaultDesignerConfig.conf index d6b28a2765e..2e19646866a 100644 --- a/designer/server/src/main/resources/defaultDesignerConfig.conf +++ b/designer/server/src/main/resources/defaultDesignerConfig.conf @@ -211,6 +211,11 @@ testDataSettings: { resultsMaxBytes: 50000000 } +stickyNotesSettings: { + maxContentLength: 5000, + maxNotesCount: 5 +} + scenarioLabelSettings: { validationRules = [ { diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/StickyNotesApiHttpService.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/StickyNotesApiHttpService.scala index d8da766e2c7..a4c5e5c3eb6 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/StickyNotesApiHttpService.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/StickyNotesApiHttpService.scala @@ -12,9 +12,15 @@ import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.{ StickyNoteCorrelationId, StickyNoteId, StickyNoteUpdateRequest, - StickyNotesError + StickyNotesError, + StickyNotesSettings +} +import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNotesError.{ + NoPermission, + NoScenario, + StickyNoteContentTooLong, + StickyNoteCountLimitReached } -import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNotesError.{NoPermission, NoScenario} import pl.touk.nussknacker.ui.process.repository.stickynotes.StickyNotesRepository import pl.touk.nussknacker.ui.process.repository.DBIOActionRunner import pl.touk.nussknacker.ui.process.ProcessService @@ -28,6 +34,7 @@ class StickyNotesApiHttpService( scenarioService: ProcessService, scenarioAuthorizer: AuthorizeProcess, dbioActionRunner: DBIOActionRunner, + stickyNotesSettings: StickyNotesSettings )(implicit executionContext: ExecutionContext) extends BaseHttpService(authManager) with LazyLogging { @@ -58,6 +65,9 @@ class StickyNotesApiHttpService( for { scenarioId <- getScenarioIdByName(scenarioName) _ <- isAuthorized(scenarioId, Permission.Write) + count <- getStickyNotesCount(scenarioId, requestBody.scenarioVersionId) + _ <- validateStickyNotesCount(count, stickyNotesSettings) + _ <- validateStickyNoteContent(requestBody.content, stickyNotesSettings) processActivity <- addStickyNote(scenarioId, requestBody) } yield processActivity } @@ -72,6 +82,7 @@ class StickyNotesApiHttpService( for { scenarioId <- getScenarioIdByName(scenarioName) _ <- isAuthorized(scenarioId, Permission.Write) + _ <- validateStickyNoteContent(requestBody.content, stickyNotesSettings) processActivity <- updateStickyNote(requestBody) } yield processActivity.toInt } @@ -122,6 +133,16 @@ class StickyNotesApiHttpService( ) ) + private def getStickyNotesCount(scenarioId: ProcessId, versionId: VersionId): EitherT[Future, StickyNotesError, Int] = + EitherT + .right( + dbioActionRunner + .run( + stickyNotesRepository.findStickyNotes(scenarioId, versionId) + ) + .map(_.length) + ) + private def deleteStickyNote(noteId: StickyNoteId)( implicit loggedUser: LoggedUser ): EitherT[Future, StickyNotesError, Int] = @@ -140,6 +161,18 @@ class StickyNotesApiHttpService( ) } yield result + private def validateStickyNotesCount( + stickyNotesCount: Int, + stickyNotesConfig: StickyNotesSettings + ): EitherT[Future, StickyNotesError, Unit] = + EitherT.fromEither( + Either.cond( + stickyNotesCount < stickyNotesConfig.maxNotesCount, + (), + StickyNoteCountLimitReached(stickyNotesConfig.maxNotesCount) + ) + ) + private def addStickyNote(scenarioId: ProcessId, requestBody: StickyNoteAddRequest)( implicit loggedUser: LoggedUser ): EitherT[Future, StickyNotesError, StickyNoteCorrelationId] = @@ -158,6 +191,18 @@ class StickyNotesApiHttpService( ) ) + private def validateStickyNoteContent( + content: String, + stickyNotesConfig: StickyNotesSettings + ): EitherT[Future, StickyNotesError, Unit] = + EitherT.fromEither( + Either.cond( + content.length <= stickyNotesConfig.maxContentLength, + (), + StickyNoteContentTooLong(content.length, stickyNotesConfig.maxContentLength) + ) + ) + private def updateStickyNote(requestBody: StickyNoteUpdateRequest)( implicit loggedUser: LoggedUser ): EitherT[Future, StickyNotesError, Int] = for { diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/StickyNotesApiEndpoints.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/StickyNotesApiEndpoints.scala index 973201c0ec5..479bccd4066 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/StickyNotesApiEndpoints.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/StickyNotesApiEndpoints.scala @@ -9,10 +9,20 @@ import pl.touk.nussknacker.restmodel.BaseEndpointDefinitions.SecuredEndpoint import pl.touk.nussknacker.security.AuthCredentials import pl.touk.nussknacker.ui.api.TapirCodecs import pl.touk.nussknacker.ui.api.TapirCodecs.ScenarioNameCodec._ -import pl.touk.nussknacker.ui.api.description.StickyNotesApiEndpoints.Examples.{noScenarioExample, noStickyNoteExample} +import pl.touk.nussknacker.ui.api.description.StickyNotesApiEndpoints.Examples.{ + noScenarioExample, + noStickyNoteExample, + stickyNoteContentTooLongExample, + stickyNoteCountLimitReachedExample +} import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNoteId -import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNotesError.{NoScenario, NoStickyNote} -import sttp.model.StatusCode.{NotFound, Ok} +import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNotesError.{ + NoScenario, + NoStickyNote, + StickyNoteContentTooLong, + StickyNoteCountLimitReached +} +import sttp.model.StatusCode.{BadRequest, NotFound, Ok} import sttp.tapir.EndpointIO.Example import sttp.tapir._ import sttp.tapir.json.circe.jsonBody @@ -102,7 +112,8 @@ class StickyNotesApiEndpoints(auth: EndpointInput[AuthCredentials]) extends Base .errorOut( oneOf[StickyNotesError]( noScenarioExample, - noStickyNoteExample + noStickyNoteExample, + stickyNoteContentTooLongExample ) ) .withSecurity(auth) @@ -122,7 +133,9 @@ class StickyNotesApiEndpoints(auth: EndpointInput[AuthCredentials]) extends Base .out(jsonBody[StickyNoteCorrelationId]) .errorOut( oneOf[StickyNotesError]( - noScenarioExample + noScenarioExample, + stickyNoteContentTooLongExample, + stickyNoteCountLimitReachedExample ) ) .withSecurity(auth) @@ -176,6 +189,32 @@ object StickyNotesApiEndpoints { ) ) + val stickyNoteContentTooLongExample: EndpointOutput.OneOfVariant[StickyNoteContentTooLong] = + oneOfVariantFromMatchType( + BadRequest, + plainBody[StickyNoteContentTooLong] + .example( + Example.of( + summary = Some("Provided note content is too long (5004 characters). Max content length is 5000."), + value = StickyNoteContentTooLong(count = 5004, max = 5000) + ) + ) + ) + + val stickyNoteCountLimitReachedExample: EndpointOutput.OneOfVariant[StickyNoteCountLimitReached] = + oneOfVariantFromMatchType( + BadRequest, + plainBody[StickyNoteCountLimitReached] + .example( + Example.of( + summary = Some( + "Cannot add another sticky note, since max number of sticky notes was reached: 5 (see configuration)." + ), + value = StickyNoteCountLimitReached(max = 5) + ) + ) + ) + } } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/stickynotes/Dtos.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/stickynotes/Dtos.scala index 94b057927b6..ce509ec388b 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/stickynotes/Dtos.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/stickynotes/Dtos.scala @@ -73,6 +73,12 @@ object Dtos { targetEdge: Option[String] ) + // TODO add this to configuration file in next iteration + case class StickyNotesSettings( + maxContentLength: Int, + maxNotesCount: Int + ) + sealed trait StickyNotesError implicit lazy val cellErrorSchema: Schema[LayoutData] = Schema.derived @@ -82,6 +88,7 @@ object Dtos { final case class NoScenario(scenarioName: ProcessName) extends StickyNotesError final case object NoPermission extends StickyNotesError with CustomAuthorizationError final case class StickyNoteContentTooLong(count: Int, max: Int) extends StickyNotesError + final case class StickyNoteCountLimitReached(max: Int) extends StickyNotesError final case class NoStickyNote(noteId: StickyNoteId) extends StickyNotesError implicit val noScenarioCodec: Codec[String, NoScenario, CodecFormat.TextPlain] = @@ -92,9 +99,14 @@ object Dtos { s"No sticky note with id: ${e.noteId} was found" ) - implicit val noCommentCodec: Codec[String, StickyNoteContentTooLong, CodecFormat.TextPlain] = + implicit val stickyNoteContentTooLongCodec: Codec[String, StickyNoteContentTooLong, CodecFormat.TextPlain] = BaseEndpointDefinitions.toTextPlainCodecSerializationOnly[StickyNoteContentTooLong](e => - s"Provided note content is too long (${e.count} characters). Max content length is ${e.max} " + s"Provided note content is too long (${e.count} characters). Max content length is ${e.max}." + ) + + implicit val stickyNoteCountLimitReachedCodec: Codec[String, StickyNoteCountLimitReached, CodecFormat.TextPlain] = + BaseEndpointDefinitions.toTextPlainCodecSerializationOnly[StickyNoteCountLimitReached](e => + s"Cannot add another sticky note, since max number of sticky notes was reached: ${e.max} (see configuration)." ) } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/config/FeatureTogglesConfig.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/config/FeatureTogglesConfig.scala index 1ee85981ab3..efd90d3d9c6 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/config/FeatureTogglesConfig.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/config/FeatureTogglesConfig.scala @@ -7,6 +7,7 @@ import net.ceedubs.ficus.readers.ValueReader import pl.touk.nussknacker.engine.definition.component.Components.ComponentDefinitionExtractionMode import pl.touk.nussknacker.engine.util.config.FicusReaders import pl.touk.nussknacker.ui.api._ +import pl.touk.nussknacker.ui.api.description.stickynotes.Dtos.StickyNotesSettings import pl.touk.nussknacker.ui.config.Implicits.parseOptionalConfig import pl.touk.nussknacker.ui.process.migrate.HttpRemoteEnvironmentConfig @@ -28,7 +29,8 @@ final case class FeatureTogglesConfig( testDataSettings: TestDataSettings, enableConfigEndpoint: Boolean, redirectAfterArchive: Boolean, - componentDefinitionExtractionMode: ComponentDefinitionExtractionMode + componentDefinitionExtractionMode: ComponentDefinitionExtractionMode, + stickyNotesSettings: StickyNotesSettings ) object FeatureTogglesConfig extends LazyLogging { @@ -56,6 +58,7 @@ object FeatureTogglesConfig extends LazyLogging { val tabs = parseOptionalConfig[List[TopTab]](config, "tabs") val intervalTimeSettings = config.as[IntervalTimeSettings]("intervalTimeSettings") val testDataSettings = config.as[TestDataSettings]("testDataSettings") + val stickyNotesSettings = config.as[StickyNotesSettings]("stickyNotesSettings") val redirectAfterArchive = config.getAs[Boolean]("redirectAfterArchive").getOrElse(true) val componentDefinitionExtractionMode = parseComponentDefinitionExtractionMode(config) @@ -76,6 +79,7 @@ object FeatureTogglesConfig extends LazyLogging { enableConfigEndpoint = enableConfigEndpoint, redirectAfterArchive = redirectAfterArchive, componentDefinitionExtractionMode = componentDefinitionExtractionMode, + stickyNotesSettings = stickyNotesSettings ) } diff --git a/designer/server/src/main/scala/pl/touk/nussknacker/ui/server/AkkaHttpBasedRouteProvider.scala b/designer/server/src/main/scala/pl/touk/nussknacker/ui/server/AkkaHttpBasedRouteProvider.scala index 3fd8625c3bf..2228bac8831 100644 --- a/designer/server/src/main/scala/pl/touk/nussknacker/ui/server/AkkaHttpBasedRouteProvider.scala +++ b/designer/server/src/main/scala/pl/touk/nussknacker/ui/server/AkkaHttpBasedRouteProvider.scala @@ -404,7 +404,8 @@ class AkkaHttpBasedRouteProvider( stickyNotesRepository = stickyNotesRepository, scenarioService = processService, scenarioAuthorizer = processAuthorizer, - dbioRunner + dbioRunner, + stickyNotesSettings = featureTogglesConfig.stickyNotesSettings ) val scenarioActivityApiHttpService = new ScenarioActivityApiHttpService( diff --git a/designer/server/src/test/resources/config/common-designer.conf b/designer/server/src/test/resources/config/common-designer.conf index d6ba6eaddd4..d5b087a8604 100644 --- a/designer/server/src/test/resources/config/common-designer.conf +++ b/designer/server/src/test/resources/config/common-designer.conf @@ -55,6 +55,11 @@ testDataSettings: { resultsMaxBytes: 50000000 } +stickyNotesSettings: { + maxContentLength: 5000, + maxNotesCount: 5 +} + scenarioLabelSettings: { validationRules = [ {