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

update GAClient to use 0.9.x google API #17277

Merged
merged 1 commit into from Aug 24, 2017
Merged

Conversation

wjordan
Copy link
Contributor

@wjordan wjordan commented Aug 24, 2017

This PR updates GAClient (a helper class used by the admin reports controller for some level-completion pages) to the >= 0.9.x google API, which was previously upgraded in #15995 causing this page to no longer work correctly.

I used #16132 as a reference for the Google API service migration, storing new JSON-format credentials in the CDO.ga_account secrets key (set via Chef cdo-secrets override_attribute on the baseline role, which gets propagated across all infrastructure instances).

Manually tested/verified locally with credentials provided via locals,yml using the following line of code (derived from the logic in the admincontroller which uses this class):

bundle exec ruby -r./deployment -r./dashboard/scripts/archive/ga_client/ga_client \
-e "puts GAClient.query_ga( \
  '2017-08-01', \
  '2017-08-02', \
  'ga:pagePath', \
  'ga:avgTimeOnPage', \
  'ga:pagePath=~^/s/|^/flappy/|^/hoc/' \
).rows"

/HOC/1
13.0
/flappy/1
[...etc...]

@tanyaparker
Copy link
Contributor

Thank you thank you thank you!

Copy link
Contributor

@ashercodeorg ashercodeorg left a comment

Choose a reason for hiding this comment

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

LGTM!

application_name: 'cdo-ga-analytics',
application_version: '0.0.1'
analytics.get_ga_data(
'ga:' + CDO.ga_profile_id.to_s, # ids
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Pretty sure you don't need the to_s conversion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

irb(main):001:0> 'ga:' + CDO.ga_profile_id
TypeError: no implicit conversion of Fixnum into String
        from (irb):1:in `+'
        from (irb):1
        from /usr/bin/irb:11:in `<main>'

Copy link
Contributor

Choose a reason for hiding this comment

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

Fascinating.

irb(main):003:0> 'ga:' + CDO.ga_profile_id
=> "ga:68074336"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

("ga:#{CDO.ga_profile_id}" is a valid alternative, because string-interpolation automatically adds #to_s to non-string objects)

@wjordan wjordan merged commit 1e344c5 into staging Aug 24, 2017
@wjordan wjordan deleted the ga_analytics_update branch August 24, 2017 15:27
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.

None yet

3 participants