AdSense notifications no longer surface alerts

[aaemnnosttv]

Bug Description

The AdSense module is the only module which currently implements the notifications datapoint which is used for surfacing module-specific notifications on the dashboard automatically.

This is currently intended to surface any "severe" alerts from AdSense via the API but does not work as expected.

Steps to reproduce

1. This requires access to an AdSense account that has at least 1 severe alert (we currently have this with one of our test accounts)
2. Connect AdSense using the aforementioned account (using a URL of an approved site)
3. Visit dashboard
4. See no notifications from AdSense

Screenshots

Example severe alert

Expected notification

Additional Context

This has not worked properly for a long time, so we may want to make some stylistic enhancements while we're at it.

Example response from the AdSense API with various alerts

```json { "alerts": [ { "name": "accounts/pub-1234567890/alerts/1a6602e9", "severity": "WARNING", "message": "We encourage you to publish your seller information in the Google sellers.json file. Visit the account settings page to review your current visibility status.", "type": "sellers-json-consent" }, { "name": "accounts/pub-1234567890/alerts/bb58073e", "severity": "INFO", "message": "You have new labs to try out. Visit the Labs tab to find out more.", "type": "new-lab-available" }, { "name": "accounts/pub-1234567890/alerts/eb9896eb", "severity": "SEVERE", "message": "Earnings at risk - You need to fix some ads.txt file issues to avoid severe impact to your revenue.", "type": "ads-txt-issues" } ] } ```

---

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

Acceptance criteria

• Severe alerts within the connected AdSense account should be surfaced automatically on the Site Kit dashboard

Implementation Brief

The main problem here is that GET:notifications calls GET:alerts internally which needs the accountID. If not provided, it falls back to the accountID in the module's settings which is relied upon in the case of the notifications endpoint. https://github.com/google/site-kit-wp/blob/109791ee1a4e426f73ddfb5ad255d8dcbac7aad3/includes/Modules/AdSense.php#L327-L335

This fails however, even when an accountID is configured in the settings because $data is a Data_Request instance, which is immutable, so the assignment is a no-op. This results in the request failing as a WP_Error which is then silently ignored in the core/modules handler.

https://github.com/google/site-kit-wp/blob/109791ee1a4e426f73ddfb5ad255d8dcbac7aad3/includes/Core/Modules/REST_Modules_Controller.php#L398-L405

• Update GET:alerts to require accountID in all cases by removing the fallback to settings
• Update GET:notifications to always pass accountID from settings, returning early with an empty array if none is set
  • Also remove format: large from mapping (invalid value anyways)
• Reduce sizing of NotificationAlertSVG in AdSenseAlerts to better align with other notifications (like module connect success)

Test Coverage

• Add test coverage for AdSense alerts and notifications datapoints

QA Brief

• Connect adsense to an account that had critical alert showing in the AdSense Console. ie. oi.ie.
• See that SK Dashboard also shows the alert.

Changelog entry

• Fix a bug that prevented severe AdSense alerts from appearing in Site Kit Dashboard.

[tofumatt]

IB ✅

[wpdarren]

QA Update: ⚠️

@kuasha420 I have two UX observations that might be out of scope, but thought I would raise.

1. The graphic does not seem lined up to the text and CTA. See screenshot below.
2. The AdSense text and logo looks small. Feel that should stand out more.

If you feel these are out of scope, then let me know and I will create a ticket.

[kuasha420]

Hi @wpdarren, those are valid observation. The notice does look a bit out of place and visually inconsistent with the current SK Dashboard, which is not surprising as it was designed for a different time.

However, I'd say those are out of scope for this ticket as it was about getting the alerts back. We can and should definitely open a new issue to improve and polish the AdSense alert further.

Cheers.

[wpdarren]

QA Update: ⚠️

@kuasha420 apologies, I have one other observation. If in AdSense there are more than 1 Severe alerts, then only the first one appears. I closed the first alert down in Site Kit and refreshed the dashboard and no other alerts appear. Is this something we have factored in, and if not, is it an issue we should fix, or, create a new ticket?

[aaemnnosttv]

@wpdarren you're correct, only the first alert is being shown. It looks like it was always this way, but I agree we should fix it so that each alert returned is shown. I gave this a quick test and it seems to work as expected if all the alerts are returned, we just need to ensure they have unique IDs which is what the dismissal references. Since this should be a simple change, I think we can include it as a follow-up here rather than as a separate issue.

[aaemnnosttv]

Moving this back to CR with a quick follow-up PR shown above.

[wpdarren]

QA Update: ✅

Verified:

• SK Dashboard also show the alert from AdSense when an error occurs.
• Tested on desktop and small viewports like mobile and tablet.
• Tested to make sure that the GDPR message appears
• Tested to make sure that all alerts appear when there is more than one.

Note: will be creating a ticket to highlight the UX/UI issues reported previously.

https://github.com/google/site-kit-wp/assets/73545194/12cacedc-18dc-4bf8-9827-f9b58e0f18a5

Screenshots