Skip to content

Commit d0a2f2a

Browse files
authored
fix: Validate required property for block editor fields (dotCMS#33937)
## Summary Fixes validation for required block editor fields in the old content edit screen. The required property was not being enforced, allowing users to save content with empty required block editor fields. **Changes:** - Updated `ESContentletAPIImpl.java` to properly validate block editor fields marked as required - Ensures validation errors are raised when required block editor fields are left empty **Closes:** dotCMS#33906 https://github.com/user-attachments/assets/1f6b6426-9793-412a-9464-3bdc35c50fab
1 parent 5891eea commit d0a2f2a

File tree

8 files changed

+1487
-108
lines changed

8 files changed

+1487
-108
lines changed

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
import com.dotcms.rendering.velocity.services.ContentletLoader;
5757
import com.dotcms.rendering.velocity.services.PageLoader;
5858
import com.dotcms.rest.AnonymousAccess;
59+
import com.dotcms.contenttype.util.StoryBlockUtil;
60+
import com.dotcms.util.JsonUtil;
5961
import com.dotcms.rest.api.v1.temp.DotTempFile;
6062
import com.dotcms.rest.api.v1.temp.TempFileAPI;
6163
import com.dotcms.storage.FileMetadataAPI;
@@ -66,7 +68,6 @@
6668
import com.dotcms.util.ConversionUtils;
6769
import com.dotcms.util.DotPreconditions;
6870
import com.dotcms.util.FunctionUtils;
69-
import com.dotcms.util.JsonUtil;
7071
import com.dotcms.util.ThreadContextUtil;
7172
import com.dotcms.util.xstream.XStreamHandler;
7273
import com.dotcms.variant.VariantAPI;
@@ -7549,7 +7550,7 @@ public static void parseDate(final Contentlet contentlet,
75497550
field.getVelocityVarName());
75507551
}
75517552
} else {
7552-
7553+
75537554
contentlet.setDateProperty(field.getVelocityVarName(), null);
75547555
}
75557556
} else if (field.isRequired() && value == null) {
@@ -7738,6 +7739,40 @@ public void validateContentlet(final Contentlet contentlet, final List<Category>
77387739
Logger.warn(this, String.format("String Field [%s] is required", field.getVelocityVarName()));
77397740
continue;
77407741
}
7742+
} else if (field.getFieldType().equals(Field.FieldType.STORY_BLOCK_FIELD.toString())) {
7743+
// Story Block validation - handle both JSON and legacy WYSIWYG content during migration
7744+
if (fieldValue == null) {
7745+
cveBuilder.addRequiredField(field, "null");
7746+
hasError = true;
7747+
Logger.warn(this, String.format("Story Block Field [%s] is required", field.getVelocityVarName()));
7748+
continue;
7749+
} else if (!(fieldValue instanceof String)) {
7750+
cveBuilder.addBadTypeField(field, fieldValue != null ? fieldValue.toString() : "null");
7751+
hasError = true;
7752+
Logger.warn(this, String.format("Story Block Field [%s] must be a String, but got: %s",
7753+
field.getVelocityVarName(), fieldValue.getClass().getSimpleName()));
7754+
continue;
7755+
} else {
7756+
String stringValue = (String) fieldValue;
7757+
if (JsonUtil.isValidJSON(stringValue)) {
7758+
// Valid JSON - validate as Story Block
7759+
if (StoryBlockUtil.isEmptyStoryBlock(stringValue)) {
7760+
cveBuilder.addRequiredField(field, fieldValue.toString());
7761+
hasError = true;
7762+
Logger.warn(this, String.format("Story Block Field [%s] is required", field.getVelocityVarName()));
7763+
continue;
7764+
}
7765+
} else {
7766+
// Legacy WYSIWYG content (including malformed JSON) - use simple string validation for backward compatibility
7767+
if (stringValue.trim().isEmpty()) {
7768+
cveBuilder.addRequiredField(field, fieldValue.toString());
7769+
hasError = true;
7770+
Logger.warn(this, String.format("Story Block Field [%s] is required", field.getVelocityVarName()));
7771+
continue;
7772+
}
7773+
// Otherwise, let legacy WYSIWYG content (including malformed JSON) pass validation during migration
7774+
}
7775+
}
77417776
} else if (fieldValue instanceof String) {
77427777
String s1 = (String) fieldValue;
77437778
if (!UtilMethods.isSet(s1.trim())
@@ -8105,7 +8140,7 @@ private void validateBinary(final File binary, final String fieldName, final Fie
81058140
UtilMethods.prettyByteify(maxLength))))
81068141
.addBadTypeField(legacyField, String.valueOf(fileLength))
81078142
.build();
8108-
Logger.warn(this, String.format("Name of Binary field [%s] has a length: %d but the max length is: %d",
8143+
Logger.warn(this, String.format("Name of Binary field [%s] has a length: %d but the max length is: %d",
81098144
fieldName, fileLength, maxLength));
81108145
throw cve;
81118146
}
@@ -8323,6 +8358,7 @@ private void validateSite(Contentlet contentlet) {
83238358
}
83248359

83258360

8361+
83268362
@CloseDBIfOpened
83278363
@Override
83288364
public void validateContentletNoRels(final Contentlet contentlet,
@@ -8383,6 +8419,7 @@ public void validateContentlet(final Contentlet contentlet,
83838419
validateContentlet(contentlet,contentRelationships, cats, false );
83848420
}
83858421

8422+
83868423
@CloseDBIfOpened
83878424
@Override
83888425
public void validateContentlet(final Contentlet contentlet,
@@ -8480,7 +8517,7 @@ private void validateRelationships(final Contentlet contentlet,
84808517

84818518
if (!foundInRelationships && !hasExistingRelatedContent) {
84828519
hasError = true;
8483-
Logger.error(this, String.format("Required %s relationship [%s] is not present for contentlet [%s]",
8520+
Logger.error(this, String.format("Required %s relationship [%s] is not present for contentlet [%s]",
84848521
(checkParent ? "child" : "parent"), rel.getRelationTypeValue(), contentletId));
84858522
builder.addRequiredRelationship(rel, new ArrayList<>());
84868523
}
@@ -8526,7 +8563,7 @@ private void validateRelationships(final Contentlet contentlet,
85268563
&& isRelationshipParent) {
85278564
if (relationship.isChildRequired() && contentsInRelationship.isEmpty()) {
85288565
hasError = true;
8529-
Logger.error(this, String.format("Error in Contentlet [%s]: Child relationship [%s] is required.",
8566+
Logger.error(this, String.format("Error in Contentlet [%s]: Child relationship [%s] is required.",
85308567
contentletId, relationship.getRelationTypeValue()));
85318568
builder.addRequiredRelationship(relationship, contentsInRelationship);
85328569
}
@@ -8564,14 +8601,14 @@ private void validateRelationships(final Contentlet contentlet,
85648601
contentsInRelationship);
85658602
}
85668603
} catch (final DotDataException e) {
8567-
Logger.error(this, String.format("An error occurred when retrieving information from related Contentlet [%s]",
8604+
Logger.error(this, String.format("An error occurred when retrieving information from related Contentlet [%s]",
85688605
contentInRelationship.getIdentifier()), e);
85698606
}
85708607
}
85718608
} else if (APILocator.getRelationshipAPI().isChild(relationship, contentType)) {
85728609
if (relationship.isParentRequired() && contentsInRelationship.isEmpty()) {
85738610
hasError = true;
8574-
Logger.error(this, String.format("Error in Contentlet [%s]: Parent relationship [%s] is required.",
8611+
Logger.error(this, String.format("Error in Contentlet [%s]: Parent relationship [%s] is required.",
85758612
contentletId, relationship.getRelationTypeValue()));
85768613
builder.addRequiredRelationship(relationship, contentsInRelationship);
85778614
}
@@ -8587,7 +8624,7 @@ private void validateRelationships(final Contentlet contentlet,
85878624
final String parentIds = contentsInRelationship.stream()
85888625
.map(Contentlet::getIdentifier)
85898626
.collect(java.util.stream.Collectors.joining(", "));
8590-
final String errorMessage = String.format("ERROR! Child content [%s] is already related to another parent content [%s]",
8627+
final String errorMessage = String.format("ERROR! Child content [%s] is already related to another parent content [%s]",
85918628
contentletId, parentIds);
85928629
Logger.error(this, errorMessage);
85938630
hasError = true;
@@ -8605,14 +8642,14 @@ private void validateRelationships(final Contentlet contentlet,
86058642
&& !contentInRelationship.getContentTypeId().equalsIgnoreCase(
86068643
relationship.getParentStructureInode())) {
86078644
hasError = true;
8608-
Logger.error(this, String.format("Content Type of Contentlet [%s] does not match the Content Type in relationship [%s]",
8645+
Logger.error(this, String.format("Content Type of Contentlet [%s] does not match the Content Type in relationship [%s]",
86098646
contentletId, relationship.getRelationTypeValue()));
86108647
builder.addInvalidContentRelationship(relationship, contentsInRelationship);
86118648
}
86128649
}
86138650
} else {
86148651
hasError = true;
8615-
Logger.error(this, String.format("Relationship [%s] is neither parent nor child of Contentlet [%s]",
8652+
Logger.error(this, String.format("Relationship [%s] is neither parent nor child of Contentlet [%s]",
86168653
relationship.getRelationTypeValue(), contentletId));
86178654
builder.addBadRelationship(relationship, contentsInRelationship);
86188655
}
@@ -8637,7 +8674,7 @@ private boolean isValidOneToOneRelationship(final Contentlet contentlet,
86378674
List<Contentlet> contentsInRelationshipSameLanguage = groupContentletsByLanguage(contentsInRelationship);
86388675
//Trying to relate more than one piece of content
86398676
if (contentsInRelationshipSameLanguage.size() > 1) {
8640-
Logger.error(this, String.format("Error in Contentlet [%s]: Relationship [%s] has been defined as One to One",
8677+
Logger.error(this, String.format("Error in Contentlet [%s]: Relationship [%s] has been defined as One to One",
86418678
contentlet.getIdentifier(), relationship.getRelationTypeValue()));
86428679
builder.addBadCardinalityRelationship(relationship, contentsInRelationship);
86438680
return false;
@@ -8651,13 +8688,13 @@ private boolean isValidOneToOneRelationship(final Contentlet contentlet,
86518688
.getSystemUser(), true, 1, 0, null);
86528689
if (relatedContents.size() > 0 && !relatedContents.get(0).getIdentifier()
86538690
.equals(contentlet.getIdentifier())) {
8654-
Logger.error(this, String.format("Error in related Contentlet [%s]: Relationship [%s] has been defined as One to One",
8691+
Logger.error(this, String.format("Error in related Contentlet [%s]: Relationship [%s] has been defined as One to One",
86558692
relatedContents.get(0).getIdentifier(), relationship.getRelationTypeValue()));
86568693
builder.addBadCardinalityRelationship(relationship, contentsInRelationship);
86578694
return false;
86588695
}
86598696
} catch (final DotDataException e) {
8660-
Logger.error(this, String.format("An error occurred when retrieving information from related Contentlet [%s]",
8697+
Logger.error(this, String.format("An error occurred when retrieving information from related Contentlet [%s]",
86618698
contentsInRelationship.get(0).getIdentifier()), e);
86628699
builder.addInvalidContentRelationship(relationship, contentsInRelationship);
86638700
return false;
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package com.dotcms.contenttype.util;
2+
3+
import com.dotcms.util.JsonUtil;
4+
import com.fasterxml.jackson.databind.JsonNode;
5+
import com.dotmarketing.util.Logger;
6+
import com.dotmarketing.util.UtilMethods;
7+
8+
/**
9+
* Utility class for Story Block field validation and processing.
10+
* Contains domain-specific business logic for determining whether a Story Block
11+
* field contains meaningful content or is effectively empty.
12+
*
13+
* @author dotCMS Team
14+
*/
15+
public class StoryBlockUtil {
16+
17+
/**
18+
* Validates if a Story Block field contains meaningful content or is effectively empty.
19+
* A Story Block is considered empty if it only contains empty text blocks (paragraphs, headings, code blocks)
20+
* without actual text content. Non-text blocks (images, videos, lists, tables, etc.) are always
21+
* considered as having content.
22+
*
23+
* @param storyBlockValue The JSON string representing the Story Block content
24+
* @return true if the Story Block is effectively empty, false if it has meaningful content
25+
*/
26+
public static boolean isEmptyStoryBlock(final String storyBlockValue) {
27+
if (!UtilMethods.isSet(storyBlockValue) || storyBlockValue.trim().isEmpty()) {
28+
return true;
29+
}
30+
31+
try {
32+
final JsonNode storyBlockJson = JsonUtil.JSON_MAPPER.readTree(storyBlockValue);
33+
34+
// Check if it has content array
35+
if (!storyBlockJson.has("content")) {
36+
return true;
37+
}
38+
39+
final JsonNode contentNode = storyBlockJson.get("content");
40+
if (!contentNode.isArray() || contentNode.size() == 0) {
41+
return true;
42+
}
43+
44+
// If any block has content, the story block has content
45+
for (JsonNode block : contentNode) {
46+
if (!isEmptyBlock(block)) {
47+
return false; // Found content, story block is not empty
48+
}
49+
}
50+
51+
// All blocks are empty, so story block is empty
52+
return true;
53+
54+
} catch (Exception e) {
55+
Logger.warn(StoryBlockUtil.class, "Error parsing Story Block JSON, treating as empty due to malformation: " + e.getMessage());
56+
return true; // Malformed JSON = empty = validation fails
57+
}
58+
}
59+
60+
/**
61+
* Helper method to determine if a specific block within a Story Block is empty.
62+
* Simple logic: If it's a text block, check if it has actual text content.
63+
* For everything else (images, videos, custom blocks, etc.), assume it has content.
64+
*
65+
* @param block The JSON node representing a single block
66+
* @return true if the block is effectively empty, false otherwise
67+
*/
68+
public static boolean isEmptyBlock(final JsonNode block) {
69+
if (!block.has("type")) {
70+
return true; // No type means invalid/empty block
71+
}
72+
73+
final String blockType = block.get("type").asText();
74+
75+
// For text-based blocks, check if they contain actual text content
76+
if ("paragraph".equals(blockType) || "heading".equals(blockType) || "codeBlock".equals(blockType)) {
77+
return isTextContentEmpty(block);
78+
}
79+
80+
// For everything else (images, videos, lists, tables, blockquotes, custom blocks, etc.):
81+
// If the block exists, it represents content,
82+
// This is to avoid recursive checking of blocks that are not text-based like lists on tables or similar.
83+
return false;
84+
}
85+
86+
/**
87+
* Helper method to check if a text block contains only empty text
88+
* @param block The block to check for text content
89+
* @return true if all text content is empty, false otherwise
90+
*/
91+
public static boolean isTextContentEmpty(final JsonNode block) {
92+
final JsonNode contentNode = block.get("content");
93+
if (contentNode == null || !contentNode.isArray() || contentNode.size() == 0) {
94+
return true; // No content property means empty
95+
}
96+
97+
// Check if all content items are empty text nodes
98+
for (JsonNode contentItem : contentNode) {
99+
if (contentItem != null) {
100+
final String itemType = contentItem.has("type") ? contentItem.get("type").asText() : "";
101+
102+
if ("text".equals(itemType)) {
103+
final String text = contentItem.has("text") ? contentItem.get("text").asText() : "";
104+
if (UtilMethods.isSet(text.trim())) {
105+
return false; // Found non-empty text
106+
}
107+
}
108+
// Note: We only consider "text" nodes for text content
109+
// Other types like "hardBreak" might exist but don't contain text
110+
}
111+
}
112+
113+
return true; // All text content is empty
114+
}
115+
}

dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.dotcms;
22

33
import com.dotcms.ai.api.OpenAIVisionAPIImplTest;
4+
import com.dotcms.contenttype.business.StoryBlockValidationTest;
5+
import com.dotcms.contenttype.test.StoryBlockUtilTest;
46
import com.dotcms.jitsu.validators.AnalyticsValidatorUtilTest;
57
import com.dotcms.junit.MainBaseSuite;
68
import com.dotcms.rest.api.v1.drive.ContentDriveHelperContentletAPIComparisonTest;
@@ -51,7 +53,9 @@
5153
OpenAIVisionAPIImplTest.class,
5254
ContentDriveHelperContentletAPIComparisonTest.class,
5355
AppsAPIImplTest.class,
54-
Task251103AddStylePropertiesColumnInMultiTreeTest.class
56+
Task251103AddStylePropertiesColumnInMultiTreeTest.class,
57+
StoryBlockValidationTest.class,
58+
StoryBlockUtilTest.class
5559
})
5660

5761
public class MainSuite3a {

0 commit comments

Comments
 (0)