-
Notifications
You must be signed in to change notification settings - Fork 842
Forms: handle first/last names in feedback classes #45944
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: changed | ||
|
|
||
| Forms: use first/last name for author. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -244,7 +244,9 @@ private function load_from_post( WP_Post $feedback_post ) { | |
| $this->author_data = new Feedback_Author( | ||
| $this->get_first_field_of_type( 'name', 'pre_comment_author_name' ), | ||
| $this->get_first_field_of_type( 'email', 'pre_comment_author_email' ), | ||
| $this->get_first_field_of_type( 'url', 'pre_comment_author_url' ) | ||
| $this->get_first_field_of_type( 'url', 'pre_comment_author_url' ), | ||
| $this->get_field_value_by_form_field_id( 'first-name' ), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also make sure that the field type is "name" ? |
||
| $this->get_field_value_by_form_field_id( 'last-name' ) | ||
| ); | ||
|
|
||
| $this->comment_content = $this->get_first_field_of_type( 'textarea' ); | ||
|
|
@@ -729,6 +731,24 @@ public function get_author_name() { | |
| return $this->author_data->get_name(); | ||
| } | ||
|
|
||
| /** | ||
| * Get the author's first name of a feedback entry. | ||
| * | ||
| * @return string | ||
| */ | ||
| public function get_author_first_name() { | ||
| return $this->author_data->get_first_name(); | ||
| } | ||
|
|
||
| /** | ||
| * Get the author's last name of a feedback entry. | ||
| * | ||
| * @return string | ||
| */ | ||
| public function get_author_last_name() { | ||
| return $this->author_data->get_last_name(); | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR does not use the two getters above, but I wanted to provide these for future handling, and we'll likely update our integration logic for MailPoet and Hostinger to leverage these methods. |
||
|
|
||
| /** | ||
| * Get the author email of a feedback entry. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,53 @@ | |
| #[CoversClass( Feedback_Author::class )] | ||
| class Feedback_Author_Test extends BaseTestCase { | ||
|
|
||
| /** | ||
| * Minimal: combining first and last name yields full name in name/display. | ||
| */ | ||
| public function test_combined_first_last_in_name_and_display() { | ||
| $author = new Feedback_Author( '', '[email protected]', '', 'John', 'Doe' ); | ||
|
|
||
| $this->assertEquals( 'John Doe', $author->get_name() ); | ||
| $this->assertEquals( 'John Doe', $author->get_display_name() ); | ||
| $this->assertSame( 'John', $author->get_first_name() ); | ||
| $this->assertSame( 'Doe', $author->get_last_name() ); | ||
| } | ||
| /** | ||
| * Minimal: when only one of first/last is present, fall back to single name. | ||
| */ | ||
| public function test_partial_first_or_last_falls_back_to_single_name() { | ||
| $author = new Feedback_Author( 'Single Name', '[email protected]', '', 'Bob', '' ); | ||
| $this->assertEquals( 'Single Name', $author->get_name() ); | ||
| $this->assertEquals( 'Single Name', $author->get_display_name() ); | ||
| } | ||
|
|
||
| /** | ||
| * When both first/last are present and differ from single name, combined name takes precedence. | ||
| */ | ||
| public function test_first_last_override_single_name_when_both_present() { | ||
| $author = new Feedback_Author( 'Some Other Name', '[email protected]', '', 'Alice', 'Jones' ); | ||
| $this->assertEquals( 'Alice Jones', $author->get_name() ); | ||
| $this->assertEquals( 'Alice Jones', $author->get_display_name() ); | ||
| } | ||
|
|
||
| /** | ||
| * When only last name is provided and single name exists, fall back to single name. | ||
| */ | ||
| public function test_only_lastname_with_single_name_falls_back_to_single() { | ||
| $author = new Feedback_Author( 'Single Name', '[email protected]', '', '', 'Jones' ); | ||
| $this->assertEquals( 'Single Name', $author->get_name() ); | ||
| $this->assertEquals( 'Single Name', $author->get_display_name() ); | ||
| } | ||
|
|
||
| /** | ||
| * When only last name is provided and single name is missing, fall back to email in display. | ||
| */ | ||
| public function test_only_lastname_without_single_name_falls_back_to_email() { | ||
| $author = new Feedback_Author( '', '[email protected]', '', '', 'Jones' ); | ||
| $this->assertSame( '', $author->get_name() ); | ||
| $this->assertEquals( '[email protected]', $author->get_display_name() ); | ||
| } | ||
|
|
||
| /** | ||
| * Test constructor with all parameters. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3140,4 +3140,33 @@ public function test_get_country_flag() { | |
|
|
||
| remove_filter( 'jetpack_get_country_from_ip', $filter_callback, 10 ); | ||
| } | ||
|
|
||
| /** | ||
| * Minimal: submission with first-name/last-name sets author name and first/last getters. | ||
| */ | ||
| public function test_author_first_last_on_submission() { | ||
| $form = new Contact_Form( | ||
| array( | ||
| 'title' => 'Test Form', | ||
| 'description' => 'This is a test form.', | ||
| ), | ||
| " | ||
| [contact-field label='First name' type='name' id='first-name'/] | ||
| [contact-field label='Last name' type='name' id='last-name'/] | ||
| [contact-field label='Email' type='email' id='email'/] | ||
| " | ||
| ); | ||
|
|
||
| $post_data = array( | ||
| 'first-name' => 'Jane', | ||
| 'last-name' => 'Doe', | ||
| 'email' => '[email protected]', | ||
| ); | ||
|
|
||
| $response = Feedback::from_submission( $post_data, $form ); | ||
|
|
||
| $this->assertEquals( 'Jane Doe', $response->get_author_name(), 'Author name should combine first and last' ); | ||
| $this->assertSame( 'Jane', $response->get_author_first_name(), 'First name getter should return raw first name' ); | ||
| $this->assertSame( 'Doe', $response->get_author_last_name(), 'Last name getter should return raw last name' ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: enhancement | ||
|
|
||
| Forms: use first/last name for author. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback_Author->get_name() will now return 'First Last' if available, or fallback to 'Single Name' as before.
I added the filter here to ensure equivalence. We filter the single 'name' field in both places before we instantiate the Feedback_Author class class.