Skip to content

Commit

Permalink
Merge pull request #578 from NDLANO/optional-language-rework
Browse files Browse the repository at this point in the history
Convert `disclaimer` to new `OptLanguageFields` type
  • Loading branch information
jnatten authored Jan 16, 2025
2 parents 944d064 + 2b73d89 commit d88a95f
Show file tree
Hide file tree
Showing 35 changed files with 560 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ abstract class HtmlMigration extends DocumentMigration {
jsoupDocumentToString(converted)
}

protected def convertColumn(document: String): String = {
def convertColumn(document: String): String = {
val oldArticle = parser.parse(document).flatMap(_.as[Article]).toTry.get
val convertedContent = oldArticle.content.map(c => {
val converted = convertContent(c.content, c.language)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Part of NDLA article-api
* Copyright (C) 2025 NDLA
*
* See LICENSE
*
*/

package no.ndla.articleapi.db.migration

import no.ndla.database.LanguageFieldMigration

class V56__DisclaimerToLanguageFields extends LanguageFieldMigration {
override val columnName: String = "document"
override val tableName: String = "contentdata"
override val fieldName: String = "disclaimer"
}
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ trait ConverterService {
val metaImage = findByLanguageOrBestEffort(article.metaImage, language).map(toApiArticleMetaImage)
val copyright = toApiCopyright(article.copyright)
val disclaimer = article.disclaimer
.flatMap(d => findByLanguageOrBestEffort(d, language))
.map(d => DisclaimerDTO(d.disclaimer, d.language))
.findByLanguageOrBestEffort(language)
.map(DisclaimerDTO.fromLanguageValue)

Success(
api.ArticleV2DTO(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import no.ndla.common.errors.{ValidationException, ValidationMessage}
import no.ndla.common.model.NDLADate
import no.ndla.common.model.domain.article.{Article, Copyright}
import no.ndla.common.model.domain.*
import no.ndla.common.model.domain.language.OptLanguageFields
import no.ndla.language.model.{Iso639, LanguageField}
import no.ndla.mapping.License.getLicense
import no.ndla.validation.HtmlTagRules.{allLegalTags, stringToJsoupDocument}
Expand Down Expand Up @@ -50,7 +51,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)) ++
validateArticleDisclaimer(article.disclaimer) ++
validateMetaDescription(article.metaDescription, isImported) ++
validateTitle(article.title) ++
validateCopyright(article.copyright) ++
Expand Down Expand Up @@ -90,12 +91,12 @@ trait ContentValidator {
}) ++ validateNonEmpty("content", contents)
}

private def validateArticleDisclaimer(disclaimers: Seq[Disclaimer]): Seq[ValidationMessage] = {
disclaimers.flatMap(disclaimer => {
private def validateArticleDisclaimer(disclaimers: OptLanguageFields[String]): Seq[ValidationMessage] = {
disclaimers.mapExisting { disclaimer =>
val field = s"disclaimer.${disclaimer.language}"
TextValidator.validate(field, disclaimer.disclaimer, allLegalTags).toList ++
validateLanguage("content.language", disclaimer.language)
})
TextValidator.validate(field, disclaimer.value, allLegalTags).toList ++
validateLanguage("disclaimer.language", disclaimer.language)
}.flatten
}

private def rootElementContainsOnlySectionBlocks(field: String, html: String): Option[ValidationMessage] = {
Expand Down Expand Up @@ -149,7 +150,7 @@ trait ContentValidator {

private def validateTitle(titles: Seq[LanguageField[String]]): Seq[ValidationMessage] = {
titles.flatMap(title => {
val field = s"title.$language"
val field = s"title.language"
TextValidator.validate(field, title.value, inlineHtmlTags).toList ++
validateLanguage("title.language", title.language) ++
validateLength("title", title.value, 256)
Expand Down
11 changes: 6 additions & 5 deletions article-api/src/test/scala/no/ndla/articleapi/TestData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import no.ndla.common.model
import no.ndla.common.model.NDLADate
import no.ndla.common.model.domain.article.{Article, Copyright}
import no.ndla.common.model.domain.*
import no.ndla.common.model.domain.language.OptLanguageFields
import no.ndla.mapping.License

trait TestData {
Expand Down Expand Up @@ -155,7 +156,7 @@ trait TestData {
relatedContent = Seq.empty,
revisionDate = Some(NDLADate.now().withNano(0)),
slug = None,
disclaimer = None
disclaimer = OptLanguageFields.empty
)

val sampleDomainArticle: Article = Article(
Expand All @@ -181,7 +182,7 @@ trait TestData {
relatedContent = Seq.empty,
revisionDate = None,
slug = None,
disclaimer = None
disclaimer = OptLanguageFields.empty
)

val sampleDomainArticle2: Article = Article(
Expand All @@ -207,7 +208,7 @@ trait TestData {
relatedContent = Seq.empty,
revisionDate = None,
slug = None,
disclaimer = None
disclaimer = OptLanguageFields.empty
)

val sampleArticleWithByNcSa: Article = sampleArticleWithPublicDomain.copy(copyright = byNcSaCopyright)
Expand Down Expand Up @@ -245,7 +246,7 @@ trait TestData {
relatedContent = Seq.empty,
revisionDate = None,
slug = None,
disclaimer = None
disclaimer = OptLanguageFields.empty
)

val apiArticleWithHtmlFaultV2: api.ArticleV2DTO = api.ArticleV2DTO(
Expand Down Expand Up @@ -316,7 +317,7 @@ trait TestData {
relatedContent = Seq.empty,
revisionDate = None,
slug = None,
disclaimer = None
disclaimer = OptLanguageFields.empty
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Part of NDLA article-api
* Copyright (C) 2024 NDLA
*
* See LICENSE
*/

package no.ndla.articleapi.db.migration

import no.ndla.articleapi.{TestEnvironment, UnitSuite}

class V55__DisclaimerToLanguageFieldsTest extends UnitSuite with TestEnvironment {
test("That old disclaimers are migrated to new language fields") {
val oldDocument =
"""{"disclaimer":[{"disclaimer":"Dette er bokmål","language":"nb"},{"disclaimer":"Dette er nynorsk","language":"nn"}]}"""
val expectedResult = """{"disclaimer":{"nb":"Dette er bokmål","nn":"Dette er nynorsk"}}"""
val migration = new V56__DisclaimerToLanguageFields
val result = migration.convertColumn(oldDocument)
result should be(expectedResult)
}

test("That no old disclaimers are migrated to new language fields") {
val oldDocument = """{}"""
val expectedResult = """{"disclaimer":{}}"""
val migration = new V56__DisclaimerToLanguageFields
val result = migration.convertColumn(oldDocument)
result should be(expectedResult)
}

test("That null disclaimers are migrated to new language fields") {
val oldDocument = """{"disclaimer":null}"""
val expectedResult = """{"disclaimer":{}}"""
val migration = new V56__DisclaimerToLanguageFields
val result = migration.convertColumn(oldDocument)
result should be(expectedResult)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ import no.ndla.common.model.domain.{
ArticleMetaImage,
Author,
Description,
Disclaimer,
Introduction,
RequiredLibrary,
Tag,
Title
}
import no.ndla.common.model.domain.article.Copyright
import no.ndla.common.model.domain.language.OptLanguageFields
import no.ndla.mapping.License.{CC_BY_SA, NA}

import scala.util.Failure
Expand Down Expand Up @@ -83,7 +83,7 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment {
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("<p><hallo>hei</hallo></p>", "nb")))
disclaimer = OptLanguageFields.withValue("<p><hallo>hei</hallo></p>", "nb")
)

val Failure(error: ValidationException) = contentValidator.validateArticle(article, false)
Expand All @@ -98,22 +98,15 @@ class ContentValidatorTest extends UnitSuite with TestEnvironment {
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"
)
)
)
disclaimer = OptLanguageFields.withValue(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")))
disclaimer = OptLanguageFields.withValue("disclaimer", "nb")
)
contentValidator.validateArticle(article, false).isSuccess should be(true)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ class WriteServiceTest extends UnitSuite with TestEnvironment {

val result = writeService.updateAudio(1, updatedAudioMeta, Some(mock[UploadedFile]), testUser)
result.isFailure should be(true)
result.failed.get.getMessage should equal(new ValidationException(errors = Seq()).getMessage)
result.failed.get.getMessage should equal(new ValidationException(errors = Seq(validationMessage)).getMessage)
}

test("that updateAudio returns Failure when audio upload fails") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@

package no.ndla.common.errors

import no.ndla.common.errors.ValidationException.formatError

case class ValidationException(
message: String = "Validation Error",
errors: Seq[ValidationMessage]
) extends RuntimeException(message)
) extends RuntimeException(formatError(message, errors))

object ValidationException {
def apply(path: String, msg: String) = new ValidationException(errors = Seq(ValidationMessage(path, msg)))

def formatError(message: String, errors: Seq[ValidationMessage]): String = {
if (errors.nonEmpty) s"$message:\n${errors.map(e => s"\t${e.field}: ${e.message}").mkString("\n")}"
else message
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package no.ndla.common.model.api

import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}
import io.circe.{Decoder, Encoder}
import no.ndla.language.model.WithLanguageAndValue
import sttp.tapir.Schema.annotations.description

case class DisclaimerDTO(
Expand All @@ -18,6 +19,8 @@ case class DisclaimerDTO(
)

object DisclaimerDTO {
def fromLanguageValue(lv: WithLanguageAndValue[String]): DisclaimerDTO = DisclaimerDTO(lv.value, lv.language)

implicit def encoder: Encoder[DisclaimerDTO] = deriveEncoder
implicit def decoder: Decoder[DisclaimerDTO] = deriveDecoder
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ package no.ndla.common.model.domain.article
import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}
import io.circe.{Decoder, Encoder}
import no.ndla.common.model.{NDLADate, RelatedContentLink}
import no.ndla.common.model.domain._
import no.ndla.common.implicits._
import no.ndla.common.model.domain.*
import no.ndla.common.implicits.*
import no.ndla.common.model.domain.language.OptLanguageFields

case class Article(
id: Option[Long],
Expand All @@ -37,7 +38,7 @@ case class Article(
relatedContent: Seq[RelatedContent],
revisionDate: Option[NDLADate],
slug: Option[String],
disclaimer: Option[Seq[Disclaimer]]
disclaimer: OptLanguageFields[String]
) extends Content

object Article {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ package no.ndla.common.model.domain.draft

import io.circe.{Decoder, Encoder}
import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}
import no.ndla.common.implicits._
import no.ndla.common.implicits.*
import no.ndla.common.model.{NDLADate, RelatedContentLink}
import no.ndla.common.model.domain._
import no.ndla.common.model.domain.*
import no.ndla.common.model.domain.language.OptLanguageFields
import no.ndla.language.Language.getSupportedLanguages

case class Draft(
Expand Down Expand Up @@ -46,11 +47,20 @@ case class Draft(
priority: Priority,
started: Boolean,
qualityEvaluation: Option[QualityEvaluation],
disclaimer: Option[Seq[Disclaimer]]
disclaimer: OptLanguageFields[String]
) extends Content {

def supportedLanguages: Seq[String] =
getSupportedLanguages(title, visualElement, introduction, metaDescription, tags, content, metaImage)
getSupportedLanguages(
title,
visualElement,
introduction,
metaDescription,
tags,
content,
metaImage,
disclaimer.getWithLanguageFields
)
}

object Draft {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Part of NDLA common
* Copyright (C) 2025 NDLA
*
* See LICENSE
*
*/

package no.ndla.common.model.domain.language

import io.circe.*
import io.circe.syntax.EncoderOps
import no.ndla.language.Language
import no.ndla.language.model.{BaseWithLanguageAndValue, WithLanguageAndValue}

case class LanguageFields[T](internal: Map[String, T]) {
def getWithLanguageFields: Seq[WithLanguageAndValue[T]] = internal.map { case (language, value) =>
BaseWithLanguageAndValue(language, value)
}.toSeq
def get(language: String): Option[WithLanguageAndValue[T]] =
internal.get(language).map(BaseWithLanguageAndValue(language, _))
def findByLanguageOrBestEffort(language: String): Option[WithLanguageAndValue[T]] =
Language.findByLanguageOrBestEffort(getWithLanguageFields, language)
}

object LanguageFields {
def empty[T]: LanguageFields[T] = LanguageFields(Map.empty)
def fromFields[T](fields: Seq[WithLanguageAndValue[T]]): LanguageFields[T] = {
val underlyingMap = fields.map(f => f.language -> f.value).toMap
LanguageFields(underlyingMap)
}

implicit def encoder[T: Encoder]: Encoder[LanguageFields[T]] = Encoder.instance { lf => lf.internal.asJson }
implicit def decoder[T: Decoder]: Decoder[LanguageFields[T]] = Decoder.instance { json =>
json.as[Map[String, T]].map { m => LanguageFields(m) }

}
}
Loading

0 comments on commit d88a95f

Please sign in to comment.