diff --git a/app/controllers/impressionist_controller.rb b/app/controllers/impressionist_controller.rb index d9a9119..b92fcbf 100644 --- a/app/controllers/impressionist_controller.rb +++ b/app/controllers/impressionist_controller.rb @@ -3,7 +3,7 @@ require 'digest/sha2' module ImpressionistController module ClassMethods def impressionist(opts={}) - before_filter { |c| c.impressionist_subapp_filter(opts)} + before_filter { |c| c.impressionist_subapp_filter(opts) } end end @@ -50,7 +50,8 @@ module ImpressionistController :request_hash => @impressionist_hash, :session_hash => session_hash, :ip_address => request.remote_ip, - :referrer => request.referer + :referrer => request.referer, + :params => params_hash ) end @@ -81,9 +82,28 @@ module ImpressionistController end def unique?(unique_opts) - return unique_opts.blank? || !Impression.where(unique_query(unique_opts)).exists? + return unique_opts.blank? || check_impression?(unique_opts) end + def check_impression?(unique_opts) + impressions = Impression.where(unique_query(unique_opts - [:params])) + check_unique_impression?(impressions, unique_opts) + end + + def check_unique_impression?(impressions, unique_opts) + impressions_present = impressions.exists? + impressions_present && unique_opts_has_params?(unique_opts) ? check_unique_with_params?(impressions) : !impressions_present + end + + def unique_opts_has_params?(unique_opts) + unique_opts.include?(:params) + end + + def check_unique_with_params?(impressions) + request_param = params_hash + impressions.detect{|impression| impression.params == request_param }.nil? + end + # creates the query to check for uniqueness def unique_query(unique_opts,impressionable=nil) full_statement = direct_create_statement({},impressionable) @@ -112,6 +132,10 @@ module ImpressionistController request.session_options[:id] end + def params_hash + request.params.except(:controller, :action, :id) + end + #use both @current_user and current_user helper def user_id user_id = @current_user ? @current_user.id : nil rescue nil diff --git a/app/models/impression.rb b/app/models/impression.rb index 9cb2d4e..0fbfd97 100644 --- a/app/models/impression.rb +++ b/app/models/impression.rb @@ -1,2 +1,4 @@ class Impression + attr_accessible :params + store :params end diff --git a/tests/test_app/app/controllers/widgets_controller.rb b/tests/test_app/app/controllers/widgets_controller.rb index b6b99a7..e533893 100644 --- a/tests/test_app/app/controllers/widgets_controller.rb +++ b/tests/test_app/app/controllers/widgets_controller.rb @@ -1,5 +1,5 @@ class WidgetsController < ApplicationController - impressionist :actions=>[:show,:index], :unique => [:controller_name,:action_name,:impressionable_id] + impressionist :actions=>[:show, :index], :unique => [:controller_name, :action_name, :impressionable_id, :params] def show end diff --git a/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb b/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb index c874863..d3dec32 100644 --- a/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb +++ b/tests/test_app/db/migrate/20130719024021_create_impressions_table.rb @@ -11,6 +11,7 @@ class CreateImpressionsTable < ActiveRecord::Migration t.string :ip_address t.string :session_hash t.text :message + t.text :params t.text :referrer t.timestamps end @@ -21,6 +22,7 @@ class CreateImpressionsTable < ActiveRecord::Migration add_index :impressions, [:controller_name,:action_name,:request_hash], :name => "controlleraction_request_index", :unique => false add_index :impressions, [:controller_name,:action_name,:ip_address], :name => "controlleraction_ip_index", :unique => false add_index :impressions, [:controller_name,:action_name,:session_hash], :name => "controlleraction_session_index", :unique => false + add_index :impressions, [:impressionable_type, :impressionable_id, :params], :name => "poly_params_request_index", :unique => false add_index :impressions, :user_id end diff --git a/tests/test_app/db/schema.rb b/tests/test_app/db/schema.rb index fd9df3f..cac1d3b 100644 --- a/tests/test_app/db/schema.rb +++ b/tests/test_app/db/schema.rb @@ -41,6 +41,7 @@ ActiveRecord::Schema.define(:version => 20150207140310) do t.string "ip_address" t.string "session_hash" t.text "message" + t.text "params" t.text "referrer" t.datetime "created_at", :null => false t.datetime "updated_at", :null => false @@ -50,6 +51,7 @@ ActiveRecord::Schema.define(:version => 20150207140310) do add_index "impressions", ["controller_name", "action_name", "request_hash"], :name => "controlleraction_request_index" add_index "impressions", ["controller_name", "action_name", "session_hash"], :name => "controlleraction_session_index" add_index "impressions", ["impressionable_type", "impressionable_id", "ip_address"], :name => "poly_ip_index" + add_index "impressions", ["impressionable_type", "impressionable_id", "params"], :name => "poly_params_request_index" add_index "impressions", ["impressionable_type", "impressionable_id", "request_hash"], :name => "poly_request_index" add_index "impressions", ["impressionable_type", "impressionable_id", "session_hash"], :name => "poly_session_index" add_index "impressions", ["impressionable_type", "message", "impressionable_id"], :name => "impressionable_type_message_index" diff --git a/tests/test_app/spec/controllers/articles_controller_spec.rb b/tests/test_app/spec/controllers/articles_controller_spec.rb index c32ea15..15454cd 100644 --- a/tests/test_app/spec/controllers/articles_controller_spec.rb +++ b/tests/test_app/spec/controllers/articles_controller_spec.rb @@ -53,8 +53,24 @@ describe ArticlesController do click_link "Same Page" Impression.last.referrer.should eq "http://test.host/articles/1" end + + it "should log request with params (checked = true)" do + get "show", id: 1, checked: true + Impression.last.params.should eq({"checked"=>true}) + Impression.last.request_hash.size.should eq 64 + Impression.last.ip_address.should eq "0.0.0.0" + Impression.last.session_hash.size.should eq 32 + Impression.last.referrer.should eq nil + end + + it "should log request with params {}" do + get "index" + Impression.last.params.should eq({}) + Impression.last.request_hash.size.should eq 64 + Impression.last.ip_address.should eq "0.0.0.0" + Impression.last.session_hash.size.should eq 32 + Impression.last.referrer.should eq nil + end end - - diff --git a/tests/test_app/spec/controllers/posts_controller_spec.rb b/tests/test_app/spec/controllers/posts_controller_spec.rb index 78cbe84..78827e2 100644 --- a/tests/test_app/spec/controllers/posts_controller_spec.rb +++ b/tests/test_app/spec/controllers/posts_controller_spec.rb @@ -16,4 +16,13 @@ describe PostsController do Post.first.impressions.last.user_id.should eq 123 end + it "should log impression at the action level with params" do + get "show", id: 1, checked: true + Impression.all.size.should eq 12 + Impression.last.params.should eq({"checked"=>true}) + Impression.last.controller_name.should eq "posts" + Impression.last.action_name.should eq "show" + Impression.last.impressionable_type.should eq "Post" + Impression.last.impressionable_id.should eq 1 + end end diff --git a/tests/test_app/spec/controllers/widgets_controller_spec.rb b/tests/test_app/spec/controllers/widgets_controller_spec.rb index 08e2569..61f7c28 100644 --- a/tests/test_app/spec/controllers/widgets_controller_spec.rb +++ b/tests/test_app/spec/controllers/widgets_controller_spec.rb @@ -44,7 +44,6 @@ describe WidgetsController do it "should log unique impressions only once per id" do get "show", :id=> 1 Impression.all.size.should eq 12 - get "show", :id=> 2 Impression.all.size.should eq 13 @@ -57,4 +56,36 @@ describe WidgetsController do end + context "Impresionist unique params options" do + it "should log unique impressions at the per action and params level" do + get "show", :id => 1 + Impression.all.size.should eq 12 + get "show", :id => 2, checked: true + Impression.all.size.should eq 13 + get "show", :id => 2, checked: false + Impression.all.size.should eq 14 + get "index" + Impression.all.size.should eq 15 + end + + it "should not log impression for same params and same id" do + get "show", :id => 1 + Impression.all.size.should eq 12 + get "show", :id => 1 + Impression.all.size.should eq 12 + get "show", :id => 1, checked: true + Impression.all.size.should eq 13 + get "show", :id => 1, checked: false + Impression.all.size.should eq 14 + get "show", :id => 1, checked: true + Impression.all.size.should eq 14 + get "show", :id => 1, checked: false + Impression.all.size.should eq 14 + get "show", :id => 1 + Impression.all.size.should eq 14 + get "show", :id => 2 + Impression.all.size.should eq 15 + end + end + end