Skip to content
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

Forms: Add unique IDs when multiple instances. #40998

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 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/update-forms-unique-id
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Forms: Add unique ids to each form
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ public function __construct( $attributes, $content = null ) {
$default_to .= $post_author->user_email;
}

if ( ! empty( self::$forms ) ) {
// Ensure 'id' exists in $attributes before trying to modify it
if ( ! isset( $attributes['id'] ) ) {
$attributes['id'] = '';
}
$attributes['id'] = $attributes['id'] . '-' . ( count( self::$forms ) + 1 );
Copy link
Member

Choose a reason for hiding this comment

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

Fine as is, but noting that wp_unique_id() can be handy here.

}

$this->hash = sha1( wp_json_encode( $attributes ) );
self::$forms[ $this->hash ] = $this;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
/**
* Unit Tests for Automattic\Jetpack\Forms\Contact_Form.
*
* To run the test visit the packages/forms directory and run composer test-php
*
* @package automattic/jetpack-forms
*/

Expand All @@ -21,6 +23,8 @@

private $post;

private $track_feedback_inserted;

private $plugin;

/**
Expand Down Expand Up @@ -98,16 +102,28 @@
remove_all_filters( 'wp_mail' );
remove_all_filters( 'grunion_still_email_spam' );
remove_all_filters( 'jetpack_contact_form_is_spam' );

// Reset the forms array
Contact_Form::$forms = array();
Contact_Form::$last = null;

Check failure on line 108 in projects/packages/forms/tests/php/contact-form/test-class.contact-form.php

View workflow job for this annotation

GitHub Actions / Static analysis

TypeError PhanTypeMismatchPropertyProbablyReal Assigning null of type null to property but \Automattic\Jetpack\Forms\ContactForm\Contact_Form::$last is \Automattic\Jetpack\Forms\ContactForm\Contact_Form (no real type) (the inferred real assigned type has nothing in common with the declared phpdoc property type)
Contact_Form::$current_form = null;

Check failure on line 109 in projects/packages/forms/tests/php/contact-form/test-class.contact-form.php

View workflow job for this annotation

GitHub Actions / Static analysis

TypeError PhanTypeMismatchPropertyProbablyReal Assigning null of type null to property but \Automattic\Jetpack\Forms\ContactForm\Contact_Form::$current_form is \Automattic\Jetpack\Forms\ContactForm\Contact_Form (no real type) (the inferred real assigned type has nothing in common with the declared phpdoc property type)
}

/**
* Adds the field values to the global $_POST value.
*
* @param array $values Array of field key value pairs.
* @param array $values Array of form fields and values.
* @param string $form_id Optional form ID. If not provided, will use $this->post->ID.
*/
private function add_field_values( $values ) {
private function add_field_values( $values, $form_id = null ) {
$prefix = $form_id ? $form_id : 'g' . $this->post->ID;
$_POST = array();
foreach ( $values as $key => $val ) {
$_POST[ 'g' . $this->post->ID . '-' . $key ] = $val;
if ( strpos( $key, 'contact-form' ) === 0 || strpos( $key, 'action' ) === 0 ) {
$_POST[ $key ] = $val;
} else {
$_POST[ $prefix . '-' . $key ] = $val;
}
}
}

Expand Down Expand Up @@ -2067,4 +2083,58 @@
Util::grunion_contact_form_apply_block_attribute( $original, array( 'foo' => 'bar' ) )
);
}
/**
* Helper function that tracks the ids of the feedbacks that got created.
*/
public function track_feedback_inserted( $post_id ) {
$this->track_feedback_inserted[] = $post_id;
}
/**
* Tests that multiple instances of the same form work correctly with unique IDs.
*/
public function test_multiple_form_instances_with_unique_ids() {
global $post;

$this->track_feedback_inserted = array();
add_action( 'grunion_after_feedback_post_inserted', array( $this, 'track_feedback_inserted' ), 10, 1 );

$this->add_field_values(
array(
'name' => 'First form name 1',
'message' => 'First form message 1',
),
'g' . $post->ID
);

$form1 = new Contact_Form( array(), "[contact-field label='Name' type='name' required='1'/][contact-field label='Message' type='textarea' required='1'/]" );
// Submit first form
$result1 = $form1->process_submission();

$this->assertTrue( is_string( $result1 ), 'First form submission should be successful' );

$this->add_field_values(
array(
'name' => 'First form name 2',
'message' => 'First form message 2',
),
'g' . $post->ID . '-2'
);

$form2 = new Contact_Form( array(), "[contact-field label='Name' type='name' required='1'/][contact-field label='Message' type='textarea' required='1'/]" );
$result2 = $form2->process_submission();

$this->assertTrue( is_string( $result2 ), 'First form submission should be successful' );

// Verify that the forms have different IDs
$this->assertNotEquals( $form1->get_attribute( 'id' ), $form2->get_attribute( 'id' ), 'Forms should have unique IDs' );

remove_action( 'grunion_after_feedback_post_inserted', array( $this, 'track_feedback_inserted' ), 10, 1 );

Check failure on line 2131 in projects/packages/forms/tests/php/contact-form/test-class.contact-form.php

View workflow job for this annotation

GitHub Actions / Static analysis

ParamError PhanParamTooMany Call with 4 arg(s) to \remove_action(string $hook_name, $callback, int $priority = 10) which only takes 3 arg(s) defined at /home/runner/work/jetpack/jetpack/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php:128840

$this->assertTrue( count( $this->track_feedback_inserted ) === 2 );

Check failure on line 2133 in projects/packages/forms/tests/php/contact-form/test-class.contact-form.php

View workflow job for this annotation

GitHub Actions / Static analysis

TypeError PhanSuspiciousValueComparison Suspicious attempt to compare count($this->track_feedback_inserted) of type 0 to 2 of type 2 with operator '==='

foreach ( $this->track_feedback_inserted as $feedback_id ) {

Check failure on line 2135 in projects/packages/forms/tests/php/contact-form/test-class.contact-form.php

View workflow job for this annotation

GitHub Actions / Static analysis

NOOPError PhanEmptyForeach Saw a foreach statement with empty iterable type array{}
$feedback = get_post( $feedback_id );
$this->assertStringContainsString( 'First form name', $feedback->post_content );
}
}
} // end class
5 changes: 5 additions & 0 deletions projects/plugins/jetpack/changelog/update-forms-unique-id
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: bugfix
Comment: Forms: Add unique ids to each form


Loading