Skip to content

Commit 98b0e2d

Browse files
committed
fix(emails): address Copilot review feedback (NPPD-1524)
- card-expiry-warning unit test: assert cancellation and woo-renewal-reminder indices are not false before comparing ordering. - card-expiry-warning smoke test: add Reader Activation prereq check so it fails fast instead of silently capturing 0 emails; drop the temporary "delete after merge" notice; add a README documenting tests/integration/. - Card_Expiry_Warning::maybe_send_warning(): use $customer->first_name as a fallback when subscription billing_first_name is empty. - Card_Expiry_Warning::clear_sent_flag(): drop the unused $new_payment_method parameter and matching accepted_args from the action.
1 parent 0e73f3f commit 98b0e2d

4 files changed

Lines changed: 46 additions & 17 deletions

File tree

includes/plugins/woocommerce-subscriptions/class-card-expiry-warning.php

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,7 @@ public static function init() {
4444
add_filter( 'newspack_email_configs', [ __CLASS__, 'add_email_config' ] );
4545
add_action( 'init', [ __CLASS__, 'schedule_cron' ] );
4646
add_action( self::CRON_HOOK, [ __CLASS__, 'scan_expiring_cards' ] );
47-
add_action(
48-
'woocommerce_subscription_payment_method_updated',
49-
[ __CLASS__, 'clear_sent_flag' ],
50-
10,
51-
2
52-
);
47+
add_action( 'woocommerce_subscription_payment_method_updated', [ __CLASS__, 'clear_sent_flag' ] );
5348
add_action( 'newspack_deactivation', [ __CLASS__, 'unschedule_cron' ] );
5449
}
5550

@@ -256,10 +251,15 @@ private static function maybe_send_warning( $subscription, $token ) {
256251
? date_i18n( get_option( 'date_format', 'F j, Y' ), $subscription->get_time( 'next_payment' ) )
257252
: __( 'your next renewal', 'newspack-plugin' );
258253

254+
$first_name = $subscription->get_billing_first_name();
255+
if ( '' === $first_name ) {
256+
$first_name = $customer->first_name;
257+
}
258+
259259
$placeholders = [
260260
[
261261
'template' => '*BILLING_FIRST_NAME*',
262-
'value' => esc_html( $subscription->get_billing_first_name() ),
262+
'value' => esc_html( $first_name ),
263263
],
264264
[
265265
'template' => '*CARD_LAST_4*',
@@ -296,10 +296,9 @@ private static function maybe_send_warning( $subscription, $token ) {
296296
*
297297
* Hooked to 'woocommerce_subscription_payment_method_updated'.
298298
*
299-
* @param \WC_Subscription $subscription The subscription.
300-
* @param string $new_payment_method The new payment method ID.
299+
* @param \WC_Subscription $subscription The subscription.
301300
*/
302-
public static function clear_sent_flag( $subscription, $new_payment_method ) {
301+
public static function clear_sent_flag( $subscription ) {
303302
$subscription->delete_meta_data( self::SENT_META );
304303
$subscription->save();
305304
}

tests/integration/README.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Integration Smoke Tests
2+
3+
Integration smoke tests that exercise Newspack features against a real WordPress + WooCommerce environment. Unlike the PHPUnit suite under `tests/unit-tests/`, these scripts are run manually via `wp eval-file` and require the full plugin stack to be active.
4+
5+
## Running
6+
7+
```bash
8+
wp eval-file tests/integration/<script>.php
9+
```
10+
11+
Each script prints `PASS`/`FAIL` lines per scenario, a final `N/M PASSED` summary, and exits non-zero on any failure. Scripts create their own fixtures and clean up after themselves.
12+
13+
## Scripts
14+
15+
### `card-expiry-warning-smoke.php`
16+
17+
End-to-end coverage for the Card Expiry Warning email (`includes/plugins/woocommerce-subscriptions/class-card-expiry-warning.php`).
18+
19+
**Requires:** WooCommerce, WooCommerce Subscriptions, Newspack Newsletters, and Reader Activation enabled.
20+
21+
Exercises cron scheduling, happy-path send, idempotency, the payment-method-update flag clear, new-card re-send, and the unattached-card no-op.

tests/integration/card-expiry-warning-smoke.php

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,18 @@
1414
* 2. Happy-path send (token → subscription → email)
1515
* 3. Idempotency (no duplicate send)
1616
* 4. clear_sent_flag() handler clears meta (called directly; the full
17-
* woocommerce_subscription_payment_method_updated action fatals on
18-
* WCS PayPal's hook which expects 3 args)
17+
* woocommerce_subscription_payment_method_updated action triggers
18+
* third-party hooks like WCS PayPal that fatal on a minimal fixture)
1919
* 5. New card triggers new send
2020
* 6. Unattached card does not trigger email
2121
* 7. Cleanup
2222
*
23-
* Delete this file after the PR merges.
24-
*
2523
* @package Newspack\Tests
2624
*/
2725

2826
use Newspack\Card_Expiry_Warning;
2927
use Newspack\Emails;
28+
use Newspack\Reader_Activation;
3029

3130
// ── Globals ──────────────────────────────────────────────────────────
3231
// wp eval-file runs via eval(), so file-scope vars are local to the eval
@@ -82,6 +81,14 @@ function smoke_fail( string $label, string $detail = '' ): void {
8281
if ( ! function_exists( 'wcs_create_subscription' ) ) {
8382
WP_CLI::error( 'wcs_create_subscription() not available. Aborting.' );
8483
}
84+
85+
// Card_Expiry_Warning::init() only registers the email config when
86+
// WooCommerce_Subscriptions::is_enabled() returns true, which in turn
87+
// requires Reader Activation to be enabled. Without this, scan_expiring_cards()
88+
// silently captures 0 emails and the test gives a misleading failure.
89+
if ( ! Reader_Activation::is_enabled() ) {
90+
WP_CLI::error( 'Reader Activation is not enabled. Aborting.' );
91+
}
8592
WP_CLI::log( 'Prerequisites OK.' );
8693

8794
// ── Intercept wp_mail via pre_wp_mail ────────────────────────────────
@@ -290,10 +297,10 @@ function () {
290297

291298
// Call clear_sent_flag() directly rather than firing the full
292299
// woocommerce_subscription_payment_method_updated action. That action
293-
// triggers third-party hooks (WCS PayPal, etc.) whose callbacks expect
294-
// 3 args and fatal with our minimal test fixture.
300+
// triggers third-party hooks (WCS PayPal, etc.) that fatal with our
301+
// minimal test fixture.
295302
$subscription = wcs_get_subscription( $sub_id );
296-
Card_Expiry_Warning::clear_sent_flag( $subscription, 'stripe' );
303+
Card_Expiry_Warning::clear_sent_flag( $subscription );
297304

298305
$subscription = wcs_get_subscription( $sub_id );
299306
$meta_val = $subscription->get_meta( '_newspack_card_expiry_warning_sent', true );

tests/unit-tests/card-expiry-warning.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ public function test_registry_entry_order() {
181181
$renewal_reminder_idx = array_search( 'woo-renewal-reminder', $slugs, true );
182182

183183
$this->assertNotFalse( $card_expiry_idx, 'card-expiry-warning should be in the registry.' );
184+
$this->assertNotFalse( $cancellation_idx, 'cancellation should be in the registry.' );
185+
$this->assertNotFalse( $renewal_reminder_idx, 'woo-renewal-reminder should be in the registry.' );
184186
$this->assertGreaterThan( $cancellation_idx, $card_expiry_idx, 'card-expiry-warning should appear after cancellation.' );
185187
$this->assertLessThan( $renewal_reminder_idx, $card_expiry_idx, 'card-expiry-warning should appear before woo-renewal-reminder.' );
186188
}

0 commit comments

Comments
 (0)