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

custom driver for Adobe DTM metrics #86

Closed

Conversation

andrewmartin
Copy link

Hi there, thanks for your hard work on an excellent add on. Over here at DSC we used ember-metrics in conjunction with the Google Tag Manager adapter, and everything worked great. Our marketing team made the decision to move to Adobe Metrics, so we found the need to build a new adapter.

This is a first pass for you to take a look at; I basically tried to follow the conventions and patterns you had set in your Google Tag Manager.

One thing I was hoping to get a second eye on is the eventQueue I set up here. Unfortunately, Adobe's solution (as far as I could tell) doesn't have any sort of a promise or trigger to alert the library has been loaded in, and we ran into a few issues with the library not being ready by the time Ember code was being executed.

I set up a pretty basic poll-based timer to check the eventQueue array for items and then fire off events in succession (see line 50 or so of the driver), but this may not be ideal. Would love a second opinion.

Also, I could use a little help with tests, if you have any suggestions (full disclosure, I'm a little new to the Ember game and whole Javascript unit testing thing).

Anyways, take a look and hopefully this is something helpful for someone down the road.

@briangonzalez briangonzalez force-pushed the adobe-metrics branch 4 times, most recently from 4fac489 to b1f4aa1 Compare May 26, 2016 16:37
return 'DynamicTagManager';
},

init () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

init() {

@poteto
Copy link
Collaborator

poteto commented Jun 29, 2016

Hey, super sorry I got to this so late.

  • I'm not sure if a queue or poll is necessary – why can't you just send the event as it comes in? You could have a sendEvent method that sends a single event, and have trackEvent and trackPage call that method
  • You can refer to the other adapter tests for an example, was there anything in particular you weren't clear about?

I apologize in advance if any of my comments were terse, it's a little late here!

@claubacher
Copy link
Collaborator

Hi @poteto. Picking this back up. I added a couple tests and made most of the changes you suggested. My understanding about the queue is that it was added because Adobe DTM (unlike other trackers like Google Tag Manager) can't handle events if they're pushed to the data layer before the DTM script has been loaded, so the queue makes sure those events still get sent.

I can go ahead and merge this if you think it's ready; otherwise, I'm happy to make further changes.

@homu
Copy link
Contributor

homu commented Nov 4, 2016

☔ The latest upstream changes (presumably 90caf4b) made this pull request unmergeable. Please resolve the merge conflicts.

@poteto
Copy link
Collaborator

poteto commented Nov 5, 2016

@andrewmartin Sorry this took so long, it completely slipped my mind and I'm super sorry for that. If you rebase this against master I'll get it merged and released

@andrewmartin
Copy link
Author

@poteto no problem. I'll make an effort or perhaps @claubacher can take over, as I'm no longer working with DSC.

andrewmartin and others added 2 commits November 7, 2016 11:45
setting up some intial DTM methods.

- satellite calls are flexible
- added satellite.pageBottom() but may want to consider adding to routing

improving driver

- removing redundant callSatellite method
- adding proper trackEvent naming with optional `label` flag
  - presently, the naming follows the format `category - action - label`
  - if `label` isn't present, the event name is simply `category - action`

fixing adobe dtm driver to avoid throwing undefined errors

- reorganization of pageView and separation of concerns to a couple new methods

changing conditional

major updates to metrics driver for event queueing, config

- when _satellite is not defined, push events to array
	- an interval runs checking for this array length
	- this interval pushes all events in array
- option to configure data layer name on window object
  - falls back to 'dtmDataLayer'
  - must be set in Adobe DTM under 'Data Elements'
  - once set, _satellite.getVar('keyName') works within DTM
- some general cleanup and removal of unused variables

adding page bottom queue handler

adding pushPayload method

removing pageBottom methods and queue handler;
rename "listen" methods to use the word "check"
@claubacher
Copy link
Collaborator

@andrewmartin Thanks, I'll take over from here!

@poteto Went ahead and rebased this against master, but our analytics person raised a concern about how we're calling _satellite.track. Going to double check before moving forward with this.

@homu
Copy link
Contributor

homu commented Dec 19, 2016

☔ The latest upstream changes (presumably 81dd219) made this pull request unmergeable. Please resolve the merge conflicts.

@kellyselden
Copy link
Collaborator

@claubacher Are you still invested in this? I have a need too and some time to help this along.

@claubacher
Copy link
Collaborator

Hi, @kellyselden. No, I'm not watching this project anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants