ualibraries / Guide-on-the-Side

The University of Arizona Libraries will no longer provide support for Guide on the Side. The code will remain openly available; however, UAL can no longer provide code fixes or upgrades.
https://ualibraries.github.io/Guide-on-the-Side/about.html
Other
66 stars 48 forks source link

Upgrade to Universal Analytics #107

Closed michaelhagedon closed 9 years ago

michaelhagedon commented 9 years ago

Because that's the only choice for new accounts.

simpsonw commented 9 years ago

I've upgraded google_analytics.ctp in the default theme with the new JavaScript for Universal Analytics. I've committed what I have to the gh_107_universal_analytics branch and deployed it to webdev to test. I was able to see my page hits in the "Real-Time" data overview, so I think everything should be working properly.

It's worth noting that the Universal Analytics code doesn't seem to want you to pass a domain name, so perhaps we should remove it from config.yml. Also, I don't believe there's anything I need to change for the UAL theme as the google_analytics.tpl file in that theme appears to be relying upon an external JavaScript, but I could be wrong.

Any feedback would be more than welcome as I don't know much about Google/Universal Analytics.

michaelhagedon commented 9 years ago

I don't know much about this stuff, either, but as long as it works for GA and UA (maybe @liquid06 would be a better reviewer) that's the main thing.

simpsonw commented 9 years ago

Ah, okay, I think there might be an issue, then, because I believe what I've done will only work for UA. I will add legacy GA support as well.

simpsonw commented 9 years ago

I have added back the legacy GA support as well as a configuration option to toggle if you're using UA or GA. I have tested it with UA and everything seems to be working properly. I'm unsure how to test it with GA since you can no longer sign up an account for "classic" Google Analytics, but I may bug the UAL team to see if I can borrow some credentials.

simpsonw commented 9 years ago

I've been added to the UAL's Google Analytics account, but I'm still having some issues testing my changes. The problem is the UAL theme for GotS uses a custom template for the Google Analytics stuff. When I switch http://webdev.library.arizona.edu/applications/quickHelp/ to use the built in Guide on the Side theme, I don't see my movements in the Overview section of the Real Time tab. I'm not too familiar with Google Analytics, but I think it may be an issue that I'm using a sub domain (e.g. webdev.library).

michaelhagedon commented 9 years ago

I'm not entirely sure GA works in core GotS because our theme overrides how that works. It might be worth searching gots-discuss; others have had trouble getting it running.

simpsonw commented 9 years ago

@liquid06 provided me with access to a legacy/classic Google Analytics account and everything seemed to work fine for me. I noticed a typo regarding a configuration option for Universal Analytics that I fixed in the commit mentioned above (53bff4b5366eb9257a4acc7ca802f305007d150e).

I believe this task is now ready for review.

liquid06 commented 9 years ago

Testing with a Universal Analytics test account:

Here's what my network tab looks like in dev tools: step-by-step-ga

Are we including analytics twice in step by step? provide_feedback and view_step_by_step_only both show up in the path in the Google Analytics interface. I saw the same thing when testing with classic analytics - it might be easier to understand the data if we only included it once.

Testing with a classic analytics account: I was able to track hits without the domain name and with setting it to a bogus value. Maybe it shouldn't be in the config options, or maybe comment it since it seems to be optional.

(I didn't test with our theme yet.)

liquid06 commented 9 years ago

Also, I reviewed the commits related to this issue and didn't see any best-practice-review problems. :+1:

liquid06 commented 9 years ago

I tested with our theme - we override and hard-code the snippet that gets embedded to be the same as the one on our main website and to alter the path of the content, so it's only possible to see hits in the main site analytics account. This is working fine from my local copy - the changes in this branch don't break anything for the UAL theme.

simpsonw commented 9 years ago

@liquid06 I think you're right; the Google Analytics script is being included more than once. This seems to be a side-effect of the fact that it's being included in public.ctp, which is probably being used as a base layout for several of the templates being used. Not sure what the best solution is just yet, will have to think it over.

simpsonw commented 9 years ago

I believe I have fixed the issue causing Google Analytics to be loaded twice (commit 099cabcd792a1a0daa0417778846335e4fe934f3). The problem seems to be that it was being loaded in the feedback element, so it was appearing twice for the step by step tutorials. Single page tutorials were preventing this by setting noGoogleAnalytics, so basically I made it so that both step by step and single page load Google Analytics, but the provide feedback function never does.

I believe this is ready for further review by either @liquid06, @michaelhagedon, or anyone else who wants to look at it.

liquid06 commented 9 years ago

@simpsonw Yay that fixed it. :+1: I tested again and I'm only seeing one pageview per actual visit in step-by-step mode. I think this branch is ready to go.

The threads I saw on gots-discuss where some people were having trouble getting analytics set up could have been due to creating a new GA account (which would have been Universal Analytics) but our snippet was including the legacy Google Analytics code. That could cause them to see no hits recorded in the Google Analytics UI. We want to make sure to tell those people to try again after upgrading. :smile: We might also want to note that enabling the analytics setup will only be recording traffic to individual tutorials and doesn't track the admin interface or the homepage of the installation.

Random thought: I haven't done any testing on piwik, but I wonder if it was also recording two hits for each single-page-view - I assume it was. We may want to mention the change from counting two page views to one specifically in the release notes, so if people were using the page views for metrics in classic analytics or in piwik they'll know what's going on if those numbers drop after upgrading gots. Also we have a couple of things awkwardly-named now that we have options for Classic Google Analytics, Universal Analytics, and Piwik. Like noGoogleAnalytics actually means no analytics at all - not even piwik.

simpsonw commented 9 years ago

I have merged the branch and created #121 to handle the refactoring of the variables as @liquid06 described. I definitely agree that in our release notes and in the Google Group we should mention the double page count problem so users know what's going on.