New spam settings section in form settings#3179
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a consolidated Spam settings tab with CAPTCHA controls and global spam status indicators, backend CAPTCHA field creation/removal during form updates, an Advanced settings redirect, and supporting JavaScript, SCSS, and regenerated admin assets. ChangesSpam Settings Consolidation and CAPTCHA Management
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SpamSettingsView
participant FrmFormsController
participant FrmField
User->>SpamSettingsView: toggle Include CAPTCHA
User->>FrmFormsController: save settings with frm_include_captcha
FrmFormsController->>FrmFormsController: update_settings()
FrmFormsController->>FrmFormsController: handle_captcha_field(form_id, include_captcha)
FrmFormsController->>FrmField: get_all_for_form(form_id)
alt CAPTCHA requested and missing
FrmFormsController->>FrmField: create CAPTCHA field before submit
else CAPTCHA not requested and present
FrmFormsController->>FrmField: destroy CAPTCHA field
end
FrmFormsController-->>User: updated settings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)js/formidable_admin.jsast-grep timed out on this file 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 | Jul 14, 2026 5:40p.m. | Review ↗ | |
| JavaScript | Jul 14, 2026 5:40p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
classes/views/frm-forms/spam-settings.php (2)
101-110: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
rel="noopener"to thetarget="_blank"link.Other
target="_blank"links in this codebase (e.g.FrmFormsController::get_form_too_long_docs_url()usage) includerel="noopener". This link omits it.🔒️ Proposed fix
'<a href="' . esc_url( admin_url( 'admin.php?page=formidable-settings&t=captcha_settings' ) ) . '" target="_blank">', + '<a href="' . esc_url( admin_url( 'admin.php?page=formidable-settings&t=captcha_settings' ) ) . '" target="_blank" rel="noopener">',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/views/frm-forms/spam-settings.php` around lines 101 - 110, The link in the spam settings notice opens a new tab via target="_blank" but is missing rel="noopener". Update the anchor markup built in the printf call in the spam settings view so the generated link includes rel="noopener", matching the existing pattern used elsewhere such as in FrmFormsController-related external links.
39-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate captcha-field lookup logic.
This loop re-implements the same "find captcha field by iterating form fields" logic that
FrmFormsController::handle_captcha_field()already performs. Extracting a shared helper (e.g. onFrmField) would keep both call sites in sync if the captcha field type or lookup ever changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/views/frm-forms/spam-settings.php` around lines 39 - 50, The captcha-field lookup is duplicated here and in FrmFormsController::handle_captcha_field(), so update this block to use a shared helper instead of iterating form fields directly. Add or reuse a single FrmField helper for finding the captcha field by form ID, then call that helper from both FrmFormsController::handle_captcha_field() and the spam-settings view so the lookup logic stays consistent if the captcha type changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@classes/controllers/FrmFormsController.php`:
- Around line 306-346: The captcha creation branch in handle_captcha_field()
calls FrmField::create() with an incomplete payload, missing the default options
and field_options that FrmField::create() expects. Update the array passed to
FrmField::create() in the include-captcha path to mirror the default field
payload used by create_submit_button_field(), so the captcha field is created
with the same required defaults and does not trigger warnings or save incomplete
data.
---
Nitpick comments:
In `@classes/views/frm-forms/spam-settings.php`:
- Around line 101-110: The link in the spam settings notice opens a new tab via
target="_blank" but is missing rel="noopener". Update the anchor markup built in
the printf call in the spam settings view so the generated link includes
rel="noopener", matching the existing pattern used elsewhere such as in
FrmFormsController-related external links.
- Around line 39-50: The captcha-field lookup is duplicated here and in
FrmFormsController::handle_captcha_field(), so update this block to use a shared
helper instead of iterating form fields directly. Add or reuse a single FrmField
helper for finding the captcha field by form ID, then call that helper from both
FrmFormsController::handle_captcha_field() and the spam-settings view so the
lookup logic stays consistent if the captcha type changes.
🪄 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: 1e2acb21-b7f9-4fc7-a9a0-f6fab4062cab
📒 Files selected for processing (8)
classes/controllers/FrmFormsController.phpclasses/helpers/FrmHtmlHelper.phpclasses/views/frm-forms/settings-advanced.phpclasses/views/frm-forms/spam-settings.phpcss/frm_admin.cssjs/formidable_admin.jsjs/src/admin/admin.jsresources/scss/admin/components/settings/_form-settings.scss
|
I'm seeing the following deprecated message in the captcha field description when I enable the
|
Thank you @garretlaxton! The code for creating the captcha field wasn't very complete. It's been updated to go through the proper functions. This should be fixed now. The old captcha would likely still throw the same issues, so I'd uncheck, save, and check it again. |
|
Thank you @garretlaxton! 🚀 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
classes/controllers/FrmFormsController.php (2)
287-290: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the existing captcha field when the checkbox is disabled (
classes/controllers/FrmFormsController.php:288-289). Whenfrm_include_captchais missing or0because the control is disabled in the Spam view, this path still reachesFrmField::destroy(). Saving unrelated form settings can remove an existing captcha field; only treat an explicit user uncheck as removal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/controllers/FrmFormsController.php` around lines 287 - 290, Update the captcha inclusion flow around FrmFormsController::handle_captcha_field so a missing or disabled frm_include_captcha value preserves the existing captcha field. Only invoke captcha removal when the request explicitly indicates the user unchecked the control, while retaining creation behavior for an enabled checkbox.
321-325: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle all CAPTCHA fields, not just the first match. The handler only records and deletes the first CAPTCHA field ID. If duplicate CAPTCHA fields are present, unchecking leaves the others in place and the form setting no longer matches the form contents. Collect all matching IDs or enforce a single-CAPTCHA invariant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/controllers/FrmFormsController.php` around lines 321 - 325, Update the CAPTCHA field handling in FrmFormsController to process every field whose type is 'captcha' rather than breaking after the first match. Collect all matching field IDs and ensure the subsequent unchecking/removal logic deletes each one, keeping the form setting synchronized with its contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@classes/controllers/FrmFormsController.php`:
- Around line 287-290: Update the captcha inclusion flow around
FrmFormsController::handle_captcha_field so a missing or disabled
frm_include_captcha value preserves the existing captcha field. Only invoke
captcha removal when the request explicitly indicates the user unchecked the
control, while retaining creation behavior for an enabled checkbox.
- Around line 321-325: Update the CAPTCHA field handling in FrmFormsController
to process every field whose type is 'captcha' rather than breaking after the
first match. Collect all matching field IDs and ensure the subsequent
unchecking/removal logic deletes each one, keeping the form setting synchronized
with its contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e1101db-de50-45d0-bd26-ca5ac0652882
📒 Files selected for processing (6)
classes/controllers/FrmFormsController.phpclasses/helpers/FrmHtmlHelper.phpclasses/views/frm-forms/spam-settings.phpcss/frm_admin.cssjs/formidable_admin.jsjs/src/admin/admin.js
🚧 Files skipped from review as they are similar to previous changes (2)
- js/src/admin/admin.js
- classes/views/frm-forms/spam-settings.php
This update aims to pull all spam settings to a single location in form settings. It includes field-level settings (adding a captcha field), and a readonly display for Global Settings with a link to make the Global spam settings easier to access.
There's a note in the old location to make them easier to locate for users who expect the old location.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Style