| @@ -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 |
| @@ -744,29 +750,17 @@ | ||
| 744 | 750 | // Fallback to old client if new one fails init (shouldn't happen if they use same creds) |
| 745 | 751 | $search_performance = $this->search_console_client->get_search_performance($site_url, $date_range, ['query'], 1000); |
| 746 | 752 | } |
| 747 | 753 | |
| 748 | - // Calculate position distribution | |
| 749 | - $position_distribution = [ | |
| 750 | - 'top_3' => 0, | |
| 751 | - '4_10' => 0, | |
| 752 | - '10_50' => 0, | |
| 753 | - '51_100' => 0 | |
| 754 | - ]; | |
| 754 | + // Position distribution over every query with an | |
| 755 | + // impression, not over the 1,000-row list above (#913). | |
| 756 | + $position_distribution = $this->search_analytics_client | |
| 757 | + ? $this->count_position_distribution($site_url, $start_date, $end_date, $search_performance['rows'] ?? []) | |
| 758 | + : self::bucket_positions( | |
| 759 | + $search_performance['rows'] ?? [], | |
| 760 | + count($search_performance['rows'] ?? []) < 1000 | |
| 761 | + ); | |
| 755 | 762 | |
| 756 | - foreach ($search_performance['rows'] ?? [] as $row) { | |
| 757 | - $position = $row['position'] ?? 0; | |
| 758 | - if ($position <= 3) { | |
| 759 | - $position_distribution['top_3']++; | |
| 760 | - } elseif ($position <= 10) { | |
| 761 | - $position_distribution['4_10']++; | |
| 762 | - } elseif ($position <= 50) { | |
| 763 | - $position_distribution['10_50']++; | |
| 764 | - } elseif ($position <= 100) { | |
| 765 | - $position_distribution['51_100']++; | |
| 766 | - } | |
| 767 | - } | |
| 768 | - | |
| 769 | 763 | $dashboard_data['search_performance'] = array_merge($search_performance, [ |
| 770 | 764 | 'totals' => $totals, |
| 771 | 765 | 'position_distribution' => $position_distribution |
| 772 | 766 | ]); |
| @@ -805,8 +799,132 @@ | ||
| 805 | 799 | // long-lived GSC payload has been stored. |
| 806 | 800 | $dashboard_data['core_web_vitals'] = $this->get_dashboard_core_web_vitals(); |
| 807 | 801 | |
| 808 | 802 | return $dashboard_data; |
| 803 | + } | |
| 804 | + | |
| 805 | + /** | |
| 806 | + * Rows per page when counting the position distribution. The most the | |
| 807 | + * Search Analytics API returns in one request. | |
| 808 | + * | |
| 809 | + * @since 2.15.0 | |
| 810 | + */ | |
| 811 | + private const POSITION_PAGE_SIZE = 25000; | |
| 812 | + | |
| 813 | + /** | |
| 814 | + * Pages read before the count stops: 200,000 queries. A property past | |
| 815 | + * that is counted over its top 200,000 by clicks and flagged incomplete. | |
| 816 | + * | |
| 817 | + * @since 2.15.0 | |
| 818 | + */ | |
| 819 | + private const POSITION_MAX_PAGES = 8; | |
| 820 | + | |
| 821 | + /** | |
| 822 | + * Count the period's queries into position buckets across the whole | |
| 823 | + * property (#913). | |
| 824 | + * | |
| 825 | + * The dashboard's query list is capped at 1,000 rows ordered by clicks, | |
| 826 | + * so counting it told any larger site it had exactly 1,000 queries and | |
| 827 | + * dropped the long tail, which is where positions 51-100 live. When that | |
| 828 | + * list came back short it already holds every query and is counted as | |
| 829 | + * is, with no extra request. Otherwise the property is paged with | |
| 830 | + * `startRow` at POSITION_PAGE_SIZE rows until a short page, counting as | |
| 831 | + * rows arrive rather than keeping them. | |
| 832 | + * | |
| 833 | + * A page that fails (other than a 401, which is re-thrown so the token | |
| 834 | + * refresh runs) leaves the count at what was read so far, flagged | |
| 835 | + * `complete: false`, rather than failing the whole dashboard. | |
| 836 | + * | |
| 837 | + * @since 2.15.0 | |
| 838 | + * | |
| 839 | + * @param string $site_url Search Console property. | |
| 840 | + * @param string $start_date Window start (Y-m-d). | |
| 841 | + * @param string $end_date Window end (Y-m-d). | |
| 842 | + * @param array $first_rows The capped query list already fetched. | |
| 843 | + * @return array{top_3:int,4_10:int,10_50:int,51_100:int,over_100:int,complete:bool} | |
| 844 | + * @throws \Exception On a 401, so get_dashboard_data() can refresh the token. | |
| 845 | + */ | |
| 846 | + private function count_position_distribution(string $site_url, string $start_date, string $end_date, array $first_rows): array { | |
| 847 | + if (count($first_rows) < 1000) { | |
| 848 | + return self::bucket_positions($first_rows, true); | |
| 849 | + } | |
| 850 | + | |
| 851 | + $distribution = self::bucket_positions([], true); | |
| 852 | + $start_row = 0; | |
| 853 | + | |
| 854 | + for ($page = 0; $page < self::POSITION_MAX_PAGES; $page++) { | |
| 855 | + try { | |
| 856 | + $rows = $this->search_analytics_client->get_search_analytics_data( | |
| 857 | + $site_url, | |
| 858 | + $start_date, | |
| 859 | + $end_date, | |
| 860 | + ['query'], | |
| 861 | + self::POSITION_PAGE_SIZE, | |
| 862 | + $start_row | |
| 863 | + )['rows'] ?? []; | |
| 864 | + } catch (\Exception $e) { | |
| 865 | + if ($e->getCode() === 401) { | |
| 866 | + throw $e; | |
| 867 | + } | |
| 868 | + // Nothing read yet: the capped list is the best there is. | |
| 869 | + $partial = $start_row === 0 ? self::bucket_positions($first_rows, false) : $distribution; | |
| 870 | + $partial['complete'] = false; | |
| 871 | + return $partial; | |
| 872 | + } | |
| 873 | + | |
| 874 | + $page_counts = self::bucket_positions($rows, true); | |
| 875 | + foreach (['top_3', '4_10', '10_50', '51_100', 'over_100'] as $bucket) { | |
| 876 | + $distribution[$bucket] += $page_counts[$bucket]; | |
| 877 | + } | |
| 878 | + | |
| 879 | + if (count($rows) < self::POSITION_PAGE_SIZE) { | |
| 880 | + return $distribution; | |
| 881 | + } | |
| 882 | + $start_row += self::POSITION_PAGE_SIZE; | |
| 883 | + } | |
| 884 | + | |
| 885 | + $distribution['complete'] = false; | |
| 886 | + return $distribution; | |
| 887 | + } | |
| 888 | + | |
| 889 | + /** | |
| 890 | + * Bucket Search Console rows by average position. | |
| 891 | + * | |
| 892 | + * `10_50` is the historical key for positions 11-50. Rows past 100 are | |
| 893 | + * counted in `over_100`: they still had impressions. | |
| 894 | + * | |
| 895 | + * @since 2.15.0 | |
| 896 | + * | |
| 897 | + * @param array $rows Search Console rows. | |
| 898 | + * @param bool $complete Whether $rows is every query in the window. | |
| 899 | + * @return array{top_3:int,4_10:int,10_50:int,51_100:int,over_100:int,complete:bool} | |
| 900 | + */ | |
| 901 | + private static function bucket_positions(array $rows, bool $complete): array { | |
| 902 | + $distribution = [ | |
| 903 | + 'top_3' => 0, | |
| 904 | + '4_10' => 0, | |
| 905 | + '10_50' => 0, | |
| 906 | + '51_100' => 0, | |
| 907 | + 'over_100' => 0, | |
| 908 | + 'complete' => $complete, | |
| 909 | + ]; | |
| 910 | + | |
| 911 | + foreach ($rows as $row) { | |
| 912 | + $position = (float) ($row['position'] ?? 0); | |
| 913 | + if ($position <= 3) { | |
| 914 | + $distribution['top_3']++; | |
| 915 | + } elseif ($position <= 10) { | |
| 916 | + $distribution['4_10']++; | |
| 917 | + } elseif ($position <= 50) { | |
| 918 | + $distribution['10_50']++; | |
| 919 | + } elseif ($position <= 100) { | |
| 920 | + $distribution['51_100']++; | |
| 921 | + } else { | |
| 922 | + $distribution['over_100']++; | |
| 923 | + } | |
| 924 | + } | |
| 925 | + | |
| 926 | + return $distribution; | |
| 809 | 927 | } |
| 810 | 928 | |
| 811 | 929 | /** |
| 812 | 930 | * Get Core Web Vitals for the analytics dashboard, cached independently |
