From 2e72b5aefa18d7064c73cfbeeed50167365627dd Mon Sep 17 00:00:00 2001
From: asharma-ror <asharma.ror@gmail.com>
Date: Tue, 21 Apr 2015 15:51:47 +0530
Subject: [PATCH 1/4] params feature

---
 app/controllers/impressionist_controller.rb   | 30 +++++++++++++++--
 app/models/impression.rb                      |  2 ++
 .../app/controllers/widgets_controller.rb     |  2 +-
 ...20130719024021_create_impressions_table.rb |  2 ++
 tests/test_app/db/schema.rb                   |  2 ++
 .../controllers/articles_controller_spec.rb   | 20 +++++++++--
 .../spec/controllers/posts_controller_spec.rb |  9 +++++
 .../controllers/widgets_controller_spec.rb    | 33 ++++++++++++++++++-
 8 files changed, 93 insertions(+), 7 deletions(-)

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

From 9d997bc600d4329aa264a761dcfa5fcc3bdd6df2 Mon Sep 17 00:00:00 2001
From: asharma-ror <asharma.ror@gmail.com>
Date: Tue, 21 Apr 2015 15:55:37 +0530
Subject: [PATCH 2/4] readme for params feature

---
 README.md | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index f2fd31f..67f0978 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:
 

From 8c3407fd3339cafa22a16c50b85acd15d5e2b8d4 Mon Sep 17 00:00:00 2001
From: asharma-ror <asharma.ror@gmail.com>
Date: Wed, 22 Apr 2015 10:31:22 +0530
Subject: [PATCH 3/4] Adding params field to migration file.

---
 app/models/impression.rb                                       | 2 --
 .../active_record/templates/create_impressions_table.rb        | 2 ++
 lib/impressionist/models/active_record/impression.rb           | 2 +-
 lib/impressionist/setup_association.rb                         | 3 ++-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/models/impression.rb b/app/models/impression.rb
index 0fbfd97..9cb2d4e 100644
--- a/app/models/impression.rb
+++ b/app/models/impression.rb
@@ -1,4 +1,2 @@
 class Impression
-  attr_accessible :params
-  store :params
 end
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

From 04d80ef1720a1dc46bd39b1dfbddf1cce8adc868 Mon Sep 17 00:00:00 2001
From: asharma-ror <asharma.ror@gmail.com>
Date: Wed, 22 Apr 2015 11:08:10 +0530
Subject: [PATCH 4/4] Fixed the failing case.

---
 tests/spec/setup_association_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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