From 405d376366ef5b49fe80813efbec63761c7703e8 Mon Sep 17 00:00:00 2001 From: cowboycoded Date: Sun, 6 Mar 2011 23:22:39 -0500 Subject: [PATCH] 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 | 6 +++++ README.rdoc | 12 ++++++--- app/controllers/impressionist_controller.rb | 2 ++ app/models/impressionist/impressionable.rb | 6 ++++- .../templates/create_impressions_table.rb | 17 +++++++++--- test_app/Gemfile.lock | 2 +- test_app/spec/controllers/controller_spec.rb | 23 ++++++++++------ test_app/spec/fixtures/impressions.yml | 5 ++++ test_app/spec/models/model_spec.rb | 4 +++ .../rails_generators/rails_generators_spec.rb | 2 +- upgrade_migrations/version_0_3_0.rb | 27 +++++++++++++++++++ 11 files changed, 87 insertions(+), 19 deletions(-) create mode 100644 upgrade_migrations/version_0_3_0.rb diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 52963f7..d1d054d 100644 --- a/CHANGELOG.rdoc +++ b/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. diff --git a/README.rdoc b/README.rdoc index 1bc5fc9..cb36b1b 100644 --- a/README.rdoc +++ b/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 diff --git a/app/controllers/impressionist_controller.rb b/app/controllers/impressionist_controller.rb index cf0b64e..1afcd53 100644 --- a/app/controllers/impressionist_controller.rb +++ b/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]) diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb index 6efc533..20abb8f 100644 --- a/app/models/impressionist/impressionable.rb +++ b/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 diff --git a/lib/generators/impressionist/templates/create_impressions_table.rb b/lib/generators/impressionist/templates/create_impressions_table.rb index 751b536..7d69cd2 100644 --- a/lib/generators/impressionist/templates/create_impressions_table.rb +++ b/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, [:controller_name,:action_name,:request_hash,:ip_address], :name => "controlleraction_index", :unique => false + 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 add_index :impressions, :user_id end def self.down - remove_index :impressions, :name => :poly_index - remove_index :impressions, :name => :controlleraction_index + 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 drop_table :impressions diff --git a/test_app/Gemfile.lock b/test_app/Gemfile.lock index 8d0d14c..bbae86d 100644 --- a/test_app/Gemfile.lock +++ b/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/ diff --git a/test_app/spec/controllers/controller_spec.rb b/test_app/spec/controllers/controller_spec.rb index b315b0a..da0522d 100644 --- a/test_app/spec/controllers/controller_spec.rb +++ b/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 \ No newline at end of file diff --git a/test_app/spec/fixtures/impressions.yml b/test_app/spec/fixtures/impressions.yml index b397089..21bf9e1 100644 --- a/test_app/spec/fixtures/impressions.yml +++ b/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 \ No newline at end of file diff --git a/test_app/spec/models/model_spec.rb b/test_app/spec/models/model_spec.rb index 439ebaf..70e230a 100644 --- a/test_app/spec/models/model_spec.rb +++ b/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 \ No newline at end of file diff --git a/test_app/spec/rails_generators/rails_generators_spec.rb b/test_app/spec/rails_generators/rails_generators_spec.rb index 90c377d..7059b22 100644 --- a/test_app/spec/rails_generators/rails_generators_spec.rb +++ b/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 \ No newline at end of file diff --git a/upgrade_migrations/version_0_3_0.rb b/upgrade_migrations/version_0_3_0.rb new file mode 100644 index 0000000..63c8e51 --- /dev/null +++ b/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 \ No newline at end of file