Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Analytics_4\Web_Tag class to function independently #7924

Closed
nfmohit opened this issue Nov 29, 2023 · 11 comments
Closed

Update Analytics_4\Web_Tag class to function independently #7924

nfmohit opened this issue Nov 29, 2023 · 11 comments
Labels
Module: Analytics Google Analytics module related issues P2 Low priority PHP Type: Enhancement Improvement of an existing feature

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Nov 29, 2023

Feature Description

The Analytics_4\Web_Tag class extends the Analytics\Web_Tag class. The Analytics_4\Web_Tag class should be able to function and output tags independently without extending its Analytics counterpart.


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

Acceptance criteria

  • The Analytics_4\Web_Tag class should no longer extend the Analytics\Web_Tag class.
  • The Analytics_4\Web_Tag class should be able to function independently and output GA4 tags on its own.

Implementation Brief

  • Edit the existing GA4 Web_Tag class at includes/Modules/Analytics_4/Web_Tag.php
    • Alter the class definition to extend Module_Web_Tag and implement Tag_Interface, i,e class Web_Tag extends Module_Web_Tag implements Tag_Interface
    • Add a use statement for the applicable Module_Web_Tag class, i.e use Google\Site_Kit\Core\Modules\Tags\Module_Web_Tag;
    • Ass a use statement for the applicable Tag_With_DNS_Prefetch_Trait class, i.e use Google\Site_Kit\Core\Tags\Tag_With_DNS_Prefetch_Trait;
    • Add the $home_domain; property as a private property of the class, i.e private $home_domain;.
    • Add the $anonymize_ip property as a private property of the class, i.e private $anonymize_ip;.
    • Add the $ads_conversion_id property as a private property of the class, i.e private $ads_conversion_id;.
    • Copy the set_home_domain() method from the previously extended Analytics/Web_Tag class and add it as a newly defined method in this now standalone class.
    • Copy the set_anonymize_ip() method from the previously extended Analytics/Web_Tag class and add it as a newly defined method in this now standalone class.
    • Copy the set_ads_conversion_id() method from the previously extended Analytics/Web_Tag class and add it as a newly defined method in this now standalone class.
    • In the register() method:
      • Move over the filter for wp_resource_hints as this will no longer be called from a parent class.
    • Implement the required render() method (required by the interface) as this is no longer rendered in a parent class. The method should be a noop method and do nothing.
    • Update the enqueue_gtag_script() method:
      • Remove the conditional did_action check entirely and replace the contents of the method with the contents from the method in the previously extended Analytics/Web_Tag class (non GA4 one).
    • Add the add_inline_config() method from the Analytics/Web_Tag class to this class as we no longer want to call a parent implemented of it.
    • Update the get_tag_config() method:
      • Remove the call to parent::get_tag_config()
      • Implement the logic of the previously extended/parent method get_tag_config() in Analytics/Web_Tag, i.e copy the contents from the once extended classes method into this method.
    • Validate that the GA4 tag output before and after the above changes are identical. This can be done by diffing the contents from the HTML comment starting at <!-- Google Analytics snippet added by Site Kit --> and ending at <!-- End Google Analytics snippet added by Site Kit -->.

Test Coverage

  • N/A - There appear to be no existing PHPUnit tests related to validation of GA4 (or UA) Web_Tag classes.

QA Brief

  • Configure GA4 within Site Kit
  • Using a branch or version prior to this change, note the contents of the GA4 tag in the web site source, this would be the script tag contents between the HTML comments <!-- Google Analytics snippet added by Site Kit --> and <!-- End Google Analytics snippet added by Site Kit -->.
  • Using the branch/version containing the changes in said ticket, note the same script tag contents as in the prior step.
  • Validate that the script contents are identical.
  • Validate that GA4 tracking is functioning correctly, i.e realtime dashboard reporting is working. Note that you may need to be logged out to avoid the opt-out snippe taking effect (if logged in as WordPress admin).

Additional QAB

  1. Verify that the GA4 tag contains custom dimensions in singular pages if relevant key metrics are selected.
  2. Verify that the GA4 tag contains ads conversion ID if set from the GA settings.
  3. Use a site that had Analytics module set up with UA property. Verify that no UA tags (or duplicate GA tags) are output.

Changelog entry

  • Update the Analytics_4\Web_Tag class to function independently from the original Analytics Web_Tag class.
@nfmohit nfmohit added Type: Enhancement Improvement of an existing feature Module: Analytics Google Analytics module related issues labels Nov 29, 2023
@bethanylang bethanylang added the P1 Medium priority label Nov 29, 2023
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Nov 29, 2023
@tofumatt tofumatt self-assigned this Dec 5, 2023
@tofumatt
Copy link
Collaborator

tofumatt commented Dec 5, 2023

ACs 👍🏻

@tofumatt tofumatt removed their assignment Dec 5, 2023
@bethanylang bethanylang added the Next Up Issues to prioritize for definition label Dec 5, 2023
@10upsimon 10upsimon self-assigned this Dec 7, 2023
@aaemnnosttv aaemnnosttv added the PHP label Dec 7, 2023
@10upsimon 10upsimon removed their assignment Dec 11, 2023
@eugene-manuilov eugene-manuilov self-assigned this Dec 11, 2023
@eugene-manuilov
Copy link
Collaborator

Thanks, @10upsimon. IB mostly looks good to me. Could you please add test coverage information and set the estimate for this task?

@10upsimon
Copy link
Collaborator

10upsimon commented Dec 12, 2023

@eugene-manuilov I have checked the existing tests/phpunit directory and can see no tests relative to GA4 (or UA) Web_Tag classes, therefore I've added test requirement as an N/A for now. Due to the lack of tests, I have kept this as a 7 estimate.

Assigning back to you @eugene-manuilov

@eugene-manuilov
Copy link
Collaborator

Thanks, @10upsimon. IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Dec 12, 2023
@10upsimon 10upsimon self-assigned this Dec 12, 2023
@10upsimon 10upsimon removed their assignment Dec 14, 2023
@eugene-manuilov eugene-manuilov self-assigned this Dec 14, 2023
@eclarke1 eclarke1 removed the Next Up Issues to prioritize for definition label Dec 15, 2023
@eugene-manuilov eugene-manuilov removed their assignment Dec 19, 2023
@mohitwp mohitwp self-assigned this Dec 20, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Jan 3, 2024

@10upsimon As per QAB - "Using the branch/version containing the changes in said ticket, note the same script tag contents as in the prior step." As related PR is already merge in develop. Just wanted to confirm that I need to compare latest and dev branch in this case. Correct ?

@10upsimon
Copy link
Collaborator

@10upsimon As per QAB - "Using the branch/version containing the changes in said ticket, note the same script tag contents as in the prior step." As related PR is already merge in develop. Just wanted to confirm that I need to compare latest and dev branch in this case. Correct ?

That is correct @mohitwp

@mohitwp
Copy link
Collaborator

mohitwp commented Jan 8, 2024

QA Update ✅

  • Tested on dev environment.
  • Verified that GA4 tracking is functioning correctly, i.e realtime dashboard reporting is working.
  • Verified that the contents of the GA4 tag in the web site source, i.e. the script tag contents between the HTML comments and .are identical on latest and dev environment.
  • Also, verified the script as website user. ( Non WP admin)

develop:

image

Latest:

image

@mohitwp mohitwp removed their assignment Jan 8, 2024
@bethanylang bethanylang added P2 Low priority and removed P1 Medium priority labels Jan 12, 2024
@aaemnnosttv
Copy link
Collaborator

❌ Approval

After some testing and review, we've found a few regressions introduced by this issue, particularly for the case where UA settings are still present. See #8078.

Additionally, the ads conversion ID is no longer output if the UA tag is not being output.

We'll address this in a follow-up PR which will also remove any UA property tagging since this is no longer possible to manage or connect.

@mohitwp
Copy link
Collaborator

mohitwp commented Jan 15, 2024

QA Update ⚠️

  • Tested on main environment.

As per QAB -
Verify that the GA4 tag contains custom dimensions in singular pages if relevant key metrics are selected.

  • Connected GA4 property have custom dimensions. I have not completed user input questions or selected any key metrics widget but GA4 showing custom dimensions. Is this expected or CD should display only when key metrics widgets are selected ?

image

image

@nfmohit
Copy link
Collaborator Author

nfmohit commented Jan 15, 2024

QA Update ⚠️

As per QAB - Verify that the GA4 tag contains custom dimensions in singular pages if relevant key metrics are selected.

  • Connected GA4 property have custom dimensions. I have not completed user input questions or selected any key metrics widget but GA4 showing custom dimensions. Is this expected or CD should display only when key metrics widgets are selected ?

image

image

Hi @mohitwp, that is expected when your GA4 property already has custom dimensions. Sorry, I probably should've clarified that in the QAB. Thanks for checking!

@mohitwp
Copy link
Collaborator

mohitwp commented Jan 15, 2024

QA Update ✅

  • Tested on main environment.
  • Verified analytics snippet.
  • Verified analytics setup and compared analytics widget with latest environment.
  • Also, verified below listed additional cases under QAB.

1. Verify that the GA4 tag contains custom dimensions in singular pages if relevant key metrics are selected

Main-

image

Latest -

image

2. Verify that the GA4 tag contains ads conversion ID if set from the GA settings.

Main

image

Latest -

image

3. Use a site that had Analytics module set up with UA property. Verify that no UA tags (or duplicate GA tags) are output.

UA connected on older version-

image

UA connected on main environment - Snippet not showing any reference of UA

image

cc @wpdarren

@mohitwp mohitwp removed their assignment Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P2 Low priority PHP Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants