Commit fb0a9b87198 for woocommerce
commit fb0a9b871988f6483bf4c4e043d628cfa34dbcbb
Author: Chris Lilitsas <1105590+xristos3490@users.noreply.github.com>
Date: Tue Oct 27 16:46:54 2026 +0200
Mark back-in-stock sign-ups as sent only when the email is actually sent (#69830)
* fix: mark back-in-stock sign-ups as sent only when the email is sent
CustomerStockNotificationEmail::trigger() ignored the result of send(),
so the notifications processor marked every sign-up Sent and logged
"Failed: 0" even when the email was disabled or wp_mail() failed.
trigger() and EmailManager::send_stock_notification_email() now return
whether the email went out, and the processor marks the sign-up Sent
only on success, counting the rest as failed.
* chore: drop stale PHPStan baseline entry for the stock notification email trigger return type
* test: report emails as sent in the e2e environment
wp-env has no mail transport, so wp_mail() returns false after WP Mail Logging
has captured the message. Now that a failed send cancels a back-in-stock
sign-up, the e2e specs saw every notification as cancelled. Short-circuit
wp_mail() with pre_wp_mail so the specs observe the production outcome.
* test: scope the e2e wp_mail short-circuit to the back-in-stock batch
The global pre_wp_mail override in the helper plugin broke the email settings
and email editor specs, which assert the no-mailer failure path. Set the filter
through the e2e-filters cookie only when a spec drains the notifications queue.
* style: fix Prettier indentation in back-in-stock e2e utils
* fix: only mark a sign-up sent from the admin edit page when the email goes out
The "Send notification" action marked the sign-up as sent regardless of the
wp_mail() result. Check the return value, keep the status untouched on failure
so the admin can retry, and show an error notice instead.
diff --git a/plugins/woocommerce/changelog/fix-wooplug-7909-bis-mark-sent-on-send b/plugins/woocommerce/changelog/fix-wooplug-7909-bis-mark-sent-on-send
new file mode 100644
index 00000000000..a01e8d319be
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-wooplug-7909-bis-mark-sent-on-send
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Mark back-in-stock sign-ups as sent only when the notification email actually goes out.
diff --git a/plugins/woocommerce/phpstan-baseline.neon b/plugins/woocommerce/phpstan-baseline.neon
index c5d6ccbf294..44958054efc 100644
--- a/plugins/woocommerce/phpstan-baseline.neon
+++ b/plugins/woocommerce/phpstan-baseline.neon
@@ -64146,12 +64146,6 @@ parameters:
count: 1
path: src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php
- -
- message: '#^Method Automattic\\WooCommerce\\Internal\\StockNotifications\\Emails\\CustomerStockNotificationEmail\:\:trigger\(\) has no return type specified\.$#'
- identifier: missingType.return
- count: 1
- path: src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php
-
-
message: '#^Parameter \#1 \$object_or_string of function is_a expects object, WP_User\|false given\.$#'
identifier: argument.type
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/Admin/NotificationEditPage.php b/plugins/woocommerce/src/Internal/StockNotifications/Admin/NotificationEditPage.php
index 68d6958f6f1..94317bc00fa 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/Admin/NotificationEditPage.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/Admin/NotificationEditPage.php
@@ -112,14 +112,16 @@ class NotificationEditPage {
if ( ! $product || ! $product->is_in_stock() ) {
$notice_message = __( 'Failed to send notification. Please make sure that the listed product is available.', 'woocommerce' );
NotificationsPage::add_notice( $notice_message, 'error' );
- } else {
- $this->email_manager->send_stock_notification_email( $notification );
+ } elseif ( $this->email_manager->send_stock_notification_email( $notification ) ) {
$notification->set_status( NotificationStatus::SENT );
$notification->set_date_notified( time() );
$notification->save();
// translators: %s user email.
$notice_message = sprintf( __( 'Notification sent to "%s".', 'woocommerce' ), $notification->get_user_email() );
NotificationsPage::add_notice( $notice_message, 'success' );
+ } else {
+ $notice_message = __( 'Failed to send notification. Please make sure that the "Back in stock notification" email is enabled and check the email logs.', 'woocommerce' );
+ NotificationsPage::add_notice( $notice_message, 'error' );
}
break;
case 'send_verification_email':
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessor.php b/plugins/woocommerce/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessor.php
index 67f086029d5..6f93d7ec2a4 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessor.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessor.php
@@ -229,9 +229,8 @@ class NotificationsProcessor {
continue;
}
- $is_sent = true;
try {
- $this->email_manager->send_stock_notification_email( $notification );
+ $is_sent = $this->email_manager->send_stock_notification_email( $notification );
} catch ( \Throwable $e ) {
$is_sent = false;
}
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php b/plugins/woocommerce/src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php
index 69131b34f76..ec522ca6b39 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/Emails/CustomerStockNotificationEmail.php
@@ -195,6 +195,7 @@ class CustomerStockNotificationEmail extends WC_Email {
* Trigger the sending of this email.
*
* @param Notification|int $notification The notification object or ID.
+ * @return bool Whether the email was sent. False when the email is disabled, has no recipient, or the mail callback failed.
*/
public function trigger( $notification ) {
$this->setup_locale();
@@ -204,25 +205,26 @@ class CustomerStockNotificationEmail extends WC_Email {
}
if ( ! $notification instanceof Notification ) {
- return;
+ return false;
}
$product = $notification->get_product();
if ( ! $product || ! is_a( $product, 'WC_Product' ) ) {
- return;
+ return false;
}
$this->maybe_setup_notification_locale( $notification );
$this->prepare_email( $notification );
+ $sent = false;
if ( $this->is_enabled() && $this->get_recipient() ) {
-
- $this->send( $this->get_recipient(), $this->get_subject(), $this->get_content(), $this->get_headers(), $this->get_attachments() );
-
+ $sent = (bool) $this->send( $this->get_recipient(), $this->get_subject(), $this->get_content(), $this->get_headers(), $this->get_attachments() );
}
$this->maybe_restore_notification_locale( $notification );
$this->restore_locale();
+
+ return $sent;
}
/**
diff --git a/plugins/woocommerce/src/Internal/StockNotifications/Emails/EmailManager.php b/plugins/woocommerce/src/Internal/StockNotifications/Emails/EmailManager.php
index 181056929c6..68c6dd7b606 100644
--- a/plugins/woocommerce/src/Internal/StockNotifications/Emails/EmailManager.php
+++ b/plugins/woocommerce/src/Internal/StockNotifications/Emails/EmailManager.php
@@ -277,7 +277,7 @@ class EmailManager {
* Send a stock notification email.
*
* @param Notification $notification The notification object.
- * @return void
+ * @return bool Whether the email was sent.
*/
public function send_stock_notification_email( Notification $notification ) {
$emails = WC()->mailer()->get_emails();
@@ -285,10 +285,10 @@ class EmailManager {
if ( ! $email instanceof CustomerStockNotificationEmail ) {
$this->log_missing_email( 'WC_Email_Customer_Stock_Notification', $notification );
- return;
+ return false;
}
- $email->trigger( $notification );
+ return $email->trigger( $notification );
}
/**
diff --git a/plugins/woocommerce/tests/e2e/utils/back-in-stock-notifications.ts b/plugins/woocommerce/tests/e2e/utils/back-in-stock-notifications.ts
index 8e8559c8b72..71e406383a2 100644
--- a/plugins/woocommerce/tests/e2e/utils/back-in-stock-notifications.ts
+++ b/plugins/woocommerce/tests/e2e/utils/back-in-stock-notifications.ts
@@ -1083,10 +1083,18 @@ export async function getEmailLinkById(
/**
* Run any pending Action Scheduler jobs via the process-waiting-actions mu-plugin.
*
+ * wp-env has no mail transport, so `wp_mail()` returns false even though WP
+ * Mail Logging has already captured the message. A failed send cancels the
+ * sign-up instead of marking it sent, so `wp_mail()` is short-circuited to
+ * report success for this context through the `e2e-filters` cookie. It is
+ * scoped here rather than in the helper plugin because other specs assert
+ * the failure path.
+ *
* @param {Page} page Playwright page (can be on any URL).
*/
export async function triggerStockNotificationsBatch(
page: Page
): Promise< void > {
+ await setFilterValue( page, 'pre_wp_mail', true );
await page.goto( '?process-waiting-actions' );
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Admin/NotificationEditPageTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Admin/NotificationEditPageTests.php
index 47708e43120..a8026e9d180 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Admin/NotificationEditPageTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Admin/NotificationEditPageTests.php
@@ -25,14 +25,22 @@ class NotificationEditPageTests extends \WC_Unit_Test_Case {
*/
private $sut;
+ /**
+ * Mocked email manager injected into the SUT.
+ *
+ * @var EmailManager|\PHPUnit\Framework\MockObject\MockObject
+ */
+ private $email_manager;
+
/**
* Set up test fixtures.
*/
public function setUp(): void {
parent::setUp();
$this->enable_stock_notifications_feature();
- $this->sut = new NotificationEditPage();
- $this->sut->init( $this->createMock( EmailManager::class ) );
+ $this->email_manager = $this->createMock( EmailManager::class );
+ $this->sut = new NotificationEditPage();
+ $this->sut->init( $this->email_manager );
add_filter( 'wp_redirect', array( $this, 'stop_redirect' ) );
}
@@ -86,4 +94,62 @@ class NotificationEditPageTests extends \WC_Unit_Test_Case {
$sent_at = (int) ( new Notification( $notification_id ) )->get_meta( NotificationManagementService::LAST_VERIFY_EMAIL_SENT_META );
$this->assertGreaterThanOrEqual( $before, $sent_at );
}
+
+ /**
+ * @testdox Sending a notification from the edit page marks it sent when the email goes out.
+ */
+ public function test_send_notification_marks_sent_when_email_is_sent(): void {
+ $this->email_manager->method( 'send_stock_notification_email' )->willReturn( true );
+ $notification_id = $this->submit_send_notification();
+
+ $notification = new Notification( $notification_id );
+ $this->assertSame( NotificationStatus::SENT, $notification->get_status() );
+ $this->assertNotEmpty( $notification->get_date_notified() );
+ $this->assertSame( 'success', get_option( NotificationsPage::ADMIN_NOTICE_OPTION_NAME )['type'] );
+ }
+
+ /**
+ * @testdox Sending a notification from the edit page leaves it active when the email fails.
+ */
+ public function test_send_notification_keeps_status_when_email_fails(): void {
+ $this->email_manager->method( 'send_stock_notification_email' )->willReturn( false );
+ $notification_id = $this->submit_send_notification();
+
+ $notification = new Notification( $notification_id );
+ $this->assertSame( NotificationStatus::ACTIVE, $notification->get_status() );
+ $this->assertEmpty( $notification->get_date_notified() );
+ $this->assertSame( 'error', get_option( NotificationsPage::ADMIN_NOTICE_OPTION_NAME )['type'] );
+ }
+
+ /**
+ * Submit the "send_notification" action for a new active sign-up on an in-stock product.
+ *
+ * @return int The notification ID.
+ */
+ private function submit_send_notification(): int {
+ $product = \WC_Helper_Product::create_simple_product();
+
+ $notification = new Notification();
+ $notification->set_user_email( 'active@test.com' );
+ $notification->set_product_id( $product->get_id() );
+ $notification->set_status( NotificationStatus::ACTIVE );
+ $notification_id = $notification->save();
+
+ $_POST = array(
+ 'wc_customer_stock_notification_action' => 'send_notification',
+ 'customer_stock_notification_edit_security' => wp_create_nonce( 'woocommerce-customer-stock-notification-edit' ),
+ );
+
+ $_REQUEST = $_POST; // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Simulates the request the handler verifies.
+
+ $redirected = false;
+ try {
+ $this->sut->process_edit_form( $notification );
+ } catch ( \Exception $e ) {
+ $redirected = 'redirect' === $e->getMessage();
+ }
+ $this->assertTrue( $redirected, 'Expected the handler to redirect.' );
+
+ return $notification_id;
+ }
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessorTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessorTests.php
index 1fd09733126..d655af10cff 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessorTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/AsyncTasks/NotificationsProcessorTests.php
@@ -31,6 +31,13 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
*/
private $sut;
+ /**
+ * What the short-circuited `wp_mail()` reports back.
+ *
+ * @var bool
+ */
+ private $wp_mail_result = true;
+
/**
* Set up test case
*/
@@ -39,13 +46,23 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
$this->enable_stock_notifications_feature();
\WC()->queue()->cancel_all( JobManager::AS_JOB_SEND_STOCK_NOTIFICATIONS );
+ // Short-circuit `wp_mail()` so the tests never attempt a real send, while still
+ // letting each test choose what the mailer reports back.
+ $this->wp_mail_result = true;
+ add_filter( 'pre_wp_mail', array( $this, 'short_circuit_wp_mail' ) );
+
$eligibility_service = new EligibilityService();
$eligibility_service->init( new StockManagementHelper() );
$job_manager = new JobManager();
$cycle_state_service = new CycleStateService();
$email_manager = new EmailManager();
- $this->sut = new NotificationsProcessor();
+ $email_manager->init();
+ $this->sut = new NotificationsProcessor();
$this->sut->init( $eligibility_service, $job_manager, $cycle_state_service, $email_manager );
+
+ // Rebuild the `WC_Emails` singleton so the BIS email registered above is part of it.
+ $this->reset_email_singleton();
+ WC()->mailer();
}
/**
@@ -53,6 +70,7 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
*/
public function tearDown(): void {
unset( $this->sut );
+ $this->reset_email_singleton();
// Clean up all notifications.
global $wpdb;
$wpdb->query( "DELETE FROM {$wpdb->prefix}wc_stock_notificationmeta" );
@@ -62,6 +80,24 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
parent::tearDown();
}
+ /**
+ * Reset the `WC_Emails` singleton so the next `WC()->mailer()` call rebuilds it.
+ */
+ private function reset_email_singleton(): void {
+ $instance_property = ( new \ReflectionClass( \WC_Emails::class ) )->getProperty( 'instance' );
+ $instance_property->setAccessible( true );
+ $instance_property->setValue( null, null );
+ }
+
+ /**
+ * `pre_wp_mail` filter: skip the actual send and report the configured result.
+ *
+ * @return bool
+ */
+ public function short_circuit_wp_mail(): bool {
+ return $this->wp_mail_result;
+ }
+
/**
* Test parse_args method.
*/
@@ -119,6 +155,7 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
$notification = new Notification();
$notification->set_product_id( $product->get_id() );
$notification->set_user_id( 1 );
+ $notification->set_user_email( 'test@test.com' );
$notification->set_status( NotificationStatus::ACTIVE );
$notification->save();
@@ -155,12 +192,14 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
$notification = new Notification();
$notification->set_product_id( $variation->get_id() );
$notification->set_user_id( 1 );
+ $notification->set_user_email( 'test@test.com' );
$notification->set_status( NotificationStatus::ACTIVE );
$notification->save();
$notification_on_parent = new Notification();
$notification_on_parent->set_product_id( $product->get_id() );
$notification_on_parent->set_user_id( 1 );
+ $notification_on_parent->set_user_email( 'test@test.com' );
$notification_on_parent->set_status( NotificationStatus::ACTIVE );
$notification_on_parent->save();
@@ -195,12 +234,14 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
$notification = new Notification();
$notification->set_product_id( $variable->get_id() );
$notification->set_user_id( 1 );
+ $notification->set_user_email( 'test@test.com' );
$notification->set_status( NotificationStatus::ACTIVE );
$notification->save();
$notification_on_variation = new Notification();
$notification_on_variation->set_product_id( $last_variation->get_id() );
$notification_on_variation->set_user_id( 1 );
+ $notification_on_variation->set_user_email( 'test@test.com' );
$notification_on_variation->set_status( NotificationStatus::ACTIVE );
$notification_on_variation->save();
@@ -370,6 +411,61 @@ class NotificationsProcessorTests extends \WC_Unit_Test_Case {
);
}
+ /**
+ * @testdox Should mark a sign-up Sent only when the email actually goes out, and count it as failed otherwise.
+ *
+ * @dataProvider send_outcome_provider
+ *
+ * @param bool $email_enabled Whether the back-in-stock email is enabled.
+ * @param bool $wp_mail_result What `wp_mail()` reports back.
+ * @param string $expected_status The status the sign-up should end up in.
+ * @param int $expected_sent Expected `sent_count` in the cycle state.
+ * @param int $expected_failed Expected `failed_count` in the cycle state.
+ */
+ public function test_process_batch_marks_sent_only_when_email_is_sent( bool $email_enabled, bool $wp_mail_result, string $expected_status, int $expected_sent, int $expected_failed ): void {
+ // Batch size 1 keeps the cycle state around after the first run, so the counters can be inspected.
+ add_filter( 'woocommerce_customer_stock_notifications_batch_size', fn() => 1 );
+ add_filter( 'woocommerce_email_enabled_customer_stock_notification', fn() => $email_enabled );
+ $this->wp_mail_result = $wp_mail_result;
+
+ $product = WC_Helper_Product::create_simple_product();
+ $notification = new Notification();
+ $notification->set_product_id( $product->get_id() );
+ $notification->set_user_email( 'test@test.com' );
+ $notification->set_status( NotificationStatus::ACTIVE );
+ $notification->save();
+
+ $this->sut->process_batch( $product->get_id() );
+
+ $notification = new Notification( $notification->get_id() );
+ $this->assertEquals( $expected_status, $notification->get_status() );
+ $this->assertEqualsWithDelta( time(), $notification->get_date_last_attempt()->getTimestamp(), 5 );
+ if ( NotificationStatus::SENT === $expected_status ) {
+ $this->assertEqualsWithDelta( time(), $notification->get_date_notified()->getTimestamp(), 5 );
+ } else {
+ $this->assertEmpty( $notification->get_date_notified(), 'A sign-up that was not emailed must not record a notified date.' );
+ }
+
+ $cycle_state = get_option( CycleStateService::STATE_OPTION_PREFIX . $product->get_id() );
+ $this->assertSame( $expected_sent, $cycle_state['sent_count'] );
+ $this->assertSame( $expected_failed, $cycle_state['failed_count'] );
+
+ WC()->queue()->cancel_all( JobManager::AS_JOB_SEND_STOCK_NOTIFICATIONS );
+ }
+
+ /**
+ * Data provider for test_process_batch_marks_sent_only_when_email_is_sent.
+ *
+ * @return array[]
+ */
+ public function send_outcome_provider(): array {
+ return array(
+ 'email sent' => array( true, true, NotificationStatus::SENT, 1, 0 ),
+ 'email disabled' => array( false, true, NotificationStatus::CANCELLED, 0, 1 ),
+ 'wp_mail failed' => array( true, false, NotificationStatus::CANCELLED, 0, 1 ),
+ );
+ }
+
/**
* Test process_batch method on a product with a notification that is throttled.
*/
diff --git a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Emails/EmailManagerTests.php b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Emails/EmailManagerTests.php
index 6c2681c9b33..c9d4060554a 100644
--- a/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Emails/EmailManagerTests.php
+++ b/plugins/woocommerce/tests/php/src/Internal/StockNotifications/Emails/EmailManagerTests.php
@@ -37,6 +37,13 @@ class EmailManagerTests extends \WC_Unit_Test_Case {
*/
private $sent_messages = array();
+ /**
+ * What the short-circuited `wp_mail()` reports back.
+ *
+ * @var bool
+ */
+ private $wp_mail_result = true;
+
/**
* Set up test fixtures.
*/
@@ -64,8 +71,9 @@ class EmailManagerTests extends \WC_Unit_Test_Case {
*/
public function tearDown(): void {
remove_filter( 'pre_wp_mail', array( $this, 'capture_pre_wp_mail' ), 10 );
- $this->sent_to = array();
- $this->sent_messages = array();
+ $this->sent_to = array();
+ $this->sent_messages = array();
+ $this->wp_mail_result = true;
$this->restore_stock_notifications_feature_option();
// Discard this test's `WC_Emails` build so later tests rebuild their own from
// whatever filters are active for them, rather than inheriting this test's list.
@@ -96,7 +104,38 @@ class EmailManagerTests extends \WC_Unit_Test_Case {
foreach ( $recipients as $recipient ) {
$this->sent_to[ $recipient ] = true;
}
- return true;
+ return $this->wp_mail_result;
+ }
+
+ /**
+ * @testdox Should report whether the back-in-stock email was actually sent when send_stock_notification_email is called.
+ *
+ * @dataProvider stock_notification_send_outcome_provider
+ *
+ * @param bool $email_enabled Whether the back-in-stock email is enabled.
+ * @param bool $wp_mail_result What `wp_mail()` reports back.
+ * @param bool $expected The expected return value.
+ */
+ public function test_send_stock_notification_email_returns_whether_sent( bool $email_enabled, bool $wp_mail_result, bool $expected ): void {
+ add_filter( 'woocommerce_email_enabled_customer_stock_notification', fn() => $email_enabled );
+ $this->wp_mail_result = $wp_mail_result;
+ $notification = $this->build_notification();
+
+ $this->assertSame( $expected, $this->sut->send_stock_notification_email( $notification ) );
+ $this->assertSame( $email_enabled, isset( $this->sent_to[ $notification->get_user_email() ] ), 'Mail should only be attempted when the email is enabled.' );
+ }
+
+ /**
+ * Data provider for test_send_stock_notification_email_returns_whether_sent.
+ *
+ * @return array[]
+ */
+ public function stock_notification_send_outcome_provider(): array {
+ return array(
+ 'email sent' => array( true, true, true ),
+ 'email disabled' => array( false, true, false ),
+ 'wp_mail failed' => array( true, false, false ),
+ );
}
/**