-
Notifications
You must be signed in to change notification settings - Fork 468
Description
Bug Report
Expected behavior
When emails are sent by EDD_SL_Emails->send_renewal_reminder() via the edd_daily_scheduled_events hook that triggers edd_sl_scheduled_reminders, a license meta with the key _edd_sl_renewal_sent_{$period} should be recorded to prevent sending new emails to the same user regarding the same notification.
Actual behavior
Multiple emails are being sent because $period is not correctly retrieved, so the meta is stored with the key _edd_sl_renewal_sent_, which doesn't prevent any email to be sent.
Steps to reproduce the behavior
- Go to
/wp-admin/edit.php?post_type=download&page=edd-emails. - Click on
Add New Email>Add License Renewal Notice. - Fill in the fields with any value and click
Save. - Get the notice's ID from the
emailURL query parameter. The value should have the following formatlicense_abc12. - Add the following commands in your theme's
function.php. These help identify the issue.if ( defined( 'WP_CLI' ) && true === WP_CLI ) { /** * Test if send_renewal_reminder() is getting the notice's period right. * * ## OPTIONS * * [--notice-id=<notice-id>] * : The notice ID. * * [--license-id=<license-id>] * : The license ID. * * @synopsis [--notice-id=<notice-id>] [--license-id=<license-id>] * * ## EXAMPLES * * # Test for notice with ID license_abc12 and for license with ID 1. * $ wp edd-ad-hoc email test send_renewal_reminder --notice-id=license_abc12 --license-id=1 * * @param array $args Positional arguments. * @param array $assoc_args Associative arguments. */ function edd_ad_hoc_email_test_send_renewal_reminder( $args, $assoc_args ) { if ( ! edd_sl_are_email_templates_registered() ) { \WP_CLI::success( 'Email templates are not registered.' ); } $notice_id = \WP_CLI\Utils\get_flag_value( $assoc_args, 'notice-id', 0 ); $email_object = edd_get_email( $notice_id ); if ( ! $email_object ) { \WP_CLI::error( 'Email object not found.' ); } $license_id = \WP_CLI\Utils\get_flag_value( $assoc_args, 'license-id', 0 ); $license = edd_software_licensing()->get_license( $license_id ); if ( false === $license ) { \WP_CLI::error( 'License not found.' ); } /* This is how send_renewal_reminder() gets the period. */ $email = EDD\Emails\Registry::get( $notice_id, array( $license->ID, $license, $email_object ) ); $period = edd_get_email_meta( $email->id, 'period', true ); $meta_key = sanitize_key( '_edd_sl_renewal_sent_' . $period ); \WP_CLI::line( 'Meta key with edd_get_email_meta( $email->id ): ' . $meta_key ); /* This is my suggested fix. */ $email = EDD\Emails\Registry::get( $notice_id, array( $license->ID, $license, $email_object ) ); $period = edd_get_email_meta( $email->email->id, 'period', true ); $meta_key = sanitize_key( '_edd_sl_renewal_sent_' . $period ); \WP_CLI::line( 'Meta key with edd_get_email_meta( $email->email->id ): ' . $meta_key ); /* This suggestion for a fix. */ $notice = edd_software_licensing()->notices->get_notice( $notice_id ); $period = edd_software_licensing()->notices->get_notice_period( $notice ); $meta_key = sanitize_key( '_edd_sl_renewal_sent_' . $period ); \WP_CLI::line( 'Meta key with get_notice_period( $notice ): ' . $meta_key ); } \WP_CLI::add_command( 'edd-ad-hoc email test send_renewal_reminder', 'edd_ad_hoc_email_test_send_renewal_reminder' ); /** * Test if edd_sl_scheduled_reminders() is getting the notice's period right. * * ## EXAMPLES * * # Test. * $ wp edd-ad-hoc email test edd_sl_scheduled_reminders * * @param array $args Positional arguments. * @param array $assoc_args Associative arguments. */ function edd_ad_hoc_email_test_edd_sl_scheduled_reminders( $args, $assoc_args ) { $notices = edd_software_licensing()->notices->get_notices(); foreach ( $notices as $notice ) { $notice_id = edd_software_licensing()->notices->get_notice_id( $notice ); $send_period = edd_software_licensing()->notices->get_notice_period( $notice ); \WP_CLI::line( 'Notice get_notice_id: ' . $notice_id ); \WP_CLI::line( 'Notice ID: ' . $notice->id ); \WP_CLI::line( 'Notice email ID: ' . $notice->email_id ); \WP_CLI::line( 'Notice subject: ' . $notice->subject ); \WP_CLI::line( 'Notice period: ' . $send_period ); \WP_CLI::line( str_repeat( '-', 30 ) ); } } \WP_CLI::add_command( 'edd-ad-hoc email test edd_sl_scheduled_reminders', 'edd_ad_hoc_email_test_edd_sl_scheduled_reminders' ); }
- (optional) Running
wp edd-ad-hoc email test edd_sl_scheduled_reminderswill give you some detail on the License Renewal Notices you have, including the notice ID we got from the URL. Something like the following:Notice get_notice_id: license_d29d5 Notice ID: 18 Notice email ID: license_d29d5 Notice subject: Your Support is About to Expire Notice period: +1month - Run
wp edd-ad-hoc email test send_renewal_reminder --notice-id=license_abc12 --license-id=1. This will compare the current implementation, which fails to append the period to the license meta in thewp_edd_licensemetatable. Something like the following:Meta key with edd_get_email_meta( $email->id ): _edd_sl_renewal_sent_ Meta key with edd_get_email_meta( $email->email->id ): _edd_sl_renewal_sent_1month Meta key with get_notice_period( $notice ): _edd_sl_renewal_sent_1month
Information (if a specific version is affected):
PHP Version: 6.6.1
EDD Version (or branch): 3.3.3
EDD Software Licensing Version: 3.8.14
WordPress Version: 6.6.1
My suggested fix
As visible in the edd_ad_hoc_email_test_send_renewal_reminder() function above, I suggest replacing the following line from EDD_SL_Emails->send_renewal_reminder() on line 96:
$period = edd_get_email_meta( $email->id, 'period', true );Either with:
$period = edd_get_email_meta( $email->email->id, 'period', true );Or with:
$notice = edd_software_licensing()->notices->get_notice( $notice_id );
$period = edd_software_licensing()->notices->get_notice_period( $notice );To understand why use $email->email->id, see the __construct() method of EDD\SoftwareLicensing\Emails\Types\Notices, which is the class registered to EDD\Emails\Registry via EDD\SoftwareLicensing\Emails\register_email_types(), via the edd_email_registered_types hook.