Fix white page when saving checkout form with validation errors#344
Fix white page when saving checkout form with validation errors#344superdav42 merged 2 commits intomainfrom
Conversation
handle_save() always returned true, causing the calling code to call exit before the page could render. When validation fails (e.g. a new step without a submit button), the parent returns false but it was being discarded. Now the parent's return value is propagated correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughMultiple admin-page classes updated so their handle_save methods now return the parent::handle_save() result (and in most cases change the declared return type to bool), propagating the parent method's outcome to callers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔨 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/admin-pages/class-checkout-form-edit-admin-page.php (1)
1514-1551: 🛠️ Refactor suggestion | 🟠 MajorPattern inconsistency in sibling overrides may cause silent post-redirect execution.
Two sibling classes call
parent::handle_save()but declarevoidand never capture or return its result:
inc/admin-pages/class-domain-edit-admin-page.php—handle_save(): voidinc/admin-pages/class-product-edit-admin-page.php—handle_save(): voidBecause
voidmethods implicitly returnnull(falsy), the outer controller never callsexitafter a successful save — even thoughparent::handle_save()already sent aLocation:redirect header. The browser follows the redirect correctly, but PHP continues executing the rest of the admin page render cycle unnecessarily. On the error path these classes happen to work (null is falsy → no exit → page renders with notice), so there's no white-page regression there.Additionally,
inc/admin-pages/class-email-edit-admin-page.php,inc/admin-pages/class-payment-edit-admin-page.php, andinc/admin-pages/class-discount-code-edit-admin-page.phphave the same pattern.
inc/admin-pages/class-event-view-admin-page.phpunconditionally returnstrue— the same root pattern as the bug fixed here.
inc/admin-pages/class-site-edit-admin-page.phpalready implements the correct fix: it captures and explicitly returns the parent result without declaringvoid.Consider aligning these overrides to match the established pattern: capture and return the parent result, and change the return type from
voidtobool.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/admin-pages/class-checkout-form-edit-admin-page.php` around lines 1514 - 1551, Several sibling admin-page overrides (e.g., handle_save() in class-domain-edit-admin-page.php, class-product-edit-admin-page.php, class-email-edit-admin-page.php, class-payment-edit-admin-page.php, class-discount-code-edit-admin-page.php, and class-event-view-admin-page.php) declare void and call parent::handle_save() without capturing/returning its result, which prevents the outer controller from exiting after a redirect; change each override to capture the parent's return value (e.g., $result = parent::handle_save();) and return it (return $result;), and update the method signature from void to bool so the redirect-exit behavior is preserved (follow the pattern used in class-site-edit-admin-page.php and the working handle_save() in class-checkout-form-edit-admin-page.php).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@inc/admin-pages/class-checkout-form-edit-admin-page.php`:
- Around line 1514-1551: Several sibling admin-page overrides (e.g.,
handle_save() in class-domain-edit-admin-page.php,
class-product-edit-admin-page.php, class-email-edit-admin-page.php,
class-payment-edit-admin-page.php, class-discount-code-edit-admin-page.php, and
class-event-view-admin-page.php) declare void and call parent::handle_save()
without capturing/returning its result, which prevents the outer controller from
exiting after a redirect; change each override to capture the parent's return
value (e.g., $result = parent::handle_save();) and return it (return $result;),
and update the method signature from void to bool so the redirect-exit behavior
is preserved (follow the pattern used in class-site-edit-admin-page.php and the
working handle_save() in class-checkout-form-edit-admin-page.php).
Domain, Product, Email, Payment, and Discount Code edit pages all declared handle_save() as void and discarded parent::handle_save() return value. This prevented the calling code from calling exit after a successful redirect, causing unnecessary page rendering after the redirect header was sent. On validation errors, the void return (null/falsy) happened to work correctly, but the success path was still broken. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 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: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/admin-pages/class-payment-edit-admin-page.php (1)
1315-1331:⚠️ Potential issue | 🟠 MajorMembership side effects fire before the parent save is confirmed, risking double-activation on retry.
membership->add_to_times_billed(1)andmembership->renew(false, 'active')(lines 1321–1328) execute unconditionally beforeparent::handle_save(). Now that this PR correctly surfaces save errors instead of a white page, the failure path is reachable: ifparent::handle_save()returnsfalse(DB error, nonce failure, etc.), the membership is already activated while the payment remains unsaved. The admin sees the error and retries, triggering a second activation.Additionally, the result of
recalculate_totals()->save()at line 1317 is silently ignored;parent::handle_save()subsequently callsload_attributes_from_post()followed by anothersave(), potentially overwriting the recalculated totals with stale POST data.The
@todoat line 1311 already acknowledges this is problematic. A safer ordering:🔒 Proposed fix — guard membership activation on save outcome
public function handle_save(): bool { - $this->get_object()->recalculate_totals()->save(); - - $should_confirm_membership = wu_request('confirm_membership'); - - if ($should_confirm_membership) { - $membership = $this->get_object()->get_membership(); - - if ($membership) { - $membership->add_to_times_billed(1); - - $membership->renew(false, 'active'); - } - } - - return parent::handle_save(); + $result = parent::handle_save(); + + if ($result) { + $should_confirm_membership = wu_request('confirm_membership'); + + if ($should_confirm_membership) { + $membership = $this->get_object()->get_membership(); + + if ($membership) { + $membership->add_to_times_billed(1); + + $membership->renew(false, 'active'); + } + } + } + + return $result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/admin-pages/class-payment-edit-admin-page.php` around lines 1315 - 1331, The code runs membership side effects before confirming the payment save and ignores the result of recalculate_totals()->save(); update handle_save() so the object's attributes are loaded and totals recalculated, then perform the definitive save and check its boolean result (use get_object(), load_attributes_from_post(), recalculate_totals(), save() and parent::handle_save() as the single successful save point); only after parent::handle_save() returns true invoke membership side effects (get_membership(), add_to_times_billed(), renew()), and remove or propagate the earlier recalculate_totals()->save() so totals are not overwritten by subsequent load/save flows.
🧹 Nitpick comments (1)
inc/admin-pages/class-domain-edit-admin-page.php (1)
577-596:wu_enqueue_async_actionfires before the save result is known, queuing spurious background work on validation failure.Since the async domain stage job is enqueued at line 594 before
parent::handle_save()is called, any save failure (nonce mismatch, DB error, validation) will still trigger a background domain stage recheck for the unchanged domain. With this PR now properly surfacing validation errors, the failure path is hit by real users more frequently.♻️ Proposed fix — enqueue only on successful save
public function handle_save(): bool { // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce verification happens in parent::handle_save() if ( ! wu_request('primary_domain')) { $_POST['primary_domain'] = false; } // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce verification happens in parent::handle_save() if ( ! wu_request('active')) { $_POST['active'] = false; } // phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nounce verification happens in parent::handle_save() if ( ! wu_request('secure')) { $_POST['secure'] = false; } - wu_enqueue_async_action('wu_async_process_domain_stage', ['domain_id' => $this->get_object()->get_id()], 'domain'); - - return parent::handle_save(); + $result = parent::handle_save(); + + if ($result) { + wu_enqueue_async_action('wu_async_process_domain_stage', ['domain_id' => $this->get_object()->get_id()], 'domain'); + } + + return $result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/admin-pages/class-domain-edit-admin-page.php` around lines 577 - 596, In handle_save(), wu_enqueue_async_action('wu_async_process_domain_stage', ...) is being called before parent::handle_save(), which enqueues background work even when the save fails; change the logic so you call parent::handle_save() first, capture its boolean return, and only when it returns true invoke wu_enqueue_async_action with ['domain_id' => $this->get_object()->get_id()] and the 'domain' group; ensure the function still returns the parent's result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/admin-pages/class-discount-code-edit-admin-page.php`:
- Around line 847-849: Customer_Edit_Admin_Page::handle_save currently doesn't
return the parent result which causes the caller to miss the boolean and leads
to the white-page bug; change the method signature to return bool (matching
Edit_Admin_Page::handle_save) and replace the bare parent::handle_save(); call
with return parent::handle_save(); so the parent boolean result is propagated
(same pattern used in Discount_Code_Edit_Admin_Page).
---
Outside diff comments:
In `@inc/admin-pages/class-payment-edit-admin-page.php`:
- Around line 1315-1331: The code runs membership side effects before confirming
the payment save and ignores the result of recalculate_totals()->save(); update
handle_save() so the object's attributes are loaded and totals recalculated,
then perform the definitive save and check its boolean result (use get_object(),
load_attributes_from_post(), recalculate_totals(), save() and
parent::handle_save() as the single successful save point); only after
parent::handle_save() returns true invoke membership side effects
(get_membership(), add_to_times_billed(), renew()), and remove or propagate the
earlier recalculate_totals()->save() so totals are not overwritten by subsequent
load/save flows.
---
Nitpick comments:
In `@inc/admin-pages/class-domain-edit-admin-page.php`:
- Around line 577-596: In handle_save(),
wu_enqueue_async_action('wu_async_process_domain_stage', ...) is being called
before parent::handle_save(), which enqueues background work even when the save
fails; change the logic so you call parent::handle_save() first, capture its
boolean return, and only when it returns true invoke wu_enqueue_async_action
with ['domain_id' => $this->get_object()->get_id()] and the 'domain' group;
ensure the function still returns the parent's result.
| * @return bool | ||
| */ | ||
| public function handle_save(): void { | ||
| public function handle_save(): bool { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "class-customer-edit-admin-page.php" -type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 133
🏁 Script executed:
# Look for the handle_save method in the customer edit admin page
rg -n "function handle_save" --type=php -A 20 | grep -A 20 "customer-edit"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 3856
🏁 Script executed:
# Search more broadly for customer edit admin page
fd "customer" --type f | grep -i adminRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 302
🏁 Script executed:
# Get the full handle_save method from Customer_Edit_Admin_Page
sed -n '1165,1250p' ./inc/admin-pages/class-customer-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2233
🏁 Script executed:
# Also check what the parent class handle_save returns
rg -n "class Customer_Edit_Admin_Page" --type=php -A 5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 555
🏁 Script executed:
# Find the parent Edit_Admin_Page class and its handle_save method
rg -n "class Edit_Admin_Page" --type=php -A 5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 500
🏁 Script executed:
# Find Edit_Admin_Page::handle_save() definition
rg -n "function handle_save" --type=php inc/admin-pages/class-edit-admin-page.php -A 30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 811
🏁 Script executed:
# Get the exact lines mentioned in the review comment - lines 847-849 in discount code edit
sed -n '840,860p' ./inc/admin-pages/class-discount-code-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 522
🏁 Script executed:
# Check return types for other similar admin page classes
rg -n "public function handle_save\(\)" --type=php inc/admin-pages/ -B 0 -A 1 | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2231
🏁 Script executed:
# Check if there are other classes extending Edit_Admin_Page and their handle_save implementations
rg -n "extends Edit_Admin_Page" --type=php inc/admin-pages/Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1625
🏁 Script executed:
# Get more context around the Discount_Code_Edit_Admin_Page handle_save
sed -n '835,880p' ./inc/admin-pages/class-discount-code-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1098
🏁 Script executed:
# Check who calls handle_save() to understand the impact of void vs bool return type
rg -n "->handle_save\(\)" --type=php inc/admin-pages/ -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 485
🏁 Script executed:
# Double-check the exact return type signature in Customer_Edit_Admin_Page one more time
sed -n '1165,1170p' ./inc/admin-pages/class-customer-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 347
🏁 Script executed:
# Search for how handle_save() is called - look for the caller pattern
rg -n "handle_save\(\)" --type=php inc/admin-pages/class-base-admin-page.php -B 3 -A 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the base admin page to understand the calling pattern
fd "class-base-admin-page" --type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 122
🏁 Script executed:
# Look for the caller - usually in a controller or dispatcher
rg -n "handle_save" --type=php inc/admin-pages/class-base-admin-page.php -B 5 -A 5 | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Look at the base admin page definition to find the caller context
cat -n ./inc/admin-pages/class-base-admin-page.php | grep -A 10 -B 10 "handle_save"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Search for where handle_save gets invoked - look in all PHP files
rg -n 'handle_save' --type=php -B 3 -A 3 | grep -A 3 -B 3 'if.*handle_save\|return.*handle_save\|exit' | head -80Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 5899
🏁 Script executed:
# Check if Customer_Edit_Admin_Page might have been modified - look at git or other evidence of what the fix should be
# Let me look at the parent class signature more carefully
sed -n '820,880p' ./inc/admin-pages/class-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1187
🏁 Script executed:
# Verify the calling pattern - line 225 in Edit_Admin_Page
sed -n '220,235p' ./inc/admin-pages/class-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 359
🏁 Script executed:
# Check other void return classes to see if they have the same issue
rg -n "public function handle_save\(\): void" --type=php inc/admin-pages/ -A 80 | grep -A 80 "webhook\|view-logs\|template-previewer\|invoice-template\|email-template"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 32566
🏁 Script executed:
# Check if Checkout_Form_Edit_Admin_Page and Broadcast_Edit_Admin_Page have return statements
# to understand the full pattern
rg -n "public function handle_save" --type=php -A 100 inc/admin-pages/class-checkout-form-edit-admin-page.php | grep -A 100 "1514:" | tail -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1748
🏁 Script executed:
# Check Broadcast_Edit_Admin_Page
fd "broadcast" --type f inc/admin-pages/Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 184
🏁 Script executed:
# Check Broadcast_Edit_Admin_Page handle_save method
rg -n "public function handle_save" --type=php -A 50 inc/admin-pages/class-broadcast-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Also verify Membership and Site_Edit_Admin_Page patterns - check if Site correctly returns
sed -n '760,775p' ./inc/admin-pages/class-site-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 614
🏁 Script executed:
# Check Membership_Edit_Admin_Page
rg -n "public function handle_save" --type=php -A 50 inc/admin-pages/class-membership-edit-admin-page.php | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1531
🏁 Script executed:
# Final verification: Check the end of Customer_Edit_Admin_Page handle_save (around line 1245-1250)
sed -n '1245,1250p' ./inc/admin-pages/class-customer-edit-admin-page.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 164
Customer_Edit_Admin_Page::handle_save() must return the parent result to prevent white-page bug after customer edits.
Customer_Edit_Admin_Page::handle_save() is declared with void return type but calls parent::handle_save(); without returning its result (line 1249). The parent Edit_Admin_Page::handle_save() returns bool, which the caller at line 225 checks to decide whether to exit. Without returning the parent's result, the exit never executes after a successful save, causing the page to continue rendering instead of redirecting.
Fix: Change the return type to bool and use return parent::handle_save();, matching the pattern already applied to Discount_Code_Edit_Admin_Page and other similar classes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/admin-pages/class-discount-code-edit-admin-page.php` around lines 847 -
849, Customer_Edit_Admin_Page::handle_save currently doesn't return the parent
result which causes the caller to miss the boolean and leads to the white-page
bug; change the method signature to return bool (matching
Edit_Admin_Page::handle_save) and replace the bare parent::handle_save(); call
with return parent::handle_save(); so the parent boolean result is propagated
(same pattern used in Discount_Code_Edit_Admin_Page).
1 participant
Summary
Checkout_Form_Edit_Admin_Page::handle_save()always returnedtrue, even whenparent::handle_save()returnedfalsedue to validation errorsEdit_Admin_Pagecallsexitwhenhandle_save()returnstrue, which terminated page rendering before any output — producing a white pageTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit