test(checkout-pages): regression tests for subsite password reset (#1168)#1172
Conversation
) Locks in the subsite-aware behaviour added in #1169 so the bug cannot silently regress again. The original GH#1168 bug was masked for years because an existing test test_replace_reset_password_link_returns_early_on_subsite was actively asserting the buggy behaviour as a contract. Replacing that test and adding nine new tests covers all four hooks introduced in #1169: - replace_reset_password_link: subsite host is preserved, wp-login.php is removed from the URL, all standard query args (action, key, login, wp_lang) are kept, and the wu_subsite_password_reset_url integration filter actually fires. - retrieve_password_message hook is registered on subsites (not just main site). - rewrite_new_user_notification_email: welcome email links stay on the subsite for users created on a subsite. - rewrite_password_notification_email: admin notification email stays on the subsite. - rewrite_email_change_content: profile email-change confirmation links stay on the subsite. Each test includes an empty-input safety case where applicable. Verification: reverted inc/checkout/class-checkout-pages.php to its pre-#1169 state and re-ran the new tests; 9/13 fail (4 errors from the new methods being absent, 5 failures from the subsite host assertions), proving the tests catch the regression. Ref #1168
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR expands test coverage for subsite password reset URL rewriting and email notification handling. It adds nine new test methods, renames one existing test to reflect actual behavior, and updates fixture data in existing tests to use full userdata objects instead of minimal ID arrays, validating GH#1168 integration work. ChangesPassword Reset and Email Rewriting Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
SummaryAdds regression tests for the subsite password-reset bug fixed in #1169 (issue #1168). What's covered
Verification that the tests catch the bugI reverted
Bisect note for #1168While reviewing #1169 I traced the bug history. The customer reported "a recent change introduced this bug", and the closest match is:
Test planvendor/bin/phpunit --filter 'replace_reset_password_link|retrieve_password_message_filter|rewrite_new_user_notification_email|rewrite_password_notification_email|rewrite_email_change_content' --no-coverage
# 13 / 13 (100%) OK (13 tests, 37 assertions)PHPCS clean for the new additions. The pre-existing aidevops.sh v3.15.29 plugin for OpenCode v1.14.41 with claude-sonnet-4-6 spent 1d 20h and 40 tokens on this as a headless worker. Merged via PR #1172 to main. |
|
Performance Test Results Performance test results for d846c04 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
Summary
Adds regression tests for the subsite password-reset bug fixed in #1169 (issue #1168).
The original GH#1168 bug was masked for years because an existing test,
test_replace_reset_password_link_returns_early_on_subsite, was actively asserting the buggy behaviour as a contract — it expected the function to return the message unchanged on a subsite, which is precisely what was breaking customer password resets. This PR replaces that test and adds nine new tests covering all four hooks introduced in #1169.What's covered
replace_reset_password_link— on a subsite the URL is rewritten away fromwp-login.php, the subsite host is preserved, and standard query args (action,key,login,wp_lang) are kept so existing handlers (WooCommerce my-account, BuddyPress, default wp-login fallback) can complete the reset.wu_subsite_password_reset_urlfilter — verified to fire on subsites so integrations can override the destination URL.retrieve_password_messagehook registration — confirmed registered on subsites whenenable_custom_login_pageis on. Before [URGENT] fix(checkout-pages): rewrite password reset URL on subsites (#1168) #1169 it was wrapped inif (is_main_site())so it never ran on subsites at all.rewrite_new_user_notification_email— welcome email "set your password" links stay on the subsite for users created there. Plus an empty-message safety test.rewrite_password_notification_email— admin notification email stays on the subsite host.rewrite_email_change_content— profile email-change confirmation links stay on the subsite. Plus an empty-input safety test.Verification that the tests catch the bug
I reverted
inc/checkout/class-checkout-pages.phpto its pre-#1169 state on the same branch and re-ran the new tests:Restoring the #1169 fix returns 13/13 green, 37 assertions. The test net is doing its job.
Bisect note for #1168
While reviewing #1169 I traced the bug history. The customer reported "a recent change introduced this bug", and the closest match is:
e8f7955c(initial commit) —replace_reset_password_link()already had theif (!is_main_site()) return $message;early-return. The bug has been present from day one but was latent.706d9637"fix: keep password reset on subsite domain (Password Reset redirect error #291)", shipped in v2.5.0 — madelostpassword_urlsubsite-aware. From v2.5.0 onward, subsite users actually started submitting reset requests on their own subsite, which firedretrieve_password()on that subsite, which hit the long-dormant guard. This is the change that turned a latent bug into a customer-visible regression.That second-order effect is exactly why this kind of regression test belongs in the suite — pinning the contract on subsites means a future "fix" to a related path can't silently re-disable it.
Test plan
PHPCS clean for the new additions. The pre-existing
test_maybe_enqueue_payment_status_poll_*failures observed locally are baseline test-isolation issues unrelated to this PR or to #1169.Ref #1168
aidevops.sh v3.15.29 plugin for OpenCode v1.14.41 with claude-sonnet-4-6 spent 1d 20h and 40 tokens on this as a headless worker.
Summary by CodeRabbit