Skip to content

Conversation

@yurkodmtr
Copy link
Collaborator

…er/issues/5654

@github-actions
Copy link

🤖 AI PR Review

Risk level: medium

Review

This PR adds form macros logic with a sizable update to CSS and JavaScript frontend assets, including calculated fields, conditional blocks, dynamic values, and media/file upload components.

Security: The JS code uses sanitized input and existing JetFormBuilder APIs. Calculated field formulas and dynamic values rely on safe parsing (e.g., use of eval is concerning but expected to be controlled in formula contexts); allowlist or validation for formulas and macros should be ensured elsewhere. No direct nonce or capability checks are impacted here as this is frontend logic.

Performance: The frontend JS handles reactive inputs, conditions, and dynamic updates efficiently with debouncing and caching strategies, and there's careful handling of file previews and sorting.

Backward Compatibility: The PR appears to extend existing APIs and patterns without breaking changes. Core class prototypes are extended properly. It includes new conditional checkers and integration with macros logic.

Styling: Large CSS changes are mostly layout and UI improvements; no regressions detected. Adheres to WP coding standards.

Testing: No explicit tests are included or referenced in the diff, especially for new or updated JS modules (calculated field formatting, conditional logic, dynamic values). Given the complexity and potential edge cases in formula evaluation and conditional display, thorough automated and manual tests should be ensured.

Multisite & Integration: No direct conflicts or issues detected, integration with Gutenberg and JetEngine data expected to continue as normal.

Recommendations:

  • Ensure the formulas/macros input is validated and securely sanitized before evaluation elsewhere in the system.
  • Add or verify existing unit and integration tests cover the new macros functionality and updates to conditional display and calculated fields.
  • Monitor performance impacts if formulas become complex or forms large.

Overall, the changes align well with JetFormBuilder architecture and standards, representing an improvement and enhancement to macros and form field dynamics.

Suggested changelog entry

- ADD: Introduced form macros logic with enhancements to calculated fields, conditional blocks, dynamic values, and media field components, improving form flexibility and interactivity.

Copy link

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

Adds “form macros” support in the editor and runtime, including a new Macros Inserter module and repeater-aware macro value resolution.

Changes:

  • Register a new Macros_Inserter module and add tooling (webpack/babel) for its editor script.
  • Enrich macro-field UI with repeater context and add frontend logic to resolve repeater macro output.
  • Rebuild multiple compiled frontend/editor assets to include the new behavior.

Reviewed changes

Copilot reviewed 45 out of 57 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
modules/modules-controller.php Registers the new Macros_Inserter module in the modules controller.
modules/macros-inserter/webpack.config.js Adds webpack config for building the macros inserter editor bundle.
modules/macros-inserter/package.json Adds build/dev scripts for the macros inserter module.
modules/macros-inserter/module.php Enqueues the macros inserter editor script via the module lifecycle hooks.
modules/macros-inserter/assets/src/editor.js Adds BlockEdit toolbar UI to insert macro placeholders into core/html blocks.
modules/macros-inserter/.babelrc Reuses the root babel configuration for module builds.
modules/html-parser/assets/build/parser.asset.php Build metadata version bump from asset rebuild.
modules/html-parser/assets/build/admin-ui.asset.php Build metadata version bump from asset rebuild.
modules/blocks-v2/repeater-field/assets/src/frontend/repeater-macros.js Adds repeater macro value resolver and change notification binding.
modules/blocks-v2/repeater-field/assets/src/frontend/index.js Hooks repeater macro resolver into the macro value filter.
modules/blocks-v2/repeater-field/assets/build/frontend.css Rebuilt CSS output for repeater-field frontend bundle.
modules/blocks-v2/repeater-field/assets/build/frontend.asset.php Build metadata version bump from asset rebuild.
modules/blocks-v2/repeater-field/assets/build/editor.asset.php Build metadata version bump from asset rebuild.
modules/blocks-v2/actions-integration/assets/build/editor.js Rebuilt actions-integration editor bundle output.
modules/blocks-v2/actions-integration/assets/build/editor.asset.php Build metadata version bump from asset rebuild.
jet-form-builder.php Updates plugin version constant.
assets/src/package/macros.button/components/MacrosFields.js Enriches macro fields with repeater context based on block tree.
assets/src/package/macros.button/components/MacroFieldItem.js Updates macro list item labeling/click payload to include context.
assets/src/frontend/main/html.macro/observeNode.js Adds macro host template caching + newline conversion behavior.
assets/src/frontend/main/html.macro/CalculatedHtmlString.js Passes macroHost through macro field value filters for context-aware resolving.
assets/build/frontend/multi.step.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/media.field.restrictions.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/media.field.js Rebuilt media field frontend bundle output.
assets/build/frontend/media.field.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/main.css Rebuilt frontend CSS output.
assets/build/frontend/main.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/dynamic.value.js Rebuilt dynamic value frontend bundle output.
assets/build/frontend/dynamic.value.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/conditional.block.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/calculated.field.js Rebuilt calculated field frontend bundle output.
assets/build/frontend/calculated.field.asset.php Build metadata version bump from asset rebuild.
assets/build/frontend/advanced.reporting.asset.php Build metadata version bump from asset rebuild.
assets/build/editor/package.asset.php Build metadata version bump from asset rebuild.
assets/build/editor/form.builder.asset.php Build metadata version bump from asset rebuild.
assets/build/editor/default.builder.asset.php Build metadata version bump from asset rebuild.
assets/build/admin/vuex.package.asset.php Build metadata version bump from asset rebuild.
assets/build/admin/pages/jfb-settings.asset.php Build metadata version bump from asset rebuild.
assets/build/admin/pages/jfb-addons.asset.php Build metadata version bump from asset rebuild.
assets/build/admin/package.asset.php Build metadata version bump from asset rebuild.
Comments suppressed due to low confidence (7)

jet-form-builder.php:1

  • The plugin version constant was changed to an invalid-looking value (3.5.6111111111111212). This will leak into asset versioning/cache busting and may break semantic-version expectations elsewhere. Please set it to the intended release version (e.g., 3.5.7 or similar) and keep it consistent with your release process.
    modules/macros-inserter/webpack.config.js:1
  • Production mode detection is incorrect: webpack CLI typically passes --mode production (separate args), so --mode:production will never match and devMode will stay true even in production. This likely enables inline source maps for production builds. Consider reading process.argv for --mode and its next argument, or rely on process.env.NODE_ENV / argv.mode.
    modules/macros-inserter/module.php:1
  • This module enqueues assets/build/editor.js, but this PR only adds assets/src/editor.js and does not add the built assets/build/editor.js artifact. If build artifacts are committed in this repo, this will result in a 404 and the feature won’t load. Please add the built file (and corresponding .asset.php if required by your conventions) or update the enqueue path to match the actual packaged output.
    modules/macros-inserter/module.php:1
  • The enqueued editor script depends on WP packages (wp.hooks, wp.compose, wp.element, wp.blockEditor, wp.components). Passing an empty dependencies array can cause load-order issues. Consider generating and using an editor.asset.php (WP build convention) or explicitly listing dependencies like wp-hooks, wp-compose, wp-element, wp-block-editor, wp-components.
    modules/macros-inserter/assets/src/editor.js:1
  • data is declared but never used. Please remove it to reduce noise and avoid confusing readers about intended behavior.
    modules/macros-inserter/assets/src/editor.js:1
  • This builds HTML via string interpolation and injects values derived from block attributes (field.repeater_name) directly into HTML. If those values contain <, \", etc., this can produce HTML injection/XSS when inserted into the HTML block content. Prefer building DOM nodes (e.g., document.createElement) and setting textContent/setAttribute, or escape/encode interpolated values before concatenation.
    modules/blocks-v2/repeater-field/assets/src/frontend/repeater-macros.js:1
  • This returns HTML assembled from user-controlled field values and joins with <br/>. If a field value contains HTML (e.g., <img onerror=...>), it will be injected into the page when the macro output is applied via innerHTML, creating an XSS vector. Escape name/value for HTML output (convert <, >, &, \", ') or return plain text and render line breaks safely.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +57 to +62
function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') {
const millis = eval(millisInput);
if (!millis || isNaN(millis) || null === millis || 0 === millis) {
return 0;
}
const date = new Date(millis);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Using eval() on millisInput is a critical code injection risk. If millisInput can be influenced by a formula or user input, this can execute arbitrary JavaScript in the browser context. Replace eval with safe numeric conversion (e.g., Number(...) / parseInt) or a restricted expression parser if expressions must be supported.

Suggested change
function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') {
const millis = eval(millisInput);
if (!millis || isNaN(millis) || null === millis || 0 === millis) {
return 0;
}
const date = new Date(millis);
function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') {
const millis = Number(millisInput);
if (!millis || isNaN(millis) || null === millis || 0 === millis) {
return 0;
}
const date = new Date(millis);

Copilot uses AI. Check for mistakes.
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