google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 292 forks source link

Ensure the "Top content" CTA for creating the custom dimension appears when the corresponding report error shows the custom dimension is missing #9218

Closed techanvil closed 1 month ago

techanvil commented 3 months ago

Feature Description

The Audience Tile's "Top content" metric area has a CTA for creating the googlesitekit_post_type custom dimension if it doesn't exist.

However, it doesn't have logic in place to determine if the custom dimension doesn't exist when requesting the report for the metric. At present it's reliant on the list of available custom dimensions being synced somewhere, which only happens during Audience Segmentation setup, or in the Key Metrics feature.

We should ensure that, if the report for the "Top content" metric returns an error because the custom dimension doesn't exist, the list of custom dimensions is resynced so the tile will immediately know that the dimension is missing and show the CTA to create it.


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

Acceptance criteria

Implementation Brief

Add Missing Custom Dimension to Top Content Report

Authenticated User Context

View-Only Context

Test Coverage

QA Brief

Authenticated user

View-only user

Changelog entry

techanvil commented 2 months ago

Hi @benbowler, thanks for drafting this IB. A couple of points:

... selectors to monitor syncAvailableCustomDimensions selector/resolver like so:

https://github.com/google/site-kit-wp/blob/56feb1f6ca227cc9fc6da3511a96fb2f4bf9d3b0/assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget/AudienceTile/AudienceTilePagesMetric.js#L94-L96

techanvil commented 2 months ago

Hi @benbowler, thanks for updating the IB. A few more points:

For the view-only context:

More fundamentally, though - I've noticed the googlesitekit_post_type custom dimension is actually missing from the "Top content" report options:

https://github.com/google/site-kit-wp/blob/119747b9e3be9bb8310faec37b7ea41c6b4ea89f/assets/js/modules/analytics-4/hooks/useAudienceTilesReports.js#L322-L329

https://github.com/google/site-kit-wp/blob/119747b9e3be9bb8310faec37b7ea41c6b4ea89f/assets/js/modules/analytics-4/hooks/useAudienceTilesReports.js#L346-L353

It needs to be added as a dimension filter, like we do here, but with value: 'post':

https://github.com/google/site-kit-wp/blob/119747b9e3be9bb8310faec37b7ea41c6b4ea89f/assets/js/modules/analytics-4/components/widgets/PopularProductsWidget.js#L79-L85

This will be needed for this issue to be complete and testable, so we should include the fix here too. There's likely to be a number of tests that need updating as a result which we should factor into the estimate.

benbowler commented 2 months ago

I chose to use the getErrorForSelector selector within the AudienceTile component instead of adding another new prop to this component, we could instead create a new prop such as hasTopContentCustomDimensionError instead if prefered.

I've increased the estimate a couple of bands because the scope has expanded slightly as we've discussed required fixes/additions to be able to work on this ticket.

techanvil commented 2 months ago

Thanks @benbowler. Using getErrorForSelector() SGTM.

A small point, you've referred/linked to topCitiesReportErrors with regard to filtering out the invalid custom dimension errors, in fact they should be filtered out from topContentReportErrors and topContentPageTitlesReportErrors. I've amended that to save a round trip.

IB :white_check_mark:

kelvinballoo commented 2 months ago

QA Update ⚠️

Hi @techanvil , I followed the steps but it doesn't seem like it's working as expected.

Let me know in case I missed any steps.

techanvil commented 2 months ago

Hey @kelvinballoo, maybe I should have mentioned this in the QAB, but you won't be able to test this issue with the Tester plugin configured to provide mock report data. This is because the underlying code relies on the error response returned by the real request for the "Top content" data once the custom dimension has been archived.

You should be able to test this with any property which is legitimately out of the gathering data state. Please give it another try and let me know if you run into any further problems.

kelvinballoo commented 2 months ago

QA Update ✅

Hi @techanvil , thanks. I managed to test it with a site, despite having 0 data, it was out of the gathering state. I was able to get the 'Update' button and in dashboard sharing view mode, the "Top content" metric area did not appear in the tiles. ✅

Clicking on 'Update' on the admin dashboard: ✅

This is working as expected. Moving ticket to approval.

https://github.com/user-attachments/assets/208a0c6b-9a9b-47ef-8416-bb1306822c93 https://github.com/user-attachments/assets/c9530fea-08bc-4fa3-9d2b-f1946b518605