Site-kit-wp: Do not make proxy notifications request when not using proxy

[felixarntz]

Bug Description

Currently, for sites that aren't using the proxy (because of using custom OAuth client credentials), Site Kit still regularly requests the notifications endpoint of the proxy, which naturally will fail, as the client ID used in that case is not a valid site ID against the proxy.

Those requests should be avoided, either by the client not making the request to the server for notifications at all, or by the server not requesting remote notifications when not using the proxy. The latter probably makes more sense because the /core/site/data/notifications endpoint is a general notifications endpoint and, while it is currently only used to return remote notifications, could also return notifications coming directly from the plugin server-side.

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

• The core/site/data/notifications endpoint should be modified to only issue its API request to the proxy endpoint if the site is actually using the proxy. If not, it should simply return an empty array of notifications. (If we need plugin-side notifications to be returned from here in the future, we could do that in this endpoint too.)

Implementation Brief

• Move the if check for using_proxy() in Notifications.php (https://github.com/google/site-kit-wp/blob/dc2cb956993b8d6e47592884264c37a70bcd9895/includes/Core/Notifications/Notifications.php#L143) above the $response = wp_remote_get( $endpoint ); line, so that a request is not made to the proxy if it is not in use.

Test Coverage

• A test to ensure wp_remote_get is not called when not using the proxy would be useful to add in PHPUnit, if straightforward.

Visual Regression Changes

• There should be no changes to VRT.

QA Brief

• Configure your site without the proxy, confirm that the core/site/data/notifications REST endpoint does not attempt to call the Google proxy.
• Configure your site with the proxy, confirm that the core/site/data/notifications REST endpoint does call the Google proxy.

Changelog entry

• Prevent a remote notifications API request for development sites where the plugin is not using the authentication service.

[felixarntz]

@tofumatt Nitpick, but I think it would be a bit better to move the check to the very top of that function because we can avoid any other logic in that case. That's a detail though that can be covered during code review. IB ✅

[aaemnnosttv]

@benbowler would you please add a QA Brief? Once added this is good to move over to QA as the PR is merged 👍

Edit I moved it to QA so just unassign yourself once you've added the QAB 🙂

[benbowler]

QA Brief added.

[asvinb]

• Configured site without the proxy, no calls to Google proxy 👍
• Configured site with the proxy, calls to Google proxy 👍

QA: ✅