Block Editor: hydrate Category/Tag fields for nested contentlets in StoryBlock responses

[Copilot]

category.mp4

Proposed Changes

• Fix List<TagField> → List<Field> in loadTagFields to resolve compilation error (incompatible types)
• Fix List<CategoryField> → List<Field> in loadCategoryFields to resolve compilation error (incompatible types)
• Always write { "categories": [...] } to dataMap for every CategoryField (even when no categories match), ensuring a consistent output type regardless of whether categories are assigned
• Add integration test test_loadCommonContentletProps_with_tag_field to verify Tag field hydration in StoryBlock refreshReferences(...) results
• Fix test data cleanup in test_loadCommonContentletProps_with_category_field to also delete childCategory in the finally block before deleting parentCategory, preventing test data leaks

Checklist

• Tests
• Translations
• Security Implications Contemplated (add notes if applicable)

Additional Info

ContentType.fields(Class<? extends Field>) always returns List<Field> — not a typed sublist — so the local variable declarations in both helper methods were updated to match the actual return type. All methods called on the iterated field elements (variable(), values()) are defined on the base Field class, so no behaviour changes result from the type correction.

CategoryAPI.delete(...) does not cascade-delete child categories; it only removes the category itself and its tree relations. The test now tracks childCategory outside the try block and explicitly deletes it in finally before parentCategory to avoid orphaned test data.

Screenshots

Original	Updated
** original screenshot **	** updated screenshot **

[Copilot]

Pull request overview

This PR enhances StoryBlock (Block Editor) nested contentlet hydration so Category/Tag fields are included in the refreshed attrs.data map, aligning nested content rendering with expected field semantics.

Changes:

• Add Category/Tag field hydration into StoryBlockAPIImpl.loadCommonContentletProps(...).
• Implement helper methods to load Tag fields (setTags() + map population) and Category fields (resolve selected categories under the field’s configured parent).
• Add an integration test validating Category field hydration for nested contentlets in StoryBlock refresh results.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
dotCMS/src/main/java/com/dotcms/contenttype/business/StoryBlockAPIImpl.java	Adds Category/Tag hydration into the common contentlet-to-map logic used by StoryBlock refresh/hydration.
dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockAPITest.java	Adds an integration test to validate Category field hydration for nested contentlets returned by refreshReferences(...).

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• opencollective.com
  • Triggering command: /home/REDACTED/work/core/core/installs/node/node /home/REDACTED/work/core/core/installs/node/node /home/REDACTED/work/core/core/core-web/node_modules/document-register-element/node_modules/.bin/lightercollective (dns block)
• repo.dotcms.com
  • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -classpath /home/REDACTED/work/core/core/.mvn/wrapper/maven-wrapper.jar -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/core/core org.apache.maven.wrapper.MavenWrapperMain compile -pl :dotcms-integration -am -q -DskipTests (dns block)
  • Triggering command: /usr/lib/jvm/temurin-21-jdk-amd64/bin/java /usr/lib/jvm/temurin-21-jdk-amd64/bin/java -classpath /home/REDACTED/work/core/core/.mvn/wrapper/maven-wrapper.jar -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/core/core org.apache.maven.wrapper.MavenWrapperMain compile -pl :dotcms-core -am -q -DskipTests (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[fmontes]

@claude review again

[claude]

Claude finished @fmontes's task in 2m 34s —— View job

---

Code Review (Re-review)

• Read changed files
• Assess previously identified issues
• Post review findings

---

What Was Fixed Since Last Review

✅ childCategory cleanup in test finally block — now deleted before parentCategory, correct order
✅ CategoryField always writes {categories: [...]} even when empty — unconditional dataMap.put(...) at line 505
✅ List<Field> type compatibility in both helper methods
✅ TagField integration test added (test_loadCommonContentletProps_with_tag_field)

---

Remaining Issues

---

loadCategoryFields partial-exception state (medium) — StoryBlockAPIImpl.java:483

The try/catch wraps the entire field loop AND the dataMap.put(...) call inside it. If categoryAPI.find(...) or isParent(...) throws on field N, fields 0..N-1 have already received {categories:[...]} while fields N+ are left absent. The generic field loop that follows (putIfAbsent at line 406 / 714) then fills the remaining category fields with raw contentlet values, producing inconsistent output shapes within the same contentlet depending on exactly where the error occurs.

// current: one big try wrapping all fields — mid-loop exception leaves partial state
try {
    final List<Category> categories = categoryAPI.getParents(contentlet, user, true);
    for (final Field categoryField : categoryFields) {
        final List<Map<String, Object>> childCategories = new ArrayList<>();
        // ... build childCategories ...
        dataMap.put(categoryField.variable(), Map.of("categories", childCategories));
    }
} catch (final DotDataException | DotSecurityException e) { ... }

// suggested: getParents() once (can throw), then per-field try so write always happens
final List<Category> categories;
try {
    categories = categoryAPI.getParents(contentlet, user, true);
} catch (final DotDataException | DotSecurityException e) {
    Logger.warn(this, ...);
    return;
}
for (final Field categoryField : categoryFields) {
    final List<Map<String, Object>> childCategories = new ArrayList<>();
    try {
        final Category parentCategory = categoryAPI.find(categoryField.values(), user, true);
        if (parentCategory != null && categories != null) {
            for (final Category category : categories) {
                if (categoryAPI.isParent(category, parentCategory, user, true)) {
                    childCategories.add(this.toCategoryMap(category));
                }
            }
        }
    } catch (final DotDataException | DotSecurityException e) {
        Logger.warn(this, String.format("Could not hydrate category field '%s' for contentlet '%s': %s",
                categoryField.variable(), contentlet.getIdentifier(), ExceptionUtil.getErrorMessage(e)));
    }
    dataMap.put(categoryField.variable(), Map.of("categories", childCategories)); // always written
}

Fix this →

---

loadTagFields shape is inconsistent with category fields (low) — StoryBlockAPIImpl.java:463

loadTagFields uses putIfAbsent and skips the key entirely when contentlet.get(tagField.variable()) is null (no tags set). Categories always write {categories: []}. This produces different output shapes for different field types in the same dataMap:

• No categories → {targeting: {categories: []}} (always present, structured)
• No tags → in addContentlet: {tags: null} (added by the generic putIfAbsent at line 406 with a null value); in refreshContentlet: key is absent entirely

For predictable frontend consumption, loadTagFields should behave the same as loadCategoryFields: always write a stable value (even "" or null) using dataMap.put(...) instead of putIfAbsent. Fix this →

---

Tag test leaks a persisted Tag record (low) — StoryBlockAPITest.java:1369 (new test)

new TagDataGen().name(tagName).nextPersisted();   // tag is persisted...
// ...
} finally {
    // ...but never deleted here
}

No reference is kept and no cleanup runs for the created tag. Add a Tag tag = null; variable above the try, assign it, and delete it in finally via APILocator.getTagAPI().deleteTag(tag) (or equivalent). Fix this →

---

N+1 query concern (carry-over, low-medium) — StoryBlockAPIImpl.java:499

Not new since the last review, but categoryAPI.isParent(category, parentCategory, user, true) is called O(M×N) times (M = category fields, N = categories on the contentlet), and each call may walk the category tree recursively via DB queries. For shallow trees and low cardinality this is tolerable, but worth tracking as a follow-up optimization.

---

Summary

The core issues from the previous review have been addressed: childCategory no longer leaks in the test, categories now always write a structured map shape, and a TagField test exists. Three items remain: the per-field exception isolation in loadCategoryFields (medium — can silently produce mixed output types on DB errors), the tag/category shape inconsistency (low), and the tag test data leak (low).