From d148372f0892b3ee9d9bdd1bf4fcd8f058b51cae Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 5 Feb 2026 16:37:35 -0500 Subject: [PATCH 1/7] feat: add TypeScript unit testing guidelines and MCP server configuration --- .cursor/agents/typescript-unit-testing.md | 55 ++++++++ core-web/.gemini/settings.json | 10 ++ core-web/.mcp.json | 9 ++ core-web/AGENTS.md | 13 +- core-web/CLAUDE.md | 151 +++++++++++----------- 5 files changed, 157 insertions(+), 81 deletions(-) create mode 100644 .cursor/agents/typescript-unit-testing.md create mode 100644 core-web/.gemini/settings.json create mode 100644 core-web/.mcp.json diff --git a/.cursor/agents/typescript-unit-testing.md b/.cursor/agents/typescript-unit-testing.md new file mode 100644 index 000000000000..c50daf6ad59a --- /dev/null +++ b/.cursor/agents/typescript-unit-testing.md @@ -0,0 +1,55 @@ +--- +name: typescript-unit-testing +description: Expert in creating and improving TypeScript unit tests. Use proactively when writing or modifying .ts/.spec.ts files, adding tests for components/services/pipes, or when asked for unit tests in Angular/TypeScript. +--- + +You are a TypeScript unit testing specialist. **Primary focus: Angular interaction** (components, templates, Spectator, TestBed). In **SDK and similar non-Angular folders**, use plain Jest and the stack appropriate to that code (no Spectator/Angular unless it's Angular SDK code). + +## Required Reading + +**Before writing any Angular test, read `docs/frontend/TESTING_FRONTEND.md`** for complete patterns including: +- Spectator API (all factories: `createComponentFactory`, `createServiceFactory`, `createDirectiveFactory`, etc.) +- `byTestId()` for element selection (required) +- `setInput()` for component inputs +- `mockProvider()` for dependencies +- `@dotcms/utils-testing` createFake functions for domain objects +- Signal conventions (`$` prefix) +- User-centric testing principles +- Common pitfalls to avoid + +## When invoked + +1. **Read `docs/frontend/TESTING_FRONTEND.md`** to get current patterns. +2. **Detect context**: Is the file under `libs/sdk/*` (or similar non-Angular libs)? If yes → use non-Angular rules. If no → use Angular rules. +3. Read the source file(s) that need tests. +4. Check for an existing `.spec.ts` file next to the source. +5. Create or extend tests following the patterns from the documentation. +6. Run or suggest the test command to verify. + +## Project context (core-web / dotCMS) + +- **Location**: Tests live in `core-web/`; `*.spec.ts` files sit alongside source files. +- **Angular code**: Jest + Spectator (`@ngneat/spectator/jest`). Follow `TESTING_FRONTEND.md`. +- **SDK / non-Angular code** (e.g. `libs/sdk/client`, `libs/sdk/analytics`): Plain Jest; no Spectator or TestBed. +- **Runner**: Nx — e.g. `cd core-web && yarn nx run :test` or `yarn nx run :test -t ComponentName`. + +## Critical Rules Summary + +| Rule | Angular | SDK/non-Angular | +|------|---------|-----------------| +| Framework | Spectator + Jest | Plain Jest | +| Selection | `byTestId()` only | N/A | +| Inputs | `setInput()` | N/A | +| Mocks | `mockProvider()`, `SpyObject` | `jest.fn()` | +| Domain data | `createFake*` from `@dotcms/utils-testing` | As needed | +| Focus | User-visible behavior | Public API | + +## Output format + +1. **Context**: Angular or SDK/non-Angular (and why). +2. **Summary**: What is under test and scenarios covered. +3. **Test file**: Full `.spec.ts` following `TESTING_FRONTEND.md` patterns. +4. **Run command**: `yarn nx run :test -t ComponentName`. +5. **Checklist**: Verify `byTestId()`, `setInput()`, `mockProvider`, `createFake*`, user flow focus. + +If the source file is missing `data-testid` attributes, suggest the minimal template changes. diff --git a/core-web/.gemini/settings.json b/core-web/.gemini/settings.json new file mode 100644 index 000000000000..4c481fb7c8d1 --- /dev/null +++ b/core-web/.gemini/settings.json @@ -0,0 +1,10 @@ +{ + "mcpServers": { + "nx-mcp": { + "type": "stdio", + "command": "npx", + "args": ["nx-mcp"] + } + }, + "contextFileName": "AGENTS.md" +} diff --git a/core-web/.mcp.json b/core-web/.mcp.json new file mode 100644 index 000000000000..7206148e4d3d --- /dev/null +++ b/core-web/.mcp.json @@ -0,0 +1,9 @@ +{ + "mcpServers": { + "nx-mcp": { + "type": "stdio", + "command": "npx", + "args": ["nx-mcp"] + } + } +} diff --git a/core-web/AGENTS.md b/core-web/AGENTS.md index 5cf3ccfda731..ec27f8330efa 100644 --- a/core-web/AGENTS.md +++ b/core-web/AGENTS.md @@ -3,11 +3,12 @@ # General Guidelines for working with Nx -- When running tasks (for example build, lint, test, e2e, etc.), always prefer running the task through `nx` (i.e. `nx run`, `nx run-many`, `nx affected`) instead of using the underlying tooling directly -- You have access to the Nx MCP server and its tools, use them to help the user -- When answering questions about the repository, use the `nx_workspace` tool first to gain an understanding of the workspace architecture where applicable. -- When working in individual projects, use the `nx_project_details` mcp tool to analyze and understand the specific project structure and dependencies -- For questions around nx configuration, best practices or if you're unsure, use the `nx_docs` tool to get relevant, up-to-date docs. Always use this instead of assuming things about nx configuration -- If the user needs help with an Nx configuration or project graph error, use the `nx_workspace` tool to get any errors +- When running tasks (for example build, lint, test, e2e, etc.), always prefer running the task through `nx` (i.e. `nx run`, `nx run-many`, `nx affected`) instead of using the underlying tooling directly +- You have access to the Nx MCP server and its tools, use them to help the user +- When answering questions about the repository, use the `nx_workspace` tool first to gain an understanding of the workspace architecture where applicable. +- When working in individual projects, use the `nx_project_details` mcp tool to analyze and understand the specific project structure and dependencies +- For questions around nx configuration, best practices or if you're unsure, use the `nx_docs` tool to get relevant, up-to-date docs. Always use this instead of assuming things about nx configuration +- If the user needs help with an Nx configuration or project graph error, use the `nx_workspace` tool to get any errors +- For Nx plugin best practices, check `node_modules/@nx//PLUGIN.md`. Not all plugins have this file - proceed without it if unavailable. diff --git a/core-web/CLAUDE.md b/core-web/CLAUDE.md index 738cda575d1b..581507a8d234 100644 --- a/core-web/CLAUDE.md +++ b/core-web/CLAUDE.md @@ -91,33 +91,33 @@ nx run-many --target=test --projects=sdk-client,sdk-react ### Monorepo Organization -- **apps/** - Main applications (dotcms-ui, dotcms-block-editor, dotcms-binary-field-builder, mcp-server) -- **libs/sdk/** - External-facing SDKs (client, react, angular, analytics, experiments, uve) -- **libs/data-access/** - Angular services for API communication -- **libs/ui/** - Shared UI components and patterns -- **libs/portlets/** - Feature-specific portlets (analytics, experiments, locales, etc.) -- **libs/dotcms-models/** - TypeScript interfaces and types -- **libs/block-editor/** - TipTap-based rich text editor -- **libs/template-builder/** - Template construction utilities +- **apps/** - Main applications (dotcms-ui, dotcms-block-editor, dotcms-binary-field-builder, mcp-server) +- **libs/sdk/** - External-facing SDKs (client, react, angular, analytics, experiments, uve) +- **libs/data-access/** - Angular services for API communication +- **libs/ui/** - Shared UI components and patterns +- **libs/portlets/** - Feature-specific portlets (analytics, experiments, locales, etc.) +- **libs/dotcms-models/** - TypeScript interfaces and types +- **libs/block-editor/** - TipTap-based rich text editor +- **libs/template-builder/** - Template construction utilities ### Technology Stack -- **Angular 19.2.9** with standalone components -- **Nx 20.5.1** for monorepo management -- **PrimeNG 17.18.11** UI components -- **TipTap 2.14.0** for rich text editing -- **NgRx 19.2.1** for state management -- **Jest 29.7.0** for testing -- **Playwright** for E2E testing -- **Node.js >=v22.15.0** requirement +- **Angular 19.2.9** with standalone components +- **Nx 20.5.1** for monorepo management +- **PrimeNG 17.18.11** UI components +- **TipTap 2.14.0** for rich text editing +- **NgRx 19.2.1** for state management +- **Jest 29.7.0** for testing +- **Playwright** for E2E testing +- **Node.js >=v22.15.0** requirement ### Component Conventions -- **Prefix**: All Angular components use `dot-` prefix -- **Naming**: Follow Angular style guide with kebab-case -- **Architecture**: Feature modules with lazy loading -- **State**: Component-store pattern with NgRx signals -- **Testing**: Jest unit tests + Playwright E2E +- **Prefix**: All Angular components use `dot-` prefix +- **Naming**: Follow Angular style guide with kebab-case +- **Architecture**: Feature modules with lazy loading +- **State**: Component-store pattern with NgRx signals +- **Testing**: Jest unit tests + Playwright E2E ### Modern Angular Syntax (REQUIRED) @@ -141,10 +141,10 @@ const button = spectator.query('[data-testid="submit-button"]'); ### Backend Integration -- **Development Proxy**: `proxy-dev.conf.mjs` routes `/api/*` to port 8080 -- **API Services**: Centralized in `libs/data-access` -- **Authentication**: Bearer token-based with `DotcmsConfigService` -- **Content Management**: Full CRUD through `DotHttpService` +- **Development Proxy**: `proxy-dev.conf.mjs` routes `/api/*` to port 8080 +- **API Services**: Centralized in `libs/data-access` +- **Authentication**: Bearer token-based with `DotcmsConfigService` +- **Content Management**: Full CRUD through `DotHttpService` ## Development Workflows @@ -167,67 +167,67 @@ const button = spectator.query('[data-testid="submit-button"]'); ### SDK Development -- **Client SDK**: Core API client in `libs/sdk/client` -- **React SDK**: React components in `libs/sdk/react` -- **Angular SDK**: Angular services in `libs/sdk/angular` -- **Publishing**: Automated via npm with proper versioning +- **Client SDK**: Core API client in `libs/sdk/client` +- **React SDK**: React components in `libs/sdk/react` +- **Angular SDK**: Angular services in `libs/sdk/angular` +- **Publishing**: Automated via npm with proper versioning ### Testing Strategy -- **Unit Tests**: Jest with comprehensive mocking utilities -- **E2E Tests**: Playwright for critical user workflows -- **Coverage**: Reports generated to `../../../target/core-web-reports/` -- **Mock Data**: Extensive mock utilities in `libs/utils-testing` +- **Unit Tests**: Jest with comprehensive mocking utilities +- **E2E Tests**: Playwright for critical user workflows +- **Coverage**: Reports generated to `../../../target/core-web-reports/` +- **Mock Data**: Extensive mock utilities in `libs/utils-testing` ### Build Targets & Configurations -- **Development**: Proxy configuration with source maps -- **Production**: Optimized builds with tree shaking -- **Library**: Rollup/Vite builds for SDK packages -- **Web Components**: Stencil.js compilation for `dotcms-webcomponents` +- **Development**: Proxy configuration with source maps +- **Production**: Optimized builds with tree shaking +- **Library**: Rollup/Vite builds for SDK packages +- **Web Components**: Stencil.js compilation for `dotcms-webcomponents` ## Important Notes ### TypeScript Configuration -- **Strict Mode**: Enabled across all projects -- **Path Mapping**: Extensive use of `@dotcms/*` barrel exports -- **Types**: Centralized in `libs/dotcms-models` and `libs/sdk/types` +- **Strict Mode**: Enabled across all projects +- **Path Mapping**: Extensive use of `@dotcms/*` barrel exports +- **Types**: Centralized in `libs/dotcms-models` and `libs/sdk/types` ### State Management -- **NgRx**: Component stores with signals pattern -- **Global Store**: Centralized state in `libs/global-store` -- **Services**: Angular services for data access and business logic +- **NgRx**: Component stores with signals pattern +- **Global Store**: Centralized state in `libs/global-store` +- **Services**: Angular services for data access and business logic ### Web Components -- **Stencil.js**: Framework-agnostic components in `libs/dotcms-webcomponents` -- **Legacy**: `libs/dotcms-field-elements` (deprecated, use Stencil components) -- **Integration**: Used across Angular, React, and vanilla JS contexts +- **Stencil.js**: Framework-agnostic components in `libs/dotcms-webcomponents` +- **Legacy**: `libs/dotcms-field-elements` (deprecated, use Stencil components) +- **Integration**: Used across Angular, React, and vanilla JS contexts ### Performance Considerations -- **Lazy Loading**: Feature modules loaded on demand -- **Tree Shaking**: Proper barrel exports for optimal bundles -- **Caching**: Nx task caching for faster builds -- **Affected**: Only build/test changed projects in CI +- **Lazy Loading**: Feature modules loaded on demand +- **Tree Shaking**: Proper barrel exports for optimal bundles +- **Caching**: Nx task caching for faster builds +- **Affected**: Only build/test changed projects in CI ## Debugging & Troubleshooting ### Common Issues -- **Proxy Errors**: Ensure backend is running on port 8080 -- **Build Failures**: Check TypeScript paths and circular dependencies -- **Test Failures**: Verify mock data and async handling -- **Linting**: Follow component naming conventions with `dot-` prefix +- **Proxy Errors**: Ensure backend is running on port 8080 +- **Build Failures**: Check TypeScript paths and circular dependencies +- **Test Failures**: Verify mock data and async handling +- **Linting**: Follow component naming conventions with `dot-` prefix ### Development Tools -- **Nx Console**: VS Code extension for Nx commands -- **Angular DevTools**: Browser extension for debugging -- **Coverage Reports**: Check `target/core-web-reports/` for test coverage -- **Dependency Graph**: Use `nx dep-graph` to visualize project relationships +- **Nx Console**: VS Code extension for Nx commands +- **Angular DevTools**: Browser extension for debugging +- **Coverage Reports**: Check `target/core-web-reports/` for test coverage +- **Dependency Graph**: Use `nx dep-graph` to visualize project relationships This codebase emphasizes consistency, testability, and maintainability through its monorepo architecture and established patterns. @@ -235,31 +235,32 @@ This codebase emphasizes consistency, testability, and maintainability through i ### Angular/TypeScript Development -- ✅ Use modern control flow: `@if`, `@for` (NOT `*ngIf`, `*ngFor`) -- ✅ Use modern inputs/outputs: `input()`, `output()` (NOT `@Input()`, `@Output()`) -- ✅ Use `data-testid` attributes for all testable elements -- ✅ Use `spectator.setInput()` for testing component inputs -- ✅ Follow `dot-` prefix convention for all components -- ✅ Use standalone components with lazy loading -- ✅ Use NgRx signals for state management -- ❌ Avoid legacy Angular syntax (`*ngIf`, `@Input()`, etc.) -- ❌ Avoid direct DOM queries without `data-testid` -- ❌ Never skip unit tests for new components +- ✅ Use modern control flow: `@if`, `@for` (NOT `*ngIf`, `*ngFor`) +- ✅ Use modern inputs/outputs: `input()`, `output()` (NOT `@Input()`, `@Output()`) +- ✅ Use `data-testid` attributes for all testable elements +- ✅ Use `spectator.setInput()` for testing component inputs +- ✅ Follow `dot-` prefix convention for all components +- ✅ Use standalone components with lazy loading +- ✅ Use NgRx signals for state management +- ❌ Avoid legacy Angular syntax (`*ngIf`, `@Input()`, etc.) +- ❌ Avoid direct DOM queries without `data-testid` +- ❌ Never skip unit tests for new components ### For Backend/Java Development -- See **[../CLAUDE.md](../CLAUDE.md)** for Java, Maven, REST API, and Git workflow standards +- See **[../CLAUDE.md](../CLAUDE.md)** for Java, Maven, REST API, and Git workflow standards # General Guidelines for working with Nx -- When running tasks (for example build, lint, test, e2e, etc.), always prefer running the task through `nx` (i.e. `nx run`, `nx run-many`, `nx affected`) instead of using the underlying tooling directly -- You have access to the Nx MCP server and its tools, use them to help the user -- When answering questions about the repository, use the `nx_workspace` tool first to gain an understanding of the workspace architecture where applicable. -- When working in individual projects, use the `nx_project_details` mcp tool to analyze and understand the specific project structure and dependencies -- For questions around nx configuration, best practices or if you're unsure, use the `nx_docs` tool to get relevant, up-to-date docs. Always use this instead of assuming things about nx configuration -- If the user needs help with an Nx configuration or project graph error, use the `nx_workspace` tool to get any errors +- When running tasks (for example build, lint, test, e2e, etc.), always prefer running the task through `nx` (i.e. `nx run`, `nx run-many`, `nx affected`) instead of using the underlying tooling directly +- You have access to the Nx MCP server and its tools, use them to help the user +- When answering questions about the repository, use the `nx_workspace` tool first to gain an understanding of the workspace architecture where applicable. +- When working in individual projects, use the `nx_project_details` mcp tool to analyze and understand the specific project structure and dependencies +- For questions around nx configuration, best practices or if you're unsure, use the `nx_docs` tool to get relevant, up-to-date docs. Always use this instead of assuming things about nx configuration +- If the user needs help with an Nx configuration or project graph error, use the `nx_workspace` tool to get any errors +- For Nx plugin best practices, check `node_modules/@nx//PLUGIN.md`. Not all plugins have this file - proceed without it if unavailable. From 4279d93aa6d36937ac32bfe23719822eee6810ad Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 5 Feb 2026 16:38:14 -0500 Subject: [PATCH 2/7] feat: implement character limit validation for Story Block fields This update introduces character limit validation for Story Block fields in the content management system. The validation checks if the character count exceeds a specified limit and logs warnings accordingly. Additionally, it enhances the to handle character limit errors, ensuring that relevant error messages are generated when limits are exceeded. Key changes include: - New method in to extract character count from Story Block JSON. - Updates to to validate character limits during content checks. - Modifications to and to manage character limit errors. - Added tests to ensure proper validation behavior for Story Block fields. This feature improves content integrity by enforcing character limits on Story Block fields, enhancing user experience and data consistency. --- .../business/ESContentletAPIImpl.java | 27 ++ .../contenttype/util/StoryBlockUtil.java | 33 +++ .../com/dotcms/exception/ExceptionUtil.java | 14 + .../contentlet/ajax/ContentletAjax.java | 13 + .../DotContentletValidationException.java | 61 ++++- .../FileAssetValidationException.java | 10 + .../importer/ImportLineValidationCodes.java | 7 +- .../WEB-INF/messages/Language.properties | 1 + .../business/StoryBlockValidationTest.java | 254 ++++++++++++++++++ .../contenttype/test/StoryBlockUtilTest.java | 152 +++++++++++ 10 files changed, 569 insertions(+), 3 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java index c939d9e004ef..f0b33b823855 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java @@ -218,6 +218,7 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; @@ -8065,6 +8066,32 @@ public void validateContentlet(final Contentlet contentlet, final List } } } + // validate charLimit for Story Block fields + if (field.getFieldType().equals(Field.FieldType.STORY_BLOCK_FIELD.toString()) + && fieldValue instanceof String) { + final Optional charLimitOpt = newField.fieldVariableValue("charLimit"); + if (charLimitOpt.isPresent()) { + try { + final int charLimit = Integer.parseInt(charLimitOpt.get()); + if (charLimit > 0) { + final OptionalInt charCountOpt = StoryBlockUtil.getCharCount((String) fieldValue); + if (charCountOpt.isPresent() && charCountOpt.getAsInt() > charLimit) { + hasError = true; + cveBuilder.addCharLimitField(field, charLimit); + Logger.warn(this, String.format( + "Story Block Field [%s] exceeds character limit: %d / %d", + field.getVelocityVarName(), charCountOpt.getAsInt(), charLimit)); + continue; + } + } + } catch (final NumberFormatException e) { + Logger.warn(this, String.format( + "Invalid charLimit value '%s' for Story Block Field [%s]", + charLimitOpt.get(), field.getVelocityVarName())); + } + } + } + // validate binary if (isFieldTypeBinary(field)) { this.validateBinary((File) fieldValue, field.getVelocityVarName(), field, contentType); diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/util/StoryBlockUtil.java b/dotCMS/src/main/java/com/dotcms/contenttype/util/StoryBlockUtil.java index 9d8df2ca8647..ea4bcf93e30e 100644 --- a/dotCMS/src/main/java/com/dotcms/contenttype/util/StoryBlockUtil.java +++ b/dotCMS/src/main/java/com/dotcms/contenttype/util/StoryBlockUtil.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.dotmarketing.util.Logger; import com.dotmarketing.util.UtilMethods; +import java.util.OptionalInt; /** * Utility class for Story Block field validation and processing. @@ -83,6 +84,38 @@ public static boolean isEmptyBlock(final JsonNode block) { return false; } + /** + * Extracts the character count from a Story Block JSON value. + * The TipTap editor stores character count metadata in the root {@code attrs.charCount} field. + * + * @param storyBlockValue The JSON string representing the Story Block content + * @return An {@link OptionalInt} containing the character count if present and valid, + * or {@link OptionalInt#empty()} if the value is not set, not valid JSON, + * or does not contain a charCount attribute. + */ + public static OptionalInt getCharCount(final String storyBlockValue) { + if (!UtilMethods.isSet(storyBlockValue)) { + return OptionalInt.empty(); + } + + try { + final JsonNode storyBlockJson = JsonUtil.JSON_MAPPER.readTree(storyBlockValue); + + if (storyBlockJson.has("attrs")) { + final JsonNode attrsNode = storyBlockJson.get("attrs"); + if (attrsNode.has("charCount") && attrsNode.get("charCount").isInt()) { + return OptionalInt.of(attrsNode.get("charCount").asInt()); + } + } + + return OptionalInt.empty(); + } catch (Exception e) { + Logger.debug(StoryBlockUtil.class, + "Unable to extract charCount from Story Block JSON: " + e.getMessage()); + return OptionalInt.empty(); + } + } + /** * Helper method to check if a text block contains only empty text * @param block The block to check for text content diff --git a/dotCMS/src/main/java/com/dotcms/exception/ExceptionUtil.java b/dotCMS/src/main/java/com/dotcms/exception/ExceptionUtil.java index fcd401c3daf3..0cbb9a79c12b 100644 --- a/dotCMS/src/main/java/com/dotcms/exception/ExceptionUtil.java +++ b/dotCMS/src/main/java/com/dotcms/exception/ExceptionUtil.java @@ -2,6 +2,7 @@ import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_BADTYPE; import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_BAD_CARDINALITY; +import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_CHAR_LIMIT; import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_BAD_REL; import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_INVALID_REL_CONTENT; import static com.dotmarketing.portlets.contentlet.business.DotContentletValidationException.VALIDATION_FAILED_PATTERN; @@ -351,6 +352,19 @@ public static Map> mapValidationException(final Us } + if (ve.hasCharLimitErrors()) { + final List reqs = ve.getNotValidFields().get(VALIDATION_FAILED_CHAR_LIMIT); + final Map charLimitMaxByFieldVar = ve.getCharLimitMaxByFieldVar(); + for (final Field field : reqs) { + final Integer maxLimit = charLimitMaxByFieldVar.get(field.getVelocityVarName()); + String errorString = LanguageUtil.get(user, "dot.edit.content.form.field.charLimitExceeded", + maxLimit != null ? maxLimit : 0); + contentValidationErrors + .computeIfAbsent(VALIDATION_FAILED_CHAR_LIMIT, k -> new ArrayList<>()) + .add(new ValidationError(field.getVelocityVarName(), errorString)); + } + } + if (ve.hasRelationshipErrors()) { final StringBuilder sb = new StringBuilder(); final Map>> notValidRelationships = ve diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/ajax/ContentletAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/ajax/ContentletAjax.java index 82f4ef65b95d..1149450ea9af 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/ajax/ContentletAjax.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/ajax/ContentletAjax.java @@ -2320,6 +2320,19 @@ private boolean handleValidationException(final User user, clearBinary = false; } + if (ve.hasCharLimitErrors()) { + final List reqs = ve.getNotValidFields() + .get(DotContentletValidationException.VALIDATION_FAILED_CHAR_LIMIT); + final Map charLimitMaxByFieldVar = ve.getCharLimitMaxByFieldVar(); + for (final Field field : reqs) { + final Integer maxLimit = charLimitMaxByFieldVar.get(field.getVelocityVarName()); + String errorString = LanguageUtil.get(user, "dot.edit.content.form.field.charLimitExceeded", + maxLimit != null ? maxLimit : 0); + saveContentErrors.add(errorString); + } + clearBinary = false; + } + if (ve.hasRelationshipErrors()) { StringBuilder sb = new StringBuilder("
"); final Map>> notValidRelationships = ve diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/DotContentletValidationException.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/DotContentletValidationException.java index ab6aef848dd3..c9ae69483c04 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/DotContentletValidationException.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/business/DotContentletValidationException.java @@ -33,6 +33,7 @@ public class DotContentletValidationException extends DotContentletStateExceptio public static final String VALIDATION_FAILED_BADTYPE = "badType"; public static final String VALIDATION_FAILED_REQUIRED = "required"; public static final String VALIDATION_FAILED_PATTERN = "pattern"; + public static final String VALIDATION_FAILED_CHAR_LIMIT = "charLimitExceeded"; public static final String VALIDATION_FAILED_REQUIRED_REL = "reqRel"; public static final String VALIDATION_FAILED_INVALID_REL_CONTENT = "badRelCon"; @@ -43,6 +44,8 @@ public class DotContentletValidationException extends DotContentletStateExceptio private static final long serialVersionUID = 1L; private final Map> notValidFields = new HashMap<>(); private final Map>> notValidRelationships = new HashMap<>(); + /** Max character limit per field (key: field velocity var name) for CHAR_LIMIT validation errors */ + private final Map charLimitMaxByFieldVar = new HashMap<>(); private ImportLineError importLineError = null; @@ -89,6 +92,22 @@ protected DotContentletValidationException(String x, ImportLineError importLineE this.notValidRelationships.putAll(notValidRelationships); } + /** + * Package-private constructor for setting ImportLineError, validation details, and char limit data from Builder + */ + protected DotContentletValidationException(String x, ImportLineError importLineError, + Map> notValidFields, + Map>> notValidRelationships, + Map charLimitMaxByFieldVar) { + super(x); + this.importLineError = importLineError; + this.notValidFields.putAll(notValidFields); + this.notValidRelationships.putAll(notValidRelationships); + if (charLimitMaxByFieldVar != null) { + this.charLimitMaxByFieldVar.putAll(charLimitMaxByFieldVar); + } + } + /** * Returns a map where the key is the validation property and the value is a List that failed @@ -192,6 +211,21 @@ public void addBadTypeField(Field field){ notValidFields.put(VALIDATION_FAILED_BADTYPE, fields); } + /** + * Add a field that failed character limit validation (e.g. Story Block charLimit exceeded). + * @param field the field that failed + * @param maxLimit the configured character limit + */ + public void addCharLimitField(Field field, int maxLimit) { + List fields = notValidFields.get(VALIDATION_FAILED_CHAR_LIMIT); + if (fields == null) { + fields = new ArrayList<>(); + } + fields.add(field); + notValidFields.put(VALIDATION_FAILED_CHAR_LIMIT, fields); + charLimitMaxByFieldVar.put(field.getVelocityVarName(), maxLimit); + } + /** * Use to add a field that failed unique field validation * @param field @@ -227,6 +261,18 @@ public boolean hasBadTypeErrors(){ return true; } + public boolean hasCharLimitErrors() { + List fields = notValidFields.get(VALIDATION_FAILED_CHAR_LIMIT); + return fields != null && !fields.isEmpty(); + } + + /** + * Returns the max character limit per field for CHAR_LIMIT validation errors (key: field velocity var name). + */ + public Map getCharLimitMaxByFieldVar() { + return new HashMap<>(charLimitMaxByFieldVar); + } + /** * use to find out if contentlet has any field validation errors * @return @@ -422,6 +468,15 @@ public Builder addBadTypeField(Field field, String value) { return this; } + /** + * Add a character limit exceeded validation error (e.g. Story Block charLimit) + */ + public Builder addCharLimitField(Field field, int maxLimit) { + exception.addCharLimitField(field, maxLimit); + captureFirstError(field, String.valueOf(maxLimit), ImportLineValidationCodes.CHAR_LIMIT_EXCEEDED.name(), null, null); + return this; + } + /** * Add a unique field validation error */ @@ -551,10 +606,12 @@ public Optional getValue() { final T newException; if (exception instanceof FileAssetValidationException) { newException = (T) new FileAssetValidationException(exception.getMessage(), lineError, - exception.getNotValidFields(), exception.getNotValidRelationship()); + exception.getNotValidFields(), exception.getNotValidRelationship(), + exception.getCharLimitMaxByFieldVar()); } else { newException = (T) new DotContentletValidationException(exception.getMessage(), lineError, - exception.getNotValidFields(), exception.getNotValidRelationship()); + exception.getNotValidFields(), exception.getNotValidRelationship(), + exception.getCharLimitMaxByFieldVar()); } return newException; diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetValidationException.java b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetValidationException.java index 38b06a0fb1d3..fe2cf8b5c18d 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetValidationException.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/fileassets/business/FileAssetValidationException.java @@ -49,4 +49,14 @@ public FileAssetValidationException(String x, ImportLineError importLineError, super(x, importLineError, notValidFields, notValidRelationships); } + /** + * Package-private constructor for setting ImportLineError, validation details, and char limit data from Builder + */ + public FileAssetValidationException(String x, ImportLineError importLineError, + Map> notValidFields, + Map>> notValidRelationships, + Map charLimitMaxByFieldVar) { + super(x, importLineError, notValidFields, notValidRelationships, charLimitMaxByFieldVar); + } + } diff --git a/dotCMS/src/main/java/com/dotmarketing/util/importer/ImportLineValidationCodes.java b/dotCMS/src/main/java/com/dotmarketing/util/importer/ImportLineValidationCodes.java index d247aa15985c..8d2fc4a201ff 100644 --- a/dotCMS/src/main/java/com/dotmarketing/util/importer/ImportLineValidationCodes.java +++ b/dotCMS/src/main/java/com/dotmarketing/util/importer/ImportLineValidationCodes.java @@ -126,6 +126,11 @@ public enum ImportLineValidationCodes { /** * Invalid Json */ - INVALID_JSON + INVALID_JSON, + + /** + * Story Block field exceeds configured character limit + */ + CHAR_LIMIT_EXCEEDED } \ No newline at end of file diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index a94310599d77..335242d926a7 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -1265,6 +1265,7 @@ dot.file.relationship.dialog.table.status=Status dot.file.relationship.dialog.table.last.modified=Last Modified dot.file.relationship.dialog.show.selected.items=Show selected items dot.edit.content.form.field.required=This field is mandatory +dot.edit.content.form.field.charLimitExceeded=Character limit exceeded. Maximum allowed: {0} characters. dot.common.apply=Apply dot.common.archived=Archived dot.common.cancel=Cancel diff --git a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java index 6cd822fc915b..4f2d5dd3455d 100644 --- a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java @@ -2,6 +2,8 @@ import com.dotcms.IntegrationTestBase; import com.dotcms.contenttype.model.field.Field; +import com.dotcms.contenttype.model.field.FieldVariable; +import com.dotcms.contenttype.model.field.ImmutableFieldVariable; import com.dotcms.contenttype.model.field.ImmutableStoryBlockField; import com.dotcms.contenttype.model.type.ContentType; import com.dotcms.datagen.ContentTypeDataGen; @@ -31,6 +33,9 @@ public class StoryBlockValidationTest extends IntegrationTestBase { private static ContentType testContentType; private static Field storyBlockField; + private static ContentType charLimitContentType; + private static Field charLimitStoryBlockField; + /** * Functional interface for operations that can throw checked exceptions */ @@ -109,6 +114,29 @@ private void expectBadTypeValidationException(CheckedOperation operation, String } } + /** + * Helper method to handle DotContentletValidationException for CHAR_LIMIT (character limit exceeded) errors + */ + private void expectCharLimitValidationException(CheckedOperation operation, String expectedErrorMessage) { + try { + operation.run(); + fail(expectedErrorMessage); + } catch (DotContentletValidationException e) { + assertTrue("Should contain char limit error", e.hasCharLimitErrors()); + } catch (DotRuntimeException e) { + if (e.getCause() instanceof DotContentletValidationException) { + DotContentletValidationException ve = (DotContentletValidationException) e.getCause(); + assertTrue("Should contain char limit error", ve.hasCharLimitErrors()); + } else { + final String message = e.getMessage(); + assertTrue("Exception message should indicate char limit error: " + message, + message != null && (message.contains("charLimitExceeded") || message.contains("character limit") || message.contains("has invalid/missing field"))); + } + } catch (DotDataException | DotSecurityException e) { + fail("Unexpected exception: " + e.getClass().getSimpleName() + " - " + e.getMessage()); + } + } + @BeforeClass public static void prepare() throws Exception { //Setting web app environment @@ -129,6 +157,29 @@ public static void prepare() throws Exception { .build(); storyBlockField = APILocator.getContentTypeFieldAPI().save(storyBlockField, systemUser); + + // Create a content type with a story block field that has a charLimit field variable + charLimitContentType = new ContentTypeDataGen() + .name("StoryBlockCharLimitTest") + .velocityVarName("storyBlockCharLimitTest") + .nextPersisted(); + + charLimitStoryBlockField = ImmutableStoryBlockField.builder() + .name("Story Block With Limit") + .variable("storyBlockWithLimit") + .contentTypeId(charLimitContentType.id()) + .required(false) + .build(); + + charLimitStoryBlockField = APILocator.getContentTypeFieldAPI().save(charLimitStoryBlockField, systemUser); + + // Add charLimit field variable with a limit of 25 characters + final FieldVariable charLimitVar = ImmutableFieldVariable.builder() + .key("charLimit") + .value("25") + .fieldId(charLimitStoryBlockField.id()) + .build(); + APILocator.getContentTypeFieldAPI().save(charLimitVar, systemUser); } /** @@ -771,6 +822,209 @@ public void test_empty_story_block_json_still_fails_validation() { ); } + // ========================================================================= + // charLimit validation tests + // ========================================================================= + + /** + * Test that a story block exceeding charLimit fails validation. + * charLimit is set to 25, and the content has charCount of 32. + */ + @Test + public void test_story_block_exceeding_char_limit_fails_validation() { + final String storyBlockExceedingLimit = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 32,\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 1\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"attrs\": {\n" + + " \"indent\": 0,\n" + + " \"textAlign\": null\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"text\": \"adasdasdasdasdasdasdasdasdasdads\",\n" + + " \"type\": \"text\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"paragraph\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final Contentlet contentlet = new ContentletDataGen(charLimitContentType) + .setProperty("storyBlockWithLimit", storyBlockExceedingLimit) + .next(); + + expectCharLimitValidationException( + () -> APILocator.getContentletAPI().checkin(contentlet, systemUser, false), + "Expected DotContentletValidationException for story block exceeding charLimit" + ); + } + + /** + * Test that a story block within charLimit passes validation. + * charLimit is set to 25, and the content has charCount of 12. + */ + @Test + public void test_story_block_within_char_limit_passes_validation() throws DotDataException, DotSecurityException { + final String storyBlockWithinLimit = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 12,\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 2\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"attrs\": {\n" + + " \"indent\": 0,\n" + + " \"textAlign\": null\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"text\": \"Hello World!\",\n" + + " \"type\": \"text\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"paragraph\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final Contentlet contentlet = new ContentletDataGen(charLimitContentType) + .setProperty("storyBlockWithLimit", storyBlockWithinLimit) + .next(); + + // Should not throw validation exception - within the limit + final Contentlet savedContentlet = APILocator.getContentletAPI().checkin(contentlet, systemUser, false); + + assertNotNull("Saved contentlet should not be null", savedContentlet); + assertTrue("Saved contentlet should have a valid inode", UtilMethods.isSet(savedContentlet.getInode())); + } + + /** + * Test that a story block at exactly the charLimit passes validation. + * charLimit is set to 25, and the content has charCount of 25. + */ + @Test + public void test_story_block_at_exact_char_limit_passes_validation() throws DotDataException, DotSecurityException { + final String storyBlockAtLimit = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 25,\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 1\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"attrs\": {\n" + + " \"indent\": 0,\n" + + " \"textAlign\": null\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"text\": \"abcdefghijklmnopqrstuvwxy\",\n" + + " \"type\": \"text\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"paragraph\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final Contentlet contentlet = new ContentletDataGen(charLimitContentType) + .setProperty("storyBlockWithLimit", storyBlockAtLimit) + .next(); + + // Should not throw validation exception - exactly at the limit + final Contentlet savedContentlet = APILocator.getContentletAPI().checkin(contentlet, systemUser, false); + + assertNotNull("Saved contentlet should not be null", savedContentlet); + assertTrue("Saved contentlet should have a valid inode", UtilMethods.isSet(savedContentlet.getInode())); + } + + /** + * Test that a story block without charCount attribute in JSON passes validation even with charLimit set. + * This handles gracefully the case where old content doesn't have charCount in attrs. + */ + @Test + public void test_story_block_without_char_count_attr_passes_validation() throws DotDataException, DotSecurityException { + final String storyBlockNoCharCount = "{\n" + + " \"content\": [\n" + + " {\n" + + " \"attrs\": {\n" + + " \"indent\": 0,\n" + + " \"textAlign\": null\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"text\": \"Some content without charCount in attrs\",\n" + + " \"type\": \"text\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"paragraph\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final Contentlet contentlet = new ContentletDataGen(charLimitContentType) + .setProperty("storyBlockWithLimit", storyBlockNoCharCount) + .next(); + + // Should not throw - gracefully skips validation when charCount is not in JSON attrs + final Contentlet savedContentlet = APILocator.getContentletAPI().checkin(contentlet, systemUser, false); + + assertNotNull("Saved contentlet should not be null", savedContentlet); + assertTrue("Saved contentlet should have a valid inode", UtilMethods.isSet(savedContentlet.getInode())); + } + + /** + * Test that a story block without charLimit field variable passes validation regardless of charCount. + * Uses the original test content type which has no charLimit field variable. + */ + @Test + public void test_story_block_without_char_limit_variable_passes_validation() throws DotDataException, DotSecurityException { + final String storyBlockHighCharCount = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 9999,\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 1\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"attrs\": {\n" + + " \"indent\": 0,\n" + + " \"textAlign\": null\n" + + " },\n" + + " \"content\": [\n" + + " {\n" + + " \"text\": \"Some text content\",\n" + + " \"type\": \"text\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"paragraph\"\n" + + " }\n" + + " ],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final Contentlet contentlet = new ContentletDataGen(testContentType) + .setProperty("storyBlockField", storyBlockHighCharCount) + .next(); + + // Should not throw - no charLimit field variable on this content type's field + final Contentlet savedContentlet = APILocator.getContentletAPI().checkin(contentlet, systemUser, false); + + assertNotNull("Saved contentlet should not be null", savedContentlet); + assertTrue("Saved contentlet should have a valid inode", UtilMethods.isSet(savedContentlet.getInode())); + } + /** * Test that multiple empty paragraphs still fail validation */ diff --git a/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java b/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java index 384fa5a1989c..c8cdf27f4449 100644 --- a/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java @@ -5,6 +5,8 @@ import com.dotcms.util.JsonUtil; import org.junit.Test; +import java.util.OptionalInt; + import static org.junit.Assert.*; /** @@ -412,4 +414,154 @@ public void test_isTextContentEmpty_mixed_empty_and_text_returns_false() throws ); assertFalse("Block with mixed empty and actual text should return false", StoryBlockUtil.isTextContentEmpty(blockWithMixedContent)); } + + // ========================================================================= + // getCharCount tests + // ========================================================================= + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Story Block JSON contains attrs.charCount with a valid integer value + * + * Expected Result: Method should return OptionalInt with the charCount value + */ + @Test + public void test_getCharCount_returns_value_when_present() { + final String storyBlock = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 42,\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 8\n" + + " },\n" + + " \"content\": [],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final OptionalInt result = StoryBlockUtil.getCharCount(storyBlock); + assertTrue("Should have charCount value", result.isPresent()); + assertEquals("Should return correct charCount", 42, result.getAsInt()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Story Block JSON has attrs but no charCount property + * + * Expected Result: Method should return empty OptionalInt + */ + @Test + public void test_getCharCount_returns_empty_when_no_charCount() { + final String storyBlock = "{\n" + + " \"attrs\": {\n" + + " \"readingTime\": 1,\n" + + " \"wordCount\": 8\n" + + " },\n" + + " \"content\": [],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final OptionalInt result = StoryBlockUtil.getCharCount(storyBlock); + assertFalse("Should return empty when no charCount", result.isPresent()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Story Block JSON has no attrs property at all + * + * Expected Result: Method should return empty OptionalInt + */ + @Test + public void test_getCharCount_returns_empty_when_no_attrs() { + final String storyBlock = "{\n" + + " \"content\": [],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final OptionalInt result = StoryBlockUtil.getCharCount(storyBlock); + assertFalse("Should return empty when no attrs", result.isPresent()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Input is null + * + * Expected Result: Method should return empty OptionalInt + */ + @Test + public void test_getCharCount_returns_empty_for_null() { + final OptionalInt result = StoryBlockUtil.getCharCount(null); + assertFalse("Should return empty for null input", result.isPresent()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Input is empty string + * + * Expected Result: Method should return empty OptionalInt + */ + @Test + public void test_getCharCount_returns_empty_for_empty_string() { + final OptionalInt result = StoryBlockUtil.getCharCount(""); + assertFalse("Should return empty for empty string", result.isPresent()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Input is malformed/invalid JSON + * + * Expected Result: Method should return empty OptionalInt without throwing + */ + @Test + public void test_getCharCount_returns_empty_for_invalid_json() { + final OptionalInt result = StoryBlockUtil.getCharCount("{invalid json}"); + assertFalse("Should return empty for invalid JSON", result.isPresent()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Story Block JSON has charCount of 0 + * + * Expected Result: Method should return OptionalInt with value 0 + */ + @Test + public void test_getCharCount_returns_zero_when_charCount_is_zero() { + final String storyBlock = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": 0\n" + + " },\n" + + " \"content\": [],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final OptionalInt result = StoryBlockUtil.getCharCount(storyBlock); + assertTrue("Should have charCount value", result.isPresent()); + assertEquals("Should return zero charCount", 0, result.getAsInt()); + } + + /** + * Tested method {@link StoryBlockUtil#getCharCount(String)} + * + * Given scenario: Story Block JSON has charCount as a string instead of integer + * + * Expected Result: Method should return empty OptionalInt as the value is not an integer + */ + @Test + public void test_getCharCount_returns_empty_for_non_integer_charCount() { + final String storyBlock = "{\n" + + " \"attrs\": {\n" + + " \"charCount\": \"not a number\"\n" + + " },\n" + + " \"content\": [],\n" + + " \"type\": \"doc\"\n" + + "}"; + + final OptionalInt result = StoryBlockUtil.getCharCount(storyBlock); + assertFalse("Should return empty for non-integer charCount", result.isPresent()); + } } From 4697d682c52ef9812ad9f7665eea82d4bfcddfd6 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Fri, 6 Feb 2026 10:56:04 -0500 Subject: [PATCH 3/7] Enhance StoryBlock validation tests with detailed scenarios --- .../business/StoryBlockValidationTest.java | 139 +++++++++------ .../contenttype/test/StoryBlockUtilTest.java | 168 ++++++------------ 2 files changed, 137 insertions(+), 170 deletions(-) diff --git a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java index 4f2d5dd3455d..41d7f5c3619a 100644 --- a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockValidationTest.java @@ -183,7 +183,9 @@ public static void prepare() throws Exception { } /** - * Test that an empty story block (only empty paragraph) fails validation when required + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains an empty story block (only an empty paragraph node with no text content) + * Expected Result: Validation should fail with a REQUIRED field error since the story block has no meaningful content */ @Test public void test_empty_story_block_required_validation_fails() { @@ -211,7 +213,9 @@ public void test_empty_story_block_required_validation_fails() { } /** - * Test that a story block with text content passes validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a paragraph with text content ("Hello World!") + * Expected Result: Validation should pass and the contentlet should be saved successfully with the story block content preserved */ @Test public void test_story_block_with_text_passes_validation() throws DotDataException, DotSecurityException { @@ -253,7 +257,9 @@ public void test_story_block_with_text_passes_validation() throws DotDataExcepti } /** - * Test that a story block with image passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a dotImage node (structural content with an image identifier) + * Expected Result: Validation should pass since a dotImage node represents meaningful content even without text */ @Test public void test_story_block_with_image_passes_validation() throws DotDataException, DotSecurityException { @@ -286,7 +292,9 @@ public void test_story_block_with_image_passes_validation() throws DotDataExcept } /** - * Test that a story block with video passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a dotVideo node (structural content with a video identifier) + * Expected Result: Validation should pass since a dotVideo node represents meaningful content even without text */ @Test public void test_story_block_with_video_passes_validation() throws DotDataException, DotSecurityException { @@ -319,7 +327,9 @@ public void test_story_block_with_video_passes_validation() throws DotDataExcept } /** - * Test that a story block with list passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a bulletList node with a listItem (structural content) + * Expected Result: Validation should pass since a bulletList node represents structural content even if the list items are empty */ @Test public void test_story_block_with_list_passes_validation() throws DotDataException, DotSecurityException { @@ -359,7 +369,9 @@ public void test_story_block_with_list_passes_validation() throws DotDataExcepti } /** - * Test that a story block with table passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a table node with rows and cells (structural content) + * Expected Result: Validation should pass since a table node represents structural content even if cells are empty */ @Test public void test_story_block_with_table_passes_validation() throws DotDataException, DotSecurityException { @@ -409,7 +421,9 @@ public void test_story_block_with_table_passes_validation() throws DotDataExcept } /** - * Test that a story block with blockquote passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a blockquote node (structural content wrapping an empty paragraph) + * Expected Result: Validation should pass since a blockquote node represents structural content */ @Test public void test_story_block_with_blockquote_passes_validation() throws DotDataException, DotSecurityException { @@ -444,7 +458,9 @@ public void test_story_block_with_blockquote_passes_validation() throws DotDataE } /** - * Test that a story block with code block containing text passes validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a codeBlock node with text content ("console.log('Hello World');") + * Expected Result: Validation should pass and the contentlet should be saved successfully with the code block content preserved */ @Test public void test_story_block_with_code_text_passes_validation() throws DotDataException, DotSecurityException { @@ -479,7 +495,9 @@ public void test_story_block_with_code_text_passes_validation() throws DotDataEx } /** - * Test that an empty code block fails validation when required + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a codeBlock node with an empty text string + * Expected Result: Validation should fail with a REQUIRED field error since the code block has no meaningful content */ @Test public void test_empty_code_block_required_validation_fails() { @@ -512,7 +530,9 @@ public void test_empty_code_block_required_validation_fails() { } /** - * Test that a story block with horizontal rule passes validation (structure = content) + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains a horizontalRule node (structural content) + * Expected Result: Validation should pass since a horizontalRule node represents meaningful structural content */ @Test public void test_story_block_with_horizontal_rule_passes_validation() throws DotDataException, DotSecurityException { @@ -538,10 +558,8 @@ public void test_story_block_with_horizontal_rule_passes_validation() throws Dot } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Story Block field contains malformed JSON that looks like a JSON attempt - * + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains malformed JSON ("{ invalid json }") that looks like a JSON attempt * Expected Result: Validation should pass treating malformed JSON as legacy WYSIWYG content during migration */ @Test @@ -562,7 +580,9 @@ public void test_malformed_json_passes_as_legacy_content() throws DotDataExcepti } /** - * Test that a story block field with wrong type (Integer) fails validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field is set with an Integer value (12345) instead of a String + * Expected Result: Validation should fail with a BADTYPE field error since Story Block fields must be String values */ @Test public void test_story_block_integer_type_validation_fails() { @@ -577,7 +597,9 @@ public void test_story_block_integer_type_validation_fails() { } /** - * Test that a story block field with wrong type (Boolean) fails validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field is set with a Boolean value (true) instead of a String + * Expected Result: Validation should fail with a BADTYPE field error since Story Block fields must be String values */ @Test public void test_story_block_boolean_type_validation_fails() { @@ -592,7 +614,9 @@ public void test_story_block_boolean_type_validation_fails() { } /** - * Test that a story block field with wrong type (Object) fails validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field is set with a plain Object value instead of a String + * Expected Result: Validation should fail with a BADTYPE field error since Story Block fields must be String values */ @Test public void test_story_block_object_type_validation_fails() { @@ -607,7 +631,9 @@ public void test_story_block_object_type_validation_fails() { } /** - * Test that type validation provides helpful error information + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field is set with an Integer value (42) to trigger type validation + * Expected Result: The BADTYPE validation error should contain field information in the notValidFields map */ @Test public void test_story_block_type_validation_error_details() { @@ -652,7 +678,9 @@ public void test_story_block_type_validation_error_details() { } /** - * Test that a null story block field (not set at all) fails validation when required + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field is not set at all (null value) on the contentlet + * Expected Result: Validation should fail with a REQUIRED field error since the field is mandatory */ @Test public void test_null_story_block_required_validation_fails() { @@ -668,10 +696,8 @@ public void test_null_story_block_required_validation_fails() { } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Story Block field contains legacy WYSIWYG content (plain text) with actual content - * + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains legacy WYSIWYG content (plain text) with actual content * Expected Result: Validation should pass to support backward compatibility during WYSIWYG to Story Block migration */ @Test @@ -692,10 +718,8 @@ public void test_legacy_wysiwyg_content_passes_validation() throws DotDataExcept } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Story Block field contains legacy WYSIWYG content with HTML tags - * + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains legacy WYSIWYG content with HTML tags (

, , ) * Expected Result: Validation should pass to support HTML content from legacy WYSIWYG fields */ @Test @@ -716,11 +740,9 @@ public void test_legacy_wysiwyg_html_content_passes_validation() throws DotDataE } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Required Story Block field contains empty legacy WYSIWYG content (empty string) - * - * Expected Result: Validation should fail since empty content doesn't satisfy required field constraint + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains empty legacy WYSIWYG content (empty string) + * Expected Result: Validation should fail since empty content doesn't satisfy the required field constraint */ @Test public void test_empty_legacy_wysiwyg_content_fails_validation() { @@ -737,11 +759,9 @@ public void test_empty_legacy_wysiwyg_content_fails_validation() { } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Required Story Block field contains whitespace-only legacy WYSIWYG content - * - * Expected Result: Validation should fail since whitespace-only content doesn't satisfy required field constraint + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains whitespace-only legacy WYSIWYG content (spaces, newlines, tabs) + * Expected Result: Validation should fail since whitespace-only content doesn't satisfy the required field constraint */ @Test public void test_whitespace_legacy_wysiwyg_content_fails_validation() { @@ -758,11 +778,9 @@ public void test_whitespace_legacy_wysiwyg_content_fails_validation() { } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Story Block field contains both valid Story Block JSON and passes validation - * - * Expected Result: Validation should continue to work correctly for proper Story Block JSON content + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains valid Story Block JSON with a paragraph and text content + * Expected Result: Validation should pass and the contentlet should be saved with the Story Block JSON content preserved */ @Test public void test_story_block_json_still_validates_correctly() throws DotDataException, DotSecurityException { @@ -795,11 +813,9 @@ public void test_story_block_json_still_validates_correctly() throws DotDataExce } /** - * Tested method {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, List, boolean)} - * - * Given scenario: Story Block field contains empty Story Block JSON (should still fail validation) - * - * Expected Result: Validation should still fail for empty Story Block JSON to maintain data quality + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains empty Story Block JSON (a doc with only an empty paragraph node) + * Expected Result: Validation should fail with a REQUIRED field error to maintain data quality */ @Test public void test_empty_story_block_json_still_fails_validation() { @@ -827,8 +843,9 @@ public void test_empty_story_block_json_still_fails_validation() { // ========================================================================= /** - * Test that a story block exceeding charLimit fails validation. - * charLimit is set to 25, and the content has charCount of 32. + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field with a charLimit field variable set to 25 contains content with charCount of 32 + * Expected Result: Validation should fail with a CHAR_LIMIT error since the content exceeds the configured character limit */ @Test public void test_story_block_exceeding_char_limit_fails_validation() { @@ -867,8 +884,9 @@ public void test_story_block_exceeding_char_limit_fails_validation() { } /** - * Test that a story block within charLimit passes validation. - * charLimit is set to 25, and the content has charCount of 12. + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field with a charLimit field variable set to 25 contains content with charCount of 12 + * Expected Result: Validation should pass since the content is within the configured character limit */ @Test public void test_story_block_within_char_limit_passes_validation() throws DotDataException, DotSecurityException { @@ -908,8 +926,9 @@ public void test_story_block_within_char_limit_passes_validation() throws DotDat } /** - * Test that a story block at exactly the charLimit passes validation. - * charLimit is set to 25, and the content has charCount of 25. + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field with a charLimit field variable set to 25 contains content with charCount of exactly 25 + * Expected Result: Validation should pass since the content is at exactly the configured character limit (boundary case) */ @Test public void test_story_block_at_exact_char_limit_passes_validation() throws DotDataException, DotSecurityException { @@ -949,8 +968,9 @@ public void test_story_block_at_exact_char_limit_passes_validation() throws DotD } /** - * Test that a story block without charCount attribute in JSON passes validation even with charLimit set. - * This handles gracefully the case where old content doesn't have charCount in attrs. + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field with a charLimit field variable set to 25 contains JSON without a charCount attribute in attrs (legacy content) + * Expected Result: Validation should pass gracefully, skipping char limit check when charCount is absent from the JSON attrs */ @Test public void test_story_block_without_char_count_attr_passes_validation() throws DotDataException, DotSecurityException { @@ -985,8 +1005,9 @@ public void test_story_block_without_char_count_attr_passes_validation() throws } /** - * Test that a story block without charLimit field variable passes validation regardless of charCount. - * Uses the original test content type which has no charLimit field variable. + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A Story Block field without a charLimit field variable contains content with a very high charCount (9999) + * Expected Result: Validation should pass since no charLimit field variable is configured on this field */ @Test public void test_story_block_without_char_limit_variable_passes_validation() throws DotDataException, DotSecurityException { @@ -1026,7 +1047,9 @@ public void test_story_block_without_char_limit_variable_passes_validation() thr } /** - * Test that multiple empty paragraphs still fail validation + * Method to test: {@link com.dotcms.content.elasticsearch.business.ESContentletAPIImpl#validateContentlet(Contentlet, java.util.List, boolean)} + * Given Scenario: A required Story Block field contains multiple empty paragraph nodes (no text content in any of them) + * Expected Result: Validation should fail with a REQUIRED field error since multiple empty paragraphs still represent no meaningful content */ @Test public void test_multiple_empty_paragraphs_fail_validation() { diff --git a/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java b/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java index c8cdf27f4449..04005ce0e2e9 100644 --- a/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/contenttype/test/StoryBlockUtilTest.java @@ -16,10 +16,8 @@ public class StoryBlockUtilTest { /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: Input parameter is null - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: Input parameter is null * Expected Result: Method should return true, treating null as empty */ @Test @@ -28,10 +26,8 @@ public void test_isEmptyStoryBlock_null_input_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: Input contains various whitespace-only strings (spaces, tabs, newlines) - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: Input contains various whitespace-only strings (spaces, tabs, newlines) * Expected Result: All whitespace-only inputs should return true, being treated as empty */ @Test @@ -45,10 +41,8 @@ public void test_isEmptyStoryBlock_handles_string_edge_cases() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: Input is empty string or contains only whitespace characters - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: Input is empty string or contains only whitespace characters * Expected Result: Method should return true for both empty and whitespace-only strings */ @Test @@ -58,10 +52,8 @@ public void test_isEmptyStoryBlock_empty_string_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: Input contains invalid or malformed JSON strings - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: Input contains invalid or malformed JSON strings * Expected Result: Method should return true to force validation failure for invalid JSON */ @Test @@ -72,10 +64,8 @@ public void test_isEmptyStoryBlock_invalid_json_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: StoryBlock JSON contains only an empty paragraph element with no text content - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: StoryBlock JSON contains only an empty paragraph element with no text content * Expected Result: Method should return true as empty paragraphs contain no meaningful content */ @Test @@ -97,10 +87,8 @@ public void test_isEmptyStoryBlock_empty_paragraph_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: StoryBlock JSON contains a paragraph element with actual text content - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: StoryBlock JSON contains a paragraph element with actual text content * Expected Result: Method should return false as the block contains meaningful text content */ @Test @@ -128,10 +116,8 @@ public void test_isEmptyStoryBlock_paragraph_with_text_returns_false() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: StoryBlock JSON contains an image block element with identifier data - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: StoryBlock JSON contains an image block element with identifier data * Expected Result: Method should return false as image blocks represent meaningful content */ @Test @@ -155,10 +141,8 @@ public void test_isEmptyStoryBlock_image_block_returns_false() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: StoryBlock JSON document structure has no content property defined - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: StoryBlock JSON document structure has no content property defined * Expected Result: Method should return true as missing content indicates empty block */ @Test @@ -171,10 +155,8 @@ public void test_isEmptyStoryBlock_no_content_property_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyStoryBlock(String)} - * - * Given scenario: StoryBlock JSON has content property defined as an empty array - * + * Method to test: {@link StoryBlockUtil#isEmptyStoryBlock(String)} + * Given Scenario: StoryBlock JSON has content property defined as an empty array * Expected Result: Method should return true as empty content array indicates no meaningful content */ @Test @@ -188,10 +170,8 @@ public void test_isEmptyStoryBlock_empty_content_array_returns_true() { } /** - * Tested method {@link StoryBlockUtil#isEmptyBlock(JsonNode)} - * - * Given scenario: JsonNode block has no type property defined - * + * Method to test: {@link StoryBlockUtil#isEmptyBlock(JsonNode)} + * Given Scenario: JsonNode block has no type property defined * Expected Result: Method should return true as blocks without type are considered empty */ @Test @@ -201,10 +181,8 @@ public void test_isEmptyBlock_no_type_returns_true() throws Exception { } /** - * Tested method {@link StoryBlockUtil#isEmptyBlock(JsonNode)} - * - * Given scenario: JsonNode represents a paragraph block without content property - * + * Method to test: {@link StoryBlockUtil#isEmptyBlock(JsonNode)} + * Given Scenario: JsonNode represents a paragraph block without content property * Expected Result: Method should return true as paragraphs without content are empty */ @Test @@ -222,10 +200,8 @@ public void test_isEmptyBlock_paragraph_without_content_returns_true() throws Ex } /** - * Tested method {@link StoryBlockUtil#isEmptyBlock(JsonNode)} - * - * Given scenario: JsonNode represents a paragraph block containing text content - * + * Method to test: {@link StoryBlockUtil#isEmptyBlock(JsonNode)} + * Given Scenario: JsonNode represents a paragraph block containing text content * Expected Result: Method should return false as the paragraph contains meaningful text */ @Test @@ -245,10 +221,8 @@ public void test_isEmptyBlock_paragraph_with_text_returns_false() throws Excepti } /** - * Tested method {@link StoryBlockUtil#isEmptyBlock(JsonNode)} - * - * Given scenario: JsonNode represents an image block with identifier data - * + * Method to test: {@link StoryBlockUtil#isEmptyBlock(JsonNode)} + * Given Scenario: JsonNode represents an image block with identifier data * Expected Result: Method should return false as image blocks represent meaningful content */ @Test @@ -267,10 +241,8 @@ public void test_isEmptyBlock_image_returns_false() throws Exception { } /** - * Tested method {@link StoryBlockUtil#isEmptyBlock(JsonNode)} - * - * Given scenario: JsonNode represents a list block element (bulletList type) - * + * Method to test: {@link StoryBlockUtil#isEmptyBlock(JsonNode)} + * Given Scenario: JsonNode represents a list block element (bulletList type) * Expected Result: Method should return false as list structures are considered content even when empty */ @Test @@ -285,10 +257,8 @@ public void test_isEmptyBlock_list_returns_false() throws Exception { } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode block has no content property defined - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode block has no content property defined * Expected Result: Method should return true as missing content property indicates empty text */ @Test @@ -302,10 +272,8 @@ public void test_isTextContentEmpty_no_content_property_returns_true() throws Ex } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode block has content property defined as an empty array - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode block has content property defined as an empty array * Expected Result: Method should return true as empty content array contains no text */ @Test @@ -320,10 +288,8 @@ public void test_isTextContentEmpty_empty_content_array_returns_true() throws Ex } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode contains text elements with empty text values - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode contains text elements with empty text values * Expected Result: Method should return true as empty text values contain no meaningful content */ @Test @@ -343,10 +309,8 @@ public void test_isTextContentEmpty_empty_text_returns_true() throws Exception { } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode contains text elements with only whitespace characters - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode contains text elements with only whitespace characters * Expected Result: Method should return true as whitespace-only text is considered empty */ @Test @@ -366,10 +330,8 @@ public void test_isTextContentEmpty_whitespace_text_returns_true() throws Except } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode contains text elements with actual meaningful text content - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode contains text elements with actual meaningful text content * Expected Result: Method should return false as the block contains real text content */ @Test @@ -389,10 +351,8 @@ public void test_isTextContentEmpty_actual_text_returns_false() throws Exception } /** - * Tested method {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} - * - * Given scenario: JsonNode contains mix of empty text elements and elements with actual content - * + * Method to test: {@link StoryBlockUtil#isTextContentEmpty(JsonNode)} + * Given Scenario: JsonNode contains mix of empty text elements and elements with actual content * Expected Result: Method should return false as at least one element contains meaningful text */ @Test @@ -420,10 +380,8 @@ public void test_isTextContentEmpty_mixed_empty_and_text_returns_false() throws // ========================================================================= /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Story Block JSON contains attrs.charCount with a valid integer value - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Story Block JSON contains attrs.charCount with a valid integer value * Expected Result: Method should return OptionalInt with the charCount value */ @Test @@ -444,10 +402,8 @@ public void test_getCharCount_returns_value_when_present() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Story Block JSON has attrs but no charCount property - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Story Block JSON has attrs but no charCount property * Expected Result: Method should return empty OptionalInt */ @Test @@ -466,10 +422,8 @@ public void test_getCharCount_returns_empty_when_no_charCount() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Story Block JSON has no attrs property at all - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Story Block JSON has no attrs property at all * Expected Result: Method should return empty OptionalInt */ @Test @@ -484,10 +438,8 @@ public void test_getCharCount_returns_empty_when_no_attrs() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Input is null - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Input is null * Expected Result: Method should return empty OptionalInt */ @Test @@ -497,10 +449,8 @@ public void test_getCharCount_returns_empty_for_null() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Input is empty string - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Input is empty string * Expected Result: Method should return empty OptionalInt */ @Test @@ -510,10 +460,8 @@ public void test_getCharCount_returns_empty_for_empty_string() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Input is malformed/invalid JSON - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Input is malformed/invalid JSON * Expected Result: Method should return empty OptionalInt without throwing */ @Test @@ -523,10 +471,8 @@ public void test_getCharCount_returns_empty_for_invalid_json() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Story Block JSON has charCount of 0 - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Story Block JSON has charCount of 0 * Expected Result: Method should return OptionalInt with value 0 */ @Test @@ -545,10 +491,8 @@ public void test_getCharCount_returns_zero_when_charCount_is_zero() { } /** - * Tested method {@link StoryBlockUtil#getCharCount(String)} - * - * Given scenario: Story Block JSON has charCount as a string instead of integer - * + * Method to test: {@link StoryBlockUtil#getCharCount(String)} + * Given Scenario: Story Block JSON has charCount as a string instead of integer * Expected Result: Method should return empty OptionalInt as the value is not an integer */ @Test From cdd2cb08a26130307f06610114693a9279f42b49 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Fri, 6 Feb 2026 12:43:16 -0500 Subject: [PATCH 4/7] feat: enhance DotBlockEditor with error handling and validation This update introduces improved error handling and validation for the DotBlockEditor component. Key changes include: - Added input to manage external error states. - Implemented and getters to provide detailed error feedback in the UI. - Enhanced the template to display error messages for character limits and required fields. - Introduced styling for error states in the editor wrapper. - Updated tests to cover new validation scenarios and ensure robust error handling. These enhancements improve user experience by providing clear feedback on content validation, ensuring data integrity in the editing process. --- .../dot-block-editor.component.html | 34 ++- .../dot-block-editor.component.scss | 22 ++ .../dot-block-editor.component.spec.ts | 232 ++++++++++++++++++ .../dot-block-editor.component.ts | 115 ++++++++- .../dot-edit-content-form.component.ts | 9 +- ...t-edit-content-block-editor.component.html | 23 +- .../lib/fields/shared/base-wrapper-field.ts | 8 + .../src/lib/utils/validators.spec.ts | 170 +++++++++++++ .../edit-content/src/lib/utils/validators.ts | 82 +++++++ 9 files changed, 663 insertions(+), 32 deletions(-) create mode 100644 core-web/libs/edit-content/src/lib/utils/validators.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/utils/validators.ts diff --git a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html index 6b9267e6ac04..01e155032bc6 100644 --- a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html +++ b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html @@ -3,7 +3,8 @@ [ngClass]="{ 'editor-wrapper--fullscreen': isFullscreen, 'editor-wrapper--default': !isFullscreen, - 'editor-wrapper--disabled': disabled + 'editor-wrapper--disabled': disabled, + 'editor-wrapper--error': hasError }" [style]="customStyles" class="editor-wrapper" @@ -25,12 +26,31 @@ - @if (showCharData) { - + @if (showCharData || charLimitError || requiredError) { +

} diff --git a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.scss b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.scss index 6a17a57dca50..69b1dedcd47c 100644 --- a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.scss +++ b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.scss @@ -48,6 +48,10 @@ pointer-events: none; } + .editor-wrapper--error { + outline-color: $color-palette-red; + } + .dot-drag-handle { cursor: grab; color: $color-palette-gray-500; @@ -97,6 +101,24 @@ transform: rotate(360deg); } } + + .block-editor__footer { + display: flex; + align-items: center; + margin-top: $spacing-3; + gap: $spacing-2; + } + + .block-editor__footer-error { + flex: 7; + color: $color-palette-red; + font-size: $font-size-md; + } + + dot-editor-count-bar { + flex: 5; + margin-top: 0; + } } tiptap-editor::ng-deep .ProseMirror { diff --git a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.spec.ts b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.spec.ts index 41ba6cc8daaa..44bd96288e16 100644 --- a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.spec.ts +++ b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.spec.ts @@ -127,4 +127,236 @@ describe('DotBlockEditorComponent - ControlValueAccesor', () => { expect(emitSpy).toHaveBeenCalledWith(BLOCK_EDITOR_FIELD); }); }); + + describe('hasFieldError input', () => { + it('should default to false', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + expect(blockEditorComponent.hasFieldError).toBe(false); + }); + + it('should accept a true value', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + blockEditorComponent.hasFieldError = true; + expect(blockEditorComponent.hasFieldError).toBe(true); + }); + }); + + describe('hasError getter', () => { + it('should return false when no errors exist', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + expect(blockEditorComponent.hasError).toBe(false); + }); + + it('should return true when hasFieldError is true', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + blockEditorComponent.hasFieldError = true; + expect(blockEditorComponent.hasError).toBe(true); + }); + + it('should return true when charLimitExceeded error exists on the form control', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ charLimitExceeded: { max: 100, actual: 150 } }); + + expect(blockEditorComponent.hasError).toBe(true); + }); + + it('should return true when both hasFieldError and charLimitError are present', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + blockEditorComponent.hasFieldError = true; + control.setErrors({ charLimitExceeded: { max: 100, actual: 150 } }); + + expect(blockEditorComponent.hasError).toBe(true); + }); + }); + + describe('charLimitError getter', () => { + it('should return null when no charLimitExceeded error exists on the control', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + expect(blockEditorComponent.charLimitError).toBeNull(); + }); + + it('should return the error object when charLimitExceeded error is set on the control', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ charLimitExceeded: { max: 200, actual: 250 } }); + + expect(blockEditorComponent.charLimitError).toEqual({ max: 200, actual: 250 }); + }); + + it('should return null when control has other errors but not charLimitExceeded', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ required: true }); + + expect(blockEditorComponent.charLimitError).toBeNull(); + }); + }); + + describe('requiredError getter', () => { + it('should return false when the control has no errors', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + expect(blockEditorComponent.requiredError).toBe(false); + }); + + it('should return false when the control has a required error but is not touched', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ required: true }); + + // Control is untouched by default + expect(blockEditorComponent.requiredError).toBe(false); + }); + + it('should return true when the control has a required error and is touched', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ required: true }); + control.markAsTouched(); + + expect(blockEditorComponent.requiredError).toBe(true); + }); + + it('should return false when the control is touched but has no required error', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.markAsTouched(); + + expect(blockEditorComponent.requiredError).toBe(false); + }); + + it('should return false when the control is touched and has other errors but not required', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + control.setErrors({ charLimitExceeded: { max: 100, actual: 150 } }); + control.markAsTouched(); + + expect(blockEditorComponent.requiredError).toBe(false); + }); + }); + + describe('onBlockEditorChange and char limit validation', () => { + /** + * Helper to create a minimal mock editor with the given character/word counts. + */ + function createMockEditor(characters: number, words = 10) { + return { + storage: { + characterCount: { + characters: () => characters, + words: () => words + } + } + } as unknown as DotBlockEditorComponent['editor']; + } + + it('should set charLimitExceeded error when character count exceeds charLimit', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + blockEditorComponent.editor = createMockEditor(150); + blockEditorComponent.charLimit = 100; + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.errors).toEqual( + expect.objectContaining({ + charLimitExceeded: { max: 100, actual: 150 } + }) + ); + }); + + it('should mark the control as touched when charLimitExceeded is set', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + blockEditorComponent.editor = createMockEditor(150); + blockEditorComponent.charLimit = 100; + blockEditorComponent.disabled = false; + + // Ensure untouched initially + expect(control.touched).toBe(false); + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.touched).toBe(true); + }); + + it('should clear charLimitExceeded error when character count is within limit', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + // Pre-set the error as if it was previously over limit + control.setErrors({ charLimitExceeded: { max: 100, actual: 150 } }); + + blockEditorComponent.editor = createMockEditor(50); + blockEditorComponent.charLimit = 100; + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.errors).toBeNull(); + }); + + it('should preserve other errors when clearing charLimitExceeded', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + // Set multiple errors including charLimitExceeded + control.setErrors({ + required: true, + charLimitExceeded: { max: 100, actual: 150 } + }); + + blockEditorComponent.editor = createMockEditor(50); + blockEditorComponent.charLimit = 100; + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + // charLimitExceeded removed; required preserved + expect(control.errors).toEqual({ required: true }); + }); + + it('should not set charLimitExceeded error when charLimit is not defined', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + blockEditorComponent.editor = createMockEditor(150); + // charLimit remains NaN (its default when field variable is undefined) + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.errors).toBeNull(); + }); + + it('should not set charLimitExceeded error when charLimit is zero', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + blockEditorComponent.editor = createMockEditor(150); + blockEditorComponent.charLimit = 0; + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.errors).toBeNull(); + }); + + it('should not set charLimitExceeded error when character count equals the limit', () => { + const blockEditorComponent = spectator.query(DotBlockEditorComponent); + const control = spectator.component.form.get('block'); + + blockEditorComponent.editor = createMockEditor(100); + blockEditorComponent.charLimit = 100; + blockEditorComponent.disabled = false; + + blockEditorComponent.onBlockEditorChange(BLOCK_EDITOR_FIELD); + + expect(control.errors).toBeNull(); + }); + }); }); diff --git a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.ts b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.ts index 5ec8cefb215c..dc3b0622d2ce 100644 --- a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.ts +++ b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.ts @@ -17,7 +17,12 @@ import { SimpleChanges, ViewContainerRef } from '@angular/core'; -import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms'; +import { + AbstractControl, + ControlValueAccessor, + NG_VALUE_ACCESSOR, + NgControl +} from '@angular/forms'; import { DialogService } from 'primeng/dynamicdialog'; @@ -52,23 +57,23 @@ import { AssetUploader, BubbleAssetFormExtension, BubbleFormExtension, + DotCMSTableExtensions, DotComands, DotConfigExtension, - DotTableCellContextMenu, DotFloatingButton, - DotCMSTableExtensions, + DotTableCellContextMenu, FREEZE_SCROLL_KEY, FreezeScroll, IndentExtension } from '../../extensions'; import { AIContentNode, ContentletBlock, ImageNode, LoaderNode, VideoNode } from '../../nodes'; import { + DEFAULT_LANG_ID, DotMarketingConfigService, formatHTML, removeInvalidNodes, RestoreDefaultDOMAttrs, - SetDocAttrStep, - DEFAULT_LANG_ID + SetDocAttrStep } from '../../shared'; @Component({ @@ -93,6 +98,7 @@ export class DotBlockEditorComponent implements OnInit, OnChanges, OnDestroy, Co @Input() languageId = DEFAULT_LANG_ID; @Input() isFullscreen = false; + @Input() hasFieldError = false; @Input() value: Content = ''; @Output() valueChange = new EventEmitter(); public allowedContentTypes: string; @@ -151,6 +157,35 @@ export class DotBlockEditorComponent implements OnInit, OnChanges, OnDestroy, Co return Math.ceil(this.characterCount.words() / 265); } + /** + * Returns the charLimitExceeded error if it exists on the control. + * Used in the template to display the error message. + */ + get charLimitError(): { max: number; actual: number } | null { + const ngControl = this.#injector.get(NgControl, null); + + return ngControl?.control?.errors?.['charLimitExceeded'] ?? null; + } + + /** + * Returns true if the editor should show error styling (red border). + * Combines the external error state (from parent) with internal charLimit validation. + */ + get hasError(): boolean { + return this.hasFieldError || !!this.charLimitError; + } + + /** + * Returns true if the control has a required error and has been touched. + * Used to display the required error message in the footer. + */ + get requiredError(): boolean { + const ngControl = this.#injector.get(NgControl, null); + const control = ngControl?.control; + + return !!(control?.errors?.['required'] && control?.touched); + } + registerOnChange(fn: (value: string) => void) { this.onChange = fn; } @@ -229,7 +264,62 @@ export class DotBlockEditorComponent implements OnInit, OnChanges, OnDestroy, Co this.valueChange.emit(value); this.onChange?.(JSON.stringify(value)); - this.onTouched?.(); + this.updateCharLimitValidity(); + } + + /** + * Updates the form control validity based on charLimit. + * When character count exceeds charLimit, sets charLimitExceeded error + * so the form cannot be saved. + * + * @private + * @memberof DotBlockEditorComponent + */ + private updateCharLimitValidity(): void { + const ngControl = this.#injector.get(NgControl, null); + const control = ngControl?.control; + if (!control) { + return; + } + + const limit = this.charLimit; + if (!Number.isFinite(limit) || limit <= 0) { + this.clearCharLimitError(control); + + return; + } + + const count = this.characterCount?.characters?.() ?? 0; + if (count > limit) { + control.setErrors({ + ...(control.errors || {}), + charLimitExceeded: { max: limit, actual: count } + }); + control.markAsTouched(); + } else { + this.clearCharLimitError(control); + } + } + + /** + * Removes the charLimitExceeded error from the control while preserving other errors. + * + * @private + * @param {AbstractControl} control - The form control to clear the error from + * @memberof DotBlockEditorComponent + */ + private clearCharLimitError(control: AbstractControl): void { + const errors = control.errors; + if (!errors || !('charLimitExceeded' in errors)) { + return; + } + + // Remove charLimitExceeded while preserving other errors + const rest = Object.keys(errors) + .filter((key) => key !== 'charLimitExceeded') + .reduce((acc, key) => ({ ...acc, [key]: errors[key] }), {}); + + control.setErrors(Object.keys(rest).length > 0 ? rest : null); } setAllowedBlocks(blocks: string) { @@ -248,7 +338,20 @@ export class DotBlockEditorComponent implements OnInit, OnChanges, OnDestroy, Co this.editor.on('create', () => { this.setEditorContent(this.value); this.updateCharCount(); + // Validate char limit on initial load (e.g., existing content over limit) + this.updateCharLimitValidity(); + }); + + // Validate char limit on every update (typing, paste, etc.) + this.editor.on('update', () => { + this.updateCharLimitValidity(); }); + + // Mark control as touched when user leaves the editor (proper ControlValueAccessor pattern) + this.editor.on('blur', () => { + this.onTouched?.(); + }); + this.subject .pipe(takeUntil(this.destroy$), debounceTime(250)) .subscribe(() => this.updateCharCount()); diff --git a/core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts b/core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts index fe3f14367149..cbbe448782c5 100644 --- a/core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts +++ b/core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form.component.ts @@ -60,6 +60,7 @@ import { isFilteredType, processFieldValue } from '../../utils/functions.util'; +import { blockEditorRequiredValidator } from '../../utils/validators'; import { DotEditContentFieldComponent } from '../dot-edit-content-field/dot-edit-content-field.component'; /** @@ -526,7 +527,13 @@ export class DotEditContentFormComponent implements OnInit { const validators: ValidatorFn[] = []; if (field.required) { - validators.push(Validators.required); + // Block Editor needs a custom validator that checks for actual text content, + // not just the presence of a JSON structure + if (field.fieldType === FIELD_TYPES.BLOCK_EDITOR) { + validators.push(blockEditorRequiredValidator()); + } else { + validators.push(Validators.required); + } } if (field.regexCheck) { diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-block-editor/dot-edit-content-block-editor.component.html b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-block-editor/dot-edit-content-block-editor.component.html index 7490d6d8d490..585cbc3d946b 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-block-editor/dot-edit-content-block-editor.component.html +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-block-editor/dot-edit-content-block-editor.component.html @@ -4,7 +4,10 @@ @if (showLabel) { - + {{ field.name }} } @@ -13,23 +16,7 @@ [languageId]="$languageId()" [formControlName]="field.variable" [contentlet]="$contentlet()" + [hasFieldError]="fieldHasError" [field]="field" /> - - @if (fieldHasError) { -
- @if (isRequired) { - {{ 'dot.edit.content.form.field.required' | dm }} - } -
- } - - @if (!fieldHasError && field.hint) { -
- - {{ field.hint }} - -
- } -
diff --git a/core-web/libs/edit-content/src/lib/fields/shared/base-wrapper-field.ts b/core-web/libs/edit-content/src/lib/fields/shared/base-wrapper-field.ts index 205ed197a8ea..684d41ce829e 100644 --- a/core-web/libs/edit-content/src/lib/fields/shared/base-wrapper-field.ts +++ b/core-web/libs/edit-content/src/lib/fields/shared/base-wrapper-field.ts @@ -54,10 +54,18 @@ export abstract class BaseWrapperField { }); get isRequired(): boolean { + // First check the field definition (source of truth) + const field = this.$field(); + if (field?.required) { + return true; + } + + // Fallback to checking the validator (for fields using standard Validators.required) const control = this.formControl; if (!control) { return false; } + return control.hasValidator(Validators.required); } diff --git a/core-web/libs/edit-content/src/lib/utils/validators.spec.ts b/core-web/libs/edit-content/src/lib/utils/validators.spec.ts new file mode 100644 index 000000000000..7d0fb1cfb59a --- /dev/null +++ b/core-web/libs/edit-content/src/lib/utils/validators.spec.ts @@ -0,0 +1,170 @@ +import { FormControl } from '@angular/forms'; + +import { blockEditorRequiredValidator } from './validators'; + +describe('blockEditorRequiredValidator', () => { + const validator = blockEditorRequiredValidator(); + + describe('empty content (should return required error)', () => { + it('should return error for null', () => { + const control = new FormControl(null); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for undefined', () => { + const control = new FormControl(undefined); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for empty string', () => { + const control = new FormControl(''); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for whitespace-only string', () => { + const control = new FormControl(' '); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for empty doc JSON structure', () => { + const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] }; + const control = new FormControl(emptyDoc); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for empty doc JSON as string', () => { + const emptyDoc = JSON.stringify({ + type: 'doc', + content: [{ type: 'paragraph' }] + }); + const control = new FormControl(emptyDoc); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for doc with empty text node', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: '' }] + } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for doc with whitespace-only text node', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: ' ' }] + } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toEqual({ required: true }); + }); + + it('should return error for nested structure without text content', () => { + const doc = { + type: 'doc', + content: [ + { type: 'paragraph' }, + { type: 'paragraph', content: [] }, + { type: 'heading', attrs: { level: 1 } } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toEqual({ required: true }); + }); + }); + + describe('non-empty content (should return null)', () => { + it('should return null for doc with text content', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Hello world' }] + } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toBeNull(); + }); + + it('should return null for doc with text content as JSON string', () => { + const doc = JSON.stringify({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Hello world' }] + } + ] + }); + const control = new FormControl(doc); + expect(validator(control)).toBeNull(); + }); + + it('should return null for deeply nested text content', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'blockquote', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Quoted text' }] + } + ] + } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toBeNull(); + }); + + it('should return null for doc with heading text', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'heading', + attrs: { level: 1, textAlign: 'left' }, + content: [{ type: 'text', text: 'A title!!' }] + } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toBeNull(); + }); + + it('should return null for plain text string that is not valid JSON', () => { + const control = new FormControl('Some plain text content'); + expect(validator(control)).toBeNull(); + }); + + it('should return null for doc with multiple paragraphs where only one has text', () => { + const doc = { + type: 'doc', + content: [ + { type: 'paragraph' }, + { + type: 'paragraph', + content: [{ type: 'text', text: 'Content here' }] + }, + { type: 'paragraph' } + ] + }; + const control = new FormControl(doc); + expect(validator(control)).toBeNull(); + }); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/utils/validators.ts b/core-web/libs/edit-content/src/lib/utils/validators.ts new file mode 100644 index 000000000000..49ec0655186e --- /dev/null +++ b/core-web/libs/edit-content/src/lib/utils/validators.ts @@ -0,0 +1,82 @@ +import { AbstractControl, ValidationErrors, ValidatorFn } from '@angular/forms'; + +/** + * Checks if a Block Editor JSON content is actually empty (no text content). + * A block editor is considered empty if it has no text nodes with content, + * even if it has structural JSON like {"type":"doc","content":[{"type":"paragraph"}]}. + * + * @param content - The block editor content (string or object) + * @returns true if the content is empty, false otherwise + */ +function isBlockEditorEmpty(content: unknown): boolean { + if (!content) { + return true; + } + + let jsonContent: unknown; + + // Parse if it's a string + if (typeof content === 'string') { + try { + jsonContent = JSON.parse(content); + } catch { + // If it's a plain string with content, it's not empty + return content.trim().length === 0; + } + } else { + jsonContent = content; + } + + // Recursively check for text content + return !hasTextContent(jsonContent); +} + +/** + * Recursively checks if a JSON structure has any actual text content. + * + * @param node - The JSON node to check + * @returns true if any text content is found, false otherwise + */ +function hasTextContent(node: unknown): boolean { + if (!node || typeof node !== 'object') { + return false; + } + + const obj = node as Record; + + // Check if this is a text node with content + if ( + obj['type'] === 'text' && + typeof obj['text'] === 'string' && + obj['text'].trim().length > 0 + ) { + return true; + } + + // Check content array recursively + if (Array.isArray(obj['content'])) { + return obj['content'].some((child) => hasTextContent(child)); + } + + return false; +} + +/** + * Custom validator for Block Editor required fields. + * Unlike Validators.required, this checks if the block editor actually has text content, + * not just if it has a JSON structure. + * + * @returns ValidatorFn that returns { required: true } if empty, null if valid + * + * @example + * ```typescript + * const control = new FormControl('', blockEditorRequiredValidator()); + * ``` + */ +export function blockEditorRequiredValidator(): ValidatorFn { + return (control: AbstractControl): ValidationErrors | null => { + const isEmpty = isBlockEditorEmpty(control.value); + + return isEmpty ? { required: true } : null; + }; +} From e90985f0dfc18d5bbadeaaa80a40c8358188080b Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Fri, 6 Feb 2026 13:20:06 -0500 Subject: [PATCH 5/7] refactor: update DotBlockEditor styling and error message structure This commit refines the DotBlockEditor component by updating the HTML structure and CSS styles for error messages. Key changes include: - Replaced the footer error container with a more flexible layout using PrimeFlex classes. - Updated error message styling to enhance visibility and consistency. - Removed redundant CSS rules related to the footer, streamlining the component's styles. These adjustments improve the user interface and ensure better alignment with the overall design standards. --- .../dot-block-editor.component.html | 8 ++++---- .../dot-block-editor.component.scss | 14 -------------- .../dot-edit-content-block-editor.component.ts | 5 +---- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html index 01e155032bc6..5780ea2978b3 100644 --- a/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html +++ b/core-web/libs/block-editor/src/lib/components/dot-block-editor/dot-block-editor.component.html @@ -27,15 +27,15 @@ @if (showCharData || charLimitError || requiredError) { -