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

Tracked events not associated with page #8

Open
marks opened this issue Jul 8, 2013 · 9 comments
Open

Tracked events not associated with page #8

marks opened this issue Jul 8, 2013 · 9 comments

Comments

@marks
Copy link

marks commented Jul 8, 2013

My understand of Google Analytics is that events should be associated with a page url/title. When I run the examples in this repo for event tracking after tracking a pageview, the event is not associated with the page like I think it should be.

Thanks,

Mark

@marks
Copy link
Author

marks commented Jul 8, 2013

My commit at marks@b6eaa6e and the one before it (marks@08d0db3) seem to do it for me but is this the correct way?

NOTE: I have not tested it to ensure it works when not all params are passed to NA.trackEvent

@iamdustan
Copy link
Contributor

The method you want is NA.trackPage https://github.com/Skookum/nodealytics#track-page

Event tracking is an entirely different piece of the GA puzzle. https://developers.google.com/analytics/devguides/collection/gajs/eventTrackerGuide

@iamdustan
Copy link
Contributor

I just realized you may be asking for a way to tie a page to a custom event. Mah bad.

That eventTracker method is asking for a ridiculous number of arguments. A pull request to refactor that is more than welcome https://github.com/Skookum/nodealytics/blob/master/lib/main.js#L103

Perhaps something like:

exports.trackEvent = function (event, options, callback) {
  if (arguments.length === 7) return doBackwardsCompatabilityWay.apply(NA, arguments);

  // do new thing with event and options object
}

This would also open up the flexibility for you to easily add that data to your event tracking.

@marks
Copy link
Author

marks commented Jul 9, 2013

Yeah - I definitely need to refactor it but I'm more looking for guidance if this is correct and why it wasn't done like this before. I want to make sure that this is the proper way to do it and the fact that the page name/url not populating for events wasnt due to a cookie or other issue.

@iamdustan
Copy link
Contributor

@rockbot – any knowledge to share on this front?

@iamdustan
Copy link
Contributor

@marks that’s a great question. I’m not really sure; I’ve just inherited responsibility for this. I invited the original author back in to provide feedback.

@rockbot
Copy link
Contributor

rockbot commented Jul 10, 2013

Hi @marks - thanks for the feedback! The only reason it wasn't done before was probably because we didn't need the functionality at the time.

So, looking at the code now, I'm not liking having so many parameters in the function call in the first place. Instead, I'd recommend changing it to an object: function (eventParams, callback), where eventParams takes in all of those extra parameters :-)

That way you can get rid of all the if (typeof blah === function) business (which is good for a shorter number of arguments, but now it's just getting silly) and simply update the eventParams object before sending it off to runQuery. It'll be simpler, shorter, etc.

Does that make sense? If you can put that together as a pull request, I think that'd make an excellent upgrade to the current codebase :-)

Hope that's helpful!

@marks
Copy link
Author

marks commented Jul 10, 2013

Yeah - no big deal... I havent refactored the arguments yet just because I wasn't sure if it was a logical change in the first place. Will try to contribute later in the week.

Mark Silverberg
@Skramhttp://twitter.com/skram | @MappyHealthhttp://twitter.com/mappyhealth
http://www.socialhealthinsights.comhttp://www.socialhealthinsights.com/
http://www.linkedin.com/in/silverbergmark

Coming late summer 2013: CheckQMhttp://checkqm.com/
"Simplifying the process of building clinical quality measures into your EHR"
[cid:0855C261-8F74-418E-9E3A-5D36710D579D]

From: Raquel Vélez <[email protected]mailto:[email protected]>
Reply-To: Skookum/nodealytics <[email protected]mailto:[email protected]>
Date: Wednesday, July 10, 2013 1:09 PM
To: Skookum/nodealytics <[email protected]mailto:[email protected]>
Cc: Mark Silverberg <[email protected]mailto:[email protected]>
Subject: Re: [nodealytics] Tracked events not associated with page (#8)

Hi @markshttps://github.com/marks - thanks for the feedback! The only reason it wasn't done before was probably because we didn't need the functionality at the time.

So, looking at the code now, I'm not liking having so many parameters in the function call in the first place. Instead, I'd recommend changing it to an object: function (eventParams, callback), where eventParams takes in all of those extra parameters :-)

That way you can get rid of all the if (typeof blah === function) business (which is good for a shorter number of arguments, but now it's just getting silly) and simply update the eventParams object before sending it off to runQuery. It'll be simpler, shorter, etc.

Does that make sense? If you can put that together as a pull request, I think that'd make an excellent upgrade to the current codebase :-)

Hope that's helpful!

Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-20757493.

@iamdustan
Copy link
Contributor

Thanks @rockbot!

Yeah, I would like to keep the old signature around for at least one version, too. Whenever you get around to it could you throw something like this into the old signature? Thanks, @marks

console.warn('Deprecated: GA.trackEvent signature has been deprecated and will be removed soon.\n See https://github.com/Skookum/nodealytics for updated docs")

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

No branches or pull requests

3 participants