-
Notifications
You must be signed in to change notification settings - Fork 152
fix: Custom Taxonomies Pro Fields #1719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
fix: Custom Taxonomies Pro Fields #1719
Conversation
WalkthroughRemoved duplicate permission checks in form-builder AJAX handlers, simplified taxonomy option labels, added detection/registration for WooCommerce/Easy Digital Downloads post types and their tag taxonomies, and adjusted pro-feature button markup and padding in form-builder templates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/Ajax/Admin_Form_Builder_Ajax.php (1)
137-139: Harden nonce verification.Require and verify the nonce consistently (matches get_roles()).
- if ( isset( $nonce ) && ! wp_verify_nonce( $post_data['wpuf_form_builder_setting_nonce'], 'form-builder-setting-nonce' ) ) { - wp_send_json_error( __( 'Unauthorized operation', 'wp-user-frontend' ) ); - } + check_ajax_referer( 'form-builder-setting-nonce', 'wpuf_form_builder_setting_nonce' );
🧹 Nitpick comments (5)
includes/Ajax/Admin_Form_Builder_Ajax.php (2)
30-36: Remove duplicate capability check.Second current_user_can() block is redundant; keep a single check.
if ( ! current_user_can( wpuf_admin_role() ) ) { wp_send_json_error( __( 'Unauthorized operation', 'wp-user-frontend' ) ); } - - if ( ! current_user_can( wpuf_admin_role() ) ) { - wp_send_json_error( __( 'Unauthorized operation', 'wp-user-frontend' ) ); - }
231-233: Escape role values and labels.Prevent accidental HTML/attribute injection.
- foreach ( $roles as $key => $role ) { - $html .= '<option value="' . $key . '">' . $role . '</option>'; - } + foreach ( $roles as $key => $role ) { + $html .= '<option value="' . esc_attr( $key ) . '">' . esc_html( $role ) . '</option>'; + }includes/Traits/FieldableTrait.php (3)
117-125: Canonical fields for product_tag/download_tag: verify free/pro gating.Good addition; ensure front-end rendering respects existing gating (Render_Form::taxonomy blocks non-builtin when Pro is inactive). Avoid confusing “field present but not rendered.”
Would you like me to add a small guard (e.g., only add when wpuf_is_pro_active() or extend wpuf_builtin_taxonomies_free to include these for their post types)?
Also applies to: 127-135
531-543: Guard taxonomy registration and handle absence gracefully.If the taxonomy isn’t registered, register_taxonomy_for_object_type() returns false. Guard with taxonomy_exists().
- if ( 'product' === $this->form_settings['post_type'] && - 'product_tag' === $taxonomy['name'] && - ! is_object_in_taxonomy( $this->form_settings['post_type'], $taxonomy['name'] ) ) { - register_taxonomy_for_object_type( 'product_tag', 'product' ); - } + if ( 'product' === $this->form_settings['post_type'] && + 'product_tag' === $taxonomy['name'] && + taxonomy_exists( 'product_tag' ) && + ! is_object_in_taxonomy( $this->form_settings['post_type'], $taxonomy['name'] ) ) { + register_taxonomy_for_object_type( 'product_tag', 'product' ); + } @@ - if ( 'download' === $this->form_settings['post_type'] && - 'download_tag' === $taxonomy['name'] && - ! is_object_in_taxonomy( $this->form_settings['post_type'], $taxonomy['name'] ) ) { - register_taxonomy_for_object_type( 'download_tag', 'download' ); - } + if ( 'download' === $this->form_settings['post_type'] && + 'download_tag' === $taxonomy['name'] && + taxonomy_exists( 'download_tag' ) && + ! is_object_in_taxonomy( $this->form_settings['post_type'], $taxonomy['name'] ) ) { + register_taxonomy_for_object_type( 'download_tag', 'download' ); + }
804-806: Typo in docblock.“Attributres” → “attributes”.
- * Get WooCommerce attributres + * Get WooCommerce attributes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Ajax/Admin_Form_Builder_Ajax.php(1 hunks)includes/Traits/FieldableTrait.php(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Traits/FieldableTrait.php (1)
includes/Render_Form.php (1)
taxonomy(1354-1518)
🪛 GitHub Actions: Inspections
includes/Ajax/Admin_Form_Builder_Ajax.php
[error] 1-1: PHPCS error: A function call to __() with texts containing placeholders was found, but was not accompanied by a 'translators:' comment on the line above to clarify the meaning of the placeholders. Command: vendor/bin/phpcs includes/Ajax/Admin_Form_Builder_Ajax.php includes/Traits/FieldableTrait.php -q --report=checkstyle | cs2pr --graceful-warnings
includes/Traits/FieldableTrait.php
[error] 1-1: PHPCS error: A function call to __() with texts containing placeholders was found, but was not accompanied by a 'translators:' comment on the line above to clarify the meaning of the placeholders. Command: vendor/bin/phpcs includes/Ajax/Admin_Form_Builder_Ajax.php includes/Traits/FieldableTrait.php -q --report=checkstyle | cs2pr --graceful-warnings
🪛 GitHub Check: Run PHPCS inspection
includes/Ajax/Admin_Form_Builder_Ajax.php
[failure] 181-181:
A function call to __() with texts containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.
includes/Traits/FieldableTrait.php
[failure] 87-87:
Whitespace found at end of line
[failure] 82-82:
Whitespace found at end of line
🔇 Additional comments (1)
includes/Traits/FieldableTrait.php (1)
78-86: LGTM: include WooCommerce and EDD post types.Clean check via post_type_exists and de-duplication looks good.
Please confirm these appear only when the respective plugins register the post types during the builder request.
| $cat .= sprintf( __( 'Default %s', 'wp-user-frontend' ), $tax->label ); | ||
| $cat .= '</label></div>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Fix i18n placeholder and escape output (pipeline blocker).
Add translators comment and escape both the string and the taxonomy label to satisfy PHPCS and prevent HTML injection.
Apply:
- $cat .= sprintf( __( 'Default %s', 'wp-user-frontend' ), $tax->label );
+ /* translators: %s: taxonomy label */
+ $cat .= sprintf( esc_html__( 'Default %s', 'wp-user-frontend' ), esc_html( $tax->label ) );Run PHPCS locally to confirm:
🏁 Script executed:
#!/bin/bash
vendor/bin/phpcs includes/Ajax/Admin_Form_Builder_Ajax.php -qLength of output: 62
I'll examine the actual code context to verify the i18n and escaping issues.
🏁 Script executed:
#!/bin/bash
# Inspect the problematic lines and surrounding context
sed -n '170,195p' includes/Ajax/Admin_Form_Builder_Ajax.phpLength of output: 1470
🏁 Script executed:
#!/bin/bash
# Check for translators comments in the file
rg -n "translators:" includes/Ajax/Admin_Form_Builder_Ajax.php -B2 -A2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for other sprintf + __() patterns in file to see if translators comments exist elsewhere
rg -n "sprintf.*__\(" includes/Ajax/Admin_Form_Builder_Ajax.php -B1 -A1Length of output: 263
🏁 Script executed:
#!/bin/bash
# Check WordPress escaping patterns for taxonomy properties
rg -n "tax->label" includes/Ajax/Admin_Form_Builder_Ajax.php -B3 -A3Length of output: 520
---
🌐 Web query:
```web
WordPress i18n translators comment requirement sprintf __ PHPCS
💡 Result:
Short answer: add a translators comment (/* translators: ... */) directly above any translation string that contains placeholders (e.g. %s, %d, %1$s), and prefer using printf()/sprintf() with (numbered) placeholders so translators can reorder them. WordPress Coding Standards (WPCS) include an i18n sniff that flags missing/misplaced translator comments.
What to do
- Use placeholders and printf/sprintf instead of interpolated variables. Example:
/* translators: %s: Name of a city */
printf( __( 'Your city is %s.', 'my-plugin' ), $city ); - Put the comment immediately above the translation call, start it with "translators:" and describe each placeholder (use numbering when multiple placeholders).
- Use numbered placeholders (%1$s, %2$s) when the argument order may change in other languages.
Why
- The WordPress i18n docs and Plugin Handbook require translator comments for strings with placeholders to give translators the necessary context. [1][2]
- Core ran a large effort to add missing translator comments and the WPCS i18n sniff now checks for missing/misplaced comments. [3][4]
Sources
- WordPress Internationalization Guidelines (placeholders & translators comments). [1]
- Plugin Handbook — translators comments and placement. [2]
- Trac ticket/history about adding missing translators comments (44360) and core changes. [3]
- WordPress Coding Standards (WPCS) changelog — added WordPress.WP.I18n sniff to detect missing translator comments. [4]
Would you like a PHPCS ruleset snippet or example to fail/auto-fix this in CI?
🌐 Web query:
WordPress esc_html__ vs __ escaping functions best practice
💡 Result:
Short answer
- Use esc_html__ (or esc_attr__, esc_html_e, esc_attr_e, etc.) when you need a translated string that is already escaped for output. [1][2]
- __ only returns the translation without escaping — wrap it with the correct escaping function or use the esc_…__ variant. [1][4]
Best-practice rules
- Escape late: escape when outputting, not when storing. Use the escaping function that fits the context (HTML text vs HTML attribute vs JS vs URL). [1]
- Prefer the combined i18n+escape helpers (esc_html__, esc_attr__, esc_html_e, esc_attr_e) to avoid forgetting to escape. [2][4]
- Choose the correct escape:
- esc_html__ / esc_html(): text inside HTML elements (strips any HTML). [1]
- esc_attr__ / esc_attr(): values used inside attributes (title, value, class). [1][4]
- esc_js(): inline JavaScript. [1]
- esc_url(): URLs (or esc_url_raw for DB/storage cases). [1]
- If HTML is expected and allowed, do NOT use esc_html — use wp_kses_post() or wp_kses() with a whitelist instead. [1][5]
- Audit for uses of __, _e, _x that are echoed without escaping and replace with escaped variants (Plugin Handbook guidance). [6]
Examples
- Safe HTML output: echo esc_html__( 'Settings', 'my-domain' ); [2]
- Attribute: printf( '', esc_attr__( 'Default', 'my-domain' ) ); [1]
- If HTML allowed: echo wp_kses_post( __( 'Some HTML', 'my-domain' ) ); [1][5]
Sources
[1] WordPress — Escaping data (Developer Handbook).
[2] Function reference: esc_html__ (WordPress Code Reference).
[3] Internationalization guidelines — escaping strings (WordPress Handbook).
[4] esc_attr() reference (WordPress Code Reference).
[5] wp_kses / wp_kses_post guidance (WordPress Handbook).
[6] Plugin Handbook — common issues: using __ without escaping.
Fix i18n placeholder and escape output (PHPCS blocker and XSS risk).
Line 181 outputs $tax->label unescaped inside an HTML label tag and is missing the required translators comment. Add the translators comment and escape both the string and label using esc_html__() and esc_html() to satisfy PHPCS i18n standards and prevent HTML injection.
- $cat .= sprintf( __( 'Default %s', 'wp-user-frontend' ), $tax->label );
+ /* translators: %s: taxonomy label */
+ $cat .= sprintf( esc_html__( 'Default %s', 'wp-user-frontend' ), esc_html( $tax->label ) );🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 181-181:
A function call to __() with texts containing placeholders was found, but was not accompanied by a "translators:" comment on the line above to clarify the meaning of the placeholders.
🤖 Prompt for AI Agents
In includes/Ajax/Admin_Form_Builder_Ajax.php around lines 181-182, the code
prints $tax->label directly into the label text and lacks a translators comment;
update the translation call to use esc_html__() (with an inline translators
comment for context) and escape the dynamic label with esc_html(), replacing the
current sprintf/concatenation so the translatable string is escaped and the
label is safely escaped to prevent XSS.
| if ( post_type_exists( 'product' ) && ! in_array( 'product', $wpuf_post_types, true ) ) { | ||
| $wpuf_post_types[] = 'product'; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Trailing whitespace trips PHPCS.
Remove trailing spaces on these lines to appease CI.
Also applies to: 87-87
🏁 Script executed:
#!/bin/bash
vendor/bin/phpcbf includes/Traits/FieldableTrait.php && vendor/bin/phpcs includes/Traits/FieldableTrait.php -qLength of output: 63
I'll directly inspect the file to verify the trailing whitespace issue since the PHP tools aren't available in the sandbox.
---
🏁 Script executed:
```shell
#!/bin/bash
# Display the file with line numbers and make trailing whitespace visible
# Using cat with -A to show whitespace characters
sed -n '80,90p' includes/Traits/FieldableTrait.php | cat -A
Length of output: 423
Remove trailing whitespace on lines 82 and 87.
Lines 82 and 87 contain 8 spaces of indentation with no other content, creating trailing whitespace that violates PHPCS standards. These should be completely blank lines with no characters.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 82-82:
Whitespace found at end of line
🤖 Prompt for AI Agents
In includes/Traits/FieldableTrait.php around lines 82 and 87, remove the
trailing whitespace on those lines so they are completely empty (no spaces or
tabs). Open the file, delete the 8-space indentation characters on lines 82 and
87, save, and run PHPCS to verify no trailing whitespace remains.
|
Issue not solved for events calendar template. Still size mismatch issue exists @sapayth vai solved for WOO commerce and EDD
|
|
fixed @Rubaiyat-E-Mohammad bhai. please have a look now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
admin/form-builder/assets/js/components/form-fields-v4-1/template.php(1 hunks)assets/js-templates/form-components.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- admin/form-builder/assets/js/components/form-fields-v4-1/template.php
| @click="alert_pro_feature(field)" | ||
| class="wpuf-relative wpuf-group/pro-field"> | ||
| <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-px-4 wpuf-py-3 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer"> | ||
| <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-p-4 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Padding inconsistency causes size mismatch with other field buttons.
The pro-feature field button uses wpuf-p-4 (16px on all sides), while failed-validation and regular field buttons use wpuf-px-3 wpuf-py-4 (12px horizontal, 16px vertical) on lines 1173 and 1191. This padding mismatch creates visual misalignment when these field types are displayed together in the form builder sidebar, which likely explains the size mismatch reported by the reviewer for The Events Calendar template.
Apply this diff to align the padding with other field button types:
- <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-p-4 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer">
+ <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-px-3 wpuf-py-4 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-p-4 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer"> | |
| <div class="wpuf-opacity-50 wpuf-field-button wpuf-flex wpuf-items-center wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-bg-white wpuf-shadow-sm wpuf-px-3 wpuf-py-4 hover:wpuf-border-gray-300 hover:wpuf-cursor-pointer"> |
🤖 Prompt for AI Agents
In assets/js-templates/form-components.php around line 1149, the pro-feature
field button uses wpuf-p-4 which causes a width/size mismatch with other field
buttons that use wpuf-px-3 wpuf-py-4; change the class list on that div to use
wpuf-px-3 wpuf-py-4 instead of wpuf-p-4 so horizontal padding matches the
failed-validation and regular buttons and restores visual alignment in the
sidebar.



close #1179
Summary by CodeRabbit
New Features
Bug Fixes
Style