Merge pull request #7608 from google/bug/#7559-fix-adsense-alert

Fix AdSense critical alerts not surfacing on SK Dashboard
google · Sep 20, 2023 · cc3a5bc · cc3a5bc
2 parents 3148c24 + 14c3f54
commit cc3a5bc
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 12 deletions.
diff --git a/assets/js/components/notifications/AdSenseAlerts.js b/assets/js/components/notifications/AdSenseAlerts.js
@@ -68,9 +68,7 @@ function AdSenseAlerts() {
 						key={ id }
 						id={ id }
 						description={ description }
-						WinImageSVG={ () => (
-							<NotificationAlertSVG height="136" />
-						) }
+						WinImageSVG={ NotificationAlertSVG }
 						format={ format || 'small' }
 						ctaLink={ ctaURL }
 						ctaLabel={ ctaLabel }

diff --git a/includes/Modules/AdSense.php b/includes/Modules/AdSense.php
@@ -326,12 +326,8 @@ protected function create_data_request( Data_Request $data ) {
 				return $service->accounts_adclients_adunits->listAccountsAdclientsAdunits( self::normalize_client_id( $data['accountID'], $data['clientID'] ) );
 			case 'GET:alerts':
 				if ( ! isset( $data['accountID'] ) ) {
-					$option            = $this->get_settings()->get();
-					$data['accountID'] = $option['accountID'];
-					if ( empty( $data['accountID'] ) ) {
-						/* translators: %s: Missing parameter name */
-						return new WP_Error( 'missing_required_param', sprintf( __( 'Request parameter is empty: %s.', 'google-site-kit' ), 'accountID' ), array( 'status' => 400 ) );
-					}
+					/* translators: %s: Missing parameter name */
+					return new WP_Error( 'missing_required_param', sprintf( __( 'Request parameter is empty: %s.', 'google-site-kit' ), 'accountID' ), array( 'status' => 400 ) );
 				}
 				$service = $this->get_service( 'adsense' );
 				return $service->accounts_alerts->listAccountsAlerts( self::normalize_account_id( $data['accountID'] ) );
@@ -348,7 +344,14 @@ protected function create_data_request( Data_Request $data ) {
 				return $service->accounts_adclients->listAccountsAdclients( self::normalize_account_id( $data['accountID'] ) );
 			case 'GET:notifications':
 				return function() {
-					$alerts = $this->get_data( 'alerts' );
+					$settings = $this->get_settings()->get();
+
+					if ( empty( $settings['accountID'] ) ) {
+						return array();
+					}
+
+					$alerts = $this->get_data( 'alerts', array( 'accountID' => $settings['accountID'] ) );
+
 					if ( is_wp_error( $alerts ) || empty( $alerts ) ) {
 						return array();
 					}
@@ -375,7 +378,6 @@ function( Google_Service_Adsense_Alert $alert ) {
 							'id'            => 'adsense-notification',
 							'description'   => $alert->getMessage(),
 							'isDismissible' => true,
-							'format'        => 'large',
 							'severity'      => 'win-info',
 							'ctaURL'        => $this->get_account_url(),
 							'ctaLabel'      => __( 'Go to AdSense', 'google-site-kit' ),

diff --git a/tests/phpunit/integration/Modules/AdSenseTest.php b/tests/phpunit/integration/Modules/AdSenseTest.php
@@ -31,12 +31,15 @@
 use Google\Site_Kit\Tests\TestCase;
 use Google\Site_Kit\Tests\FakeHttp;
 use Google\Site_Kit_Dependencies\Google\Service\Adsense\AdBlockingRecoveryTag;
+use Google\Site_Kit_Dependencies\Google\Service\Adsense\Alert;
+use Google\Site_Kit_Dependencies\Google\Service\Adsense\ListAlertsResponse;
 use Google\Site_Kit_Dependencies\GuzzleHttp\Psr7\Response;
 use ReflectionMethod;
 use WP_REST_Request;
 
 /**
  * @group Modules
+ * @group AdSense
  */
 class AdSenseTest extends TestCase {
 	use Module_With_Scopes_ContractTests;
@@ -336,7 +339,7 @@ public function test_get_module_scope() {
 		);
 	}
 
-	public function test_get_data__sync_ad_blocking_recovery_tags() {
+	public function test_set_data__sync_ad_blocking_recovery_tags() {
 		$user = $this->factory()->user->create_and_get( array( 'role' => 'administrator' ) );
 		wp_set_current_user( $user->ID );
 		do_action( 'wp_login', $user->user_login, $user );
@@ -377,6 +380,92 @@ function() {
 		$this->assertOptionExists( Ad_Blocking_Recovery_Tag::OPTION );
 	}
 
+	public function test_get_data__notifications() {
+		$user = $this->factory()->user->create_and_get( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user->ID );
+		do_action( 'wp_login', $user->user_login, $user );
+
+		$context        = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE );
+		$options        = new Options( $context );
+		$user_options   = new User_Options( $context, $user->ID );
+		$authentication = new Authentication( $context, $options, $user_options );
+		$adsense        = new AdSense( $context, $options, $user_options, $authentication );
+
+		$authentication->get_oauth_client()->set_granted_scopes(
+			$adsense->get_scopes()
+		);
+
+		FakeHttp::fake_google_http_handler(
+			$adsense->get_client(),
+			function() {
+				$mock_alert_severe = new Alert();
+				$mock_alert_severe->setSeverity( 'SEVERE' );
+
+				$mock_alert_warning = new Alert();
+				$mock_alert_warning->setSeverity( 'WARNING' );
+
+				$response = new ListAlertsResponse();
+				$response->setAlerts( array( $mock_alert_severe, $mock_alert_warning ) );
+
+				return new Response( 200, array(), json_encode( $response ) );
+			}
+		);
+
+		// Should return empty array when account ID is not available in settings.
+		$response = $adsense->get_data( 'notifications' );
+		$this->assertCount( 0, $response );
+
+		// Should return an array of `adsense-notification` with `SEVERE` severity when account ID is available.
+		$adsense->get_settings()->merge( array( 'accountID' => 'pub-1234567890' ) );
+
+		$response = $adsense->get_data( 'notifications' );
+		$this->assertNotWPError( $response );
+		$this->assertCount( 1, $response );
+	}
+
+	public function test_get_data__alerts() {
+		$user = $this->factory()->user->create_and_get( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user->ID );
+		do_action( 'wp_login', $user->user_login, $user );
+
+		$context        = new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE );
+		$options        = new Options( $context );
+		$user_options   = new User_Options( $context, $user->ID );
+		$authentication = new Authentication( $context, $options, $user_options );
+		$adsense        = new AdSense( $context, $options, $user_options, $authentication );
+
+		$authentication->get_oauth_client()->set_granted_scopes(
+			$adsense->get_scopes()
+		);
+
+		FakeHttp::fake_google_http_handler(
+			$adsense->get_client(),
+			function() {
+				$mock_alert_severe = new Alert();
+				$mock_alert_severe->setSeverity( 'SEVERE' );
+
+				$mock_alert_warning = new Alert();
+				$mock_alert_warning->setSeverity( 'WARNING' );
+
+				$response = new ListAlertsResponse();
+				$response->setAlerts( array( $mock_alert_severe, $mock_alert_warning ) );
+
+				return new Response( 200, array(), json_encode( $response ) );
+			}
+		);
+
+		// Should return WP Error when account ID is not provided.
+		$response = $adsense->get_data( 'alerts' );
+		$this->assertWPError( $response );
+
+		// Should return an array of alerts when account ID is provided.
+		$response = $adsense->get_data( 'alerts', array( 'accountID' => 'pub-1234567890' ) );
+		$this->assertNotWPError( $response );
+		$this->assertCount( 2, $response );
+		$this->assertInstanceOf( 'Google\Site_Kit_Dependencies\Google\Service\Adsense\Alert', $response[0] );
+		$this->assertInstanceOf( 'Google\Site_Kit_Dependencies\Google\Service\Adsense\Alert', $response[1] );
+	}
+
 	public function test_is_connected() {
 		$adsense  = new AdSense( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) );
 		$options  = new Options( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) );