make criteo events more generic

* use only one loop in the .erb template
* add global events (setSiteType, setAccount, setCustomerId) as Rack::Tracker::Criteo::Events

Author: jhilden

lib/rack/tracker/criteo/criteo.rb

@@ -1,4 +1,12 @@
 class Criteo <  Rack::Tracker::Handler
+
+  TRACKER_OPTIONS = {
+    # option/event name => event key name, e.g. { event: 'setSiteType', type: '' }
+    set_site_type: :type,
+    set_account: :account,
+    set_customer_id: :id
+  }
+
   class Event < OpenStruct
     def write
       to_h.to_json
@@ -7,17 +15,16 @@ def write
 
   self.position = :body
 
-  def tracker_options
-    @tracker_options ||= begin
-      tracker_options = {}
-
-      user_id = options[:user_id].call(env) if options[:user_id]
-      tracker_options[:user_id] = "#{user_id}" if user_id.present?
-
-      site_type = options[:site_type].call(env) if options[:site_type]
-      tracker_options[:site_type] = "#{site_type}" if site_type.present?
-
-      tracker_options
+  # global events for each tracker instance
+  def tracker_events
+    @tracker_events ||= begin
+      tracker_events = []
+      options.slice(*TRACKER_OPTIONS.keys).each do |key, value|
+        if option_value = value.respond_to?(:call) ? value.call(env) : value
+          tracker_events << Event.new(:event => "#{key}".camelize(:lower),  TRACKER_OPTIONS[key] => "#{option_value}")
+        end
+      end
+      tracker_events
     end
   end
 

lib/rack/tracker/criteo/template/criteo.erb

@@ -2,18 +2,7 @@
 <script type: 'text/javascript', src: '//static.criteo.net/js/ld/ld.js', async: 'true' />
 <script type='text/javascript'>
 window.criteo_q = window.criteo_q || [];
-
-window.criteo_q.push({ event: 'setAccount', account: '<%= options[:account_id] %>' });
-
-<% if tracker_options[:site_type] %>
-window.criteo_q.push({ event: 'setSiteType', type: '<%= tracker_options[:site_type] %>' });
-<% end %>
-
-<% if tracker_options[:user_id] %>
-window.criteo_q.push({ event: 'setCustomerId', id: '<%= tracker_options[:user_id] %>' });
-<% end %>
-
-<% events.each do |event| %>
+<% (tracker_events + events).each do |event| %>
 window.criteo_q.push(<%= event.write %>);
 <% end %>
 </script>

spec/handler/criteo_spec.rb

@@ -59,41 +59,14 @@ def env
     end
   end
 
-  describe '#tracker_options' do
-    describe 'with site_type' do
-      context 'returning a value' do
-        subject { described_class.new(env, { site_type: ->(env){ 'd' } }) }
-
-        it 'returns hash with userId' do
-          expect(subject.tracker_options).to eql ({ site_type: 'd' })
-        end
-      end
-
-      context 'returning nil' do
-        subject { described_class.new(env, { site_type: ->(env){ nil } }) }
-
-        it 'returns hash without userId' do
-          expect(subject.tracker_options).to eql ({ })
-        end
-      end
-    end
-
-    describe 'with user_id option' do
-      context 'returning a value' do
-        subject { described_class.new(env, { user_id: ->(env){ '123' } }) }
-
-        it 'returns hash with userId' do
-          expect(subject.tracker_options).to eql ({ user_id: '123' })
-        end
-      end
-
-      context 'returning nil' do
-        subject { described_class.new(env, { user_id: ->(env){ nil } }) }
-
-        it 'returns hash without userId' do
-          expect(subject.tracker_options).to eql ({ })
-        end
-      end
+  describe '#tracker_events' do
+    subject { described_class.new(env, { set_account: '1234', set_site_type: ->(env){ 'd' }, set_customer_id: ->(env){ nil } }) }
+
+    specify do
+      expect(subject.tracker_events).to match_array [
+        Rack::Tracker::Criteo::Event.new(event: 'setSiteType', type: 'd'),
+        Rack::Tracker::Criteo::Event.new(event: 'setAccount', account: '1234')
+      ]
     end
   end
 

spec/integration/criteo_integration_spec.rb

@@ -4,9 +4,9 @@
   before do
     setup_app(action: :criteo) do |tracker|
       tracker.handler(:criteo, {
-        account_id: '1234',
-        user_id: ->(env){ '4711' },
-        site_type: ->(env) { 'm' }
+        set_account: '1234',
+        set_customer_id: ->(env){ '4711' },
+        set_site_type: ->(env){ 'm' }
       })
     end
     visit '/'
@@ -15,22 +15,22 @@
   subject { page }
 
   it 'should include all the basic pushes' do
-    expect(page.find("body")).to have_content("window.criteo_q.push({ event: 'setAccount', account: '1234' });")
-    expect(page.find("body")).to have_content("window.criteo_q.push({ event: 'setSiteType', type: 'm' });")
-    expect(page.find("body")).to have_content("window.criteo_q.push({ event: 'setCustomerId', id: '4711' });")
+    expect(page.find("body")).to have_content("window.criteo_q.push({\"event\":\"setAccount\",\"account\":\"1234\"});")
+    expect(page.find("body")).to have_content("window.criteo_q.push({\"event\":\"setSiteType\",\"type\":\"m\"});")
+    expect(page.find("body")).to have_content("window.criteo_q.push({\"event\":\"setCustomerId\",\"id\":\"4711\"});")
   end
 
   describe 'adjust tracker position via options' do
     before do
       setup_app(action: :criteo) do |tracker|
-        tracker.handler :criteo, { account_id: '1234', position: :head }
+        tracker.handler :criteo, { set_account: '1234', position: :head }
       end
       visit '/'
     end
 
     it "will be placed in the specified tag" do
      expect(page.find("body")).to_not have_content('criteo')
-     expect(page.find("head")).to have_content("{ event: 'setAccount', account: '1234' }")
+     expect(page.find("head")).to have_content("{\"event\":\"setAccount\",\"account\":\"1234\"}")
     end
 
   end