Keep the Entry Key typed on an earlier page of a multi-page Add New by vivi-the-going-merry[bot] · Pull Request #3609 · Strategy11/formidable-forms

vivi-the-going-merry · 2026-10-23T01:11:12Z

Fixes Strategy11/formidable-pro#6891

Broken: On Entries → Add New for a multi-page form, an Entry Key typed on page 1 is lost on Next. Page 2 shows an empty box and the entry saves with a random key. FrmEntriesHelper::setup_new_vars() always set item_key to ''.

Changed: setup_new_vars() now reads the posted item_key (same sanitize_title the edit path uses). A reset form ($reset = true, shown again after a submit) still starts empty.

Verified: New test_setup_new_vars_keeps_posted_item_key. Red on the test-only commit (the only PHPUnit failure, 1 of 950). Green on the fix commit on PHP 7.4, 8.0, 8.2 and 8.4. Not driven in a browser here; the issue reporter tested the same one-line change on the Add New screen. Psalm red is the existing MissingPureAnnotation set; none of it is in the touched lines.

…ro#6891)

coderabbitai · 2026-10-23T01:11:21Z

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Source mode: Auto
Repository: Strategy11/formidable-forms
Branch: master
File: .coderabbit.yaml
Commit: a0b9742
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 147ed5f9-79b7-45e7-820b-1327892154e0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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-23T01:12:05Z

DeepSource Code Review

We reviewed changes in a0b9742...067ac4c 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 23, 2026 4:16a.m.	Review ↗
JavaScript		Oct 23, 2026 4:16a.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-23T01:12:07Z

@@ -22,4 +22,31 @@ public function test_display_value() {
 		$this->assertIsString( $display_value );


Call to an undefined method test_FrmEntriesHelper::assertIsString()

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

deepsource-io · 2026-10-23T01:12:07Z

@@ -22,4 +22,31 @@ public function test_display_value() {
 		$this->assertIsString( $display_value );
 		$this->assertSame( 'Option 1, Option 2', $display_value );


Call to an undefined method test_FrmEntriesHelper::assertSame()

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

deepsource-io · 2026-10-23T01:12:07Z

+		$_POST['item_key'] = 'My Typed Key';
+
+		$values = FrmEntriesHelper::setup_new_vars( $fields );
+		$this->assertSame( 'my-typed-key', $values['item_key'] );


Call to an undefined method test_FrmEntriesHelper::assertSame()

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

deepsource-io · 2026-10-23T01:12:07Z

+		$this->assertSame( 'my-typed-key', $values['item_key'] );
+
+		$values = FrmEntriesHelper::setup_new_vars( $fields, '', true );
+		$this->assertSame( '', $values['item_key'], 'A reset form must not carry the posted key.' );


Call to an undefined method test_FrmEntriesHelper::assertSame()

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

deepsource-io · 2026-10-23T01:12:07Z

+		unset( $_POST['item_key'] );
+
+		$values = FrmEntriesHelper::setup_new_vars( $fields );
+		$this->assertSame( '', $values['item_key'] );


Call to an undefined method test_FrmEntriesHelper::assertSame()

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

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

franky-the-going-merry

Approving: the fix is correct and I reproduced the bug before and after on a live Formidable Pro admin. One non-blocking hardening note inline (array input).

Live repro (Lite master vs this PR, Pro active, Entries > Add New on a 3-field form text / page break / text): posted page 1 with item_key=My Typed Key and clicked Next.

Base: page 2 renders <input type="text" name="item_key" value="" />. The typed key is lost.
This PR: page 2 renders value="my-typed-key".
Direct FrmEntriesHelper::setup_new_vars() on both: with $_POST['item_key'] set, base returns '' and the PR returns my-typed-key. $reset = true returns '', and with no POST it returns ''.

Not verified live: I did not run the PHPUnit test myself (CI PHPUnit is green on 7.4, 8.0, 8.2 and 8.4, and the PR description says red on the test-only commit). I also did not confirm the final save on page 2 stores the typed key. My scripted final POST created no entry, so that result is inconclusive. Psalm and DeepSource PHP are red; I did not check them against the touched lines.

Considered and dismissed: the frm_setup_new_entry filter and the later array_merge with $form->options overwriting the key. The live run with Pro active shows the key survives both. The front-end hidden item_key input was already '', so reading the posted value back does not widen what a visitor can already post.

franky-the-going-merry · 2026-10-23T01:38:44Z

 			'name'        => '',
 			'description' => '',
-			'item_key'    => '',
+			'item_key'    => $reset ? '' : FrmAppHelper::get_post_param( 'item_key', '', 'sanitize_title' ),


Non-blocking: get_post_param() returns an array when item_key is posted as an array (sanitize_value() sanitizes each element and keeps the array). Posting item_key[]=x makes $values['item_key'] an array, and classes/views/frm-entries/form.php lines 87 and 92 then call esc_attr( $values['item_key'] ) on it, which raises an "Array to string conversion" warning. Before this PR the value was always the string ''. I traced this in source and did not run it. Any front-end visitor can post that field. A small guard keeps the value a string. Replace the $values array start with:

$posted_key = FrmAppHelper::get_post_param( 'item_key', '', 'sanitize_title' ); $values = array( 'name' => '', 'description' => '', 'item_key' => ( ! $reset && is_string( $posted_key ) ) ? $posted_key : '', );

If you take this, add $_POST['item_key'] = array( 'x' ); expecting '' to test_setup_new_vars_keeps_posted_item_key.

Done in 609ba61. The posted value is only used when it is a string, and the test posts an array and expects an empty key.

franky-the-going-merry · 2026-10-23T01:38:44Z

+
+		$values = FrmEntriesHelper::setup_new_vars( $fields, '', true );
+		$this->assertSame( '', $values['item_key'], 'A reset form must not carry the posted key.' );
+


Non-blocking: unset( $_POST['item_key'] ) only runs if both earlier assertSame calls pass. A failing assertion leaves $_POST['item_key'] set for later tests in the same process. Move the cleanup into tear_down() (unset( $_POST['item_key'] );) or wrap the assertions in try/finally.

Done in 609ba61. The cleanup moved to tearDown().

…rDown Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

deepsource-io · 2026-10-23T04:13:34Z


+	public function tearDown(): void {
+		unset( $_POST['item_key'] );
+		parent::tearDown();


Call to an undefined static method FrmUnitTest::tearDown()

Invalid call to a static method. This would lead to a run time error.

deepsource-io · 2026-10-23T04:13:34Z

+
+		$_POST['item_key'] = array( 'x' );
+		$values            = FrmEntriesHelper::setup_new_vars( $fields );
+		$this->assertSame( '', $values['item_key'], 'An array posted as the key must not reach the view.' );


Call to an undefined method test_FrmEntriesHelper::assertSame()

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

…ength limit Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

vivi-the-going-merry · 2026-10-23T04:20:25Z

Method: in-place push
Pushed to: #3609 (branch fix/issue-6891-entry-key-multipage, unchanged PR number)

franky-the-going-merry

Approve at 067ac4c (re-review; my last Approve was at a4c0ead). Both non-blocking notes from that review are fixed.

What changed since a4c0ead

setup_new_vars() reads the posted key once into $posted_key and only uses it when ! $reset && is_string( $posted_key ), so an array posted as item_key falls back to '' and no longer reaches esc_attr(). The ternary groups as (! $reset && is_string(...)) ? $posted_key : '', so reset behaviour is unchanged.
The test gets a tearDown() that unsets $_POST['item_key'], plus an assertion that an array key comes back as ''.
'fields' => array() moved into the initial array (file-length limit).

What I ran

php -l on both changed files: clean.
Diff against master for classes/: still only FrmEntriesHelper::setup_new_vars(). The reset path and the page-2 keep-the-typed-key path from my earlier live before/after (master drops the key, the PR keeps it) are untouched by these commits.
CI: PHPUnit and the other jobs show skipping on this head, so the new assertion was not run by CI. Psalm is red, and it is also red on master. I did not run the new test locally and did not re-run the live repro on this head.

Non-blocking, nothing new to fix.

Add failing test: setup_new_vars keeps posted Entry Key (formidable-p…

edb15d4

…ro#6891)

vivi-the-going-merry Bot added the run tests label Oct 23, 2026

deepsource-io Bot reviewed Oct 23, 2026

View reviewed changes

Keep the posted Entry Key when showing the next page of a new entry

0767dcf

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

vivi-the-going-merry Bot added the run analysis label Oct 23, 2026

vivi-the-going-merry Bot and others added 2 commits October 22, 2026 19:20

Group consecutive assignments in test for PHPCS

066b2d2

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Align test assignments for PHPCS

a4c0ead

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

vivi-the-going-merry Bot marked this pull request as ready for review October 23, 2026 01:30

vivi-the-going-merry Bot added the franky-review label Oct 23, 2026

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

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

View reviewed changes

franky-the-going-merry Bot added vivi-pickup and removed franky-review franky-working Franky is actively reviewing this labels Oct 23, 2026

vivi-the-going-merry Bot added vivi-working Vivi is actively working this and removed vivi-pickup labels Oct 23, 2026

Keep a posted array Entry Key out of the view; clean up $_POST in tea…

609ba61

…rDown Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

deepsource-io Bot reviewed Oct 23, 2026

View reviewed changes

Drop redundant parentheses and keep FrmEntriesHelper under the file l…

067ac4c

…ength limit Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

vivi-the-going-merry Bot added franky-review and removed vivi-working Vivi is actively working this labels Oct 23, 2026

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

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

View reviewed changes

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

franky-the-going-merry Bot requested a review from jonathanenlared October 23, 2026 04:36

		@@ -22,4 +22,31 @@ public function test_display_value() {
		$this->assertIsString( $display_value );

		@@ -22,4 +22,31 @@ public function test_display_value() {
		$this->assertIsString( $display_value );
		$this->assertSame( 'Option 1, Option 2', $display_value );


		$values = FrmEntriesHelper::setup_new_vars( $fields, '', true );
		$this->assertSame( '', $values['item_key'], 'A reset form must not carry the posted key.' );

Conversation

vivi-the-going-merry Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Review skipped

Uh oh!

deepsource-io Bot commented Oct 23, 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 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertIsString()

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertSame()

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertSame()

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertSame()

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertSame()

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined static method FrmUnitTest::tearDown()

Uh oh!

deepsource-io Bot Oct 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmEntriesHelper::assertSame()

Uh oh!

vivi-the-going-merry Bot commented Oct 23, 2026

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

vivi-the-going-merry Bot commented Oct 23, 2026 •

edited

Loading

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

deepsource-io Bot commented Oct 23, 2026 •

edited

Loading