Skip to content

Update pricing fields modal#3052

Merged
Crabcyborg merged 13 commits intomasterfrom
fix-pricing-modal-show-in-pro
Mar 31, 2026
Merged

Update pricing fields modal#3052
Crabcyborg merged 13 commits intomasterfrom
fix-pricing-modal-show-in-pro

Conversation

@truongwp
Copy link
Copy Markdown
Contributor

@truongwp truongwp commented Mar 31, 2026

This PR does:

  • Only shows the pricing fields modal if Pro isn't installed.
  • Change the modal message if a payment gateway is installed.
  • Improve the modal code.

Summary by CodeRabbit

  • Refactor

    • Consolidated payment and pricing modals into a single structured payload delivered to the admin UI; admin scripts now consume that payload directly for consistent titles, messages, images and actions.
    • Migration now sets the pricing-fields prompt only for non‑Pro installs.
  • Bug Fixes

    • Modals appear only when appropriate, reflect accurate gateway connection status, and the pricing-fields prompt is cleared after use.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Caution

Review failed

Pull request was closed or merged during review

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Backend 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

Cohort / File(s) Summary
PHP helper / modal data
classes/helpers/FrmAppHelper.php
Removed ad-hoc localized keys; added add_form_builder_modal_data(&$admin_script_strings) which early-returns unless on the form-builder and Pro is not installed, checks Stripe/Square connection state, builds paymentsSettingsModal when no gateway is connected, conditionally builds pricingFieldsModal when frm_show_pricing_fields_modal is truthy, and deletes that option after use.
Admin JS consumption
js/src/admin/admin.js
Replaced boolean/string guards with object type checks for paymentsSettingsModal and pricingFieldsModal; calls infoModal() with backend-provided modal objects (and width for pricing modal) instead of constructing inline config and using separate localized strings/URLs.
Migration tweak
classes/models/FrmMigrate.php
migrate_to_105() no longer unconditionally sets frm_show_pricing_fields_modal; it sets that option only when FrmAppHelper::pro_is_installed() returns false.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through PHP fields and JS glades,

I tuck modal carrots in tidy shades,
If gateways nap, a helpful pop appears,
If Pro arrived, I tidy up my gears,
A little rabbit nudge to calm admin fears.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Update pricing fields modal' is partially related to the changeset, referring to a real aspect of the changes, but does not clearly highlight the main objective: only showing the modal when Pro is not installed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-pricing-modal-show-in-pro

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@deepsource-io
Copy link
Copy Markdown

deepsource-io bot commented Mar 31, 2026

DeepSource Code Review

We reviewed changes in c1ae373...db1ef00 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

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 ↗

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
classes/helpers/FrmAppHelper.php (1)

4033-4035: Remove unused method at_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_data to 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

📥 Commits

Reviewing files that changed from the base of the PR and between e0c1736 and 0e2031a.

📒 Files selected for processing (3)
  • classes/helpers/FrmAppHelper.php
  • js/formidable_admin.js
  • js/src/admin/admin.js

@truongwp truongwp requested a review from Crabcyborg March 31, 2026 14:55
@Crabcyborg Crabcyborg added this to the 6.30 milestone Mar 31, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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)
classes/helpers/FrmAppHelper.php (1)

3951-3961: ⚠️ Potential issue | 🟡 Minor

Only build and clear modal data inside the ! $data branch.

add_form_builder_modal_data() runs before the formidable_admin data check. If that handle already has localized data and the pricing flag is set, Line 3960 skips wp_localize_script(), but the helper can already delete frm_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

📥 Commits

Reviewing files that changed from the base of the PR and between 0e2031a and 458e343.

📒 Files selected for processing (3)
  • classes/helpers/FrmAppHelper.php
  • js/formidable_admin.js
  • js/src/admin/admin.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • js/src/admin/admin.js

Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg left a comment

Choose a reason for hiding this comment

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

Thanks @truongwp!

This should be good.

🚀

@Crabcyborg Crabcyborg merged commit 3015c6f into master Mar 31, 2026
21 of 22 checks passed
@Crabcyborg Crabcyborg deleted the fix-pricing-modal-show-in-pro branch March 31, 2026 18:38
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 0% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.59%. Comparing base (576a819) to head (db1ef00).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
classes/helpers/FrmAppHelper.php 0.00% 40 Missing ⚠️
classes/models/FrmMigrate.php 0.00% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants