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

Implement option to switch between conversion_async and gtag #20

Merged
merged 7 commits into from
Oct 25, 2021

Conversation

alexs-mparticle
Copy link
Contributor

Google is pushing customers to start using Google Site Tags (gtag) for all analytic tracking. As Google is handling backwards compatibility themselves, we are offering our users a way to opt into gtag so that they can begin transitioning to the latest and greatest versions of Google Ads analytics.

src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
src/GoogleAdWordsEventForwarder.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. Please do the manual tests and let's sync before merging.

@alexs-mparticle alexs-mparticle added the wait Wait to Merge label Oct 4, 2021
@@ -739,8 +743,11 @@ describe('Adwords forwarder', function () {
}
});

// debugger;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

var conversionPayload = {
'send-to': gtagSiteId + '/' + conversionLabel
};
if (!conversionLabel) { return };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should return an explicit null, so that it's not just returning a payload of undefined which looks like it could be a bug.

var conversionPayload = {
'send-to': gtagSiteId + '/' + conversionLabel
};
if (!conversionLabel) { return };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto to returning null

Copy link
Collaborator

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nits

@alexs-mparticle alexs-mparticle removed the wait Wait to Merge label Oct 25, 2021
@alexs-mparticle alexs-mparticle merged commit 198e232 into master Oct 25, 2021
@alexs-mparticle alexs-mparticle deleted the feat/71444-implement-enhanced-conversion branch October 25, 2021 15:34
github-actions bot pushed a commit that referenced this pull request Nov 11, 2021
# [2.1.0](v2.0.1...v2.1.0) (2021-11-11)

### Bug Fixes

* Correct send_to in gtag API ([#23](#23)) ([af01adc](af01adc))
* Prevent unmapped events from being forwarded ([#22](#22)) ([50e6d27](50e6d27))

### Features

* Implement option to switch between conversion_async and gtag ([#20](#20)) ([198e232](198e232))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants