Conversation
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBackend now builds structured modal payloads for payments and pricing during script localization and exposes them to admin JS; admin JS uses those objects to show info modals. Migration now sets the pricing-modal-on-upgrade flag only when Pro is not installed. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUI as Admin UI (browser)
participant AdminJS as admin.js
participant Server as PHP (FrmAppHelper / FrmMigrate)
participant Gateways as Stripe/Square checks
AdminUI->>AdminJS: user inserts payment/pricing field
AdminJS->>Server: reads `frm_admin_js` (localized script data)
Note right of Server: FrmAppHelper builds modal objects during localization
Server->>Gateways: check Stripe and Square connection status
Gateways-->>Server: connection states
Server-->>AdminJS: `paymentsSettingsModal` / `pricingFieldsModal` objects (or none)
AdminJS->>AdminUI: call infoModal(modalObj) if modal object present
AdminUI-->>AdminUI: render modal and user interacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP | Mar 31, 2026 6:35p.m. | Review ↗ | |
| JavaScript | Mar 31, 2026 6:35p.m. | Review ↗ |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
classes/helpers/FrmAppHelper.php (1)
4033-4035: Remove unused methodat_least_one_payment_gateway_is_setup().This private method has no usages in the codebase. Either remove it if it's dead code, or refactor
add_form_builder_modal_datato use it for consistency instead of directly calling the helper methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/helpers/FrmAppHelper.php` around lines 4033 - 4035, The private helper at_least_one_payment_gateway_is_setup() is dead code; either delete this unused method (remove the private static function at_least_one_payment_gateway_is_setup) or refactor add_form_builder_modal_data to call this method instead of directly calling FrmStrpLiteConnectHelper::at_least_one_mode_is_setup() and FrmSquareLiteConnectHelper::at_least_one_mode_is_setup(); pick one approach and update the class accordingly so there are no unused private methods and the payment-gateway existence check is centralized in at_least_one_payment_gateway_is_setup().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/helpers/FrmAppHelper.php`:
- Line 3951: The CI failure is caused by a PHP-CS-Fixer
blank_line_before_statement violation around the new call to
self::add_form_builder_modal_data; open the FrmAppHelper class where that call
was inserted and adjust surrounding blank lines to conform with the project's
style (add a single blank line before the self::add_form_builder_modal_data(...)
statement or remove an extra blank line as required), then run
./vendor/bin/php-cs-fixer fix --allow-risky=yes classes/helpers/FrmAppHelper.php
to auto-correct and verify the fix.
In `@js/src/admin/admin.js`:
- Line 10725: The call to infoModal is failing ESLint for missing spaces inside
parentheses; update the invocation of infoModal so there is a space after the
opening parenthesis and before the closing parenthesis (i.e., change
infoModal(frm_admin_js.pricingFieldsModal, '550px'); to use spaces inside the
parens), touching the invocation that references infoModal and
frm_admin_js.pricingFieldsModal to satisfy the spacing rule.
- Around line 2255-2258: The check using "'object' === typeof
frm_admin_js.paymentsSettingsModal" can pass for null and lead
infoModal(frm_admin_js.paymentsSettingsModal) to be called with null (causing
Object.assign to throw); update both occurrences (the block around
frm_admin_js.paymentsSettingsModal and the other check at the later occurrence)
to also assert truthiness before treating as an object (e.g., ensure
frm_admin_js.paymentsSettingsModal is not null/undefined) so infoModal is only
called with a real object; locate uses of frm_admin_js.paymentsSettingsModal and
the infoModal call to add the null/undefined guard.
---
Nitpick comments:
In `@classes/helpers/FrmAppHelper.php`:
- Around line 4033-4035: The private helper
at_least_one_payment_gateway_is_setup() is dead code; either delete this unused
method (remove the private static function
at_least_one_payment_gateway_is_setup) or refactor add_form_builder_modal_data
to call this method instead of directly calling
FrmStrpLiteConnectHelper::at_least_one_mode_is_setup() and
FrmSquareLiteConnectHelper::at_least_one_mode_is_setup(); pick one approach and
update the class accordingly so there are no unused private methods and the
payment-gateway existence check is centralized in
at_least_one_payment_gateway_is_setup().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6197ba1c-c332-4c83-b599-7e1bac229d77
📒 Files selected for processing (3)
classes/helpers/FrmAppHelper.phpjs/formidable_admin.jsjs/src/admin/admin.js
There was a problem hiding this comment.
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)
classes/helpers/FrmAppHelper.php (1)
3951-3961:⚠️ Potential issue | 🟡 MinorOnly build and clear modal data inside the
! $databranch.
add_form_builder_modal_data()runs before theformidable_admindata check. If that handle already has localized data and the pricing flag is set, Line 3960 skipswp_localize_script(), but the helper can already deletefrm_show_pricing_fields_modal.Suggested fix
- self::add_form_builder_modal_data( $admin_script_strings ); - - /** - * `@param` array $admin_script_strings - */ - $admin_script_strings = apply_filters( 'frm_admin_script_strings', $admin_script_strings ); - $data = $wp_scripts->get_data( 'formidable_admin', 'data' ); if ( ! $data ) { + self::add_form_builder_modal_data( $admin_script_strings ); + + /** + * `@param` array $admin_script_strings + */ + $admin_script_strings = apply_filters( 'frm_admin_script_strings', $admin_script_strings ); wp_localize_script( 'formidable_admin', 'frm_admin_js', $admin_script_strings ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/helpers/FrmAppHelper.php` around lines 3951 - 3961, add_form_builder_modal_data() is being called before checking whether the 'formidable_admin' handle already has localized data ($data), which can cause frm_show_pricing_fields_modal to be removed even when wp_localize_script() is skipped; move the call to self::add_form_builder_modal_data($admin_script_strings) (and any logic that clears frm_show_pricing_fields_modal) inside the if (! $data) branch so the modal data is only built and cleared when you're actually localizing the 'formidable_admin' script (referencing add_form_builder_modal_data, $data, wp_localize_script, and frm_show_pricing_fields_modal).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/helpers/FrmAppHelper.php`:
- Around line 4014-4025: The sprintf call that assigns
$admin_script_strings['pricingFieldsModal']['msg'] double-escapes the gateway
names because $gateway_texts entries and the 'and' separator are already escaped
with esc_html__(), then the entire implode(...) is wrapped in esc_html(...);
remove the outer esc_html() and pass the plain implode result (built from the
already-escaped $gateway_texts) into sprintf so the translated entities are not
double-escaped; locate the sprintf block that references $gateway_texts and
update it accordingly.
- Around line 3972-3990: The early return uses "self::is_form_builder_page() ||
self::pro_is_installed()" which prevents building the paymentsSettingsModal
payload for Pro installs; remove or change the pro check so the modal is still
added on form builder pages. Update the conditional that gates the block (the
call to self::is_form_builder_page() and self::pro_is_installed()) to only bail
out when not on the form builder (i.e., use only self::is_form_builder_page()),
ensuring the paymentsSettingsModal assignment (and the checks using
FrmStrpLiteConnectHelper::at_least_one_mode_is_setup(),
FrmSquareLiteConnectHelper::at_least_one_mode_is_setup(), and
FrmStrpLiteAppController::get_payments_settings_url()) runs for Pro and non‑Pro
users.
---
Outside diff comments:
In `@classes/helpers/FrmAppHelper.php`:
- Around line 3951-3961: add_form_builder_modal_data() is being called before
checking whether the 'formidable_admin' handle already has localized data
($data), which can cause frm_show_pricing_fields_modal to be removed even when
wp_localize_script() is skipped; move the call to
self::add_form_builder_modal_data($admin_script_strings) (and any logic that
clears frm_show_pricing_fields_modal) inside the if (! $data) branch so the
modal data is only built and cleared when you're actually localizing the
'formidable_admin' script (referencing add_form_builder_modal_data, $data,
wp_localize_script, and frm_show_pricing_fields_modal).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58d098ee-0a38-48f2-bb95-3587759040d4
📒 Files selected for processing (3)
classes/helpers/FrmAppHelper.phpjs/formidable_admin.jsjs/src/admin/admin.js
🚧 Files skipped from review as they are similar to previous changes (1)
- js/src/admin/admin.js
…/formidable-forms into fix-pricing-modal-show-in-pro
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3052 +/- ##
============================================
- Coverage 26.61% 26.59% -0.03%
- Complexity 9072 9078 +6
============================================
Files 148 148
Lines 30444 30476 +32
============================================
+ Hits 8103 8104 +1
- Misses 22341 22372 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR does:
Summary by CodeRabbit
Refactor
Bug Fixes