Avoiding redefinition of core function wp_notify_moderator #202

Merged
merged 2 commits into from Jun 17, 2014

Conversation

Projects
None yet
3 participants
@fpozzer
Contributor

fpozzer commented May 27, 2014

This patch is to replace the redefinition of a core function by adding a filter for comment notification recipients.
Instead of redefining wp_modify_moderator() only to be able to get the co-authors e-mails, I've added a filter to the plugin, "filter_comment_moderation_email_recipients", which filters the comment moderation recipient list and adds the co-authors e-mails.

On WP 3.7.0, a new filter was applied to the core wp_modify_moderator() function ("comment_moderation_recipients"), and the lack of the application of this filter on the modified plugin version was preventing other plugins that use "comment_moderation_recipients" from working.

Probably the same approach can be used on the wp_notify_postauthor() function on this plugin on a future work.

@larruda

This comment has been minimized.

Show comment
Hide comment
@larruda

larruda May 27, 2014

This patch resolved a bug on my project which was preventing other plug-ins from interacting correctly with the e-mail notification flow within WordPress. Please consider integrating this into the project's repository.

larruda commented May 27, 2014

This patch resolved a bug on my project which was preventing other plug-ins from interacting correctly with the e-mail notification flow within WordPress. Please consider integrating this into the project's repository.

@larruda

This comment has been minimized.

Show comment
Hide comment
@larruda

larruda May 27, 2014

FYI although Travis CI build failed, you can check that 3 of 4 tests passed and the fourth one failed because the plug-in implementation uses mysql_connect() instead of PDO. The scenario which fails is based on PHP 5.5 thus this is the reason is fails, for all commits as well.

larruda commented May 27, 2014

FYI although Travis CI build failed, you can check that 3 of 4 tests passed and the fourth one failed because the plug-in implementation uses mysql_connect() instead of PDO. The scenario which fails is based on PHP 5.5 thus this is the reason is fails, for all commits as well.

co-authors-plus.php
- // The blogname option is escaped with esc_html on the way into the database in sanitize_option
- // we want to reverse this for the plain text arena of emails.
- $blogname = wp_specialchars_decode(get_option('blogname'), ENT_QUOTES);
+function filter_comment_moderation_email_recipients( $recipients ) {

This comment has been minimized.

@mjangda

mjangda May 27, 2014

Member

The function here should be prefixed with cap_ or coauthors_

@mjangda

mjangda May 27, 2014

Member

The function here should be prefixed with cap_ or coauthors_

co-authors-plus.php
- // we want to reverse this for the plain text arena of emails.
- $blogname = wp_specialchars_decode(get_option('blogname'), ENT_QUOTES);
+function filter_comment_moderation_email_recipients( $recipients ) {
+ global $post;

This comment has been minimized.

@mjangda

mjangda May 27, 2014

Member

Instead of the global $post, it would be better to check the comment_post_ID property for the comment. The comment ID is passed in as the second arg that we can use for the lookup.

@mjangda

mjangda May 27, 2014

Member

Instead of the global $post, it would be better to check the comment_post_ID property for the comment. The comment ID is passed in as the second arg that we can use for the lookup.

co-authors-plus.php
+ $coauthors = get_coauthors( $post->ID );
+ foreach ( $coauthors as $user ) {
+ // "edit_comment" capability == "edit_post" capability
+ if ( user_can($user->ID, 'edit_post', $post->ID) && !empty($user->user_email) )

This comment has been minimized.

@mjangda

mjangda May 27, 2014

Member

I'm not sure why they user_can check is necessary here. The plugin adds the edit_post cap to all coauthors anyway.

@mjangda

mjangda May 27, 2014

Member

I'm not sure why they user_can check is necessary here. The plugin adds the edit_post cap to all coauthors anyway.

@fpozzer

This comment has been minimized.

Show comment
Hide comment
@fpozzer

fpozzer May 28, 2014

Contributor

The suggested changes were done on the last commit.

Contributor

fpozzer commented May 28, 2014

The suggested changes were done on the last commit.

@larruda

This comment has been minimized.

Show comment
Hide comment
@larruda

larruda May 30, 2014

@mjangda please review the pull request once you are able to. Thanks and have a blessed day!

larruda commented May 30, 2014

@mjangda please review the pull request once you are able to. Thanks and have a blessed day!

@larruda

This comment has been minimized.

Show comment
Hide comment
@larruda

larruda Jun 16, 2014

Hello. @mjangda @danielbachhuber @nickdaugherty could you kindly review and approve this pull request? Thanks.

larruda commented Jun 16, 2014

Hello. @mjangda @danielbachhuber @nickdaugherty could you kindly review and approve this pull request? Thanks.

@mjangda

This comment has been minimized.

Show comment
Hide comment
@mjangda

mjangda Jun 17, 2014

isset is the wrong check here since the var is defined on the line above; empty would be better.

isset is the wrong check here since the var is defined on the line above; empty would be better.

mjangda added a commit that referenced this pull request Jun 17, 2014

Merge pull request #202 from fpozzer/master
Avoiding redefinition of core function wp_notify_moderator

@mjangda mjangda merged commit 3f4b672 into Automattic:master Jun 17, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment