From aea2c2a1eb581204239967bb431db1da9999ea52 Mon Sep 17 00:00:00 2001 From: David Stone Date: Tue, 5 May 2026 01:30:19 -0600 Subject: [PATCH] fix(template-switch): emit JSON on AJAX failure so UI never hangs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Site_Duplicator::override_site() returned false the customer-panel 'Switch Template' AJAX handler returned void. The HTTP body was empty, the JS success callback threw on results.data.redirect_url (which was undefined), the exception was swallowed inside jQuery's success handler, and unblock() was never called. The customer was left staring at an indefinite loading spinner with no error indication. This change: - Adds an explicit wp_send_json_error() on the override_site() failure path so the JS error branch always fires. - Adds a defensive guard for the case where wu_get_current_site() returns null (e.g. the request runs outside a customer-site context), so the handler never tries to dereference a missing Site object. - Adds the missing 'return' after wp_send_json_error() in the not_authorized branch — execution previously continued into the override_site() call after the error response, which could double up output. - Moves the do_action('wu_after_switch_template') call inside the success branch. Previously it fired even when the switch failed, causing hooked code (cache clears, audit logs, notifications) to run for switches that did not actually happen. Front-end JS now also defends against a missing or malformed response body, so a future regression in the PHP contract cannot leave the loading spinner stuck. Adds Template_Switching_Element_Test exercising the input-validation failure paths to confirm they emit a parsable JSON error body. --- assets/js/template-switching.js | 48 ++++- inc/ui/class-template-switching-element.php | 52 +++-- .../UI/Template_Switching_Element_Test.php | 199 ++++++++++++++++++ 3 files changed, 281 insertions(+), 18 deletions(-) create mode 100644 tests/WP_Ultimo/UI/Template_Switching_Element_Test.php diff --git a/assets/js/template-switching.js b/assets/js/template-switching.js index 655fe5c7..317f9e2d 100644 --- a/assets/js/template-switching.js +++ b/assets/js/template-switching.js @@ -150,6 +150,27 @@ template_id: that.template_id, }, function(results) { + /* + * Defensive guard — if the server responds with an empty, + * non-JSON, or otherwise malformed body (which previously + * happened on the override_site() failure path), treat it + * as a generic failure so the customer is not left staring + * at the loading spinner forever. + */ + if ( ! results || typeof results !== 'object') { + + that.unblock(); + + that.confirm_switch = false; + + that.ready = false; + + wu_ajax_error('An error occurred while switching templates.'); + + return; + + } + /* * Handle error responses. */ @@ -173,21 +194,38 @@ } - wu_ajax_error(errorMessage); + wu_ajax_error(errorMessage); - return; + return; } /* - * Redirect of we get a redirect URL back. + * Redirect if we get a redirect URL back. Guard against + * a missing or malformed data payload so a server that + * answers success without a redirect URL does not throw + * inside this callback (which would silently leave the + * page-blocking spinner active). */ - if (typeof results.data.redirect_url === 'string') { + if (results.data && typeof results.data.redirect_url === 'string') { window.location.href = results.data.redirect_url; + return; + } // end if; + /* + * Success without a redirect URL — unblock and reset + * state so the customer can try again instead of being + * stuck on a loading overlay. + */ + that.unblock(); + + that.confirm_switch = false; + + that.ready = false; + }, function() { /* @@ -199,7 +237,7 @@ that.ready = false; - wu_ajax_error(null); + wu_ajax_error(null); }); diff --git a/inc/ui/class-template-switching-element.php b/inc/ui/class-template-switching-element.php index 442a4b02..aa32218c 100644 --- a/inc/ui/class-template-switching-element.php +++ b/inc/ui/class-template-switching-element.php @@ -272,6 +272,15 @@ public function switch_template() { $this->site = wu_get_current_site(); } + // Defensive guard — wu_get_current_site() can return false when the request + // runs outside a customer-site context. Without this, dereferencing + // $this->site below would emit no JSON body and the AJAX caller would + // hang on its loading spinner. + if ( ! $this->site || ! $this->site->get_id()) { + wp_send_json_error(new \WP_Error('site_context_missing', __('Could not determine which site to switch. Please reload the page and try again.', 'ultimate-multisite'))); + return; + } + $template_id = (int) wu_request('template_id', ''); // false means MODE_DEFAULT (no restriction) — all templates are allowed. @@ -279,6 +288,7 @@ public function switch_template() { if (false !== $available_templates && ! in_array($template_id, array_map('intval', $available_templates), true)) { wp_send_json_error(new \WP_Error('not_authorized', __('You are not allowed to use this template.', 'ultimate-multisite'))); + return; } if ( ! $template_id) { @@ -288,28 +298,44 @@ public function switch_template() { $switch = \WP_Ultimo\Helpers\Site_Duplicator::override_site($template_id, $this->site->get_id()); + if ( ! $switch) { + /* + * Site_Duplicator::override_site() returns false on any failure + * (user cap missing, copy_data error, Elementor Kit copy failure, + * etc.) without surfacing a reason. Without an explicit error + * response here, the AJAX call would close with an empty body, + * the JS success handler would throw on results.data.redirect_url, + * and the customer would see an indefinite loading spinner. + */ + wp_send_json_error(new \WP_Error('switch_failed', __('Could not switch the template. Please contact your network administrator.', 'ultimate-multisite'))); + return; + } + /** - * Allow plugin developers to hook functions after a user or super admin switches the site template + * Allow plugin developers to hook functions after a user or super admin switches the site template. + * + * Only fires on a successful switch — previously this fired even on failure, + * which caused hooked code (cache clears, notifications, audit logs) to run + * for switches that did not actually happen. * * @since 1.9.8 * @param int $id Site ID * @return void */ do_action('wu_after_switch_template', $this->site->get_id()); + $referer = isset($_SERVER['HTTP_REFERER']) ? sanitize_url(wp_unslash($_SERVER['HTTP_REFERER'])) : ''; - if ($switch) { - wp_send_json_success( - [ - 'redirect_url' => add_query_arg( - [ - 'updated' => 1, - ], - $referer - ), - ] - ); - } + wp_send_json_success( + [ + 'redirect_url' => add_query_arg( + [ + 'updated' => 1, + ], + $referer + ), + ] + ); } /** diff --git a/tests/WP_Ultimo/UI/Template_Switching_Element_Test.php b/tests/WP_Ultimo/UI/Template_Switching_Element_Test.php new file mode 100644 index 00000000..7f4711af --- /dev/null +++ b/tests/WP_Ultimo/UI/Template_Switching_Element_Test.php @@ -0,0 +1,199 @@ +setAccessible( true ); + } + + $ref->setValue( $element, null ); + + parent::tear_down(); + } + + /** + * Install AJAX die handler so wp_send_json_* don't kill PHPUnit. + * + * @return callable The installed handler. + */ + private function install_ajax_die_handler(): callable { + + add_filter( 'wp_doing_ajax', '__return_true' ); + + $handler = function () { + return function ( $message ) { + throw new \WPAjaxDieContinueException( (string) $message ); + }; + }; + + add_filter( 'wp_die_ajax_handler', $handler, 1 ); + + return $handler; + } + + /** + * Remove the AJAX die handler. + * + * @param callable $handler The handler returned by install_ajax_die_handler(). + */ + private function remove_ajax_die_handler( callable $handler ): void { + + remove_filter( 'wp_doing_ajax', '__return_true' ); + remove_filter( 'wp_die_ajax_handler', $handler, 1 ); + } + + /** + * Call switch_template() inside an AJAX context, capturing JSON output. + * + * @return array{output: string, exception: bool} + */ + private function call_switch_template(): array { + + $handler = $this->install_ajax_die_handler(); + $exception_caught = false; + + ob_start(); + + try { + Template_Switching_Element::get_instance()->switch_template(); + } catch ( \WPAjaxDieContinueException $e ) { + $exception_caught = true; + } + + $output = ob_get_clean(); + + $this->remove_ajax_die_handler( $handler ); + + return [ + 'output' => $output, + 'exception' => $exception_caught, + ]; + } + + /** + * Decode a JSON response body, asserting it is a non-empty array. + * + * @param string $output Raw output from the AJAX handler. + * @return array + */ + private function decode_json( string $output ): array { + + $this->assertNotEmpty( + $output, + 'AJAX handler emitted an empty body. The front-end JS will hang on its loading spinner without a parsable JSON response.' + ); + + $decoded = json_decode( $output, true ); + + $this->assertIsArray( + $decoded, + 'AJAX handler emitted output that was not valid JSON: ' . $output + ); + + return $decoded; + } + + /** + * Missing site context must yield a JSON error body, not silence. + * + * Regression guard for the indefinite-spinner bug: a NULL $this->site + * with no current-site fallback used to dereference NULL and produce + * no body at all. + */ + public function test_switch_template_missing_site_emits_json_error(): void { + + $_REQUEST['template_id'] = '0'; + + $result = $this->call_switch_template(); + + $this->assertTrue( + $result['exception'], + 'wp_send_json_error must be reached so the AJAX response terminates cleanly.' + ); + + $decoded = $this->decode_json( $result['output'] ); + + $this->assertSame( false, $decoded['success'] ); + $this->assertNotEmpty( $decoded['data'], 'Error payload must include a message for the JS to display.' ); + } + + /** + * Empty template_id must yield a JSON error body, not silence. + */ + public function test_switch_template_missing_template_id_emits_json_error(): void { + + // Force a fake site object so the "missing site context" guard is bypassed + // and we hit the template_id check. + $site_id = $this->factory()->blog->create(); + $site = wu_get_site( $site_id ); + $site->set_type( 'customer_owned' ); + $site->save(); + + $element = Template_Switching_Element::get_instance(); + $ref = new \ReflectionProperty( $element, 'site' ); + + if ( PHP_VERSION_ID < 80100 ) { + $ref->setAccessible( true ); + } + + $ref->setValue( $element, $site ); + + // No template_id set in $_REQUEST. + $result = $this->call_switch_template(); + + $this->assertTrue( + $result['exception'], + 'wp_send_json_error must be reached for missing template_id.' + ); + + $decoded = $this->decode_json( $result['output'] ); + $this->assertSame( false, $decoded['success'] ); + } + +}