Forms: Include HTML id in Feedback classes #44760

edanzer · 2025-08-12T18:13:21Z

Proposed changes:

Note: The diff for this is a bit easier to read if you check the remove whitespace option.

What's is added? This PR adds logic to the Feedback and Feedback_Field classes to preserve and handle the html id for each field.

The Feedback Class
- $field_id is passed in when instantiating Feedback_Field, when relevant.
- There are two new methods: get_field_by_id() and get_field_value_by_id()
The Feedback_Field Class
- Now has a $form_field_id property, which is set in the constructor.
- Has has a simple getter, get_form_field_id() to fetch the id.
Tests are added or updated for both classes.

Why? The new methods are not used in production code yet, but would be used by the MailPoet integration class. We want to be get first and last name when adding subscribers to MailPoet lists, and one of several methods we want to give users is the ability to set those filed by using ids like 'first_name' and 'last_name'. These changes allow us to do something like get_field_value_by_id( 'first-name' ) and confirm a non-null value is returned.

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

None.

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Review new properties and methods and confirm they make sense.
Review code to look for any possible regressions or backwards compat issues.
Run test and confirm they all pass. Naviage to to projects/packages/forms and run composer test-php.

The new property and methods are not used yet, so there's no way to test them in production use.

github-actions · 2025-08-12T18:14:09Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the add/forms-feedback-field-id branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack add/forms-feedback-field-id

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2025-08-12T18:14:20Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Review, ...).
✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

WordPress.com Simple releases happen as soon as you deploy your changes after merging this PR (PCYsg-Jjm-p2).
WoA releases happen weekly.
Releases to self-hosted sites happen monthly:
- Scheduled release: September 2, 2025
- Code freeze: September 1, 2025

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

jp-launch-control · 2025-08-12T18:22:37Z

Code Coverage Summary

Coverage changed in 2 files.

File	Coverage	Δ%	Δ Uncovered
projects/packages/forms/src/contact-form/class-feedback.php	437/446 (97.98%)	-0.17%	1 ❤️‍🩹
projects/packages/forms/src/contact-form/class-feedback-field.php	77/80 (96.25%)	0.20%	0 💚

Full summary · PHP report · JS report

enejb · 2025-08-13T16:45:53Z

projects/packages/forms/tests/php/contact-form/Feedback_Test.php

+				'title'       => 'Test Form',
+				'description' => 'This is a test form.',
+			),
+			"[contact-field label='Name' type='name' required='1'/][contact-field label='Email' type='email' required='1'/][contact-field label='Message' type='textarea' required='1'/]"


It would be good to check - [contact-field label='Name' type='name' required='1' id="foo" /]
And

that

$this->assertEquals( 'John Doe', $response->get_field_value_by_id( 'foo' ) );

This is apparently not that simple. As written above, this will not pass. Our testing code (if not our production code) seems to be set up in a way where id=foo will be converted to something like gXXX-ID-foo. And then there's some kind of disconnect in the data. I'm having a hard time figuring out how to fix/resolve it in a non-artificial way.

enejb · 2025-08-13T16:48:06Z

projects/packages/forms/tests/php/contact-form/Feedback_Test.php

+
+		$field = $response->get_field_by_id( $email_id );
+		$this->assertInstanceOf( Feedback_Field::class, $field );
+		$this->assertEquals( $email_id, $field->get_form_field_id() );


It would also be good to check that saving worked as expected and that the field_id would also be there after we save the response.

So

$saved_id = $response->save(); $saved_response = Feedback::get( $saved_id ); $this->assertEquals( '[email protected]', $saved_response->get_field_value_by_id( $email_id ) );

enejb · 2025-08-13T16:50:53Z

projects/packages/forms/src/contact-form/class-feedback-field.php

+		$this->value         = $value;
+		$this->type          = $type;
+		$this->meta          = $meta;
+		$this->form_field_id = is_string( $form_field_id ) && $form_field_id !== '' ? $form_field_id : null;


Do we need to be able to tell is form_field_id is null or '' ?
I think it would simplify things to always return a string?

Agree, let's try not to polymorph the variable types. I'm fine with empty string and keep it a string all the way.

projects/packages/forms/src/contact-form/class-feedback.php

CGastrell · 2025-08-19T19:25:20Z

projects/packages/forms/src/contact-form/class-feedback-field.php

@@ -266,7 +289,8 @@ public static function from_serialized( $data ) {
 			$data['label'],
 			$data['value'],
 			$data['type'] ?? 'basic',
-			$data['meta'] ?? array()
+			$data['meta'] ?? array(),
+			$data['form_field_id'] ?? null


Given the comment above, this should default to '', if we decide to go with it.

CGastrell · 2025-08-19T19:26:50Z

projects/packages/forms/src/contact-form/class-feedback.php

+		}
+		foreach ( $this->fields as $field ) {
+			$form_field_id = $field->get_form_field_id();
+			if ( is_string( $form_field_id ) && $form_field_id !== '' && $form_field_id === $id ) {


Again, given the null vs string discussion above, this comparison should be simpler, you wouldn't need to check if is_string, you'd only check for equality :)

In fact, if $this->fields is always an array, you could even just return array_find( $this->fields, function( $a ) { $a->get_form_field_id() === $id ); }

edanzer self-assigned this Aug 12, 2025

edanzer added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Pri] Normal [Package] Forms labels Aug 12, 2025

github-actions bot added [Feature] Contact Form [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] In Progress labels Aug 12, 2025

github-actions bot added the [Tests] Includes Tests label Aug 13, 2025

edanzer force-pushed the add/forms-feedback-field-id branch from ff892d6 to 28920e2 Compare August 13, 2025 05:33

edanzer requested a review from enejb August 13, 2025 05:33

edanzer marked this pull request as ready for review August 13, 2025 05:33

enejb reviewed Aug 13, 2025

View reviewed changes

projects/packages/forms/src/contact-form/class-feedback.php Outdated Show resolved Hide resolved

edanzer added 7 commits August 15, 2025 10:40

Forms: Add id to Feedback classes

8bd08d0

changelog

b05e2de

Make form_field_id a main property of Feedback_Field

00b055e

Remove unneeded paramters

302f259

Simplify new Feedback methods

57c42db

Update tests

e13ebf9

Add Feedback tests

f9229c3

edanzer force-pushed the add/forms-feedback-field-id branch from 28920e2 to f9229c3 Compare August 15, 2025 16:40

edanzer and others added 3 commits August 15, 2025 12:02

Update method names

78b3129

Add saved post test

121de15

Check that field id works as expected.

50616a2

edanzer mentioned this pull request Aug 19, 2025

Forms: Update MailPoet to use Feedback class #44852

Open

3 tasks

CGastrell reviewed Aug 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Forms: Include HTML id in Feedback classes #44760

Forms: Include HTML id in Feedback classes #44760

edanzer commented Aug 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

jp-launch-control bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

enejb Aug 13, 2025

Uh oh!

edanzer Aug 15, 2025

Uh oh!

enejb Aug 13, 2025

Uh oh!

edanzer Aug 15, 2025

Uh oh!

enejb Aug 15, 2025

Uh oh!

enejb Aug 13, 2025

Uh oh!

CGastrell Aug 19, 2025

Uh oh!

Uh oh!

CGastrell Aug 19, 2025

Uh oh!

CGastrell Aug 19, 2025

Uh oh!

CGastrell Aug 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Forms: Include HTML id in Feedback classes #44760

Are you sure you want to change the base?

Forms: Include HTML id in Feedback classes #44760

Conversation

edanzer commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp-launch-control bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CGastrell Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edanzer commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading

jp-launch-control bot commented Aug 12, 2025 •

edited

Loading

CGastrell Aug 19, 2025 •

edited

Loading