-
Notifications
You must be signed in to change notification settings - Fork 58
feat: require reader to set name when commenting #4647
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
079c35e
feat(reader-activation): add display name field to comment form
miguelpeixe 288121d
feat(reader-activation): validate display name on comment submission
miguelpeixe da26e3e
feat(reader-activation): save display name on comment submission
miguelpeixe 6761fea
fix: ensure the display name is used for the current comment
miguelpeixe 913aaf3
chore: update input label
miguelpeixe efdd6eb
refactor: move init calls to the class files
miguelpeixe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
121 changes: 121 additions & 0 deletions
121
includes/reader-activation/class-comment-display-name.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| <?php | ||
| /** | ||
| * Comment Display Name — requires readers with auto-generated display names | ||
| * to choose a proper display name before commenting. | ||
| * | ||
| * @package Newspack | ||
| */ | ||
|
|
||
| namespace Newspack\Reader_Activation; | ||
|
|
||
| use Newspack\Reader_Activation; | ||
|
|
||
| defined( 'ABSPATH' ) || exit; | ||
|
|
||
| /** | ||
| * Comment Display Name class. | ||
| */ | ||
| final class Comment_Display_Name { | ||
|
|
||
| /** | ||
| * Initialize hooks. | ||
| */ | ||
| public static function init() { | ||
| \add_filter( 'comment_form_submit_field', [ __CLASS__, 'render_display_name_field' ] ); | ||
| \add_filter( 'preprocess_comment', [ __CLASS__, 'validate_display_name' ] ); | ||
| } | ||
|
|
||
| /** | ||
| * Whether the current user should be prompted for a display name. | ||
| * | ||
| * @return bool | ||
| */ | ||
| private static function should_prompt() { | ||
| if ( ! \is_user_logged_in() ) { | ||
| return false; | ||
| } | ||
| $user = \wp_get_current_user(); | ||
| if ( ! Reader_Activation::is_user_reader( $user ) ) { | ||
| return false; | ||
| } | ||
| return Reader_Activation::reader_has_generic_display_name( $user->ID ); | ||
| } | ||
|
|
||
| /** | ||
| * Validate and save the display name before a comment is inserted. | ||
| * | ||
| * Runs on `preprocess_comment` so the updated display name is used as the | ||
| * comment author, rather than the stale email-derived name WordPress read | ||
| * from the user profile earlier in the request. | ||
| * | ||
| * @param array $commentdata Comment data. | ||
| * @return array Comment data with updated comment_author. | ||
| */ | ||
| public static function validate_display_name( $commentdata ) { | ||
| if ( ! self::should_prompt() ) { | ||
| return $commentdata; | ||
| } | ||
|
|
||
| $display_name = isset( $_POST['comment_display_name'] ) ? \sanitize_text_field( \wp_unslash( $_POST['comment_display_name'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Missing | ||
|
|
||
| if ( empty( $display_name ) ) { | ||
| \wp_die( | ||
| esc_html__( 'Please enter a display name.', 'newspack-plugin' ), | ||
| esc_html__( 'Comment Submission Failure', 'newspack-plugin' ), | ||
| [ 'back_link' => true ] | ||
| ); | ||
| } | ||
|
|
||
| $user = \wp_get_current_user(); | ||
| $email = $user->user_email; | ||
| if ( | ||
| Reader_Activation::generate_user_nicename( $email ) === $display_name || | ||
| Reader_Activation::strip_email_domain( $email ) === $display_name | ||
| ) { | ||
| \wp_die( | ||
| esc_html__( 'Please choose a display name that is not derived from your email address.', 'newspack-plugin' ), | ||
| esc_html__( 'Comment Submission Failure', 'newspack-plugin' ), | ||
| [ 'back_link' => true ] | ||
| ); | ||
| } | ||
|
|
||
| // Update the user profile. | ||
| $user_data = [ | ||
| 'ID' => $user->ID, | ||
| 'display_name' => $display_name, | ||
| ]; | ||
|
|
||
| $name_parts = explode( ' ', $display_name, 2 ); | ||
| $user_data['first_name'] = $name_parts[0]; | ||
| $user_data['last_name'] = $name_parts[1] ?? ''; | ||
|
|
||
| \wp_update_user( $user_data ); | ||
|
|
||
| // Override the comment author so this comment uses the new name. | ||
| $commentdata['comment_author'] = $display_name; | ||
|
|
||
| return $commentdata; | ||
| } | ||
|
|
||
| /** | ||
| * Render the display name field before the comment form submit button. | ||
| * | ||
| * @param string $submit_field The submit field HTML. | ||
| * @return string The submit field HTML, with display name field prepended if needed. | ||
| */ | ||
| public static function render_display_name_field( $submit_field ) { | ||
| if ( ! self::should_prompt() ) { | ||
| return $submit_field; | ||
| } | ||
|
|
||
| $field = '<p class="comment-form-display-name">' | ||
| . '<label for="comment_display_name">' | ||
| . esc_html__( 'Name', 'newspack-plugin' ) | ||
| . ' <span class="required" aria-hidden="true">*</span>' | ||
| . '</label>' | ||
| . '<input id="comment_display_name" name="comment_display_name" type="text" required="required" style="display:block;width:100%" />' | ||
| . '</p>'; | ||
|
|
||
| return $field . $submit_field; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| <?php | ||
| /** | ||
| * Tests the Comment Display Name functionality. | ||
| * | ||
| * @package Newspack\Tests | ||
| */ | ||
|
|
||
| use Newspack\Reader_Activation; | ||
| use Newspack\Reader_Activation\Comment_Display_Name; | ||
|
|
||
| /** | ||
| * Tests the Comment Display Name functionality. | ||
| * | ||
| * @group comment-display-name | ||
| */ | ||
| class Newspack_Test_Comment_Display_Name extends WP_UnitTestCase { | ||
|
|
||
| /** | ||
| * Create a reader with a generic (email-derived) display name. | ||
| * | ||
| * @param string $email Reader email. | ||
| * @return int User ID. | ||
| */ | ||
| private function create_generic_reader( $email = 'jane.doe@example.com' ) { | ||
| $user_id = Reader_Activation::register_reader( $email ); | ||
| wp_set_current_user( $user_id ); | ||
| return $user_id; | ||
| } | ||
|
|
||
| /** | ||
| * Test that the display name field renders for a reader with a generic display name. | ||
| */ | ||
| public function test_renders_field_for_generic_display_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
|
|
||
| $output = Comment_Display_Name::render_display_name_field( '<submit />' ); | ||
|
|
||
| $this->assertStringContainsString( 'name="comment_display_name"', $output ); | ||
| $this->assertStringContainsString( 'required', $output ); | ||
| $this->assertStringContainsString( '<submit />', $output ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that the display name field does not render for a reader with a custom display name. | ||
| */ | ||
| public function test_does_not_render_for_custom_display_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
| wp_update_user( | ||
| [ | ||
| 'ID' => $user_id, | ||
| 'display_name' => 'Jane Doe', | ||
| ] | ||
| ); | ||
|
|
||
| $output = Comment_Display_Name::render_display_name_field( '<submit />' ); | ||
|
|
||
| $this->assertEquals( '<submit />', $output ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that the display name field does not render for non-reader users. | ||
| */ | ||
| public function test_does_not_render_for_non_reader() { | ||
| $admin_id = wp_insert_user( | ||
| [ | ||
| 'user_login' => 'test-admin', | ||
| 'user_pass' => wp_generate_password(), | ||
| 'user_email' => 'admin@example.com', | ||
| 'role' => 'administrator', | ||
| ] | ||
| ); | ||
| wp_set_current_user( $admin_id ); | ||
|
|
||
| $output = Comment_Display_Name::render_display_name_field( '<submit />' ); | ||
|
|
||
| $this->assertEquals( '<submit />', $output ); | ||
|
|
||
| wp_delete_user( $admin_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that the display name field does not render for logged-out users. | ||
| */ | ||
| public function test_does_not_render_for_logged_out() { | ||
| wp_set_current_user( 0 ); | ||
|
|
||
| $output = Comment_Display_Name::render_display_name_field( '<submit />' ); | ||
|
|
||
| $this->assertEquals( '<submit />', $output ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation rejects an empty display name. | ||
| */ | ||
| public function test_validate_rejects_empty_display_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
| $_POST['comment_display_name'] = ''; | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $user_id, | ||
| ]; | ||
|
|
||
| $this->expectException( WPDieException::class ); | ||
| Comment_Display_Name::validate_display_name( $commentdata ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| unset( $_POST['comment_display_name'] ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation rejects a display name that matches the generic pattern. | ||
| */ | ||
| public function test_validate_rejects_generic_display_name() { | ||
| $user_id = $this->create_generic_reader( 'john.smith@example.com' ); | ||
| $_POST['comment_display_name'] = 'john.smith'; | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $user_id, | ||
| ]; | ||
|
|
||
| $this->expectException( WPDieException::class ); | ||
| Comment_Display_Name::validate_display_name( $commentdata ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| unset( $_POST['comment_display_name'] ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation passes with a valid display name and sets comment_author. | ||
| */ | ||
| public function test_validate_accepts_valid_display_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
| $_POST['comment_display_name'] = 'Jane Doe'; | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $user_id, | ||
| ]; | ||
|
|
||
| $result = Comment_Display_Name::validate_display_name( $commentdata ); | ||
| $this->assertEquals( 'Jane Doe', $result['comment_author'] ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| unset( $_POST['comment_display_name'] ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation passes for non-reader users without the field. | ||
| */ | ||
| public function test_validate_skips_non_reader() { | ||
| $admin_id = wp_insert_user( | ||
| [ | ||
| 'user_login' => 'test-admin-2', | ||
| 'user_pass' => wp_generate_password(), | ||
| 'user_email' => 'admin2@example.com', | ||
| 'role' => 'administrator', | ||
| ] | ||
| ); | ||
| wp_set_current_user( $admin_id ); | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $admin_id, | ||
| ]; | ||
|
|
||
| $result = Comment_Display_Name::validate_display_name( $commentdata ); | ||
| $this->assertEquals( $commentdata, $result ); | ||
|
|
||
| wp_delete_user( $admin_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation saves the display name to the user profile and updates comment_author. | ||
| */ | ||
| public function test_validate_saves_display_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
| $_POST['comment_display_name'] = 'Jane Doe'; | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $user_id, | ||
| 'comment_author' => 'jane-doe', // Generic name set by WP before preprocess_comment. | ||
| ]; | ||
|
|
||
| $result = Comment_Display_Name::validate_display_name( $commentdata ); | ||
|
|
||
| // Comment author should be updated. | ||
| $this->assertEquals( 'Jane Doe', $result['comment_author'] ); | ||
|
|
||
| // User profile should be updated. | ||
| $user = get_userdata( $user_id ); | ||
| $this->assertEquals( 'Jane Doe', $user->display_name ); | ||
| $this->assertEquals( 'Jane', $user->first_name ); | ||
| $this->assertEquals( 'Doe', $user->last_name ); | ||
| $this->assertFalse( Reader_Activation::reader_has_generic_display_name( $user_id ) ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| unset( $_POST['comment_display_name'] ); | ||
| } | ||
|
|
||
| /** | ||
| * Test that validation handles single-word display names. | ||
| */ | ||
| public function test_validate_saves_single_word_name() { | ||
| $user_id = $this->create_generic_reader(); | ||
| $_POST['comment_display_name'] = 'Madonna'; | ||
|
|
||
| $commentdata = [ | ||
| 'comment_post_ID' => $this->factory->post->create(), | ||
| 'user_id' => $user_id, | ||
| 'comment_author' => 'jane-doe', | ||
| ]; | ||
|
|
||
| $result = Comment_Display_Name::validate_display_name( $commentdata ); | ||
|
|
||
| $this->assertEquals( 'Madonna', $result['comment_author'] ); | ||
|
|
||
| $user = get_userdata( $user_id ); | ||
| $this->assertEquals( 'Madonna', $user->display_name ); | ||
| $this->assertFalse( Reader_Activation::reader_has_generic_display_name( $user_id ) ); | ||
|
|
||
| wp_delete_user( $user_id ); | ||
| unset( $_POST['comment_display_name'] ); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.