Skip to content

ADD: Ability to rename form actions https://github.com/Crocoblock/suggestions/issues/7857#643

Merged
Gawuww merged 2 commits into
release/3.6.1from
issue/7857
May 26, 2026
Merged

ADD: Ability to rename form actions https://github.com/Crocoblock/suggestions/issues/7857#643
Gawuww merged 2 commits into
release/3.6.1from
issue/7857

Conversation

@Gawuww
Copy link
Copy Markdown
Collaborator

@Gawuww Gawuww commented May 18, 2026

@github-actions
Copy link
Copy Markdown

🤖 AI PR Review

Risk level: medium

Review

Summary

  • This PR changes the action edit modal to show a display name for actions (using a new helper) and includes a large number of rebuilt/compiled frontend/admin assets.

What I looked at

  • Source change: assets/src/editor/plugins/actions/edit.settings.modal.js
  • Built artifacts under assets/build/... (lots of generated files changed) — these appear to be the result of rebuilding JS bundles after the source change.

Notes / Findings

  1. Functional change (small, focused)
  • File: assets/src/editor/plugins/actions/edit.settings.modal.js
    • Adds import: getActionDisplayName from modules/actions-v2 assets.
    • Uses actionName = getActionDisplayName(currentAction, actionType?.label ?? '');
    • Replaces actionType.label in the modal header with actionName.
  • This implements the requested UX: show the (possibly renamed) action display name when editing action settings.
  1. Security / escaping
  • The header text is constructed via wp.i18n.sprintf( __('Edit %s', 'jet-form-builder'), actionName ). In React, plain string children are rendered safely (React escapes text nodes), so there is no XSS concern here as long as actionName is a plain string. If actionName could contain HTML that must be rendered as HTML, that would be a separate decision — currently we're rendering plain text which is safer.
  • If actionName is derived from user input (i.e. admin-provided action names), consider explicitly sanitizing on save (server-side) and/or normalizing the value. But displaying it here as text is acceptable.
  1. Import path and bundling
  • The import path is a deep relative path: ../../../../../modules/actions-v2/assets/src/utils/getActionDisplayName
    • Please verify this helper is exported/stable in that location and that the build system includes it into the bundle as expected. Deep relative paths are brittle; if that util has a stable public import path (e.g. modules/actions-v2/src/utils or an index export), prefer that.
    • The PR includes rebuilt assets, so it looks like your build picked up the new util. Ensure CI/build uses the same path.
  1. Backward compatibility & behavior
  • This changes the text shown to users in the action edit modal — previously it used actionType.label; now it uses getActionDisplayName(currentAction, actionType?.label). This is a user-visible change but is expected and matches the requested feature (ability to rename actions).
  • If any third-party plugin or integration relied on the exact previous label string for automated parsing of admin UI text (rare), this changes that string. I consider this low-impact.
  1. Tests
  • There are no new tests included. For high-risk UI logic it is fine to rely on manual QA, but a small unit test for getActionDisplayName (if not covered) or an integration test for the modal rendering would be helpful.
  1. Built asset volume
  • Many built files were changed (assets/build/...). That is expected when committing a UI change if the repo commits built artifacts. Make sure these are intentionally included (they are in this PR). Confirm bundle versions/pass lint/build on CI.

Suggestions / Recommendations

  • Verify the import path for getActionDisplayName is the intended, supported path. If possible, import from a module index or a shorter stable path so refactors are less likely to break imports.
  • Consider adding a tiny unit/integration test that the edit modal header shows the new actionName when a custom name is set, to prevent regressions.
  • If action names can be user-provided, ensure server-side sanitization on save (not done here) so stored names are normalized; displaying them in React text nodes is safe.

Minor code notes

  • The existing sprintf + __ usage is appropriate for translation. React will escape the final string so no extra esc_html is needed in JS.

Conclusion

  • Change is small and addresses the requested feature (ability to rename form actions). The main open item is to verify the util import path and to add a small test if you want stronger coverage. Built assets were updated accordingly.

Suggested changelog entry

- ADD: Ability to rename form actions — display custom action names in the action edit modal (actions UI)

@github-actions
Copy link
Copy Markdown

🤖 AI PR Review

Risk level: medium

Review

Summary

  • This PR adds the ability to rename form actions in the editor UI. The main functional changes are:
    • includes/actions/action-handler.php: new property $action->editor_name is saved from request payload.
    • assets/src/editor/plugins/actions/edit.settings.modal.js: the modal title now uses a computed action display name (getActionDisplayName()) instead of the raw actionType.label.
    • Large number of built JS/CSS assets were updated (assets/build/**) — these are generated bundles.

What I checked

  • The new persistent property (editor_name) is introduced and written when saving an action.
  • The editor modal uses a helper to get the display name.
  • Many built assets were changed — likely result of a build after the code change.

Issues / Concerns

  1. Sanitization of incoming value
  • File: includes/actions/action-handler.php
    • The new line sets $action->editor_name = $form_action['editor_name'] ?? '';
    • The value comes from the request (untrusted). It must be sanitized before being persisted. Use sanitize_text_field() (and wp_unslash if necessary), e.g.:
      $action->editor_name = isset( $form_action['editor_name'] ) ? sanitize_text_field( wp_unslash( $form_action['editor_name'] ) ) : '';
    • Please ensure this (or equivalent) is applied. If save_action() already sanitizes settings for other fields, mention it in code comments; otherwise sanitize here.
  1. Escaping when rendering
  • Any place that outputs editor_name in admin UI or frontend must escape output appropriately (esc_html for text nodes, esc_attr for attributes, wp_kses_post if HTML allowed). Search codebase for usages of this property and ensure proper escaping is used when rendering.
  1. Backward compatibility / data loading
  • Adding a new property to saved actions is compatible in principle, but ensure that:
    • Loading logic that hydrates action objects (loading from DB / postmeta / options) recognizes editor_name and restores it when editing existing forms/actions.
    • REST API and other persistence/serialization layers include editor_name where needed (if you provide an actions REST endpoint or send actions to JS). Otherwise the editor name will not persist across save/load.
    • If any add-ons or third-party code rely on strict action object shapes, document the addition (should be non-breaking but worth calling out).
  1. Authorization / capability checks
  • I did not see the surrounding save endpoint in this diff. Ensure the endpoint that accepts form_action has proper capability checks and nonce verification. The repo guidelines require nonces and capability checks for write operations. If these are already present earlier in this request path then fine; otherwise enforce them.
  1. Built assets committed
  • Many assets in assets/build/** were updated. Please ensure these builds are intentional (version bump in package.asset.php files) — if this PR should only contain PHP changes, consider committing only the minimal built bundles or confirm release process. Large generated diffs make code reviews harder and can risk shipping unintended changes.
  1. Tests and edge cases
  • There's no test coverage added for this change. Add unit/integration tests that:
    • Save an action with editor_name and verify that loading returns the same editor_name.
    • Ensure that invalid/malicious input in editor_name is sanitized correctly.
  • Consider an end-to-end test that renames an action in the editor and confirms persistence after save/load.
  1. Multisite / duplicates / uniqueness
  • If you expect editor_name to be human-friendly and not necessarily unique, that's fine. If you need uniqueness per form, consider checks or document behaviour. No functional breakage, but note potential for duplicate names.
  1. UI / i18n
  • The editor modal now shows a computed display name. Confirm getActionDisplayName() returns proper translated labels if appropriate, and that it falls back safely.

Suggested code change (includes sanitization):

  • Replace in includes/actions/action-handler.php:
    $action->editor_name = $form_action['editor_name'] ?? '';
    With:
    $action->editor_name = isset( $form_action['editor_name'] )
    ? sanitize_text_field( wp_unslash( $form_action['editor_name'] ) )
    : '';

Other small suggestions

  • Add a unit test for the save_form_action() behaviour.
  • Add a short comment where the property is added, explaining purpose and persistence expectations.
  • Grep for any other places that need to read/write editor_name (load, REST responses, JS initial data) and update them if they don’t already include it.

Risk assessment

  • The change touches persistence (adds a stored field) and touches the editor UI. It's a small change conceptually, but because it introduces a persisted value and many built asset updates, there is some risk around missing sanitization, missing restoration on load, and lack of tests. See specific concerns above.

Conclusion

  • The feature is straightforward and useful. Before merge:
    • Sanitize input before saving.
    • Ensure all renderers escape output.
    • Verify load/serialization layers include editor_name so the value persists.
    • Add tests and confirm build assets are intentionally updated.

Files of interest

  • includes/actions/action-handler.php (save_form_action change)
  • assets/src/editor/plugins/actions/edit.settings.modal.js (use of getActionDisplayName)
  • assets/build/** (many generated files) — verify build is intentional

If you want, I can propose a small patch for includes/actions/action-handler.php to add sanitization and a corresponding unit test skeleton.

Suggested changelog entry

- ADD: Ability to rename form actions in the editor (action editor name saved with form action)

@Gawuww Gawuww merged commit a43fbd7 into release/3.6.1 May 26, 2026
1 check passed
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.

1 participant