Fix/issue 36360 #37883
Fix/issue 36360 #37883
Conversation
…ails when the field is empty
…ails when the field is empty for Admin New Order email
github-actions
bot
added
plugin: woocommerce
woocommercebot
requested review from
a team and
barryhughes
and removed request for
a team
| * @return string | ||
| */ | ||
| public function get_additional_content() { | ||
| return apply_filters( 'woocommerce_email_additional_content_' . $this->id, $this->format_string( $this->get_option( 'additional_content' ) ), $this->object, $this ); // phpcs:ignore WooCommerce.Commenting.CommentHooks.MissingHookComment |
There was a problem hiding this comment.
Thinking that rather than suppress the linter warning, which makes it a little harder to surface missing docblocks in the future (because PHPCS will stop complaining about any with this phpcs:ignore comment), probably the best course of action would be to document the original hook in WC_Email and then mark this one as a duplicate hook.
There was a problem hiding this comment.
@barryhughes I was discussing this with @jorgeatorres and he mentioned using the phpcs:ignore comment because it was a duplicate. Maybe he can jump in to this conversation and give his opinion.
Did you see my comment on the PHPCS @barryhughes ? |
|
Hi , Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Yes, thank you. We just discussed this and I pushed a small change—we'll aim to re-review shortly! |
There was a problem hiding this comment.
LGTM 👍.
Tested that the "Congratulations on your sale" message no longer appears when removed from the e-mail via Settings. Also, when the woocommerce_new_order_settings option doesn't exist, the default e-mail message gets used (which does include the additional content).
Thank you @shadimanna for your contribution and @barryhughes for the first round of review.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Remove the default text in "Additional content" being sent for all emails when the field is empty for Admin New Order email
Closes #36360 .
How to test the changes in this Pull Request: