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.23k stars 286 forks source link

Update WP Consent API plugin detection to account for non-standard location #8307

Closed techanvil closed 2 months ago

techanvil commented 7 months ago

Feature Description

As mentioned in this https://github.com/google/site-kit-wp/pull/8305#discussion_r1499937157, it's possible for the WP Consent API plugin entry point to reside in a location other than wp-consent-api/wp-consent-api.php.

https://github.com/google/site-kit-wp/blob/b19bdd5bf6c3e0b53d3a157022927672c9df88b3/includes/Core/Consent_Mode/REST_Consent_Mode_Controller.php#L143-L144

We should update the above code to account for alternative locations for the plugin.


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

image image

Changelog entry

eugene-manuilov commented 3 months ago

AC ✔️, I have adjusted it a bit to add a note that the controller should use the right path to the plugin.

tofumatt commented 3 months ago

IB ✅

mohitwp commented 2 months ago

QA Update ✅

![image](https://github.com/google/site-kit-wp/assets/94359491/63c098c0-4b79-491a-8c14-43f85e4eec00) ![image](https://github.com/google/site-kit-wp/assets/94359491/c0014908-9200-41ce-b4d7-b888ae600ef9) ![image](https://github.com/google/site-kit-wp/assets/94359491/85c7d9fe-4fd3-447a-bf40-800b8a8b7212) https://github.com/google/site-kit-wp/assets/94359491/c5afaa8f-1a33-4306-95e7-d9e930ad1436
wpdarren commented 2 months ago

@mohitwp as per my conversation on Slack. I am a little paranoid with this feature due to past issues 😄 so could you test the sceario in the QAB, but also check that the snippet still appears in the source code and ensure that tracking is working as expected for EU and Non EU traffic. More info on the snippet can be found in the testing instructions. Let me know if you have any questions.

mohitwp commented 2 months ago

QA Update ✅

![image](https://github.com/google/site-kit-wp/assets/94359491/63c098c0-4b79-491a-8c14-43f85e4eec00) ![image](https://github.com/google/site-kit-wp/assets/94359491/c0014908-9200-41ce-b4d7-b888ae600ef9) ![image](https://github.com/google/site-kit-wp/assets/94359491/85c7d9fe-4fd3-447a-bf40-800b8a8b7212) https://github.com/google/site-kit-wp/assets/94359491/c5afaa8f-1a33-4306-95e7-d9e930ad1436
https://github.com/google/site-kit-wp/assets/94359491/f58c36a7-6421-4d60-b39c-630adcdc51f5 ![image](https://github.com/google/site-kit-wp/assets/94359491/5f201302-0ce5-457e-aedc-c26016eb4642)
https://github.com/google/site-kit-wp/assets/94359491/f1b1e21f-be10-4990-836c-4e8c8e8c90cd
![image](https://github.com/google/site-kit-wp/assets/94359491/6536d582-095f-4495-be70-e674cb3c6bb9) ![image](https://github.com/google/site-kit-wp/assets/94359491/cfa8ca0f-43d8-46f6-9490-617b658a9ee0) ![image](https://github.com/google/site-kit-wp/assets/94359491/beb23e5a-c20f-482e-9d7e-b3f3f7415dff) ![image](https://github.com/google/site-kit-wp/assets/94359491/019f75c0-bf47-4943-a772-8622fe99f9a4) ![image](https://github.com/google/site-kit-wp/assets/94359491/97f652fc-2be8-42a4-b294-c65ea59df476) ![image](https://github.com/google/site-kit-wp/assets/94359491/1c709f0f-6138-481d-8eda-19857622fe99) ![image](https://github.com/google/site-kit-wp/assets/94359491/9ed2f55c-50c8-4762-8363-45a1da77fa3a) ![image](https://github.com/google/site-kit-wp/assets/94359491/3e7db2bb-7ba3-4614-ac44-19b9746e6a9a) ![image](https://github.com/google/site-kit-wp/assets/94359491/18adbc8e-1afb-4977-bf3f-78adb33be8d1)
aaemnnosttv commented 2 months ago

⚠️ Approval

There is a small change we should make to the implementation here. See https://github.com/google/site-kit-wp/pull/8959#pullrequestreview-2175494399

I can raise a quick follow up here if needed.

nfmohit commented 2 months ago

⚠️ Approval

There is a small change we should make to the implementation here. See #8959 (review)

I can raise a quick follow up here if needed.

I've created a follow-up PR.

aaemnnosttv commented 2 months ago

Thanks @nfmohit, this has been merged and tested so this is good to go 👍

It's worth noting that this issue's changes were somehow reverted (no longer in main or develop) in https://github.com/google/site-kit-wp/commit/9ab0412a6047e43781a011d06293adfedea7001e after it had gone through QA, so the follow-up PR actually re-introduced the change while addressing the points I raised.