fix(woocommerce): scope payment notice to recoverable statuses#301
Open
jason10lee wants to merge 7 commits into
Open
fix(woocommerce): scope payment notice to recoverable statuses#301jason10lee wants to merge 7 commits into
jason10lee wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows when the WooCommerce “update payment method” notice is generated by gating it to recoverable subscription statuses only, preventing notices on terminal or otherwise non-actionable subscriptions (e.g., expired subscriptions with stale unpaid renewal orders).
Changes:
- Added an explicit subscription-status allowlist (
on-hold,pending) for when the “needs payment” notice is actionable. - Added unit tests covering excluded statuses, the reported expired+stale-order incident, and regression guards for
on-hold/pending. - Extended WooCommerce test mocks to support the new notice code paths (
needs_payment(),get_view_order_url(), andwcs_get_subscriptions()).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| plugins/newspack-plugin/includes/plugins/woocommerce/class-woocommerce-update-payment-notice.php | Adds a recoverable-status allowlist and uses it to gate notice generation. |
| plugins/newspack-plugin/tests/unit-tests/plugins/woocommerce-update-payment-notice.php | Introduces a focused unit test suite for the allowlist/status behavior and incident regression. |
| plugins/newspack-plugin/tests/mocks/wc-mocks.php | Updates WC/WCS mocks to support wcs_get_subscriptions() and subscription methods used by the notice logic. |
…t.1 [skip ci] # newspack-components [4.4.0-hotfix-nppm-2926-payment-notice-status-allowlist.1](https://github.com/Automattic/newspack-workspace/compare/newspack-components@4.3.0...newspack-components@4.4.0-hotfix-nppm-2926-payment-notice-status-allowlist.1) (2026-06-13) ### Bug Fixes * **components:** restore className and disabled props on CardSettingsGroup ([226762e](226762e)) ### Features * **reader-activation:** replace RAS auto-enable with manual toggle ([#196](#196)) ([71d2165](71d2165))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
Users continued to see the My Account / snackbar "Your '…' subscription is not active. Please update your payment method." notice following previous efforts to limit where it appeared.
This PR further limits that notice to subscriptions in a status where paying can actually restore them —
on-hold(failed renewal) andpending(awaiting initial payment).Previously we skipped only
cancelleditems and otherwise trustedneeds_payment(). An expired (or otherwise terminal) subscription that still carries a stale, unpaid failed-renewal order tripsneeds_payment(), so finished subscriptions kept firing a notice the reader had no way to resolve — telling active subscribers they were "not active."This replaces the
cancelled-only skip with an explicit recoverable-status allowlist.expired/cancelled/switched(terminal) andactive/pending-cancel(reader still has access) are intentionally excluded. A guard test pins the allowlist against the known WooCommerce Subscriptions status set so any future/custom status surfaces for a human decision rather than being silently skipped.Scope: This is a hotfix that fully resolves the reported incident. A broader preventative adjustment that will suppress the notice when an active subscription grants the same membership through a different product (the "same product was too narrow" follow-up) will ship separately on our typical release cadence and is not included here.
Closes NPPM-2926: Update Payment Notice still fires for subscribers holding both active and expired subscriptions.
How to test the changes in this Pull Request:
on-holdwith a payment due → the notice does appear with an "update your payment method" link.pending(initial payment not completed) also still shows the notice.plugins/newspack-plugin, runn test-php --group update_payment_notice→ OK (10 tests). The suite covers each excluded status (incl. the expired-with-stale-order incident), theon-hold/pendingfire paths, the not-needs-payment case, and the allowlist-vs-WCS-status-registry guard.Other information:
Cross-publisher impact was reviewed (reader-revenue / third-party-integrations surface): no Newspack repo references this class, the new constant, or the touched filter; the change only ever narrows when the notice shows, so there is no silent-breakage vector. Production diff is one new constant plus the allowlist gate; the rest is tests + test mocks.