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.

Sign up for GitHub

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

Jump to bottom

#759 google ecommerce #760

Merged
merged 7 commits into from
Mar 7, 2019
Merged

#759 google ecommerce #760

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6f425d0
Remove dangling todo
ArtemijRodionov Mar 6, 2019
66b37ce
Dispatch gtm_loaded event
ArtemijRodionov Mar 6, 2019
dd0aca1
Create PublishedGATracker class for GATracker
ArtemijRodionov Mar 6, 2019
18bdf8b
Fix typo
ArtemijRodionov Mar 6, 2019
8f0bc7f
Review fixes
ArtemijRodionov Mar 7, 2019
89ddfeb
Move class declaration upper than instantiation
ArtemijRodionov Mar 7, 2019
92c13e7
Apply linter rules
ArtemijRodionov Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 39 additions & 12 deletions front/js/shared/tracking.es6
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,8 @@

// Sync container for yaTracker
window.dataLayer = window.dataLayer || [];
// Load ecommerce plugin for gaTracker
try {
ga('require', 'ecommerce'); // Ignore ESLintBear (block-scoped-var)
} catch (e) {
Sentry.captureException(e); // Ignore ESLintBear (no-undef)
var ga = console.log; // Ignore ESLintBear (no-var)
console.error(`GaTracker failed to load. Traceback: ${e}`);
}

const yaTracker = new YATracker(window.dataLayer, 'RUB'); // Ignore ESLintBear (no-undef)
const gaTracker = new GATracker(ga, 'ecommerce'); // Ignore ESLintBear (block-scoped-var)

// @todo #504:30m Send info about product's brand to YA and GA.
const gaTracker = new PublishedGATracker(); // Ignore ESLintBear (block-scoped-var)

const init = () => {
setUpListeners();
Expand Down Expand Up @@ -78,6 +67,44 @@
});
}

// @todo #759:30m Move PublishedGATracker to a separate file.

class PublishedGATracker {
Copy link
Contributor

Choose a reason for hiding this comment

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

provide doc plz why it's published, but not common GATracker

Copy link
Contributor

Choose a reason for hiding this comment

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

publish is not the most good name for classes purpose. LimitedGATracher or SafeGATracker might be more good. Up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe LoadedGATracker would be the best name

constructor() {
this.published = false;
}

purchase(productsData, txData) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's possible to add tests for ecommerce behaviour.
We are already testing Ya.Metrika at SE: shopelectro.tests.tests_selenium.YandexMetrika

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will create todo for it

const publishOnce = () => {
// Publish only once
if (this.published) return;

// Load ecommerce plugin for gaTracker
ga('require', 'ecommerce'); // Ignore ESLintBear (block-scoped-var)
const tracker = new GATracker(ga, 'ecommerce'); // Ignore ESLintBear (block-scoped-var)

tracker.purchase(productsData, txData);
Copy link
Contributor

Choose a reason for hiding this comment

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

hope data is consistent with gAnalytics api docs. Looks too simple and suspicious =)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Data were consistent before, so what could be wrong?

this.published = true;
};

window.addEventListener('gtm_loaded', () => {
try {
publishOnce();
} catch (e) {
Sentry.captureException(e); // Ignore ESLintBear (no-undef)
console.error(e);
}
});

try {
publishOnce();
} catch(e) {
// Error occured because of unloaded Google tag manager.
// `gtm_loaded` event will try to publish again.
}
}
}

function publishPurchase() {
if (!DOM.$purchasedOrder.length) return;
const orderData = {
Expand Down
9 changes: 8 additions & 1 deletion templates/layout/google_tag_manager.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@
(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],
j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f);
'https://www.googletagmanager.com/gtm.js?id='+i+dl;

j.addEventListener('load', function() {
// notify listeners about loaded gtm
d.dispatchEvent(new CustomEvent('gtm_loaded', { bubbles: true }));
});

f.parentNode.insertBefore(j,f);
})(window,document,'script','dataLayer','GTM-P6RH2S');
</script>
<!-- End Google Tag Manager -->
Expand Down