| @@ -601,66 +601,72 @@ | ||
| 601 | 601 | * |
| 602 | 602 | * @return array Connection test results |
| 603 | 603 | */ |
| 604 | 604 | public function test_connections(): array { |
| 605 | + // Every entry carries a `message`, configured or not: the result is | |
| 606 | + // handed to the Integrations screen as JSON, and a key that exists on | |
| 607 | + // some services and not others reads as `undefined` there. | |
| 605 | 608 | $results = [ |
| 606 | - 'google_analytics' => ['status' => 'not_configured'], | |
| 607 | - 'search_console' => ['status' => 'not_configured'], | |
| 608 | - 'pagespeed' => ['status' => 'not_configured'] | |
| 609 | + 'google_analytics' => ['status' => 'not_configured', 'message' => ''], | |
| 610 | + 'search_console' => ['status' => 'not_configured', 'message' => ''], | |
| 611 | + 'pagespeed' => ['status' => 'not_configured', 'message' => ''] | |
| 609 | 612 | ]; |
| 610 | 613 | |
| 611 | - // Test Google Analytics connection | |
| 612 | - if ($this->analytics_client) { | |
| 613 | - try { | |
| 614 | - $test_result = $this->analytics_client->test_connection(); | |
| 615 | - $results['google_analytics'] = [ | |
| 616 | - 'status' => $test_result['success'] ? 'connected' : 'error', | |
| 617 | - 'message' => $test_result['message'], | |
| 618 | - 'details' => $test_result | |
| 619 | - ]; | |
| 620 | - } catch (\Exception $e) { | |
| 621 | - $results['google_analytics'] = [ | |
| 622 | - 'status' => 'error', | |
| 623 | - 'message' => $e->getMessage() | |
| 624 | - ]; | |
| 614 | + // One shape for all three. They were three copies of the same block | |
| 615 | + // differing only in the client, which is how the same unguarded read | |
| 616 | + // came to exist in triplicate (#852). | |
| 617 | + $clients = [ | |
| 618 | + 'google_analytics' => $this->analytics_client, | |
| 619 | + 'search_console' => $this->search_console_client, | |
| 620 | + 'pagespeed' => $this->pagespeed_client, | |
| 621 | + ]; | |
| 622 | + | |
| 623 | + foreach ($clients as $service => $client) { | |
| 624 | + if (!$client) { | |
| 625 | + continue; | |
| 625 | 626 | } |
| 627 | + | |
| 628 | + $results[$service] = $this->describe_connection_test($client); | |
| 626 | 629 | } |
| 627 | 630 | |
| 628 | - // Test Search Console connection | |
| 629 | - if ($this->search_console_client) { | |
| 630 | - try { | |
| 631 | - $test_result = $this->search_console_client->test_connection(); | |
| 632 | - $results['search_console'] = [ | |
| 633 | - 'status' => $test_result['success'] ? 'connected' : 'error', | |
| 634 | - 'message' => $test_result['message'], | |
| 635 | - 'details' => $test_result | |
| 636 | - ]; | |
| 637 | - } catch (\Exception $e) { | |
| 638 | - $results['search_console'] = [ | |
| 639 | - 'status' => 'error', | |
| 640 | - 'message' => $e->getMessage() | |
| 641 | - ]; | |
| 631 | + return $results; | |
| 632 | + } | |
| 633 | + | |
| 634 | + /** | |
| 635 | + * Run one client's connection test and report it in a fixed shape. | |
| 636 | + * | |
| 637 | + * The clients answer success with `message` and failure with `error`, and | |
| 638 | + * this read `$test_result['message']` unconditionally — so a failed test | |
| 639 | + * raised "Undefined array key" and handed the admin an error with | |
| 640 | + * `message => null`. The reason the client had in hand was the one thing | |
| 641 | + * the screen needed. The clients now set `message` on both branches; the | |
| 642 | + * fallbacks here cover a client that does not, including one that answers | |
| 643 | + * with neither key. | |
| 644 | + * | |
| 645 | + * @since 2.12.0 | |
| 646 | + * | |
| 647 | + * @param object $client A client exposing test_connection(): array. | |
| 648 | + * @return array{status:string, message:string, details?:array} | |
| 649 | + */ | |
| 650 | + private function describe_connection_test($client): array { | |
| 651 | + try { | |
| 652 | + $test_result = $client->test_connection(); | |
| 653 | + | |
| 654 | + if (!is_array($test_result)) { | |
| 655 | + return ['status' => 'error', 'message' => '']; | |
| 642 | 656 | } |
| 643 | - } | |
| 644 | 657 | |
| 645 | - // Test PageSpeed connection | |
| 646 | - if ($this->pagespeed_client) { | |
| 647 | - try { | |
| 648 | - $test_result = $this->pagespeed_client->test_connection(); | |
| 649 | - $results['pagespeed'] = [ | |
| 650 | - 'status' => $test_result['success'] ? 'connected' : 'error', | |
| 651 | - 'message' => $test_result['message'], | |
| 652 | - 'details' => $test_result | |
| 653 | - ]; | |
| 654 | - } catch (\Exception $e) { | |
| 655 | - $results['pagespeed'] = [ | |
| 656 | - 'status' => 'error', | |
| 657 | - 'message' => $e->getMessage() | |
| 658 | - ]; | |
| 659 | - } | |
| 658 | + return [ | |
| 659 | + 'status' => !empty($test_result['success']) ? 'connected' : 'error', | |
| 660 | + 'message' => (string) ($test_result['message'] ?? $test_result['error'] ?? ''), | |
| 661 | + 'details' => $test_result, | |
| 662 | + ]; | |
| 663 | + } catch (\Exception $e) { | |
| 664 | + return [ | |
| 665 | + 'status' => 'error', | |
| 666 | + 'message' => $e->getMessage(), | |
| 667 | + ]; | |
| 660 | 668 | } |
| 661 | - | |
| 662 | - return $results; | |
| 663 | 669 | } |
| 664 | 670 | |
| 665 | 671 | /** |
| 666 | 672 | * Get analytics dashboard data |