Skip to content

Forms: Include HTML id in Feedback classes #44760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/packages/forms/changelog/add-forms-feedback-field-id
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Forms: Preserve html ids when processing feedback.
58 changes: 41 additions & 17 deletions projects/packages/forms/src/contact-form/class-feedback-field.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,32 @@ class Feedback_Field {
*/
private $meta;

/**
* The original form field ID from the form schema.
*
* @since $$next-version$$
*
* @var string
*/
protected $form_field_id = '';

/**
* Constructor.
*
* @param string $key The key of the field.
* @param mixed $label The label of the field. Non-string values will be converted to empty string.
* @param mixed $value The value of the field.
* @param string $type The type of the field (default is 'basic').
* @param array $meta Additional metadata for the field (default is an empty array).
* @param string $key The key of the field.
* @param mixed $label The label of the field. Non-string values will be converted to empty string.
* @param mixed $value The value of the field.
* @param string $type The type of the field (default is 'basic').
* @param array $meta Additional metadata for the field (default is an empty array).
* @param string|null $form_field_id The original form field ID (default is null).
*/
public function __construct( $key, $label, $value, $type = 'basic', $meta = array() ) {
$this->key = $key;
$this->label = is_string( $label ) ? html_entity_decode( $label, ENT_QUOTES | ENT_HTML5, 'UTF-8' ) : '';
$this->value = $value;
$this->type = $type;
$this->meta = $meta;
public function __construct( $key, $label, $value, $type = 'basic', $meta = array(), $form_field_id = null ) {
$this->key = $key;
$this->label = is_string( $label ) ? html_entity_decode( $label, ENT_QUOTES | ENT_HTML5, 'UTF-8' ) : '';
$this->value = $value;
$this->type = $type;
$this->meta = $meta;
$this->form_field_id = is_string( $form_field_id ) ? $form_field_id : '';
}

/**
Expand Down Expand Up @@ -107,6 +118,17 @@ public function get_value() {
return $this->value;
}

/**
* Get the original form field ID.
*
* @since $$next-version$$
*
* @return string
*/
public function get_form_field_id() {
return $this->form_field_id;
}

/**
* Get the value of the field for rendering.
*
Expand Down Expand Up @@ -242,11 +264,12 @@ public function get_meta_key_value( $meta_key ) {
*/
public function serialize() {
return array(
'key' => $this->get_key(),
'label' => $this->get_label(),
'value' => $this->get_value(),
'type' => $this->get_type(),
'meta' => $this->get_meta(),
'key' => $this->get_key(),
'label' => $this->get_label(),
'value' => $this->get_value(),
'type' => $this->get_type(),
'meta' => $this->get_meta(),
'form_field_id' => $this->get_form_field_id(),
);
}
/**
Expand All @@ -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'] ?? ''
);
}

Expand Down
40 changes: 39 additions & 1 deletion projects/packages/forms/src/contact-form/class-feedback.php
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,8 @@ private function get_computed_fields( $post_data, $form ) {
$label = wp_strip_all_tags( $field->get_attribute( 'label' ) );
$key = $i . '_' . $label;

$fields[ $key ] = new Feedback_Field( $key, $label, $value, $type );
$meta = array();
$fields[ $key ] = new Feedback_Field( $key, $label, $value, $type, $meta, $field_id );
if ( ! $this->has_file && $fields[ $key ]->has_file() ) {
$this->has_file = true;
}
Expand Down Expand Up @@ -1221,4 +1222,41 @@ private function get_computed_consent( $post_data, $form ) {

return false;
}

/**
* Get a field by its original form ID.
*
* @since $$next-version$$
*
* @param string $id Original form field ID.
* @return Feedback_Field|null
*/
public function get_field_by_form_field_id( $id ) {
if ( ! is_string( $id ) || $id === '' ) {
return null;
}
foreach ( $this->fields as $field ) {
if ( $field->get_form_field_id() === $id ) {
return $field;
}
}
return null;
}

/**
* Get a field render value by its original form ID.
*
* @since $$next-version$$
*
* @param string $id Original form field ID.
* @param string $context Render context.
* @return string
*/
public function get_field_value_by_form_field_id( $id, $context = 'default' ) {
$field = $this->get_field_by_form_field_id( $id );
if ( ! $field ) {
return '';
}
return (string) $field->get_render_value( $context );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,22 @@ public function test_Feedback_Field_can_be_instantiated() {
$this->assertEquals( 'test_value', $field->get_value() );
$this->assertEquals( 'basic', $field->get_type() );
$this->assertEquals( array(), $field->get_meta() );
$this->assertSame( '', $field->get_form_field_id() );
}

/**
* Test that the Feedback_Field class can be instantiated with additional parameters.
*/
public function test_Feedback_Field_with_additional_parameters() {
$field = new Feedback_Field( 'test_key', 'test_label', 'test_value', 'text', array( 'meta_key' => 'meta_value' ) );
$field = new Feedback_Field( 'test_key', 'test_label', 'test_value', 'text', array( 'meta_key' => 'meta_value' ), 'firstname' );
$this->assertEquals( 'test_key', $field->get_key() );
$this->assertEquals( 'test_label', $field->get_label() );
$this->assertEquals( 'test_value', $field->get_value() );
$this->assertEquals( 'text', $field->get_type() );
$this->assertEquals( array( 'meta_key' => 'meta_value' ), $field->get_meta() );
$this->assertEquals( 'meta_value', $field->get_meta_key_value( 'meta_key' ) );
$this->assertNull( $field->get_meta_key_value( 'non_existant' ) );
$this->assertEquals( 'firstname', $field->get_form_field_id() );
}

/**
Expand All @@ -59,6 +61,7 @@ public function test_Feedback_Field_with_empty_values() {
$this->assertSame( '', $field->get_value() );
$this->assertEquals( 'basic', $field->get_type() );
$this->assertEquals( array(), $field->get_meta() );
$this->assertSame( '', $field->get_form_field_id() );
}

/**
Expand Down
47 changes: 47 additions & 0 deletions projects/packages/forms/tests/php/contact-form/Feedback_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1983,4 +1983,51 @@ public function test_validate_radio_form() {

Contact_Form::reset_errors();
}

public function test_get_field_by_id_and_value_by_id_new_submission() {
$form_id = Utility::get_form_id();
$_post_data = Utility::get_post_request(
array(
'name' => 'John Doe',
'email' => '[email protected]',
'message' => 'Hello!',
),
'g' . $form_id
);

$form = new Contact_Form(
array(
'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'/]"
Copy link
Member

Choose a reason for hiding this comment

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

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' ) );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

);

$response = Feedback::from_submission( $_post_data, $form );
$field_ids = $form->get_field_ids();
$email_id = $field_ids['email'];

$this->assertNotEmpty( $email_id );
$this->assertEquals( '[email protected]', $response->get_field_value_by_form_field_id( $email_id ) );

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

// Save and reload; ensure the field id and value persist correctly
$saved_post_id = $response->save();
$saved_response = Feedback::get( $saved_post_id );
$this->assertEquals( '[email protected]', $saved_response->get_field_value_by_form_field_id( $email_id ) );
$saved_field = $saved_response->get_field_by_form_field_id( $email_id );
$this->assertInstanceOf( Feedback_Field::class, $saved_field );
$this->assertEquals( $email_id, $saved_field->get_form_field_id() );
}

public function test_get_field_by_id_and_value_by_id_legacy() {
$post_id = Utility::create_legacy_feedback( array() );
$response = Feedback::get( $post_id );

$this->assertSame( '', $response->get_field_value_by_form_field_id( 'email' ) );
$this->assertNull( $response->get_field_by_form_field_id( 'email' ) );
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: enhancement

Forms: Preserve html ids when processing feedback.
Loading