fix(site-duplication): balance switch_to_blog stack so welcome emails send#1167
Conversation
… send MUCD_Files::copy_files() pushed two switch_to_blog() frames (source then target) onto the WordPress blog stack but only popped one. After the function returned the caller was still on the source/template blog context instead of the original network/admin context. Customers reported that new-site / welcome emails were silently dropped during template-based signups while a manual "send test" from the network admin worked. The leak puts wp_mail() into the template site's context, so it picks up that site's options (admin_email, blogname) and any per-site SMTP plugin set — almost always different from the network configuration the test path uses. Add a second restore_current_blog() to fully unwind the stack, and add a regression test class that drives MUCD_Files::copy_files() directly and asserts the caller's blog id and the global _wp_switched_stack are both unchanged after the call. The full duplicate_site() pipeline gets the same context-preservation assertion as a higher-level guard.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
Summary
Why test emails work but checkout emails don't
TestsNew file
PHPCS and PHPStan are clean on both modified files. Risk
Merged via PR #1167 to main. aidevops.sh v3.15.12 spent 34s on this as a headless bash routine. |
Summary
Why test emails work but checkout emails don't
TestsNew file
PHPCS and PHPStan are clean on both modified files. Risk
Merged via PR #1167 to main. aidevops.sh v3.15.12 spent 30s on this as a headless bash routine. |
🔨 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: |
|
Performance Test Results Performance test results for 20113c8 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
MUCD_Files::copy_files(): it pushes twoswitch_to_blog()frames onto the WordPress blog stack but only pops one. When duplication finishes, the request is still on the template/source blog instead of the network context.wp_mail()then runs against the template'sadmin_email,blogname, and any per-site SMTP plugin instead of the network's, so the welcome email never reaches the customer.restore_current_blog()so the stack is fully unwound. The bug has been there since the initial MUCD import, so no other call sites needed adjusting.Why test emails work but checkout emails don't
wp_mail()runswpmu_create_blog)Site_Duplicator::duplicate_site→copy_files)That asymmetry exactly matches what customers reported: emails send when an admin tests them, fail during template-based signup, and succeed for non-template signups.
Tests
New file
tests/WP_Ultimo/Helpers/Site_Duplicator_Context_Test.phpadds two regression tests that fail onmainand pass after the fix:test_copy_files_does_not_leak_blog_context— drivesMUCD_Files::copy_files()directly with the actual filesystem recursion stubbed out via the existingmucd_copy_dirsfilter, and asserts thatget_current_blog_id()andms_is_switched()are both unchanged after the call.test_duplicate_site_restores_blog_context— higher-level guard that runs the fullSite_Duplicator::duplicate_site()pipeline and asserts the caller's blog context is preserved end-to-end.Verified locally:
PHPCS and PHPStan are clean on both modified files.
Risk
restore_current_blog(); it does not change copy semantics or the order in which uploads info is read.switch_to_blog()calls already happened on every prior duplication, so the secondrestore_current_blog()matches what was already pushed onto the stack.