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 bb4fdd2358..9d6e07095b 100644 --- a/article-api/src/test/scala/no/ndla/articleapi/TestData.scala +++ b/article-api/src/test/scala/no/ndla/articleapi/TestData.scala @@ -137,7 +137,7 @@ class TestData { Seq(), Seq( VisualElement( - s"<$EmbedTagName data-resource=\"image\" data-resource_id=\"1\" data-size=\"large\" data-align=\"center\" data-alt=\"alt\">", + s"<$EmbedTagName data-resource=\"image\" data-resource_id=\"1\" data-size=\"full\" data-align=\"center\" data-alt=\"alt\">", "en" ) ), @@ -302,7 +302,7 @@ class TestData { val sampleTitle: Title = Title("title", "en") val visualElement: VisualElement = VisualElement( - s"""<$EmbedTagName data-align="" data-alt="" data-caption="" data-resource="image" data-resource_id="1" data-size="">""", + s"""<$EmbedTagName data-align="" data-alt="" data-caption="" data-resource="image" data-resource_id="1" data-size="full">""", "nb" ) 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 0e0fb85ca1..2787adc67e 100644 --- a/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala +++ b/draft-api/src/test/scala/no/ndla/draftapi/TestData.scala @@ -572,7 +572,7 @@ object TestData { val sampleTitle: common.Title = common.Title("title", "en") val visualElement: common.VisualElement = common.VisualElement( - s"""<$EmbedTagName data-align="" data-alt="" data-caption="" data-resource="image" data-resource_id="1" data-size="">""", + s"""<$EmbedTagName data-align="" data-alt="" data-caption="" data-resource="image" data-resource_id="1" data-size="full">""", "nb" ) diff --git a/learningpath-api/src/main/scala/no/ndla/learningpathapi/validation/LearningPathValidator.scala b/learningpath-api/src/main/scala/no/ndla/learningpathapi/validation/LearningPathValidator.scala index aeeaf7787c..b0c1e569f7 100644 --- a/learningpath-api/src/main/scala/no/ndla/learningpathapi/validation/LearningPathValidator.scala +++ b/learningpath-api/src/main/scala/no/ndla/learningpathapi/validation/LearningPathValidator.scala @@ -18,7 +18,7 @@ import no.ndla.mapping.License.getLicense import no.ndla.common.model.domain.RevisionStatus import no.ndla.common.model.NDLADate import no.ndla.common.model.domain.learningpath.Introduction -import no.ndla.validation.HtmlTagRules.{stringToJsoupDocument} +import no.ndla.validation.HtmlTagRules.stringToJsoupDocument import scala.jdk.CollectionConverters.* import scala.util.{Failure, Success, Try} diff --git a/validation/src/main/resources/embed-tag-rules.json b/validation/src/main/resources/embed-tag-rules.json index 7e4f7480cb..aedc2ebb15 100644 --- a/validation/src/main/resources/embed-tag-rules.json +++ b/validation/src/main/resources/embed-tag-rules.json @@ -24,7 +24,8 @@ { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { @@ -74,7 +75,15 @@ { "name": "data-disclaimer", "validation": { - "required": true + "required": true, + "dataType": "TEXT" + } + }, + { + "name": "data-skip-content", + "validation": { + "required": false, + "dataType": "BOOLEAN" } } ], @@ -119,13 +128,19 @@ { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "inline", + "block" + ] } }, { "name": "data-text", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } } ], @@ -173,7 +188,8 @@ { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { @@ -186,7 +202,8 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -209,19 +226,25 @@ { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "external" + ] } }, { "name": "data-caption", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { @@ -239,6 +262,7 @@ "name": "data-alt", "validation": { "required": false, + "dataType": "TEXT", "mustCoexistWith": [ "data-imageid" ] @@ -257,7 +281,8 @@ { "name": "data-message", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } } ] @@ -280,19 +305,26 @@ { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "standard", + "minimal" + ] } }, { "name": "data-caption", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -338,13 +370,22 @@ { "name": "data-content-type", "validation": { - "required": false + "required": false, + "dataType": "ENUM", + "allowedValues": [ + "article" + ] } }, { "name": "data-open-in", "validation": { - "required": false + "required": false, + "dataType": "ENUM", + "allowedValues": [ + "current-context", + "new-context" + ] } } ], @@ -375,6 +416,7 @@ "name": "data-caption", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -406,13 +448,15 @@ { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -435,19 +479,28 @@ { "name": "data-align", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "", + "center", + "right", + "left" + ] } }, { "name": "data-alt", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { "name": "data-caption", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -461,13 +514,21 @@ { "name": "data-size", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "full", + "medium", + "small", + "xsmall" + ] } }, { "name": "data-border", "validation": { - "required": false + "required": false, + "dataType": "BOOLEAN" } }, { @@ -579,7 +640,13 @@ { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "gloss", + "inline", + "block" + ] } }, { @@ -685,19 +752,25 @@ { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "iframe" + ] } }, { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { "name": "data-caption", "validation": { "required": false, + "dataType": "TEXT", "mustCoexistWith": [ "data-title", "data-imageid" @@ -755,13 +828,15 @@ { "name": "data-title", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { "name": "data-type", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { @@ -773,19 +848,22 @@ { "name": "data-edition", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { "name": "data-publisher", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { "name": "data-authors", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } } ] @@ -813,7 +891,8 @@ { "name": "data-title", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -847,6 +926,7 @@ "name": "data-title", "validation": { "required": false, + "dataType": "TEXT", "mustCoexistWith": [ "data-url" ] @@ -875,7 +955,8 @@ { "name": "data-title", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { @@ -893,13 +974,19 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { "name": "data-display", "validation": { - "required": false + "required": false, + "dataType": "ENUM", + "allowedValues": [ + "block", + "inline" + ] } } ], @@ -924,7 +1011,8 @@ { "name": "data-job-title", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { @@ -943,7 +1031,8 @@ { "name": "data-description", "validation": { - "required": true + "required": true, + "dataType": "TEXT" } }, { @@ -956,7 +1045,8 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { @@ -986,6 +1076,7 @@ "name": "data-title", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1000,6 +1091,7 @@ "name": "data-description", "validation": { "required": false, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1020,7 +1112,8 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -1044,6 +1137,7 @@ "name": "data-title", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1058,6 +1152,7 @@ "name": "data-subtitle", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1090,7 +1185,8 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } } ] @@ -1107,6 +1203,7 @@ "name": "data-title", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1127,6 +1224,7 @@ "name": "data-description", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1146,7 +1244,15 @@ { "name": "data-heading-level", "validation": { - "required": true + "required": true, + "dataType": "ENUM", + "allowedValues": [ + "h1", + "h2", + "h3", + "h4", + "h5" + ] } }, { @@ -1160,6 +1266,7 @@ "name": "data-url-text", "validation": { "required": false, + "dataType": "TEXT", "allowedHtml": [ "code", "em", @@ -1180,7 +1287,12 @@ { "name": "data-image-side", "validation": { - "required": false + "required": false, + "dataType": "ENUM", + "allowedValues": [ + "left", + "right" + ] } }, { @@ -1193,7 +1305,8 @@ { "name": "data-alt", "validation": { - "required": false + "required": false, + "dataType": "TEXT" } }, { @@ -1216,6 +1329,7 @@ "name": "data-title", "validation": { "required": true, + "dataType": "TEXT", "allowedHtml": [ "code", "em", diff --git a/validation/src/main/scala/no/ndla/validation/TagRules.scala b/validation/src/main/scala/no/ndla/validation/TagRules.scala index c60835ebea..89a12d518b 100644 --- a/validation/src/main/scala/no/ndla/validation/TagRules.scala +++ b/validation/src/main/scala/no/ndla/validation/TagRules.scala @@ -45,6 +45,7 @@ object TagRules { case class Validation( dataType: AttributeType = AttributeType.STRING, required: Boolean = false, + allowedValues: Set[String] = Set.empty, allowedHtml: Set[String] = Set.empty, allowedDomains: Set[String] = Set.empty, mustCoexistWith: List[TagAttribute] = List.empty @@ -56,12 +57,14 @@ object TagRules { for { dataType <- getOrDefault[AttributeType](cur, "dataType", AttributeType.STRING) required <- getOrDefault(cur, "required", false) + allowedValues <- getOrDefault(cur, "allowedValues", Set.empty[String]) allowedHtml <- getOrDefault(cur, "allowedHtml", Set.empty[String]) allowedDomains <- getOrDefault(cur, "allowedDomains", Set.empty[String]) mustCoexistWith <- getOrDefault(cur, "mustCoexistWith", List.empty[TagAttribute]) } yield Validation( dataType = dataType, required = required, + allowedValues = allowedValues, allowedHtml = allowedHtml, allowedDomains = allowedDomains, mustCoexistWith = mustCoexistWith @@ -120,10 +123,12 @@ object AttributeType extends Enum[AttributeType] with CirceEnum[A val values: IndexedSeq[AttributeType] = findValues case object BOOLEAN extends AttributeType case object EMAIL extends AttributeType + case object ENUM extends AttributeType case object JSON extends AttributeType case object LIST extends AttributeType case object NUMBER extends AttributeType case object STRING extends AttributeType + case object TEXT extends AttributeType case object URL extends AttributeType } @@ -296,6 +301,7 @@ object TagAttribute extends Enum[TagAttribute] with CirceEnum[TagAttribute] { case object DataResource extends TagAttribute("data-resource") case object DataResource_Id extends TagAttribute("data-resource_id") case object DataSize extends TagAttribute("data-size") + case object DataSkipContent extends TagAttribute("data-skip-content") case object DataSubjectId extends TagAttribute("data-subject-id") case object DataSubtitle extends TagAttribute("data-subtitle") case object DataTag extends TagAttribute("data-tag") diff --git a/validation/src/main/scala/no/ndla/validation/TagValidator.scala b/validation/src/main/scala/no/ndla/validation/TagValidator.scala index e912a7af22..4071bbb7c9 100644 --- a/validation/src/main/scala/no/ndla/validation/TagValidator.scala +++ b/validation/src/main/scala/no/ndla/validation/TagValidator.scala @@ -459,10 +459,12 @@ object TagValidator { f.validation.dataType match { case BOOLEAN => validateBooleanField(fieldName, partialErrorMessage, key, value, f) case EMAIL => validateEmailField(fieldName, partialErrorMessage, key, value, f) + case ENUM => validateEnumField(fieldName, partialErrorMessage, key, value, f) case JSON => validateJsonField(fieldName, partialErrorMessage, key, value, f) case LIST => validateListField(fieldName, partialErrorMessage, key, value, f) case NUMBER => validateNumberField(fieldName, partialErrorMessage, key, value, f) case STRING => None + case TEXT => None case URL => validateUrlField(fieldName, partialErrorMessage, key, value, f) } ) @@ -511,6 +513,27 @@ object TagValidator { } } + private def validateEnumField( + fieldName: String, + partialErrorMessage: String, + key: TagAttribute, + value: String, + field: TagRules.Field + ): Option[ValidationMessage] = { + field.validation.allowedValues.find(_ == value) match { + case Some(_) => None + case None => + Some( + ValidationMessage( + fieldName, + s"$partialErrorMessage and $key can only contain the following values: ${field.validation.allowedValues + .mkString(", ")}" + ) + ) + + } + } + private def validateJsonField( fieldName: String, partialErrorMessage: String, diff --git a/validation/src/test/scala/no/ndla/validation/EmbedTagRulesTest.scala b/validation/src/test/scala/no/ndla/validation/EmbedTagRulesTest.scala index aeade5c603..2f1861bec1 100644 --- a/validation/src/test/scala/no/ndla/validation/EmbedTagRulesTest.scala +++ b/validation/src/test/scala/no/ndla/validation/EmbedTagRulesTest.scala @@ -64,7 +64,7 @@ class EmbedTagRulesTest extends UnitSuite { s"""<$EmbedTagName | data-resource="image" | data-resource_id="" - | data-size="" + | data-size="full" | data-align="" | data-alt="" | data-caption="" @@ -422,7 +422,7 @@ class EmbedTagRulesTest extends UnitSuite { s"""<$EmbedTagName | data-resource="image" | data-resource_id="1" - | data-size="" + | data-size="full" | data-align="" | data-alt="Bilde på engelsk" | data-caption="Bilde på engelsk" diff --git a/validation/src/test/scala/no/ndla/validation/EmbedTagValidatorTest.scala b/validation/src/test/scala/no/ndla/validation/EmbedTagValidatorTest.scala index 41c53ecf5a..c81513b8f5 100644 --- a/validation/src/test/scala/no/ndla/validation/EmbedTagValidatorTest.scala +++ b/validation/src/test/scala/no/ndla/validation/EmbedTagValidatorTest.scala @@ -76,6 +76,18 @@ class EmbedTagValidatorTest extends UnitSuite { findErrorByMessage(res, "contains attributes with HTML: data-url").size should be(1) } + test("validate should return no validation errors if data-resource is invalid") { + val tag = generateTagWithAttrs( + Map( + TagAttribute.DataResource -> "whatever", + TagAttribute.DataResource_Id -> "1234", + TagAttribute.DataCaption -> "", + TagAttribute.DataType -> "standard" + ) + ) + TagValidator.validate("content", tag).size should be(1) + } + test( "validate should return validation error if embed tag does not contain required attributes for data-resource=image" ) { @@ -92,7 +104,7 @@ class EmbedTagValidatorTest extends UnitSuite { Map( TagAttribute.DataResource -> ResourceType.Image.toString, TagAttribute.DataResource_Id -> "1234", - TagAttribute.DataSize -> "fullbredde", + TagAttribute.DataSize -> "full", TagAttribute.DataAlign -> "", TagAttribute.DataAlt -> "alttext" ) @@ -110,7 +122,7 @@ class EmbedTagValidatorTest extends UnitSuite { Map( TagAttribute.DataResource -> ResourceType.Image.toString, TagAttribute.DataResource_Id -> "1234", - TagAttribute.DataSize -> "fullbredde", + TagAttribute.DataSize -> "full", TagAttribute.DataAlt -> "alternative text", TagAttribute.DataCaption -> "here is a rabbit", TagAttribute.DataAlign -> "left" @@ -255,7 +267,7 @@ class EmbedTagValidatorTest extends UnitSuite { Map( TagAttribute.DataResource -> ResourceType.Image.toString, TagAttribute.DataResource_Id -> "1234", - TagAttribute.DataSize -> "fullbredde", + TagAttribute.DataSize -> "full", TagAttribute.DataAlt -> "alternative text", TagAttribute.DataCaption -> "here is a rabbit", TagAttribute.DataAlign -> "left" @@ -338,7 +350,7 @@ class EmbedTagValidatorTest extends UnitSuite { Map( TagAttribute.DataResource -> ResourceType.ExternalContent.toString, TagAttribute.DataUrl -> "https://www.youtube.com/watch?v=pCZeVTMEsik", - TagAttribute.DataType -> "iframe" + TagAttribute.DataType -> "external" ) ) TagValidator.validate("content", tag).size should be(0) @@ -366,6 +378,29 @@ class EmbedTagValidatorTest extends UnitSuite { TagValidator.validate("content", tag).size should be(0) } + test("validate should fail if ENUM field has wrong value") { + val tag = generateTagWithAttrs( + Map( + TagAttribute.DataResource -> ResourceType.Image.toString, + TagAttribute.DataAlt -> "123", + TagAttribute.DataCaption -> "123", + TagAttribute.DataResource_Id -> "123", + TagAttribute.DataSize -> "fullscreen", + TagAttribute.DataAlign -> "left", + TagAttribute.DataUpperLeftX -> "0", + TagAttribute.DataUpperLeftY -> "0", + TagAttribute.DataLowerRightX -> "1", + TagAttribute.DataLowerRightY -> "1", + TagAttribute.DataFocalX -> "0", + TagAttribute.DataFocalY -> "1", + TagAttribute.DataIsDecorative -> "false" + ) + ) + val messages = TagValidator.validate("content", tag) + messages.size should be(1) + messages.head.message.contains("can only contain the following values:") should be(true) + } + test("validate should fail if only one optional attribute is specified") { val tag = generateTagWithAttrs( Map(