add session_hash to impression model

changed migration template
added upgrade migration template
added unique_impression_count_session to impressionable model
fixed controller_spec

CHANGELOG.rdoc

@@ -1,3 +1,9 @@
+== 0.3.0 (2011-03-06)
+* added session_hash to impression model
+* migration template updated to add session_hash
+* new count instance method for impressionable model - unique_impression_count_session
+* NOTE: if you are upgrading from 0.2.5, then run the migration in the 'upgrade_migrations' dir
+
 == 0.2.5 (2011-02-17)
 * New model method - @widget.unique_impression_count_ip  - This gives you unique impression account filtered by IP (and in turn request_hash since they have same IPs)
 * @widget.unique_impression_count now uses request_hash.  This was incorrectly stated in the README, since it was using ip_address.  The README is correct as a result of the method change.

README.rdoc

@@ -25,7 +25,7 @@ Rails 3.0.4 and Ruby 1.9.2 (also tested on REE 1.8.7) - Sorry, but you need to u
 
 Add it to your Gemfile
 
-  gem 'impressionist', :git => 'git@github.com:cowboycoded/impressionist.git'
+  gem 'impressionist'
 
 Install with Bundler
 
@@ -81,7 +81,7 @@ The following fields are provided in the migration:
       impressionist(@widget,message:"wtf is a widget?") #message is optional
     end
 
-5. Get unique impression count from a model.  This groups impressions by request_hash, so if you logged multiple impressions per request, it will only count them one time.
+5. Get unique impression count from a model.  This groups impressions by request_hash, so if you logged multiple impressions per request, it will only count them one time.  This unique impression count will not filter out unique users, only unique requests
     @widget.unique_impression_count
     @widget.unique_impression_count("2011-01-01","2011-01-02")  # start date, end date
     @widget.unique_impression_count("2011-01-01")  #specify start date only, end date = now
@@ -90,8 +90,13 @@ The following fields are provided in the migration:
     @widget.unique_impression_count_ip
     @widget.unique_impression_count_ip("2011-01-01","2011-01-02")  # start date, end date
     @widget.unique_impression_count_ip("2011-01-01")  #specify start date only, end date = now
+
+7. 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 user could clear session data in their browser and skew the results.
+    @widget.unique_impression_count_session
+    @widget.unique_impression_count_session("2011-01-01","2011-01-02")  # start date, end date
+    @widget.unique_impression_count_session("2011-01-01")  #specify start date only, end date = now
     
-7. Get total impression count.  This may return more than 1 impression per http request, depending on how you are logging impressions
+8. Get total impression count.  This may return more than 1 impression per http request, depending on how you are logging impressions
     @widget.impression_count
     @widget.impression_count("2011-01-01","2011-01-02")  # start date, end date
     @widget.impression_count("2011-01-01")  #specify start date only, end date = now
@@ -100,7 +105,6 @@ Logging impressions for authenticated users happens automatically.  If you have
 
 
 == Development Roadmap
-TODO (soon): more helpers for impression count to filter controllers, actions, messages, and user_ids  
 * Automatic impression logging in views.  For example, log initial view, and any partials called from initial view
 * Customizable black list for user-agents or IP addresses.  Impressions will be ignored.  Web admin as part of the Engine.
 * Reporting engine

app/controllers/impressionist_controller.rb

@@ -17,6 +17,7 @@ module ImpressionistController
         if obj.respond_to?("impressionable?")
           obj.impressions.create(:message=> message,
                                  :request_hash=> @impressionist_hash,
+                                 :session_hash=> request.session_options[:id],
                                  :ip_address=> request.remote_ip,
                                  :user_id=> user_id,
                                  :controller_name=>controller_name,
@@ -39,6 +40,7 @@ module ImpressionistController
                             :action_name=> action_name,
                             :user_id=> user_id,
                             :request_hash=> @impressionist_hash,
+                            :session_hash=> request.session_options[:id],
                             :ip_address=> request.remote_ip,
                             :impressionable_type=> controller_name.singularize.camelize,
                             :impressionable_id=> params[:id])

app/models/impressionist/impressionable.rb

@@ -18,9 +18,13 @@ module Impressionist
         start_date.blank? ? impressions.group(:request_hash).all.size : impressions.where("created_at>=? and created_at<=?",start_date,end_date).group(:request_hash).all.size
       end
       
-      def unique_impression_count_ip(start_date=nil,end_date=nil)
+      def unique_impression_count_ip(start_date=nil,end_date=Time.now)
         start_date.blank? ? impressions.group(:ip_address).all.size : impressions.where("created_at>=? and created_at<=?",start_date,end_date).group(:ip_address).all.size
       end
+      
+      def unique_impression_count_session(start_date=nil,end_date=Time.now)
+        start_date.blank? ? impressions.group(:session_hash).all.size : impressions.where("created_at>=? and created_at<=?",start_date,end_date).group(:session_hash).all.size
+      end
     end
   end
 end

lib/generators/impressionist/templates/create_impressions_table.rb

@@ -8,18 +8,27 @@ class CreateImpressionsTable < ActiveRecord::Migration
       t.string :action_name
       t.string :view_name
       t.string :request_hash
+      t.string :session_hash
       t.string :ip_address
       t.string :message
       t.timestamps
     end
-    add_index :impressions, [:impressionable_type, :impressionable_id, :request_hash, :ip_address], :name => "poly_index", :unique => false
+    add_index :impressions, [:impressionable_type, :impressionable_id, :request_hash], :name => "poly_request_index", :unique => false
-    add_index :impressions, [:controller_name,:action_name,:request_hash,:ip_address], :name => "controlleraction_index", :unique => false
+    add_index :impressions, [:impressionable_type, :impressionable_id, :ip_address], :name => "poly_ip_index", :unique => false
+    add_index :impressions, [:impressionable_type, :impressionable_id, :session_hash], :name => "poly_session_index", :unique => false    
+    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, :user_id
   end
 
   def self.down
-    remove_index :impressions, :name => :poly_index
+    remove_index :impressions, :name => :poly_request_index
-    remove_index :impressions, :name => :controlleraction_index
+    remove_index :impressions, :name => :poly_ip_index
+    remove_index :impressions, :name => :poly_session_index        
+    remove_index :impressions, :name => :controlleraction_request_index
+    remove_index :impressions, :name => :controlleraction_ip_index
+    remove_index :impressions, :name => :controlleraction_session_index        
     remove_index :impressions, :user_id
     
     drop_table :impressions

test_app/Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: /rails_plugins/mine/impressionist
   specs:
-    impressionist (0.2.4)
+    impressionist (0.2.5)
 
 GEM
   remote: http://rubygems.org/

test_app/spec/controllers/controller_spec.rb

@@ -11,7 +11,7 @@ describe ArticlesController do
   
   it "should log an impression with a message" do
     get "index"
-    Impression.all.size.should eq 11
+    Impression.all.size.should eq 12
     Article.first.impressions.last.message.should eq "this is a test article impression"
     Article.first.impressions.last.controller_name.should eq "articles"
     Article.first.impressions.last.action_name.should eq "index"
@@ -19,7 +19,7 @@ describe ArticlesController do
   
   it "should log an impression without a message" do
     get "show", :id=> 1
-    Impression.all.size.should eq 11
+    Impression.all.size.should eq 12
     Article.first.impressions.last.message.should eq nil
     Article.first.impressions.last.controller_name.should eq "articles"
     Article.first.impressions.last.action_name.should eq "show"
@@ -35,12 +35,19 @@ describe ArticlesController do
     get "show", :id=> 1
     Article.first.impressions.last.user_id.should eq nil
   end
+  
+  it "should log the request_hash, ip_address, and session_hash" do
+    get "show", :id=> 1
+    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
+  end
 end  
 
 describe PostsController do
   it "should log impression at the action level" do
     get "show", :id=> 1
-    Impression.all.size.should eq 11
+    Impression.all.size.should eq 12
     Impression.last.controller_name.should eq "posts"
     Impression.last.action_name.should eq "show"
     Impression.last.impressionable_type.should eq "Post"
@@ -57,23 +64,23 @@ end
 describe WidgetsController do
   it "should log impression at the per action level" do
     get "show", :id=> 1
-    Impression.all.size.should eq 11
+    Impression.all.size.should eq 12
     get "index"
-    Impression.all.size.should eq 12
+    Impression.all.size.should eq 13
     get "new"
-    Impression.all.size.should eq 12
+    Impression.all.size.should eq 13
   end
   
   it "should not log impression when user-agent is in wildcard list" do
     request.stub!(:user_agent).and_return('somebot')
     get "show", :id=> 1
-    Impression.all.size.should eq 10
+    Impression.all.size.should eq 11
   end
   
   it "should not log impression when user-agent is in the bot list" do
     request.stub!(:user_agent).and_return('Acoon Robot v1.50.001')
     get "show", :id=> 1
-    Impression.all.size.should eq 10    
+    Impression.all.size.should eq 11    
   end
 end
   

test_app/spec/fixtures/impressions.yml

@@ -3,6 +3,7 @@ impression<%= i %>:
   impressionable_type: Article
   impressionable_id: 1
   request_hash: a<%=i%>
+  session_hash: b<%=i%>
   ip_address: 127.0.0.<%=i%>
   created_at: 2011-01-01
 <% end %>
@@ -12,6 +13,7 @@ impression8:
   impressionable_type: Article
   impressionable_id: 1
   request_hash: a1
+  session_hash: b1
   ip_address: 127.0.0.1
   created_at: 2010-01-01
   
@@ -19,6 +21,7 @@ impression9:
   impressionable_type: Article
   impressionable_id: 1
   request_hash: a1
+  session_hash: b2
   ip_address: 127.0.0.1
   created_at: 2011-01-03
     
@@ -26,6 +29,7 @@ impression10:
   impressionable_type: Article
   impressionable_id: 1
   request_hash: a9
+  session_hash: b3
   ip_address: 127.0.0.8
   created_at: 2010-01-01  
   
@@ -33,6 +37,7 @@ impression11:
   impressionable_type: Article
   impressionable_id: 1
   request_hash: a10
+  session_hash: b4  
   ip_address: 127.0.0.1
   created_at: 2010-01-01  
   

test_app/spec/models/model_spec.rb

@@ -44,4 +44,8 @@ describe Impression do
   it "should return unique impression count using ip address (which in turn eliminates duplicate request_hashes)" do
     @article.unique_impression_count_ip.should eq 8
   end
+
+  it "should return unique impression count using session_hash (which in turn eliminates duplicate request_hashes)" do
+    @article.unique_impression_count_session.should eq 7
+  end
 end

test_app/spec/rails_generators/rails_generators_spec.rb

@@ -13,7 +13,7 @@ describe Impressionist do
   end
   
   it "should run the migration created in the previous spec" do
-    migrate_output = systemu("rake db:migrate")
+    migrate_output = systemu("rake db:migrate RAILS_ENV=test")
     migrate_output[1].include?("CreateImpressionsTable: migrated").should be true
   end
 end

upgrade_migrations/version_0_3_0.rb

@@ -0,0 +1,27 @@
+class CreateImpressionsTable < ActiveRecord::Migration
+  def self.up
+    add_column :impressions, :session_hash, :string
+    remove_index :impressions, :name => :poly_index
+    remove_index :impressions, :name => :controlleraction_index
+    add_index :impressions, [:impressionable_type, :impressionable_id, :request_hash], :name => "poly_request_index", :unique => false
+    add_index :impressions, [:impressionable_type, :impressionable_id, :ip_address], :name => "poly_ip_index", :unique => false
+    add_index :impressions, [:impressionable_type, :impressionable_id, :session_hash], :name => "poly_session_index", :unique => false    
+    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        
+    
+  end
+
+  def self.down
+    remove_column :impressions, :session_hash
+    remove_index :impressions, :name => :poly_request_index
+    remove_index :impressions, :name => :poly_ip_index
+    remove_index :impressions, :name => :poly_session_index        
+    remove_index :impressions, :name => :controlleraction_request_index
+    remove_index :impressions, :name => :controlleraction_ip_index
+    remove_index :impressions, :name => :controlleraction_session_index        
+    remove_index :impressions, :user_id
+    add_index :impressions, [:impressionable_type, :impressionable_id, :request_hash, :ip_address], :name => "poly_index", :unique => false
+    add_index :impressions, [:controller_name,:action_name,:request_hash,:ip_address], :name => "controlleraction_index", :unique => false    
+  end
+end