Merge pull request #188 from asharma-ror/params_feature

Params feature

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:
 

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

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
 

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

lib/impressionist/setup_association.rb

@@ -35,7 +35,8 @@ module Impressionist
                         :view_name,
                         :referrer,
                         :message,
-                        :user_id)
+                        :user_id,
+                        :params)
       end
 
       def toggle

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

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

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
 

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"

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
 
 
-
-

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

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