Sharing : Add wp_sharing_email_send_post_subject filter by Umangvaghela · Pull Request #8284 · Automattic/jetpack

Umangvaghela · 2017-12-01T06:58:58Z

Fixes #8165

Changes proposed in this Pull Request:

add wp_sharing_email_send_post_subject for allow modifying the email share Subject Line

Testing instructions:

jeherve

Thanks for the contribution.

In my review I just suggested a few changes to match coding standards.

jeherve · 2017-12-01T20:28:37Z

modules/sharedaddy/sharedaddy.php

 	$title = $data['sharing_source']->get_share_title( $data['post']->ID );

-	wp_mail( $data['target'], '[' . __( 'Shared Post', 'jetpack' ) . '] ' . $title, $content, $headers );
+	$subject = apply_filters('wp_sharing_email_send_post_subject',sprintf(__('Shared Post','jetpack')));


Could you add a docblock before the filter, as with the other filters in this file?

jeherve · 2017-12-01T20:29:19Z

modules/sharedaddy/sharedaddy.php

 	$title = $data['sharing_source']->get_share_title( $data['post']->ID );

-	wp_mail( $data['target'], '[' . __( 'Shared Post', 'jetpack' ) . '] ' . $title, $content, $headers );
+	$subject = apply_filters('wp_sharing_email_send_post_subject',sprintf(__('Shared Post','jetpack')));


Could you add spacing around quotes, parenthesis, and before sprintf?

Umangvaghela · 2017-12-02T03:21:14Z

@jeherve

Greetings of the day !!

Thanks for reply.
I add changes which is you suggested me. Please check it and provide the feedback for the same.

Thanks

jeherve

A few changes to match the previous subject line.

jeherve · 2017-12-15T21:36:38Z

modules/sharedaddy/sharedaddy.php

+	 *
+	 * @param string $var Sharing Email Send Post Subject. Default is "Shared Post".
+	 */
+	$subject = apply_filters( 'wp_sharing_email_send_post_subject', __( 'Shared Post', 'jetpack' ) );


Could you add square brackets ([]) around the subject line, like in the original message?

Thanks!

Umangvaghela · 2017-12-17T05:36:59Z

@jeherve

Changes Done.

Thanks

jeherve

One last change and we should be good to go!

Thank you!

jeherve · 2017-12-18T08:33:09Z

modules/sharedaddy/sharedaddy.php

+	 * Filter the Sharing Email Send Post Subject.
+	 *
+	 * @module sharedaddy
+	 *


Could you add @since 5.7.0 here too?

Umangvaghela · 2017-12-18T13:51:09Z

@jeherve

Done.

Thanks

oskosk

Thanks for the changes @Umangvaghela .

I left some comments about the usage of $title.

oskosk · 2017-12-20T17:44:25Z

modules/sharedaddy/sharedaddy.php

+	 *
+	 * @param string $var Sharing Email Send Post Subject. Default is "Shared Post".
+	 */
+	$subject = apply_filters( 'wp_sharing_email_send_post_subject', '[' . __( 'Shared Post', 'jetpack' ) . '] ' );


I think something is missing here.

The $title needs to be concatenated in the subject.

oskosk · 2017-12-20T17:46:18Z

modules/sharedaddy/sharedaddy.php

+	 */
+	$subject = apply_filters( 'wp_sharing_email_send_post_subject', '[' . __( 'Shared Post', 'jetpack' ) . '] ' );
+
+	wp_mail( $data['target'], $subject, $title, $content, $headers );


Here $title shouldn't be the third argument. $title should be concatenated into the subject`.

Umangvaghela · 2017-12-21T14:13:17Z

@oskosk

Changes Done.

Thanks

oskosk · 2017-12-21T14:14:06Z

Thanks for the changes @Umangvaghela ! I've relabeled as needs review...

Code looks good now, but how do I test that the funcionality updated here is not affected? "Testing instructions" on the description of the PR, pleaseee 😄

Umangvaghela · 2017-12-24T07:08:29Z

@oskosk

Testing instruction.

1 ) Enable sharing email option.
2 ) Add filter "wp_sharing_email_send_post_subject" in active theme function.php.
3 ) Update the subject if you want.
4 ) Share post by email sharing option.

Thanks

Umangvaghela · 2017-12-31T07:43:59Z

@oskosk

Hello Sir

Can you told me anything is missing for "testing description" or In PR. If you find anyhting else so told me I will change or add it as soon as possible. :)

Thanks,

zinigor

Sorry, but this PR missed the 5.7.0 release, and we're aiming for 5.8.0 now. Can you please fix the version number?
Thank you!

Umangvaghela · 2018-01-25T06:20:14Z

@zinigor

changes done.

Thanks

Thanks for the review, Osk! Your comments have been addressed.

* Changelog 5.8: create base for changelog. * Update 5.8 release post link * fix 5.8 release date * Updates to plugin description * Changelog: add #8499 * Changelog: add #8506 * Changelog: add #8509 * Changelog: add #8516 * Changelog: add #8517 * Changelog: add #8523 * Changelog: add #8547 * Changelog: add #8496 * Changelog: add #8584 * Changelog: add #8595 * Changelog: add #8445 * Changelog: add #8431 * Changelog: add #8284 * Changelog: add #8270 * Changelog: add #8124 * Changelog: add #8581 * Changelog: add #8463 * Changelog: add #8568 (#8646) * Updates to testing list and changelog * Changelog: add #8443 * Changelog: add #8459 * Changelog: add #8469 * Changelog: add #8464 * Changelog: add #8478 and #8479 * Changelog: add #8483 * Changelog: add #8488 * Changelog: add #8513 * Changelog: add #8555 * Changelog: add #8565 * Changelog: add #8601 * Changelog: add #8612 * Changelog: add first pass at Search items. * Changelog: add more info to help test Search. * Changelog: add #8144 * Changelog: add #8313 * Changelog: add #8419 * Changelog: add #8465 * Changelog: add #8515 * Changelog: add #8587 * Changelog: add #8591 * Changelog: add #8659 * Changelog: add #8661 * Changelog: add #8671 * Changelog: add 5.7.1 to archived changelog too. * Reverted changes to readme, removed entry about backups.

add wp_sharing_email_send_post_subject filter

1db8f6f

Umangvaghela requested a review from a team as a code owner December 1, 2017 06:58

Umangvaghela changed the title ~~add wp_sharing_email_send_post_subject filter~~ Sharing : Add wp_sharing_email_send_post_subject filter Dec 1, 2017

jeherve added [Feature] Sharing Post sharing, sharing buttons [Pri] Normal [Status] Needs Review This PR is ready for review. Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Dec 1, 2017

jeherve requested changes Dec 1, 2017

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 1, 2017

Update sharedaddy.php

c67d4b0

jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 4, 2017

jeherve requested changes Dec 15, 2017

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 15, 2017

Update sharedaddy.php

f46966c

jeherve requested changes Dec 18, 2017

View reviewed changes

Update sharedaddy.php

2fe7883

jeherve approved these changes Dec 18, 2017

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 18, 2017

oskosk added this to the 5.7 milestone Dec 20, 2017

oskosk previously requested changes Dec 20, 2017

View reviewed changes

oskosk added [Status] Needs Review This PR is ready for review. [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 20, 2017

oskosk removed the [Status] Needs Review This PR is ready for review. label Dec 21, 2017

Update sharedaddy.php

f42513e

oskosk added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 21, 2017

oskosk modified the milestones: 5.7, 5.8 Dec 26, 2017

Automattic deleted a comment from Umangvaghela Jan 19, 2018

zinigor suggested changes Jan 23, 2018

View reviewed changes

zinigor added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jan 23, 2018

Update sharedaddy.php

e1a329d

jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jan 25, 2018

zinigor approved these changes Jan 25, 2018

View reviewed changes

zinigor merged commit 1f7eccb into Automattic:master Jan 25, 2018

matticbot added the [Status] Needs Changelog label Jan 25, 2018

zinigor removed the [Status] Needs Review This PR is ready for review. label Jan 25, 2018

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Jan 25, 2018

jeherve added a commit that referenced this pull request Jan 25, 2018

Changelog: add #8284

a346988

Conversation

Umangvaghela commented Dec 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Testing instructions:

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve Dec 1, 2017

Choose a reason for hiding this comment

Uh oh!

jeherve Dec 1, 2017

Choose a reason for hiding this comment

Uh oh!

Umangvaghela commented Dec 2, 2017

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

Umangvaghela commented Dec 17, 2017

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

jeherve Dec 18, 2017

Choose a reason for hiding this comment

Uh oh!

Umangvaghela commented Dec 18, 2017

Uh oh!

oskosk left a comment

Choose a reason for hiding this comment

Uh oh!

oskosk Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oskosk Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Umangvaghela commented Dec 21, 2017

Uh oh!

oskosk commented Dec 21, 2017

Uh oh!

Umangvaghela commented Dec 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Umangvaghela commented Dec 31, 2017

Uh oh!

zinigor left a comment

Choose a reason for hiding this comment

Uh oh!

Umangvaghela commented Jan 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Umangvaghela commented Dec 1, 2017 •

edited

Loading

oskosk Dec 20, 2017 •

edited

Loading

oskosk Dec 20, 2017 •

edited

Loading

Umangvaghela commented Dec 24, 2017 •

edited

Loading