Skip to content

refactor: review 2.6 clarify CND ownership and engine form semantics#86

Open
hduchesne wants to merge 2 commits into
review_2.1245from
review_2.67
Open

refactor: review 2.6 clarify CND ownership and engine form semantics#86
hduchesne wants to merge 2 commits into
review_2.1245from
review_2.67

Conversation

@hduchesne
Copy link
Copy Markdown
Member

Description

Move engine-owned logic CND definitions out of formidable-elements/settings
into formidable-engine, and introduce explicit structural semantics for form
containers instead of inferring them from non-submittable field types.

Add engine-owned semantic mixins for form protection:

  • fmdbmix:captchaProtectedForm
  • fmdbmix:authenticatedOnlyForm

Keep fmdbmix:captcha and fmdbmix:requireAuthentication as author-facing
wrappers on fmdb:form in formidable-elements, while making runtime Java code
read the engine-owned mixins directly.

Move the Content Editor form definitions for fmdbmix:formLogicElement to the
engine module, update fieldset/container classification, and align technical
documentation and unit tests with the new ownership rules.

Checklist

Source code

  • I've shared and documented any breaking change
  • I've reviewed and updated the jahia-depends

Tests

  • I've provided Unit and/or Integration Tests
  • I've updated the parent issue with required manual validations

Tip

Documentation to guide the reviews: How to do a code review

hduchesne added 2 commits May 29, 2026 17:27
Move engine-owned logic CND definitions out of formidable-elements/settings
into formidable-engine, and introduce explicit structural semantics for form
containers instead of inferring them from non-submittable field types.

Add engine-owned semantic mixins for form protection:
- fmdbmix:captchaProtectedForm
- fmdbmix:authenticatedOnlyForm

Keep fmdbmix:captcha and fmdbmix:requireAuthentication as author-facing
wrappers on fmdb:form in formidable-elements, while making runtime Java code
read the engine-owned mixins directly.

Move the Content Editor form definitions for fmdbmix:formLogicElement to the
engine module, update fieldset/container classification, and align technical
documentation and unit tests with the new ownership rules.
Move fmdbmix:formLogicElement labels from formidable-elements to
formidable-engine. Update the conditional-logic empty-state message
to mention rules above.
@hduchesne hduchesne requested a review from a team as a code owner May 29, 2026 17:45
@github-actions
Copy link
Copy Markdown
Contributor

📝 Documentation Guidelines

Thank you for contributing to our documentation! To ensure your contributions meet our standards, please review these resources:

This comment is posted automatically when changes are detected in the docs/ folder.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR clarifies CND ownership between formidable-elements and formidable-engine, moving engine-interpreted form semantics into the engine module while keeping author-facing wrappers in elements.

Changes:

  • Added engine-owned semantic and structural mixins for form logic, CAPTCHA/auth protection, and field classification.
  • Updated submission parsing/pipeline code to consume semantic mixins instead of concrete element node types.
  • Moved conditional-logic editor definitions/i18n to the engine module and updated related documentation.

Reviewed changes

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

Show a summary per file
File Description
README.md Adds links to core technical documentation.
formidable-engine/src/main/resources/META-INF/definitions.cnd Defines engine-owned form logic and semantic mixins.
formidable-engine/src/main/java/org/jahia/modules/formidable/engine/servlet/FormSubmissionPipeline.java Reads auth/CAPTCHA engine semantic mixins.
formidable-engine/src/main/java/org/jahia/modules/formidable/engine/servlet/FormFieldMetadataCollector.java Collects parser metadata from semantic mixins.
formidable-engine/src/main/java/org/jahia/modules/formidable/engine/actions/FormDataParser.java Uses FieldInfo semantic flags for validation.
formidable-engine/src/main/java/org/jahia/modules/formidable/engine/captcha/CaptchaRenderFilter.java Applies CAPTCHA rendering via engine-owned semantic mixin.
formidable-engine/src/main/java/org/jahia/modules/formidable/engine/servlet/ErrorCode.java Updates auth error-code comment.
formidable-engine/src/test/java/org/jahia/modules/formidable/engine/servlet/FormSubmissionPipelineTest.java Updates auth mixin tests.
formidable-engine/src/test/java/org/jahia/modules/formidable/engine/actions/FormDataParserFieldMetadataTest.java Updates FieldInfo tests for semantic flags.
formidable-engine/src/main/resources/resources/formidable-engine*.properties Adds conditional-logic labels to engine resources.
formidable-engine/src/main/resources/META-INF/jahia-content-editor-forms/**/fmdbmix_formLogicElement.json Moves conditional-logic editor form definitions to engine.
formidable-engine/src/main/resources/javascript/locales/*.json Adjusts conditional-logic UI wording.
formidable-elements/settings/definitions.cnd Removes engine-owned logic definitions from elements.
formidable-elements/src/components/**/definition.cnd Adds semantic mixins to concrete field/container definitions.
formidable-elements/src/components/Form/definition.cnd Keeps author-facing wrappers extending engine semantic mixins.
formidable-elements/settings/resources/formidable-elements*.properties Removes conditional-logic labels from elements resources.
formidable-elements/README.md Adds CND ownership documentation link.
docs/cnd-module-ownership.md Documents CND ownership rules.
docs/form-submission-flow.md Aligns submission flow docs with semantic mixins.
docs/error-codes.md Updates auth/CAPTCHA error descriptions.
docs/conditional-logic-field-resolution.md Updates CND location and shape.
docs/captcha-server-side-validation.md Aligns CAPTCHA docs with pipeline terminology.

boolean hasCaptcha;
try {
hasCaptcha = formNode.isNodeType("fmdbmix:captcha");
hasCaptcha = formNode.isNodeType(CAPTCHA_PROTECTED_FORM_MIXIN);
Comment on lines +204 to +215
private static FormDataParser.FieldInfo buildFieldInfo(JCRNodeWrapper node, String nodeType) throws RepositoryException {
boolean nonSubmittable = node.isNodeType("fmdbmix:nonSubmittable");
boolean choiceField = node.isNodeType("fmdbmix:choiceField");
boolean fileField = node.isNodeType("fmdbmix:fileField");
boolean emailField = node.isNodeType("fmdbmix:emailField");
boolean dateField = node.isNodeType("fmdbmix:dateField");
boolean datetimeLocalField = node.isNodeType("fmdbmix:datetimeLocalField");
boolean colorField = node.isNodeType("fmdbmix:colorField");

Set<String> choices = choiceField ? collectChoices(node, node.getName(), resolveChoicePropertyName(node)) : Set.of();
Set<String> acceptedTypes = fileField ? collectAcceptTypes(node) : Set.of();
FormDataParser.FieldConstraints constraints = readConstraints(node, dateField, datetimeLocalField);
@jahia-sonarqube
Copy link
Copy Markdown

Quality Gate failed Quality Gate failed

Failed conditions
B Security Rating on New Code (required ≥ A)
E Reliability Rating on New Code (required ≥ A)
9 New Reliability Issues (required ≤ 0)
895 Added Technical Debt (required ≤ 0)
61 New Maintainability Issues (required ≤ 0)
13 New Security Issues (required ≤ 0)

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants