Merge pull request #144 from beghbali/master

added :if and :unless conditionals to impressionist

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[:actions], opts[:unique])}
+      before_filter { |c| c.impressionist_subapp_filter(opts)}
     end
   end
 
@@ -13,7 +13,7 @@ module ImpressionistController
     end
 
     def impressionist(obj,message=nil,opts={})
-      unless bypass
+      if should_count_impression?(opts)
         if obj.respond_to?("impressionable?")
           if unique_instance?(obj, opts[:unique])
             obj.impressions.create(associative_create_statement({:message => message}))
@@ -29,10 +29,11 @@ module ImpressionistController
       @impressionist_hash = Digest::SHA2.hexdigest(Time.now.to_f.to_s+rand(10000).to_s)
     end
 
-    def impressionist_subapp_filter(actions=nil,unique_opts=nil)
+    def impressionist_subapp_filter(opts = {})
-      unless bypass
+      if should_count_impression?(opts)
+        actions = opts[:actions]
         actions.collect!{|a|a.to_s} unless actions.blank?
-        if (actions.blank? || actions.include?(action_name)) && unique?(unique_opts)
+        if (actions.blank? || actions.include?(action_name)) && unique?(opts[:unique])
           Impression.create(direct_create_statement)
         end
       end
@@ -59,6 +60,22 @@ module ImpressionistController
       Impressionist::Bots.bot?(request.user_agent)
     end
 
+    def should_count_impression?(opts)
+      !bypass && condition_true?(opts[:if]) && condition_false?(opts[:unless])
+    end
+
+    def condition_true?(condition)
+      condition.present? ? conditional?(condition) : true
+    end
+
+    def condition_false?(condition)
+      condition.present? ? !conditional?(condition) : true
+    end
+
+    def conditional?(condition)
+      condition.is_a?(Symbol) ? self.send(condition) : condition.call
+    end
+
     def unique_instance?(impressionable, unique_opts)
       return unique_opts.blank? || !impressionable.impressions.where(unique_query(unique_opts)).exists?
     end

tests/test_app/config/initializers/impression.rb

@@ -5,4 +5,4 @@
 #end
 
 
-Impressionist.orm = :active_record
+

tests/test_app/spec/controllers/impressionist_uniqueness_spec.rb

@@ -10,63 +10,63 @@ describe DummyController do
   describe "impressionist filter uniqueness" do
 
     it "should ignore uniqueness if not requested" do
-      controller.impressionist_subapp_filter(nil, nil)
+      controller.impressionist_subapp_filter
-      controller.impressionist_subapp_filter(nil, nil)
+      controller.impressionist_subapp_filter
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize unique session" do
       controller.stub(:session_hash).and_return(request.session_options[:id])
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique ip" do
       controller.request.stub(:remote_ip).and_return("1.2.3.4")
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique request" do
-      controller.impressionist_subapp_filter(nil, [:request_hash])
+      controller.impressionist_subapp_filter(unique: [:request_hash])
-      controller.impressionist_subapp_filter(nil, [:request_hash])
+      controller.impressionist_subapp_filter(unique: [:request_hash])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique action" do
       controller.stub(:action_name).and_return("test_action")
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique controller" do
       controller.stub(:controller_name).and_return("post")
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique user" do
       controller.stub(:user_id).and_return(42)
-      controller.impressionist_subapp_filter(nil, [:user_id])
+      controller.impressionist_subapp_filter(unique: [:user_id])
-      controller.impressionist_subapp_filter(nil, [:user_id])
+      controller.impressionist_subapp_filter(unique: [:user_id])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique referer" do
       controller.request.stub(:referer).and_return("http://foo/bar")
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize unique id" do
       controller.stub(:params).and_return({:id => "666"}) # for correct impressionable id in filter
-      controller.impressionist_subapp_filter(nil, [:impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_id])
-      controller.impressionist_subapp_filter(nil, [:impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_id])
       Impression.should have(@impression_count + 1).records
     end
 
@@ -74,104 +74,104 @@ describe DummyController do
     it "should recognize different controller and action" do
       controller.stub(:controller_name).and_return("post")
       controller.stub(:action_name).and_return("test_action")
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
       Impression.should have(@impression_count + 1).records
       controller.stub(:action_name).and_return("another_action")
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
       Impression.should have(@impression_count + 2).records
       controller.stub(:controller_name).and_return("article")
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name, :action_name])
       Impression.should have(@impression_count + 3).records
     end
 
     it "should recognize different action" do
       controller.stub(:action_name).and_return("test_action")
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
       Impression.should have(@impression_count + 1).records
       controller.stub(:action_name).and_return("another_action")
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize different controller" do
       controller.stub(:controller_name).and_return("post")
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
       Impression.should have(@impression_count + 1).records
       controller.stub(:controller_name).and_return("article")
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
-      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(unique: [:controller_name])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize different session" do
       controller.stub(:session_hash).and_return("foo")
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
       Impression.should have(@impression_count + 1).records
       controller.stub(:session_hash).and_return("bar")
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
-      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(unique: [:session_hash])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize different ip" do
       controller.request.stub(:remote_ip).and_return("1.2.3.4")
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
       Impression.should have(@impression_count + 1).records
       controller.request.stub(:remote_ip).and_return("5.6.7.8")
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize different referer" do
       controller.request.stub(:referer).and_return("http://foo/bar")
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
       Impression.should have(@impression_count + 1).records
       controller.request.stub(:referer).and_return("http://bar/fo")
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
-      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(unique: [:referrer])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize different id" do
       controller.stub(:params).and_return({:id => "666"}) # for correct impressionable id in filter
-      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_type, :impressionable_id])
-      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_type, :impressionable_id])
       controller.stub(:params).and_return({:id => "42"}) # for correct impressionable id in filter
-      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_type, :impressionable_id])
-      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(unique: [:impressionable_type, :impressionable_id])
       Impression.should have(@impression_count + 2).records
     end
 
     it "should recognize combined uniqueness" do
       controller.stub(:action_name).and_return("test_action")
-      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash, :action_name])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :request_hash, :action_name])
-      controller.impressionist_subapp_filter(nil, [:request_hash, :ip_address, :action_name])
+      controller.impressionist_subapp_filter(unique: [:request_hash, :ip_address, :action_name])
-      controller.impressionist_subapp_filter(nil, [:request_hash, :action_name])
+      controller.impressionist_subapp_filter(unique: [:request_hash, :action_name])
-      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :action_name])
-      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :request_hash])
-      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(unique: [:action_name])
-      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(unique: [:ip_address])
-      controller.impressionist_subapp_filter(nil, [:request_hash])
+      controller.impressionist_subapp_filter(unique: [:request_hash])
       Impression.should have(@impression_count + 1).records
     end
 
     it "should recognize combined non-uniqueness" do
       controller.stub(:action_name).and_return(nil)
-      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :action_name])
       controller.stub(:action_name).and_return("test_action")
-      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :action_name])
       controller.stub(:action_name).and_return("another_action")
-      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :action_name])
       Impression.should have(@impression_count + 3).records
     end
 
@@ -299,12 +299,83 @@ describe DummyController do
       controller.stub(:session_hash).and_return("foo")
       controller.request.stub(:remote_ip).and_return("1.2.3.4")
       # order of the following methods is important for the test!
-      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash, :session_hash])
+      controller.impressionist_subapp_filter(unique: [:ip_address, :request_hash, :session_hash])
       controller.impressionist(impressionable, nil, :unique => [:ip_address, :request_hash, :session_hash])
       Impression.should have(@impression_count + 1).records
     end
 
   end
 
+  shared_examples_for 'an impressionable action' do
+    it 'should record an impression' do
+      controller.impressionist_subapp_filter(condition)
+      Impression.should have(@impression_count + 1).records
+    end
+  end
+
+  shared_examples_for 'an unimpressionable action' do
+    it 'should record an impression' do
+      controller.impressionist_subapp_filter(condition)
+      Impression.should have(@impression_count).records
+    end
+  end
+
+  describe "conditional impressions" do
+    describe ":if condition" do
+      context "true condition" do
+        before do
+          controller.stub(:true_condition).and_return(true)
+        end
+        it_behaves_like 'an impressionable action' do
+          let(:condition) {{ if: :true_condition }}
+        end
+
+        it_behaves_like 'an impressionable action' do
+          let(:condition) {{ if: lambda { true } }}
+        end
+      end
+
+      context "false condition" do
+        before do
+          controller.stub(:false_condition).and_return(false)
+        end
+        it_behaves_like 'an unimpressionable action' do
+          let(:condition) {{ if: :false_condition }}
+        end
+
+        it_behaves_like 'an unimpressionable action' do
+          let(:condition) {{ if: lambda { false } }}
+        end
+      end
+    end
+
+    describe ":unless condition" do
+      context "true condition" do
+        before do
+          controller.stub(:true_condition).and_return(true)
+        end
+        it_behaves_like 'an unimpressionable action' do
+          let(:condition) {{ unless: :true_condition }}
+        end
+
+        it_behaves_like 'an unimpressionable action' do
+          let(:condition) {{ unless: lambda { true } }}
+        end
+      end
+
+      context "false condition" do
+        before do
+          controller.stub(:false_condition).and_return(false)
+        end
+        it_behaves_like 'an impressionable action' do
+          let(:condition) {{ unless: :false_condition }}
+        end
+
+        it_behaves_like 'an impressionable action' do
+          let(:condition) {{ unless: lambda { false } }}
+        end
+      end
+    end
+  end
 end