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

Create decoupled infrastructure for gtag tag #8269

Closed
2 tasks
aaemnnosttv opened this issue Feb 19, 2024 · 8 comments
Closed
2 tasks

Create decoupled infrastructure for gtag tag #8269

aaemnnosttv opened this issue Feb 19, 2024 · 8 comments
Labels
Module: Ads Google Ads module related issues P0 High priority PHP QA: Eng Requires specialized QA by an engineer Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Feb 19, 2024

Feature Description

GTag rendering should be lifted out of Analytics into a core location where it can be used by multiple modules, initially the Ads module in addition to Analytics.

Note: This work has largely been completed as part of the POC for the Consent Mode epic, please see 82d9f94


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

Acceptance criteria

  • The GTag rendering should be extracted from the Analytics_4\Web_Tag class into a reusable class in the Core namespace.
  • It should be modified to allow multiple tag IDs to be queued along with their initial tag config.
  • Additional gtag() commands should be able to be queued for insertion either before or after the gtag.js script is requested.

Implementation Brief

Test Coverage

  • Provide PHPUnit test coverage for the above changes.

QA Brief (ENG:QA)

  • Validate general logic of the GTag class.
  • Create a new instance of the class in a theme functions.php file, an mu-plugin, or even in the Plugin class of SK
    • Add a new dummy GTag, i.e $gtag->add_tag( 'GT-12345', array( 'foo' => 'bar' ) ); and validate that the gtag script is successfully output on the page, and that the src correctly reflects the chosen tag ID.
      • Validate that the applicable gtag( 'config' ) call is present for the tag in question.
    • Add a dummy command, i.e $gtag->add_command( 'foo', array('bar') ); and validate that the gtag command call is successfully output within the gtag snippet.
      • Play around with the $position function arg and ensure that the position of the command changes accordingly.
    • Add another tag via the add_tag() method, such as GT-54321.
      • Validate that the second tag is present, but not the main src of the snippet.
      • Validate that there are 2x gtag('config') calls, one for each tag ID.
      • Validate that inline tag configs are specific to their tags and in the correct gtag('config') function calls.

Changelog entry

  • Add decoupled infrastructure for GTag.
@aaemnnosttv aaemnnosttv added P0 High priority Type: Enhancement Improvement of an existing feature labels Feb 19, 2024
@aaemnnosttv
Copy link
Collaborator Author

aaemnnosttv commented Feb 20, 2024

Thanks @techanvil – thinking ahead just a little bit, one thing that comes to mind re #8274 is that we want to do this for GTM as well which doesn't use an enqueued WP script the way GA does. All that to say that we might want to decouple the gtag infra for the consent ("before") from the rest of the tag so that this part can be output independently. I suppose this part could be implemented by the consent mode infra almost treating it as its own tag that would always be printed very early relative to other scripts. WDYT?

If we want to address this in this issue then it should probably have a point in the AC, otherwise I'm okay with this proceeding if we want to handle it in a later issue.

@aaemnnosttv aaemnnosttv assigned techanvil and unassigned aaemnnosttv Feb 20, 2024
@techanvil
Copy link
Collaborator

Thanks @aaemnnosttv, good spot. I think we can implement that in #8264, but I've updated the POC to accommodate this in a slightly cheap and cheerful manner.

Actually this makes me wonder if we even need to decouple the GTag as part of this epic - at least, as things stand we don't have it specced out to enqueue any additional GTag consent commands, only to call gtag() on the fly in response to the wp_listen_for_consent_change event. Are we missing something whereby we should be enqueuing some additional events, or could we avoid this issue and implement it in the Ads Module epic instead? 🤔

@techanvil techanvil assigned aaemnnosttv and unassigned techanvil Feb 20, 2024
@techanvil
Copy link
Collaborator

I'm moving this back to AC as we need to determine whether or not it's actually needed for the Consent Mode MVP.

@bethanylang
Copy link
Collaborator

@aaemnnosttv I see this is still set as blocking #8273, which is a Consent Mode issue. Does this mean that we'll need to complete this Ads Module issue ASAP in order to deliver Consent Mode at the end of Sprint 122 as planned?

@techanvil techanvil assigned 10upsimon and unassigned aaemnnosttv Feb 22, 2024
@techanvil
Copy link
Collaborator

@bethanylang thanks for spotting this. With this issue removed from the CoMo epic, #8273 can also be removed.

I have removed that issue from CoMo and assigned both of these to @10upsimon for evaluation and reuse in the Ads Module epic.

@10upsimon 10upsimon removed their assignment Feb 26, 2024
@10upsimon
Copy link
Collaborator

Note to AC reviewer, this has largely been kept as is from the Consent Mode epic, having reviewed the POC at 82d9f94 this should be a "lift and shift" effort.

@10upsimon 10upsimon added PHP Module: Ads Google Ads module related issues labels Feb 26, 2024
@tofumatt tofumatt self-assigned this Feb 26, 2024
@tofumatt
Copy link
Collaborator

ACs here look good 👍🏻

Since there's an IB as well (which also looks good) I'll move this right to EB

IB ✅

@tofumatt tofumatt removed their assignment Feb 26, 2024
@10upsimon 10upsimon self-assigned this Mar 11, 2024
@10upsimon 10upsimon added the QA: Eng Requires specialized QA by an engineer label Mar 18, 2024
@10upsimon 10upsimon removed their assignment Mar 18, 2024
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Mar 20, 2024
@kuasha420 kuasha420 self-assigned this Mar 25, 2024
@kuasha420
Copy link
Collaborator

kuasha420 commented Mar 28, 2024

QA:Eng ✔️

  • Went through the QA brief and verified every point.

Screenshot_20240328_125428

LGTM ✅

@kuasha420 kuasha420 removed their assignment Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Ads Google Ads module related issues P0 High priority PHP QA: Eng Requires specialized QA by an engineer Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants