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 291 forks source link

Filter out ACR metric items from saved ones if feature flag is disabled #9364

Closed zutigrm closed 1 month ago

zutigrm commented 1 month ago

Feature Description

We should prevent metric tiles from still appearing in the widget area after the conversionReporting feature flag is disabled.


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

zutigrm commented 1 month ago

Since AC is very straightforward, I added both AC and IB and moved it to the IBR

eugene-manuilov commented 1 month ago
  • userPickedMetrics and answerBasedMetrics should be filtered out for any saved ACR items, only if conversionReporting feature flag is disabled.

@zutigrm let's make it clear which items should be filtered out exactly. Could you please list them in IB?

zutigrm commented 1 month ago

@eugene-manuilov Sure, I added the list, and expanded the IB

eugene-manuilov commented 1 month ago

Thanks, IB ✔️

10upsimon commented 1 month ago

@zutigrm and I chatted about this via DM, and it made more sense to define a constant that holds non ACR metrics only, and filter against this list. This saves having to add metrics to the array as they become available in code, thus reducing the chance for oversight thereof.

I'll update the IB shortly to reflect this change, cc @eugene-manuilov

10upsimon commented 1 month ago

@zutigrm @eugene-manuilov the IB has been updated accordingly.

10upsimon commented 1 month ago

@zutigrm :

Noting that we are not able to add test cases to assets/js/googlesitekit/datastore/user/key-metrics.test.js until such time that we have the mapping defined, as these tests rely on the real values returned by registry.select( CORE_USER ).getAnswerBasedMetrics().

We should make the addition of this test as part of the GH issue that maps ACR metrics to site purpose answers, once that task is defined.

mohitwp commented 1 month ago

QA Update ✅

https://github.com/user-attachments/assets/0da6da13-5420-435d-baeb-5b03dd7becd1