Fix download MCP blocked when toggle was off by Crabcyborg · Pull Request #3569 · Strategy11/formidable-forms

Crabcyborg · 2026-10-13T16:18:13Z

This fixes a bug I noticed, where the download button would fail if the toggle wasn't saved yet.

coderabbitai · 2026-10-13T16:18:23Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: Strategy11/formidable-forms/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 6e5d0674-b315-4bbb-a80b-5ee58ae4eee8

📥 Commits

Reviewing files that changed from the base of the PR and between 1c85367 and 2115081.

📒 Files selected for processing (2)

classes/controllers/FrmMcpSkillEnvController.php
tests/phpunit/abilities/test_FrmMcpSkillEnvController.php

 ___________________________________________________________
< Your API returns 200 OK; your users return 404 Not Found. >
 -----------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

deepsource-io · 2026-10-13T16:20:08Z

DeepSource Code Review

We reviewed changes in 1c85367...2115081 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 ↗

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
PHP		Oct 13, 2026 4:18p.m.	Review ↗
JavaScript		Oct 13, 2026 4:18p.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.

deepsource-io · 2026-10-13T16:20:37Z

+		$settings->mcp = $initial;
+		$settings->store();
+		FrmMcpController::reset();
+		$this->assertSame( (bool) $initial, FrmMcpController::is_enabled(), 'The initial MCP setting should be cached.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+
+		try {
+			FrmMcpSkillEnvController::download();
+			$this->fail( 'The download should stop before creating a credential.' );


Call to an undefined method test_FrmMcpSkillEnvController::fail()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+		try {
+			FrmMcpSkillEnvController::download();
+			$this->fail( 'The download should stop before creating a credential.' );
+		} catch ( WPDieException $exception ) {


Caught class WPDieException not found

The Exception class you are trying to use seems to be invalid. This will result in a run time fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			FrmMcpSkillEnvController::download();
+			$this->fail( 'The download should stop before creating a credential.' );
+		} catch ( WPDieException $exception ) {
+			$this->assertSame( 403, $exception->getCode(), 'The blocked download should return a forbidden response.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			FrmMcpSkillEnvController::download();
+			$this->fail( 'The download should stop before creating a credential.' );
+		} catch ( WPDieException $exception ) {
+			$this->assertSame( 403, $exception->getCode(), 'The blocked download should return a forbidden response.' );


Call to method getCode() on an unknown class WPDieException

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			$this->fail( 'The download should stop before creating a credential.' );
+		} catch ( WPDieException $exception ) {
+			$this->assertSame( 403, $exception->getCode(), 'The blocked download should return a forbidden response.' );
+			$this->assertSame( $expected, $settings->mcp, 'The submitted toggle should be saved only after authorization.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+		} catch ( WPDieException $exception ) {
+			$this->assertSame( 403, $exception->getCode(), 'The blocked download should return a forbidden response.' );
+			$this->assertSame( $expected, $settings->mcp, 'The submitted toggle should be saved only after authorization.' );
+			$this->assertSame( $expected, get_option( 'frm_options' )->mcp, 'The MCP setting should persist for the next request.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			$this->assertSame( 403, $exception->getCode(), 'The blocked download should return a forbidden response.' );
+			$this->assertSame( $expected, $settings->mcp, 'The submitted toggle should be saved only after authorization.' );
+			$this->assertSame( $expected, get_option( 'frm_options' )->mcp, 'The MCP setting should persist for the next request.' );
+			$this->assertSame( (bool) $expected, FrmMcpController::is_enabled(), 'The cached MCP setting should reflect the saved value.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			$this->assertSame( $expected, $settings->mcp, 'The submitted toggle should be saved only after authorization.' );
+			$this->assertSame( $expected, get_option( 'frm_options' )->mcp, 'The MCP setting should persist for the next request.' );
+			$this->assertSame( (bool) $expected, FrmMcpController::is_enabled(), 'The cached MCP setting should reflect the saved value.' );
+			$this->assertSame( $original_currency, $settings->currency, 'Other settings should remain unchanged.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

deepsource-io · 2026-10-13T16:20:37Z

+			$this->assertSame( $expected, get_option( 'frm_options' )->mcp, 'The MCP setting should persist for the next request.' );
+			$this->assertSame( (bool) $expected, FrmMcpController::is_enabled(), 'The cached MCP setting should reflect the saved value.' );
+			$this->assertSame( $original_currency, $settings->currency, 'Other settings should remain unchanged.' );
+			$this->assertSame( array(), FrmMcpSkillEnvController::get_passwords( $user_id ), 'A blocked download should not create a credential.' );


Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

The method you are trying to call is not defined, which can result in a fatal error.

franky-the-going-merry

Approve. The download handler now saves the submitted MCP toggle (only after the permission and nonce checks), resets the memoized is_enabled(), and loads the adapter, so an unsaved "on" toggle no longer gets a 403 from the Download button.

Live check: ran the PR's new test_download_saves_submitted_mcp_setting (6 scenarios: unsaved on, unchecked, nothing posted, no marker, bad nonce, subscriber) under PHPUnit. It passes with the PR (4 tests, 62 assertions) and fails with the base FrmMcpSkillEnvController.php swapped back in. The failure text came out garbled by a harness quirk, so I can't say which scenario failed.
Not exercised: the real Download click in a browser. The Playground env isn't treated as local, so Application Passwords are unavailable and the button doesn't render. The success path (credential created, file streamed) isn't covered either, since the tests stop at the 403 before the file response.
Not run: security-review/simplify (two-file diff, read in full instead), PHPCS/PHPStan locally.

Non-blocking:

A failed download still keeps the toggle change. If Application Passwords are off or the site isn't HTTPS, the user sees the 403 page and MCP is now saved as on, without ever pressing Save. The test asserts this, so it looks intended, but it's worth a deliberate yes.
FrmSettings::update_settings() (L661) has the same "marker present, then empty( frm_mcp ) ? 0 : 1" rule. This copies it into a second place, so the two can drift. Consider a small shared method on FrmSettings.

Fix downlaod MCP blocked when toggle was off

2115081

Crabcyborg added priority: high franky-review labels Oct 13, 2026

Crabcyborg added this to the 7.0 milestone Oct 13, 2026

franky-the-going-merry Bot added the franky-working Franky is actively reviewing this label Oct 13, 2026

deepsource-io Bot reviewed Oct 13, 2026

View reviewed changes

franky-the-going-merry Bot approved these changes Oct 13, 2026

View reviewed changes

franky-the-going-merry Bot removed franky-review franky-working Franky is actively reviewing this labels Oct 13, 2026

Crabcyborg marked this pull request as ready for review October 13, 2026 17:48

Crabcyborg changed the title ~~Fix downlaod MCP blocked when toggle was off~~ Fix download MCP blocked when toggle was off Oct 13, 2026

Crabcyborg merged commit ad6cf22 into master Oct 13, 2026
57 of 59 checks passed

Crabcyborg deleted the fix_download_mcp_file_when_toggle_was_off_bug branch October 13, 2026 17:48

Conversation

Crabcyborg commented Oct 13, 2026

Uh oh!

coderabbitai Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepsource-io Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::fail()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Caught class WPDieException not found

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to method getCode() on an unknown class WPDieException

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

deepsource-io Bot Oct 13, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmMcpSkillEnvController::assertSame()

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 13, 2026 •

edited

Loading

deepsource-io Bot commented Oct 13, 2026 •

edited

Loading