From 5bc9b0e2d16663752fae4fb8de8e68282f7590fc Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Fri, 13 Dec 2024 13:05:05 +0100 Subject: [PATCH 1/6] draft-api/article-api: Add disclaimer field --- .../scala/no/ndla/articleapi/TestData.scala | 15 +++++--- .../ndla/common/model/domain/Disclaimer.scala | 22 +++++++++++ .../common/model/domain/article/Article.scala | 3 +- .../common/model/domain/draft/Draft.scala | 3 +- .../ndla/draftapi/model/api/ArticleDTO.scala | 1 + .../draftapi/model/api/DisclaimerDTO.scala | 23 +++++++++++ .../draftapi/model/api/NewArticleDTO.scala | 1 + .../model/api/UpdatedArticleDTO.scala | 4 +- .../draftapi/service/ConverterService.scala | 35 ++++++++++++++--- .../scala/no/ndla/draftapi/TestData.scala | 24 ++++++++---- .../service/ConverterServiceTest.scala | 38 +++++++++++++++---- .../service/StateTransitionRulesTest.scala | 18 ++++++--- .../articleapi/ArticleApiClientTest.scala | 3 +- .../scala/no/ndla/searchapi/TestData.scala | 18 ++++++--- 14 files changed, 167 insertions(+), 41 deletions(-) create mode 100644 common/src/main/scala/no/ndla/common/model/domain/Disclaimer.scala create mode 100644 draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala diff --git a/article-api/src/test/scala/no/ndla/articleapi/TestData.scala b/article-api/src/test/scala/no/ndla/articleapi/TestData.scala index c7e09fd4e..4f401dc60 100644 --- a/article-api/src/test/scala/no/ndla/articleapi/TestData.scala +++ b/article-api/src/test/scala/no/ndla/articleapi/TestData.scala @@ -152,7 +152,8 @@ trait TestData { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = Some(NDLADate.now().withNano(0)), - slug = None + slug = None, + disclaimer = Seq.empty ) val sampleDomainArticle: Article = Article( @@ -177,7 +178,8 @@ trait TestData { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = None + slug = None, + disclaimer = Seq.empty ) val sampleDomainArticle2: Article = Article( @@ -202,7 +204,8 @@ trait TestData { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = None + slug = None, + disclaimer = Seq.empty ) val sampleArticleWithByNcSa: Article = sampleArticleWithPublicDomain.copy(copyright = byNcSaCopyright) @@ -239,7 +242,8 @@ trait TestData { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = None + slug = None, + disclaimer = Seq.empty ) val apiArticleWithHtmlFaultV2: api.ArticleV2DTO = api.ArticleV2DTO( @@ -308,7 +312,8 @@ trait TestData { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = None + slug = None, + disclaimer = Seq.empty ) } diff --git a/common/src/main/scala/no/ndla/common/model/domain/Disclaimer.scala b/common/src/main/scala/no/ndla/common/model/domain/Disclaimer.scala new file mode 100644 index 000000000..a4f0f257b --- /dev/null +++ b/common/src/main/scala/no/ndla/common/model/domain/Disclaimer.scala @@ -0,0 +1,22 @@ +/* + * Part of NDLA common + * Copyright (C) 2024 NDLA + * + * See LICENSE + */ + +package no.ndla.common.model.domain + +import io.circe.{Decoder, Encoder} +import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} +import no.ndla.language.model.LanguageField + +case class Disclaimer(disclaimer: String, language: String) extends LanguageField[String] { + override def value: String = disclaimer + override def isEmpty: Boolean = disclaimer.isEmpty +} + +object Disclaimer { + implicit def encoder: Encoder[Disclaimer] = deriveEncoder[Disclaimer] + implicit def decoder: Decoder[Disclaimer] = deriveDecoder[Disclaimer] +} diff --git a/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala b/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala index a05dbbc98..3d060e711 100644 --- a/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala +++ b/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala @@ -36,7 +36,8 @@ case class Article( availability: Availability, relatedContent: Seq[RelatedContent], revisionDate: Option[NDLADate], - slug: Option[String] + slug: Option[String], + disclaimer: Seq[Disclaimer] ) extends Content object Article { diff --git a/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala b/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala index 16b4cbce8..31100c127 100644 --- a/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala +++ b/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala @@ -45,7 +45,8 @@ case class Draft( comments: Seq[Comment], priority: Priority, started: Boolean, - qualityEvaluation: Option[QualityEvaluation] + qualityEvaluation: Option[QualityEvaluation], + disclaimer: Seq[Disclaimer] ) extends Content { def supportedLanguages: Seq[String] = diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/ArticleDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/ArticleDTO.scala index 72995ecf9..c875294b9 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/model/api/ArticleDTO.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/ArticleDTO.scala @@ -51,6 +51,7 @@ case class ArticleDTO( @description("If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") priority: String, @description("If the article has been edited after last status or responsible change") started: Boolean, @description("The quality evaluation of the article. Consist of a score from 1 to 5 and a comment.") qualityEvaluation : Option[QualityEvaluationDTO], + @description("The disclaimer of the article") disclaimer: Option[DisclaimerDTO] ) object ArticleDTO { diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala new file mode 100644 index 000000000..d8954d4b4 --- /dev/null +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala @@ -0,0 +1,23 @@ +/* + * Part of NDLA draft-api + * Copyright (C) 2024 NDLA + * + * See LICENSE + */ + +package no.ndla.draftapi.model.api + +import io.circe.{Decoder, Encoder} +import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} +import sttp.tapir.Schema.annotations.description + +case class DisclaimerDTO( + @description("The freetext content of the disclaimer") disclaimer: String, + @description("The freetext html content of the disclaimer") htmlDisclaimer: String, + @description("ISO 639-1 code that represents the language used in the disclaimer") language: String +) + +object DisclaimerDTO { + implicit def encoder: Encoder[DisclaimerDTO] = deriveEncoder + implicit def decoder: Decoder[DisclaimerDTO] = deriveDecoder +} diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/NewArticleDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/NewArticleDTO.scala index f9fd4c3fd..2502a3af2 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/model/api/NewArticleDTO.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/NewArticleDTO.scala @@ -42,6 +42,7 @@ case class NewArticleDTO( @description("If the article should be prioritized") prioritized: Option[Boolean], @description("If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") priority: Option[String], @description("The quality evaluation of the article. Consist of a score from 1 to 5 and a comment.") qualityEvaluation : Option[QualityEvaluationDTO], + @description("The disclaimer of the article") disclaimer: Option[String] ) // format: on diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala index cb24244a2..d73d99a25 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala @@ -47,7 +47,9 @@ case class UpdatedArticleDTO( @description("If the article should be prioritized") prioritized: Option[Boolean], @description("If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") priority: Option[String], @description("The quality evaluation of the article. Consist of a score from 1 to 5 and a comment.") qualityEvaluation : Option[QualityEvaluationDTO], -) + @description("The disclaimer of the article") disclaimer: Option[String] + + ) // format: on object UpdatedArticleDTO { diff --git a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala index 0d6f41e17..09d384b98 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala @@ -55,6 +55,7 @@ trait ConverterService { val domainContent = newArticle.content .map(content => common.ArticleContent(removeUnknownEmbedTagAttribute(content), newArticle.language)) .toSeq + val domainDisclaimer = Seq(common.Disclaimer(newArticle.disclaimer.getOrElse(""), newArticle.language)) val status = externalIds match { case Nil => common.Status(PLANNED, Set.empty) @@ -124,7 +125,8 @@ trait ConverterService { comments = newCommentToDomain(newArticle.comments.getOrElse(List.empty)), priority = priority, started = false, - qualityEvaluation = qualityEvaluationToDomain(newArticle.qualityEvaluation) + qualityEvaluation = qualityEvaluationToDomain(newArticle.qualityEvaluation), + disclaimer = domainDisclaimer ) ) } @@ -257,6 +259,9 @@ trait ConverterService { private def toDomainTitle(articleTitle: api.ArticleTitleDTO): common.Title = common.Title(articleTitle.title, articleTitle.language) + private def toDomainDisclaimer(articleDisclaimer: api.DisclaimerDTO): common.Disclaimer = + common.Disclaimer(articleDisclaimer.disclaimer, articleDisclaimer.language) + private def toDomainContent(articleContent: api.ArticleContentDTO): common.ArticleContent = { common.ArticleContent(removeUnknownEmbedTagAttribute(articleContent.content), articleContent.language) } @@ -386,6 +391,7 @@ trait ConverterService { val metaImage = findByLanguageOrBestEffort(article.metaImage, language).map(toApiArticleMetaImage) val revisionMetas = article.revisionMeta.map(toApiRevisionMeta) val responsible = article.responsible.map(toApiResponsible) + val disclaimer = findByLanguageOrBestEffort((article.disclaimer), language).map(toApiArticleDisclaimer) Success( api.ArticleDTO( @@ -421,7 +427,8 @@ trait ConverterService { prioritized = article.priority == Priority.Prioritized, priority = article.priority.entryName, started = article.started, - qualityEvaluation = toApiQualityEvaluation(article.qualityEvaluation) + qualityEvaluation = toApiQualityEvaluation(article.qualityEvaluation), + disclaimer = disclaimer ) ) } else { @@ -453,6 +460,13 @@ trait ConverterService { def toApiArticleTitle(title: common.Title): api.ArticleTitleDTO = api.ArticleTitleDTO(Jsoup.parseBodyFragment(title.title).body().text(), title.title, title.language) + def toApiArticleDisclaimer(disclaimer: common.Disclaimer): api.DisclaimerDTO = + api.DisclaimerDTO( + Jsoup.parseBodyFragment(disclaimer.disclaimer).body().text(), + disclaimer.disclaimer, + disclaimer.language + ) + private def toApiArticleContent(content: common.ArticleContent): api.ArticleContentDTO = api.ArticleContentDTO(content.content, content.language) @@ -620,7 +634,8 @@ trait ConverterService { availability = draft.availability, relatedContent = draft.relatedContent, revisionDate = getNextRevision(draft.revisionMeta).map(_.revisionDate), - slug = draft.slug + slug = draft.slug, + disclaimer = draft.disclaimer ) ) } @@ -781,6 +796,14 @@ trait ConverterService { .traverse(lang => articleWithNewContent.title.toSeq.map(t => toDomainTitle(api.ArticleTitleDTO(t, t, lang)))) .flatten ) + val updatedDisclaimer = mergeLanguageFields( + toMergeInto.disclaimer, + maybeLang + .traverse(lang => + articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, d, lang))) + ) + .flatten + ) val updatedContents = mergeLanguageFields( toMergeInto.content, maybeLang @@ -845,7 +868,8 @@ trait ConverterService { comments = updatedComments, priority = priority, started = toMergeInto.started, - qualityEvaluation = qualityEvaluationToDomain(article.qualityEvaluation) + qualityEvaluation = qualityEvaluationToDomain(article.qualityEvaluation), + disclaimer = updatedDisclaimer ) Success(converted) @@ -933,7 +957,8 @@ trait ConverterService { comments = comments, priority = priority, started = false, - qualityEvaluation = qualityEvaluationToDomain(article.qualityEvaluation) + qualityEvaluation = qualityEvaluationToDomain(article.qualityEvaluation), + disclaimer = article.disclaimer.map(d => common.Disclaimer(d, lang)).toSeq ) } diff --git a/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala b/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala index b7f480d93..ef879f8ab 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala @@ -111,7 +111,8 @@ object TestData { priority = Priority.Unspecified.entryName, started = false, prioritized = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = None ) val blankUpdatedArticle: UpdatedArticleDTO = api.UpdatedArticleDTO( @@ -142,7 +143,8 @@ object TestData { comments = None, prioritized = None, priority = None, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = None ) val sampleApiUpdateArticle: UpdatedArticleDTO = blankUpdatedArticle.copy( @@ -230,6 +232,7 @@ object TestData { false, Priority.Unspecified.entryName, false, + None, None ) @@ -283,6 +286,7 @@ object TestData { false, Priority.Unspecified.entryName, false, + None, None ) @@ -317,7 +321,8 @@ object TestData { Seq.empty, Priority.Unspecified, false, - None + None, + Seq.empty ) val sampleArticleWithPublicDomain: Draft = Draft( @@ -351,7 +356,8 @@ object TestData { Seq.empty, Priority.Unspecified, false, - None + None, + Seq.empty ) val sampleDomainArticle: Draft = Draft( @@ -387,7 +393,8 @@ object TestData { Seq.empty, Priority.Unspecified, false, - None + None, + Seq.empty ) val newArticle: NewArticleDTO = api.NewArticleDTO( @@ -426,6 +433,7 @@ object TestData { None, None, None, + None, None ) @@ -476,7 +484,8 @@ object TestData { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val apiArticleWithHtmlFaultV2: api.ArticleDTO = api.ArticleDTO( @@ -532,7 +541,8 @@ object TestData { prioritized = false, priority = Priority.Unspecified.entryName, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = None ) val (nodeId, nodeId2) = ("1234", "4321") diff --git a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala index b3afe298e..69c88a237 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala @@ -58,6 +58,20 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { ) } + test("toApiDisclaimer returns both disclaimer and plainDisclaimer") { + val disclaimer = Disclaimer( + "Denne ressursen har innhold som kan være utfordrende dersom du bruker: ", + "nb" + ) + service.toApiArticleDisclaimer(disclaimer) should equal( + api.DisclaimerDTO( + "Denne ressursen har innhold som kan være utfordrende dersom du bruker: skjermleser tastatur", + "Denne ressursen har innhold som kan være utfordrende dersom du bruker: ", + "nb" + ) + ) + } + test("toApiArticleIntroduction returns both introduction and plainIntroduction") { val introduction = Introduction("

Introduction with emphasis

", "en") service.toApiArticleIntroduction(introduction) should equal( @@ -332,7 +346,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq(Disclaimer("Disclaimer test", "nb")) ) val updatedNothing = TestData.blankUpdatedArticle.copy( @@ -379,7 +394,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq(Disclaimer("Disclaimer test", "nb")) ) val expectedArticle = Draft( @@ -413,7 +429,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq(Disclaimer("NyDisclaimer test", "nb")) ) val updatedEverything = TestData.blankUpdatedArticle.copy( @@ -435,7 +452,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { editorLabels = None, grepCodes = None, conceptIds = None, - createNewVersion = None + createNewVersion = None, + disclaimer = Some("NyDisclaimer test") ) val user = TokenUser("theuserthatchangeditid", Set.empty, None) @@ -477,7 +495,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val expectedArticle = Draft( @@ -519,7 +538,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val updatedEverything = TestData.blankUpdatedArticle.copy( @@ -1120,7 +1140,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq(Disclaimer("articleDisclaimer", "nb")) ) val article = common.model.domain.article.Article( id = Some(articleId), @@ -1145,7 +1166,8 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { availability = Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = Some("kjempe-slug") + slug = Some("kjempe-slug"), + disclaimer = Seq(Disclaimer("articleDisclaimer", "nb")) ) val result = service.toArticleApiArticle(draft) diff --git a/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala index 191918598..a1e17f136 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala @@ -355,7 +355,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val article = common.article.Article( id = Some(articleId), @@ -379,7 +380,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { availability = common.Availability.everyone, relatedContent = Seq.empty, revisionDate = None, - slug = None + slug = None, + disclaimer = Seq.empty ) val status = common.Status(END_CONTROL, Set.empty) @@ -476,7 +478,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == PUBLISHED) @@ -532,7 +535,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == ARCHIVED) @@ -592,7 +596,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == UNPUBLISHED) @@ -653,7 +658,8 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val status = common.Status(PUBLISHED, Set.empty) val transitionToTest: StateTransition = PUBLISHED -> IN_PROGRESS diff --git a/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala b/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala index c24045fcc..64bdc0e3a 100644 --- a/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala +++ b/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala @@ -125,7 +125,8 @@ class ArticleApiClientTest comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val exampleToken = diff --git a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala index a2b27d818..1d91ab9b9 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala @@ -201,7 +201,8 @@ object TestData { Availability.everyone, Seq.empty, None, - slug = None + slug = None, + Seq.empty ) val sampleDomainArticle: Article = Article( @@ -226,7 +227,8 @@ object TestData { Availability.everyone, Seq.empty, None, - slug = None + slug = None, + Seq.empty ) val sampleDomainArticle2: Article = Article( @@ -251,7 +253,8 @@ object TestData { Availability.everyone, Seq.empty, None, - slug = None + slug = None, + Seq.empty ) val sampleArticleWithByNcSa: Article = @@ -533,7 +536,8 @@ object TestData { availability = Availability.everyone, relatedContent = Seq.empty, None, - slug = None + slug = None, + Seq.empty ) val emptyDomainDraft: Draft = Draft( @@ -567,7 +571,8 @@ object TestData { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + Seq.empty ) val draftStatus: Status = Status(DraftStatus.PLANNED, Set.empty) @@ -629,7 +634,8 @@ object TestData { comments = Seq.empty, priority = Priority.Unspecified, started = false, - qualityEvaluation = None + qualityEvaluation = None, + disclaimer = Seq.empty ) val sampleDraftWithByNcSa: Draft = sampleDraftWithPublicDomain.copy(copyright = Some(draftByNcSaCopyright)) From 5344a05d3de22a772e0f65062cb17ea6cd574975 Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Mon, 16 Dec 2024 08:50:14 +0100 Subject: [PATCH 2/6] Remove htmlDisclaimer + format --- .../no/ndla/draftapi/model/api/DisclaimerDTO.scala | 3 +-- .../draftapi/model/api/UpdatedArticleDTO.scala | 3 +-- .../ndla/draftapi/service/ConverterService.scala | 3 +-- .../draftapi/service/ConverterServiceTest.scala | 14 -------------- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala index d8954d4b4..6c318748c 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/DisclaimerDTO.scala @@ -12,8 +12,7 @@ import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} import sttp.tapir.Schema.annotations.description case class DisclaimerDTO( - @description("The freetext content of the disclaimer") disclaimer: String, - @description("The freetext html content of the disclaimer") htmlDisclaimer: String, + @description("The freetext html content of the disclaimer") disclaimer: String, @description("ISO 639-1 code that represents the language used in the disclaimer") language: String ) diff --git a/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala b/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala index d73d99a25..c555d9dae 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/model/api/UpdatedArticleDTO.scala @@ -48,8 +48,7 @@ case class UpdatedArticleDTO( @description("If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") priority: Option[String], @description("The quality evaluation of the article. Consist of a score from 1 to 5 and a comment.") qualityEvaluation : Option[QualityEvaluationDTO], @description("The disclaimer of the article") disclaimer: Option[String] - - ) +) // format: on object UpdatedArticleDTO { diff --git a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala index 09d384b98..a4755572b 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala @@ -462,7 +462,6 @@ trait ConverterService { def toApiArticleDisclaimer(disclaimer: common.Disclaimer): api.DisclaimerDTO = api.DisclaimerDTO( - Jsoup.parseBodyFragment(disclaimer.disclaimer).body().text(), disclaimer.disclaimer, disclaimer.language ) @@ -800,7 +799,7 @@ trait ConverterService { toMergeInto.disclaimer, maybeLang .traverse(lang => - articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, d, lang))) + articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, lang))) ) .flatten ) diff --git a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala index 69c88a237..9150be30f 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala @@ -58,20 +58,6 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { ) } - test("toApiDisclaimer returns both disclaimer and plainDisclaimer") { - val disclaimer = Disclaimer( - "Denne ressursen har innhold som kan være utfordrende dersom du bruker: ", - "nb" - ) - service.toApiArticleDisclaimer(disclaimer) should equal( - api.DisclaimerDTO( - "Denne ressursen har innhold som kan være utfordrende dersom du bruker: skjermleser tastatur", - "Denne ressursen har innhold som kan være utfordrende dersom du bruker: ", - "nb" - ) - ) - } - test("toApiArticleIntroduction returns both introduction and plainIntroduction") { val introduction = Introduction("

Introduction with emphasis

", "en") service.toApiArticleIntroduction(introduction) should equal( From 378ee68144001f17c689b87c4e3a4545032f130a Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Mon, 16 Dec 2024 10:04:15 +0100 Subject: [PATCH 3/6] Disclaimer is optional --- .../scala/no/ndla/articleapi/TestData.scala | 10 +++---- .../common/model/domain/article/Article.scala | 2 +- .../common/model/domain/draft/Draft.scala | 2 +- .../draftapi/service/ConverterService.scala | 29 +++++++++++-------- .../scala/no/ndla/draftapi/TestData.scala | 8 ++--- .../service/ConverterServiceTest.scala | 14 ++++----- .../service/StateTransitionRulesTest.scala | 12 ++++---- .../articleapi/ArticleApiClientTest.scala | 2 +- .../scala/no/ndla/searchapi/TestData.scala | 12 ++++---- 9 files changed, 48 insertions(+), 43 deletions(-) diff --git a/article-api/src/test/scala/no/ndla/articleapi/TestData.scala b/article-api/src/test/scala/no/ndla/articleapi/TestData.scala index 4f401dc60..5570aeba3 100644 --- a/article-api/src/test/scala/no/ndla/articleapi/TestData.scala +++ b/article-api/src/test/scala/no/ndla/articleapi/TestData.scala @@ -153,7 +153,7 @@ trait TestData { relatedContent = Seq.empty, revisionDate = Some(NDLADate.now().withNano(0)), slug = None, - disclaimer = Seq.empty + disclaimer = None ) val sampleDomainArticle: Article = Article( @@ -179,7 +179,7 @@ trait TestData { relatedContent = Seq.empty, revisionDate = None, slug = None, - disclaimer = Seq.empty + disclaimer = None ) val sampleDomainArticle2: Article = Article( @@ -205,7 +205,7 @@ trait TestData { relatedContent = Seq.empty, revisionDate = None, slug = None, - disclaimer = Seq.empty + disclaimer = None ) val sampleArticleWithByNcSa: Article = sampleArticleWithPublicDomain.copy(copyright = byNcSaCopyright) @@ -243,7 +243,7 @@ trait TestData { relatedContent = Seq.empty, revisionDate = None, slug = None, - disclaimer = Seq.empty + disclaimer = None ) val apiArticleWithHtmlFaultV2: api.ArticleV2DTO = api.ArticleV2DTO( @@ -313,7 +313,7 @@ trait TestData { relatedContent = Seq.empty, revisionDate = None, slug = None, - disclaimer = Seq.empty + disclaimer = None ) } diff --git a/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala b/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala index 3d060e711..f41cf6220 100644 --- a/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala +++ b/common/src/main/scala/no/ndla/common/model/domain/article/Article.scala @@ -37,7 +37,7 @@ case class Article( relatedContent: Seq[RelatedContent], revisionDate: Option[NDLADate], slug: Option[String], - disclaimer: Seq[Disclaimer] + disclaimer: Option[Seq[Disclaimer]] ) extends Content object Article { diff --git a/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala b/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala index 31100c127..e4f3cce33 100644 --- a/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala +++ b/common/src/main/scala/no/ndla/common/model/domain/draft/Draft.scala @@ -46,7 +46,7 @@ case class Draft( priority: Priority, started: Boolean, qualityEvaluation: Option[QualityEvaluation], - disclaimer: Seq[Disclaimer] + disclaimer: Option[Seq[Disclaimer]] ) extends Content { def supportedLanguages: Seq[String] = diff --git a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala index a4755572b..3150ae078 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala @@ -55,7 +55,7 @@ trait ConverterService { val domainContent = newArticle.content .map(content => common.ArticleContent(removeUnknownEmbedTagAttribute(content), newArticle.language)) .toSeq - val domainDisclaimer = Seq(common.Disclaimer(newArticle.disclaimer.getOrElse(""), newArticle.language)) + val domainDisclaimer = newArticle.disclaimer.map { d => Seq(common.Disclaimer(d, newArticle.language)) } val status = externalIds match { case Nil => common.Status(PLANNED, Set.empty) @@ -391,7 +391,8 @@ trait ConverterService { val metaImage = findByLanguageOrBestEffort(article.metaImage, language).map(toApiArticleMetaImage) val revisionMetas = article.revisionMeta.map(toApiRevisionMeta) val responsible = article.responsible.map(toApiResponsible) - val disclaimer = findByLanguageOrBestEffort((article.disclaimer), language).map(toApiArticleDisclaimer) + val disclaimer = + article.disclaimer.flatMap { d => findByLanguageOrBestEffort(d, language).map(toApiArticleDisclaimer) } Success( api.ArticleDTO( @@ -460,7 +461,7 @@ trait ConverterService { def toApiArticleTitle(title: common.Title): api.ArticleTitleDTO = api.ArticleTitleDTO(Jsoup.parseBodyFragment(title.title).body().text(), title.title, title.language) - def toApiArticleDisclaimer(disclaimer: common.Disclaimer): api.DisclaimerDTO = + private def toApiArticleDisclaimer(disclaimer: common.Disclaimer): api.DisclaimerDTO = api.DisclaimerDTO( disclaimer.disclaimer, disclaimer.language @@ -795,14 +796,18 @@ trait ConverterService { .traverse(lang => articleWithNewContent.title.toSeq.map(t => toDomainTitle(api.ArticleTitleDTO(t, t, lang)))) .flatten ) - val updatedDisclaimer = mergeLanguageFields( - toMergeInto.disclaimer, - maybeLang - .traverse(lang => - articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, lang))) - ) - .flatten - ) + + val updatedDisclaimer = toMergeInto.disclaimer.map { disclaimer => + mergeLanguageFields( + disclaimer, + maybeLang + .traverse(lang => + articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, lang))) + ) + .flatten + ) + } + val updatedContents = mergeLanguageFields( toMergeInto.content, maybeLang @@ -957,7 +962,7 @@ trait ConverterService { priority = priority, started = false, qualityEvaluation = qualityEvaluationToDomain(article.qualityEvaluation), - disclaimer = article.disclaimer.map(d => common.Disclaimer(d, lang)).toSeq + disclaimer = article.disclaimer.map { d => Seq(common.Disclaimer(d, lang)) } ) } diff --git a/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala b/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala index ef879f8ab..27054f7a1 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala @@ -322,7 +322,7 @@ object TestData { Priority.Unspecified, false, None, - Seq.empty + None ) val sampleArticleWithPublicDomain: Draft = Draft( @@ -357,7 +357,7 @@ object TestData { Priority.Unspecified, false, None, - Seq.empty + None ) val sampleDomainArticle: Draft = Draft( @@ -394,7 +394,7 @@ object TestData { Priority.Unspecified, false, None, - Seq.empty + None ) val newArticle: NewArticleDTO = api.NewArticleDTO( @@ -485,7 +485,7 @@ object TestData { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val apiArticleWithHtmlFaultV2: api.ArticleDTO = api.ArticleDTO( diff --git a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala index 9150be30f..3b4c02ecf 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/service/ConverterServiceTest.scala @@ -333,7 +333,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq(Disclaimer("Disclaimer test", "nb")) + disclaimer = Some(Seq(Disclaimer("Disclaimer test", "nb"))) ) val updatedNothing = TestData.blankUpdatedArticle.copy( @@ -381,7 +381,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq(Disclaimer("Disclaimer test", "nb")) + disclaimer = Some(Seq(Disclaimer("Disclaimer test", "nb"))) ) val expectedArticle = Draft( @@ -416,7 +416,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq(Disclaimer("NyDisclaimer test", "nb")) + disclaimer = Some(Seq(Disclaimer("NyDisclaimer test", "nb"))) ) val updatedEverything = TestData.blankUpdatedArticle.copy( @@ -482,7 +482,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val expectedArticle = Draft( @@ -525,7 +525,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val updatedEverything = TestData.blankUpdatedArticle.copy( @@ -1127,7 +1127,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq(Disclaimer("articleDisclaimer", "nb")) + disclaimer = Some(Seq(Disclaimer("articleDisclaimer", "nb"))) ) val article = common.model.domain.article.Article( id = Some(articleId), @@ -1153,7 +1153,7 @@ class ConverterServiceTest extends UnitSuite with TestEnvironment { relatedContent = Seq.empty, revisionDate = None, slug = Some("kjempe-slug"), - disclaimer = Seq(Disclaimer("articleDisclaimer", "nb")) + disclaimer = Some(Seq(Disclaimer("articleDisclaimer", "nb"))) ) val result = service.toArticleApiArticle(draft) diff --git a/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala index a1e17f136..bbad36f92 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/service/StateTransitionRulesTest.scala @@ -356,7 +356,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val article = common.article.Article( id = Some(articleId), @@ -381,7 +381,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { relatedContent = Seq.empty, revisionDate = None, slug = None, - disclaimer = Seq.empty + disclaimer = None ) val status = common.Status(END_CONTROL, Set.empty) @@ -479,7 +479,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == PUBLISHED) @@ -536,7 +536,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == ARCHIVED) @@ -597,7 +597,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val status = common.Status(PLANNED, Set.empty) val transitionsToTest = StateTransitionRules.StateTransitions.filter(_.to == UNPUBLISHED) @@ -659,7 +659,7 @@ class StateTransitionRulesTest extends UnitSuite with TestEnvironment { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val status = common.Status(PUBLISHED, Set.empty) val transitionToTest: StateTransition = PUBLISHED -> IN_PROGRESS diff --git a/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala b/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala index 64bdc0e3a..a370e954b 100644 --- a/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala +++ b/integration-tests/src/test/scala/no/ndla/integrationtests/draftapi/articleapi/ArticleApiClientTest.scala @@ -126,7 +126,7 @@ class ArticleApiClientTest priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val exampleToken = diff --git a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala index 1d91ab9b9..ee1974653 100644 --- a/search-api/src/test/scala/no/ndla/searchapi/TestData.scala +++ b/search-api/src/test/scala/no/ndla/searchapi/TestData.scala @@ -202,7 +202,7 @@ object TestData { Seq.empty, None, slug = None, - Seq.empty + None ) val sampleDomainArticle: Article = Article( @@ -228,7 +228,7 @@ object TestData { Seq.empty, None, slug = None, - Seq.empty + None ) val sampleDomainArticle2: Article = Article( @@ -254,7 +254,7 @@ object TestData { Seq.empty, None, slug = None, - Seq.empty + None ) val sampleArticleWithByNcSa: Article = @@ -537,7 +537,7 @@ object TestData { relatedContent = Seq.empty, None, slug = None, - Seq.empty + None ) val emptyDomainDraft: Draft = Draft( @@ -572,7 +572,7 @@ object TestData { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - Seq.empty + None ) val draftStatus: Status = Status(DraftStatus.PLANNED, Set.empty) @@ -635,7 +635,7 @@ object TestData { priority = Priority.Unspecified, started = false, qualityEvaluation = None, - disclaimer = Seq.empty + disclaimer = None ) val sampleDraftWithByNcSa: Draft = sampleDraftWithPublicDomain.copy(copyright = Some(draftByNcSaCopyright)) From 2a98bbef5428b524f54c61b28504673c1d0a00c7 Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Mon, 16 Dec 2024 13:43:40 +0100 Subject: [PATCH 4/6] Add content validation for disclaimer --- .../validation/ContentValidator.scala | 9 +++++ .../validation/ContentValidatorTest.scala | 38 +++++++++++++++++++ .../validation/ContentValidator.scala | 8 ++++ .../validation/ContentValidatorTest.scala | 31 +++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/article-api/src/main/scala/no/ndla/articleapi/validation/ContentValidator.scala b/article-api/src/main/scala/no/ndla/articleapi/validation/ContentValidator.scala index 4b556f08d..90fc36721 100644 --- a/article-api/src/main/scala/no/ndla/articleapi/validation/ContentValidator.scala +++ b/article-api/src/main/scala/no/ndla/articleapi/validation/ContentValidator.scala @@ -50,6 +50,7 @@ trait ContentValidator { def validateArticle(article: Article, isImported: Boolean): Try[Article] = { val validationErrors = validateArticleContent(article.content) ++ article.introduction.flatMap(i => validateIntroduction(i)) ++ + validateArticleDisclaimer(article.disclaimer.getOrElse(Seq.empty)) ++ validateMetaDescription(article.metaDescription, isImported) ++ validateTitle(article.title) ++ validateCopyright(article.copyright) ++ @@ -89,6 +90,14 @@ trait ContentValidator { }) ++ validateNonEmpty("content", contents) } + private def validateArticleDisclaimer(disclaimers: Seq[Disclaimer]): Seq[ValidationMessage] = { + disclaimers.flatMap(disclaimer => { + val field = s"disclaimer.${disclaimer.language}" + TextValidator.validate(field, disclaimer.disclaimer, allLegalTags).toList ++ + validateLanguage("content.language", disclaimer.language) + }) + } + private def rootElementContainsOnlySectionBlocks(field: String, html: String): Option[ValidationMessage] = { val legalTopLevelTag = "section" val topLevelTags = stringToJsoupDocument(html).children().asScala.map(_.tagName()) diff --git a/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala b/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala index c4f191bed..f8b4a626b 100644 --- a/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala +++ b/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala @@ -15,6 +15,7 @@ import no.ndla.common.model.domain.{ ArticleMetaImage, Author, Description, + Disclaimer, Introduction, RequiredLibrary, Tag, @@ -30,6 +31,8 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { val validDocument = """

heisann

heia

""" val validIntroduction = """

heisann heia

hopp

""" val invalidDocument = """
""" + val validDisclaimer = + """

hallo!test

""" test("validateArticle does not throw an exception on a valid document") { val article = TestData.sampleArticleWithByNcSa.copy(content = Seq(ArticleContent(validDocument, "nb"))) @@ -77,6 +80,41 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { contentValidator.validateArticle(article, false).isSuccess should be(true) } + test("validateArticle should throw an error if disclaimer contains illegal HTML tags") { + val article = TestData.sampleArticleWithByNcSa.copy( + content = Seq(ArticleContent(validDocument, "nb")), + disclaimer = Some(Seq(Disclaimer("

hei

", "nb"))) + ) + val result = contentValidator.validateArticle(article, false) + result.failed.get.asInstanceOf[ValidationException].errors.length should be(1) + result.failed.get.asInstanceOf[ValidationException].errors.head.message should be( + "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + ) + } + + test("validateArticle should not throw an error if disclaimer contains legal HTML tags") { + val article = TestData.sampleArticleWithByNcSa.copy( + content = Seq(ArticleContent(validDocument, "nb")), + disclaimer = Some( + Seq( + Disclaimer( + validDisclaimer, + "nb" + ) + ) + ) + ) + contentValidator.validateArticle(article, false).isSuccess should be(true) + } + + test("validateArticle should not throw an error if disclaimer contains plain text") { + val article = TestData.sampleArticleWithByNcSa.copy( + content = Seq(ArticleContent(validDocument, "nb")), + disclaimer = Some(Seq(Disclaimer("disclaimer", "nb"))) + ) + contentValidator.validateArticle(article, false).isSuccess should be(true) + } + test("validateArticle should throw an error if metaDescription contains HTML tags") { val article = TestData.sampleArticleWithByNcSa.copy( content = Seq(ArticleContent(validDocument, "nb")), diff --git a/draft-api/src/main/scala/no/ndla/draftapi/validation/ContentValidator.scala b/draft-api/src/main/scala/no/ndla/draftapi/validation/ContentValidator.scala index d1bd62e04..c9ff0c12b 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/validation/ContentValidator.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/validation/ContentValidator.scala @@ -84,6 +84,7 @@ trait ContentValidator { if (shouldValidateEntireArticle) article.content.flatMap(c => validateArticleContent(c)) ++ article.introduction.flatMap(i => validateIntroduction(i)) ++ + validateArticleDisclaimer(article.disclaimer.getOrElse(Seq.empty)) ++ article.metaDescription.flatMap(m => validateMetaDescription(m)) ++ validateTitles(article.title) ++ article.copyright.map(x => validateCopyright(x)).toSeq.flatten ++ @@ -164,6 +165,13 @@ trait ContentValidator { validateLanguage("content.language", content.language) } + private def validateArticleDisclaimer(disclaimers: Seq[Disclaimer]): Seq[ValidationMessage] = { + disclaimers.flatMap(disclaimer => { + TextValidator.validate("disclaimer", disclaimer.disclaimer, allLegalTags).toList ++ + validateLanguage("disclaimer.language", disclaimer.language) + }) + } + private def rootElementContainsOnlySectionBlocks(field: String, html: String): Option[ValidationMessage] = { val legalTopLevelTag = "section" val topLevelTags = stringToJsoupDocument(html).children().asScala.map(_.tagName()) diff --git a/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala index 9d626f17c..413498b69 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala @@ -21,6 +21,8 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { override val converterService: ConverterService = new ConverterService val validDocument = """

heisann

heia

""" val invalidDocument = """
""" + val validDisclaimer = + """

hallo!test

""" val articleToValidate: Draft = TestData.sampleArticleWithByNcSa.copy(responsible = Some(Responsible("hei", TestData.today))) @@ -58,7 +60,36 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { contentValidator.validateArticle(article).isSuccess should be(true) } + test("validateArticle should throw an error if disclaimer contains illegal HTML tags") { + val article = articleToValidate.copy( + content = Seq(ArticleContent(validDocument, "nb")), + disclaimer = Some(Seq(Disclaimer("

hei

", "nb"))) + ) + val result = contentValidator.validateArticle(article) + result.failed.get.asInstanceOf[ValidationException].errors.length should be(1) + result.failed.get.asInstanceOf[ValidationException].errors.head.message should be( + "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + ) + } + + test("validateArticle should not throw an error if disclaimer contains legal HTML tags") { + val article = articleToValidate.copy( + content = Seq(ArticleContent(validDocument, "nb")), + disclaimer = Some(Seq(Disclaimer(validDisclaimer, "nb"))) + ) + contentValidator.validateArticle(article).isSuccess should be(true) + + } + test("validateArticle should throw an error if metaDescription contains HTML tags") { + val article = articleToValidate.copy( + content = Seq(ArticleContent(validDocument, "nb")), + metaDescription = Seq(Description(validDisclaimer, "nb")) + ) + contentValidator.validateArticle(article).isFailure should be(true) + } + + test("validateArticle should throw an error if metaDescription contains plain text") { val article = articleToValidate.copy( content = Seq(ArticleContent(validDocument, "nb")), metaDescription = Seq(Description(validDocument, "nb")) From f74f8314e7a36e304dc36ecd470fca412962b567 Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Tue, 17 Dec 2024 14:11:23 +0100 Subject: [PATCH 5/6] Update disclaimer tests --- .../articleapi/validation/ContentValidatorTest.scala | 11 +++++++---- .../draftapi/validation/ContentValidatorTest.scala | 10 ++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala b/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala index f8b4a626b..e9e6298e5 100644 --- a/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala +++ b/article-api/src/test/scala/no/ndla/articleapi/validation/ContentValidatorTest.scala @@ -85,10 +85,13 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { content = Seq(ArticleContent(validDocument, "nb")), disclaimer = Some(Seq(Disclaimer("

hei

", "nb"))) ) - val result = contentValidator.validateArticle(article, false) - result.failed.get.asInstanceOf[ValidationException].errors.length should be(1) - result.failed.get.asInstanceOf[ValidationException].errors.head.message should be( - "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + + val Failure(error: ValidationException) = contentValidator.validateArticle(article, false) + error should be( + ValidationException( + "disclaimer.nb", + "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + ) ) } diff --git a/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala b/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala index 413498b69..f75a74233 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/validation/ContentValidatorTest.scala @@ -65,10 +65,12 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment { content = Seq(ArticleContent(validDocument, "nb")), disclaimer = Some(Seq(Disclaimer("

hei

", "nb"))) ) - val result = contentValidator.validateArticle(article) - result.failed.get.asInstanceOf[ValidationException].errors.length should be(1) - result.failed.get.asInstanceOf[ValidationException].errors.head.message should be( - "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + val Failure(error: ValidationException) = contentValidator.validateArticle(article) + error should be( + ValidationException( + "disclaimer", + "The content contains illegal tags and/or attributes. Allowed HTML tags are: h3, msgroup, a, article, sub, sup, mtext, msrow, tbody, mtd, pre, thead, figcaption, mover, msup, semantics, ol, span, mroot, munder, h4, mscarries, dt, nav, mtr, ndlaembed, li, br, mrow, merror, mphantom, u, audio, ul, maligngroup, mfenced, annotation, div, strong, section, i, mspace, malignmark, mfrac, code, h2, td, aside, em, mstack, button, dl, th, tfoot, math, tr, b, blockquote, msline, col, annotation-xml, mstyle, caption, mpadded, mo, mlongdiv, msubsup, p, munderover, maction, menclose, h1, details, mmultiscripts, msqrt, mscarry, mstac, mi, mglyph, mlabeledtr, mtable, mprescripts, summary, mn, msub, ms, table, colgroup, dd" + ) ) } From 894d7defa74744755e7434f3df51e6def15f6dfb Mon Sep 17 00:00:00 2001 From: Katrine Wist Date: Tue, 17 Dec 2024 14:11:40 +0100 Subject: [PATCH 6/6] Update logic to actually update disclaimer --- .../draftapi/service/ConverterService.scala | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala index 3150ae078..5f7287055 100644 --- a/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala +++ b/draft-api/src/main/scala/no/ndla/draftapi/service/ConverterService.scala @@ -797,15 +797,14 @@ trait ConverterService { .flatten ) - val updatedDisclaimer = toMergeInto.disclaimer.map { disclaimer => - mergeLanguageFields( - disclaimer, - maybeLang - .traverse(lang => - articleWithNewContent.disclaimer.toSeq.map(d => toDomainDisclaimer(api.DisclaimerDTO(d, lang))) - ) - .flatten - ) + val updatedDisclaimer = articleWithNewContent.disclaimer match { + case None => toMergeInto.disclaimer + case Some(newDisclaimer) => + val updated = mergeLanguageFields( + toMergeInto.disclaimer.getOrElse(Seq.empty), + maybeLang.map(lang => toDomainDisclaimer(api.DisclaimerDTO(newDisclaimer, lang))).toSeq + ) + Option.when(updated.nonEmpty)(updated) } val updatedContents = mergeLanguageFields(