fix(woo-memberships): prevent fatal when membership plan has been deleted

[wil-gerken]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

In Woocommerce_Memberships\Events, listener methods called $user_membership->get_plan()->get_id() without checking if get_plan() returns null. When a membership's plan post has been deleted, get_plan() returns false, and calling ->get_id() on false is a PHP fatal.

This fatal can kill the node pull process mid-execution via the network_user_deleted path (where $pause_events is not set), leaving the pull pointer stuck.

Adds null checks on get_plan() in membership_status_changed() and membership_deleted(), matching the existing get_user() null-check pattern already present in those methods.

Related to NPPM-2742.

⚠️ Parked draft - membership_transferred() needs the same guard before this is ready. Reopening this as a fresh PR when that work resumes.

How to test the changes in this Pull Request:

Reproduce the problem:

1. On a node site, create a WC membership plan and assign a user to it
2. To simulate the problem, delete the plan post directly from the database (bypassing WC Memberships hooks): wp db query 'DELETE FROM wp_posts WHERE ID = <plan_id>;' Then wp cache flush
3. Call the listener directly. The process will die silently after the call with no output (PHP fatal, exit code 1).
   wp eval '$m = wc_memberships_get_user_membership(<membership_id>); $r = Newspack_Network\Woocommerce_Memberships\Events::membership_deleted($m); var_dump($r);'

Verify the fix:

1. With the fix applied, repeat step 3 above
2. The process completes and prints NULL. The listener returns early cleanly instead of fataling
3. Run tests: n test-php --group membership-events — all tests pass

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[Copilot]

Pull request overview

Prevents PHP fatals in WooCommerce Memberships event listeners when a membership’s plan has been deleted (i.e., get_plan() returns falsey), which could otherwise halt node pull processing.

Changes:

• Add guard clauses in Events::membership_status_changed() and Events::membership_deleted() to safely handle missing plans.
• Add a new unit test file covering the missing-plan scenario for membership_deleted().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
includes/woocommerce-memberships/class-events.php	Adds get_plan() null/falsey checks before calling ->get_id() in two event listeners.
tests/unit-tests/test-membership-events-null-plan.php	Adds regression tests for deleted/missing membership plans (currently focused on membership_deleted()).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The file docblock references Events::membership_transferred(), but includes/woocommerce-memberships/class-events.php does not define that method; the regression described in this PR appears to be in membership_status_changed() and membership_deleted(). Please update the docblock to reference the correct listener(s) so the test documentation matches reality.

Suggested change

	* Events::membership_transferred() when calling get_plan()->get_id() on null.
	* Events::membership_status_changed() when calling get_plan()->get_id() on null.

[Copilot]

The real failure mode described in the PR is get_plan() returning false when the plan post is deleted, but this test simulates a missing plan with null and the mock documents get_plan() as object|null. Consider using false (and updating the mock/docblocks) so the test matches WooCommerce Memberships behavior more closely and continues to validate the guard even if the production code changes to strict checks.

[Copilot]

In this test, $plan and the assigned $plan->get_id closure are unused (the test uses $mock_plan instead). Removing the dead code will reduce confusion about how the mock plan is supposed to behave.

Suggested change

	$plan = (object) [ 'id' => $plan_id ];
	$plan->get_id = function() use ( $plan_id ) {
	return $plan_id;
	};

[Copilot]

This PR adds a new early-return path in membership_status_changed() when the plan is missing, but the new unit test file only exercises membership_deleted(). Please add a regression test for membership_status_changed() with a membership whose get_plan() returns falsey to ensure this listener also avoids the fatal and returns early as intended.

[Copilot]

The plan null-check + $plan_id extraction logic is now duplicated in both membership_status_changed() and membership_deleted(). Consider extracting a small private helper (e.g., get_plan_id_from_membership( $user_membership )) to keep the guard behavior consistent and reduce the chance of future divergence.

[github-actions]

🎉 This PR is included in version 2.20.0-hotfix-woo-membership-null-plan-fatal.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀