-
Notifications
You must be signed in to change notification settings - Fork 120
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
add support for google analytics userId #15
Changes from all commits
28d59f2
621731f
17000d3
e1b0f9e
af64923
ba3af18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
class Rack::Tracker::GoogleAnalytics < Rack::Tracker::Handler | ||
|
||
ALLOWED_TRACKER_OPTIONS = [:cookie_domain, :user_id] | ||
|
||
class Send < OpenStruct | ||
def initialize(attrs = {}) | ||
attrs.reverse_merge!(type: 'event') | ||
|
@@ -28,6 +31,18 @@ def tracker | |
options[:tracker].respond_to?(:call) ? options[:tracker].call(env) : options[:tracker] | ||
end | ||
|
||
def tracker_options | ||
@tracker_options ||= begin | ||
tracker_options = {} | ||
options.slice(*ALLOWED_TRACKER_OPTIONS).each do |key, value| | ||
if option_value = value.respond_to?(:call) ? value.call(env) : value | ||
tracker_options["#{key}".camelize(:lower).to_sym] = "#{option_value}" | ||
end | ||
end | ||
tracker_options | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this maybe more generic? always camelize the keys, support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point and useful for the cookie_domain with an i18n multihost setup 17000d3 |
||
|
||
def render | ||
Tilt.new( File.join( File.dirname(__FILE__), 'template', 'google_analytics.erb') ).render(self) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
RSpec.describe Rack::Tracker::GoogleAnalytics do | ||
|
||
def env | ||
{misc: 'foobar'} | ||
{ | ||
misc: 'foobar', | ||
user_id: '123' | ||
} | ||
end | ||
|
||
it 'will be placed in the head' do | ||
|
@@ -30,6 +33,44 @@ def env | |
end | ||
end | ||
|
||
describe '#tracker_options' do | ||
before do | ||
stub_const("#{described_class}::ALLOWED_TRACKER_OPTIONS", [:some_option]) | ||
end | ||
|
||
context 'with an allowed option configured with a static value' do | ||
subject { described_class.new(env, { some_option: 'value' }) } | ||
|
||
it 'returns hash with option set' do | ||
expect(subject.tracker_options).to eql ({ someOption: 'value' }) | ||
end | ||
end | ||
|
||
context 'with an allowed option configured with a block' do | ||
subject { described_class.new(env, { some_option: lambda { |env| return env[:misc] } }) } | ||
|
||
it 'returns hash with option set' do | ||
expect(subject.tracker_options).to eql ({ someOption: 'foobar' }) | ||
end | ||
end | ||
|
||
context 'with an allowed option configured with a block returning nil' do | ||
subject { described_class.new(env, { some_option: lambda { |env| return env[:non_existing_key] } }) } | ||
|
||
it 'returns an empty hash' do | ||
expect(subject.tracker_options).to eql ({}) | ||
end | ||
end | ||
|
||
context 'with a non allowed option' do | ||
subject { described_class.new(env, { new_option: 'value' }) } | ||
|
||
it 'returns an empty hash' do | ||
expect(subject.tracker_options).to eql ({}) | ||
end | ||
end | ||
end | ||
|
||
describe "with events" do | ||
describe "default" do | ||
def env | ||
|
@@ -93,6 +134,15 @@ def env | |
end | ||
end | ||
|
||
describe "with user_id tracking" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't that something that should only be tested in the integration spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the cookieDomain thingy is tested here too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok i'm just a bit confused as to what i should test where |
||
subject { described_class.new(env, tracker: 'somebody', user_id: lambda { |env| return env[:user_id] } ).render } | ||
|
||
it "will show asyncronous tracker with userId" do | ||
expect(subject).to match(%r{ga\('create', 'somebody', {\"userId\":\"123\"}\)}) | ||
expect(subject).to match(%r{ga\('send', 'pageview'\)}) | ||
end | ||
end | ||
|
||
describe "with enhanced_link_attribution" do | ||
subject { described_class.new(env, tracker: 'happy', enhanced_link_attribution: true).render } | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know its a matter of taste, but personally i don't like these temporary variables,
tap
is your fried to get rid of 'em