Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #188 from asharma-ror/params_feature

Params feature
  • Loading branch information...
commit 729e4fca3ba414842fbf357b8b18bcbdf2ee2371 2 parents ae01d1c + 04d80ef
@johnmcaliley johnmcaliley authored
View
15 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:
View
30 app/controllers/impressionist_controller.rb
@@ -3,7 +3,7 @@
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 @@ def associative_create_statement(query_params={})
: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 @@ def unique_instance?(impressionable, unique_opts)
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 @@ def session_hash
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
View
2  lib/generators/active_record/templates/create_impressions_table.rb
@@ -12,6 +12,7 @@ def self.up
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 @@ def self.up
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
View
2  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
View
3  lib/impressionist/setup_association.rb
@@ -35,7 +35,8 @@ def make_accessible
:view_name,
:referrer,
:message,
- :user_id)
+ :user_id,
+ :params)
end
def toggle
View
4 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
View
2  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
View
2  tests/test_app/db/migrate/20130719024021_create_impressions_table.rb
@@ -11,6 +11,7 @@ def self.up
t.string :ip_address
t.string :session_hash
t.text :message
+ t.text :params
t.text :referrer
t.timestamps
end
@@ -21,6 +22,7 @@ def self.up
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
View
2  tests/test_app/db/schema.rb
@@ -41,6 +41,7 @@
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 @@
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"
View
18 tests/test_app/spec/controllers/articles_controller_spec.rb
@@ -53,8 +53,24 @@
click_link "Same Page"
Impression.last.referrer.should eq "http://test.host/articles/1"
end
-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
View
9 tests/test_app/spec/controllers/posts_controller_spec.rb
@@ -16,4 +16,13 @@
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
View
33 tests/test_app/spec/controllers/widgets_controller_spec.rb
@@ -44,7 +44,6 @@
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 @@
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
Please sign in to comment.
Something went wrong with that request. Please try again.