diff --git a/README.md b/README.md index 85bd6e2..96de0c5 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ The following fields are provided in the migration: t.string "request_hash" # unique ID per request, in case you want to log multiple impressions and group them t.string "session_hash" # logs the rails session t.string "ip_address" # request.remote_ip + t.text "params" # request.params, except action name, controller name and resource id t.string "referrer" # request.referer t.string "message" # custom message you can add t.datetime "created_at" # I am not sure what this is.... Any clue? @@ -114,7 +115,12 @@ Usage @widget.impressionist_count(:filter=>:ip_address) -7. Get the unique impression count from a model filtered by session hash. Same +7. Get the unique impression count from a model filtered by params. This + in turn will give you impressions with unique params. + + @widget.impressionist_count(:filter => :params) + +8. Get the unique impression count from a model filtered by session hash. Same as #6 regarding request hash. This may be more desirable than filtering by IP address depending on your situation, since filtering by IP may ignore visitors that use the same IP. The downside to this filtering is that a @@ -122,12 +128,12 @@ Usage @widget.impressionist_count(:filter=>:session_hash) -8. Get total impression count. This may return more than 1 impression per http +9. Get total impression count. This may return more than 1 impression per http request, depending on how you are logging impressions @widget.impressionist_count(:filter=>:all) -9. Get impression count by message. This only counts impressions of the given message. +10. Get impression count by message. This only counts impressions of the given message. @widget.impressionist_count(:message=>"pageview", :filter=>:all) @@ -182,6 +188,9 @@ impressions in your controller: # only record impression if session is unique impressionist :unique => [:session_hash] + + # only record impression if param is unique + impressionist :unique => [:params] Or you can use the `impressionist` method directly: 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/lib/generators/active_record/templates/create_impressions_table.rb b/lib/generators/active_record/templates/create_impressions_table.rb index c874863..a9f8248 100644 --- a/lib/generators/active_record/templates/create_impressions_table.rb +++ b/lib/generators/active_record/templates/create_impressions_table.rb @@ -12,6 +12,7 @@ class CreateImpressionsTable < ActiveRecord::Migration t.string :session_hash t.text :message t.text :referrer + t.text :params t.timestamps end add_index :impressions, [:impressionable_type, :message, :impressionable_id], :name => "impressionable_type_message_index", :unique => false, :length => {:message => 255 } @@ -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/lib/impressionist/models/active_record/impression.rb b/lib/impressionist/models/active_record/impression.rb index b32a8b8..7670160 100644 --- a/lib/impressionist/models/active_record/impression.rb +++ b/lib/impressionist/models/active_record/impression.rb @@ -9,6 +9,6 @@ class Impression < ActiveRecord::Base # sets belongs_to and attr_accessible depending on Rails version Impressionist::SetupAssociation.new(self).set + store :params after_save :impressionable_counter_cache_updatable? - end diff --git a/lib/impressionist/setup_association.rb b/lib/impressionist/setup_association.rb index 95286e8..1bc5227 100644 --- a/lib/impressionist/setup_association.rb +++ b/lib/impressionist/setup_association.rb @@ -35,7 +35,8 @@ module Impressionist :view_name, :referrer, :message, - :user_id) + :user_id, + :params) end def toggle diff --git a/tests/spec/setup_association_spec.rb b/tests/spec/setup_association_spec.rb index ee189c6..644ddc3 100644 --- a/tests/spec/setup_association_spec.rb +++ b/tests/spec/setup_association_spec.rb @@ -9,10 +9,10 @@ module Impressionist before do # expects attr_accessible to return true - # and pass 11 arguments + # and pass 12 arguments mock. expect(:attr_accessible, true) do |args| - args.size == 11 + args.size == 12 end 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