Fixed the not condition in the parser. Add test for Operations. Added

method in Field Handler called handlers.  Makes sure end user fields are
returned first.

app/models/end_user_segment_field.rb

@@ -15,5 +15,6 @@ def self.user_segment_fields_handler_info
   register_field :registered, UserSegment::CoreType::BooleanType
   register_field :activated, UserSegment::CoreType::BooleanType
   register_field :id, UserSegment::CoreType::NumberType
+  register_field :user_level, UserSegment::CoreType::NumberType
 
 end

app/models/user_segment/field.rb

@@ -80,7 +80,7 @@ def model_field
   end
 
   def handler
-    @handler ||= self.get_handler_info(:user_segment, :fields).find { |info| info[:class].has_field?(self.field) }
+    @handler ||= UserSegment::FieldHandler.handlers.find { |info| info[:class].has_field?(self.field) }
   end
 
   def handler=(handler)

app/models/user_segment/field_handler.rb

@@ -1,5 +1,6 @@
 
 class UserSegment::FieldHandler
+  include HandlerActions
 
   def self.user_segment_fields; {}; end
 
@@ -19,4 +20,10 @@ def self.register_field(field, type, options={})
       fields
     end 
   end
+
+  def self.handlers
+    ([ self.get_handler_info(:user_segment, :fields, 'end_user_segment_field'),
+       self.get_handler_info(:user_segment, :fields, 'end_user_action_segment_field')] +
+       self.get_handler_info(:user_segment, :fields)).uniq
+  end
 end

app/models/user_segment/operations.rb

@@ -21,6 +21,9 @@ def parser
   def parse(text)
     if options = self.parser.parse(text)
       self.operations = options.eval
+      true
+    else
+      false
     end
   end
 

app/models/user_segment_option_parser.rb

@@ -1324,14 +1324,20 @@ module Not0
   end
 
   module Not1
-    def space
-      elements[1]
+    def space1
+      elements[0]
+    end
+
+    def space2
+      elements[2]
     end
   end
 
   module Not2
     def eval(env={})
-      empty? ? nil : 'not'
+	return nil if empty?
+      return 'not' if text_value.strip.downcase == 'not' || text_value.strip.downcase == '!'
+	nil
     end
   end
 
@@ -1347,76 +1353,80 @@ def _nt_not
     end
 
     i0, s0 = index, []
-    i2 = index
-    i3, s3 = index, []
-    if has_terminal?('\G[nN]', true, index)
-      r4 = true
-      @index += 1
-    else
-      r4 = nil
-    end
-    s3 << r4
-    if r4
-      if has_terminal?('\G[oO]', true, index)
+    r1 = _nt_space
+    s0 << r1
+    if r1
+      i3 = index
+      i4, s4 = index, []
+      if has_terminal?('\G[nN]', true, index)
         r5 = true
         @index += 1
       else
         r5 = nil
       end
-      s3 << r5
+      s4 << r5
       if r5
-        if has_terminal?('\G[tT]', true, index)
+        if has_terminal?('\G[oO]', true, index)
           r6 = true
           @index += 1
         else
           r6 = nil
         end
-        s3 << r6
+        s4 << r6
         if r6
-          if has_terminal?(' ', false, index)
-            r7 = instantiate_node(SyntaxNode,input, index...(index + 1))
+          if has_terminal?('\G[tT]', true, index)
+            r7 = true
             @index += 1
           else
-            terminal_parse_failure(' ')
             r7 = nil
           end
-          s3 << r7
+          s4 << r7
+          if r7
+            if has_terminal?(' ', false, index)
+              r8 = instantiate_node(SyntaxNode,input, index...(index + 1))
+              @index += 1
+            else
+              terminal_parse_failure(' ')
+              r8 = nil
+            end
+            s4 << r8
+          end
         end
       end
-    end
-    if s3.last
-      r3 = instantiate_node(SyntaxNode,input, i3...index, s3)
-      r3.extend(Not0)
-    else
-      @index = i3
-      r3 = nil
-    end
-    if r3
-      r2 = r3
-    else
-      if has_terminal?('!', false, index)
-        r8 = instantiate_node(SyntaxNode,input, index...(index + 1))
-        @index += 1
+      if s4.last
+        r4 = instantiate_node(SyntaxNode,input, i4...index, s4)
+        r4.extend(Not0)
       else
-        terminal_parse_failure('!')
-        r8 = nil
+        @index = i4
+        r4 = nil
       end
-      if r8
-        r2 = r8
+      if r4
+        r3 = r4
       else
-        @index = i2
-        r2 = nil
+        if has_terminal?('!', false, index)
+          r9 = instantiate_node(SyntaxNode,input, index...(index + 1))
+          @index += 1
+        else
+          terminal_parse_failure('!')
+          r9 = nil
+        end
+        if r9
+          r3 = r9
+        else
+          @index = i3
+          r3 = nil
+        end
+      end
+      if r3
+        r2 = r3
+      else
+        r2 = instantiate_node(SyntaxNode,input, index...index)
+      end
+      s0 << r2
+      if r2
+        r10 = _nt_space
+        s0 << r10
       end
-    end
-    if r2
-      r1 = r2
-    else
-      r1 = instantiate_node(SyntaxNode,input, index...index)
-    end
-    s0 << r1
-    if r1
-      r9 = _nt_space
-      s0 << r9
     end
     if s0.last
       r0 = instantiate_node(SyntaxNode,input, i0...index, s0)

app/models/user_segment_option_parser.treetop

@@ -122,9 +122,11 @@ grammar UserSegmentOption
   end
 
   rule not
-    ([nN] [oO] [tT] ' ' / '!')? space {
+    space ([nN] [oO] [tT] ' ' / '!')? space {
       def eval(env={})
-        empty? ? nil : 'not'
+	return nil if empty?
+        return 'not' if text_value.strip.downcase == 'not' || text_value.strip.downcase == '!'
+	nil
       end
     }
   end

spec/models/user_segment/field_handler_spec.rb

@@ -15,4 +15,8 @@
     UserSegment::FieldHandler.user_segment_fields[:created][:type].should == UserSegment::CoreType::DateTimeType
     UserSegment::FieldHandler.user_segment_fields[:created][:name].should == 'EndUser.created'
   end
+
+  it "should always return the EndUserSegmentField as the first handler" do
+    UserSegment::FieldHandler.handlers[0][:class] == EndUserSegmentField
+  end
 end

spec/models/user_segment/field_spec.rb

@@ -4,8 +4,6 @@
 
   reset_domain_tables :end_users
 
-  @handler = EndUserSegmentField.user_segment_fields_handler_info
-
   before(:each) do
     EndUser.push_target('test1@test.dev', :created_at => 2.days.ago, :activated => true)
     EndUser.push_target('test2@test.dev', :created_at => 5.days.ago, :activated => false)
@@ -14,7 +12,6 @@
 
   it "should be able to use handler" do
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days']
-    @field.handler = @handler
 
     @field.handler_class.should == EndUserSegmentField
     @field.domain_model_class.should == EndUser
@@ -28,33 +25,29 @@
 
   it "should return the count" do
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days']
-    @field.handler = @handler
+
     @field.valid?.should == true
     @field.count.should == 2
     @field.end_user_ids.length.should == 2
 
     @field = UserSegment::Field.new :field => 'created', :operation => 'since', :arguments => [1, 'days']
-    @field.handler = @handler
     @field.valid?.should == true
     @field.count.should == 1
     @field.end_user_ids.length.should == 1
   end
 
   it "should work with children" do
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days'], :child => {:field => 'activated', :operation => 'is', :arguments => [true]}
-    @field.handler = @handler
     @field.valid?.should == true
     @field.count.should == 1
     @field.end_user_ids.length.should == 1
 
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days'], :child => {:field => 'activated', :operation => 'is', :arguments => [true], :child => {:field => 'email', :operation => 'like', :arguments => ['test%@test.dev']}}
-    @field.handler = @handler
     @field.valid?.should == true
     @field.count.should == 1
     @field.end_user_ids.length.should == 1
 
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days'], :child => {:field => 'activated', :operation => 'is', :arguments => [false], :child => {:field => 'email', :operation => 'like', :arguments => ['test%@test.dev']}}
-    @field.handler = @handler
     @field.valid?.should == true
     @field.count.should == 1
     @field.end_user_ids.length.should == 1

spec/models/user_segment/operation_spec.rb

@@ -4,44 +4,46 @@
 
   reset_domain_tables :end_users
 
-  @handler = EndUserSegmentField.user_segment_fields_handler_info
-
   before(:each) do
     EndUser.push_target('test1@test.dev', :created_at => 2.days.ago, :activated => true)
     EndUser.push_target('test2@test.dev', :created_at => 5.days.ago, :activated => false)
     EndUser.push_target('test3@test.dev', :activated => false)
+    EndUser.push_target('test4@test.dev', :activated => true)
+    EndUser.push_target('test5@test.dev', :activated => false)
+    EndUser.push_target('test6@test.dev', :created_at => 10.days.ago, :activated => true)
+    EndUser.push_target('test7@test.dev', :activated => false)
   end
 
   it "should be able to get the count" do
     @field = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days']
-    @field.handler = @handler
 
     @operation = UserSegment::Operation.new nil, [@field]
     @operation.valid?.should be_true
-    @operation.count.should == 2
-    @operation.end_user_ids.length.should == 2
+    @operation.count.should == 3
+    @operation.end_user_ids.length.should == 3
+    @operation.to_a.should == [nil, @field.to_h]
 
     @operation = UserSegment::Operation.new 'not', [@field]
     @operation.valid?.should be_true
-    @operation.count.should == 1
-    @operation.end_user_ids.length.should == 1
+    @operation.count.should == 4
+    @operation.end_user_ids.length.should == 4
+    @operation.to_a.should == ['not', @field.to_h]
   end
 
   it "should be able to concat fields" do
     @field1 = UserSegment::Field.new :field => 'created', :operation => 'before', :arguments => [1, 'days']
-    @field1.handler = @handler
-
     @field2 = UserSegment::Field.new :field => 'activated', :operation => 'is', :arguments => [false]
-    @field2.handler = @handler
 
     @operation = UserSegment::Operation.new nil, [@field1, @field2]
     @operation.valid?.should be_true
-    @operation.count.should == 4
-    @operation.end_user_ids.length.should == 3
+    @operation.count.should == 7
+    @operation.end_user_ids.length.should == 6
+    @operation.to_a.should == [nil, @field1.to_h, @field2.to_h]
 
     @operation = UserSegment::Operation.new 'not', [@field1, @field2]
     @operation.valid?.should be_true
-    @operation.count.should == 2
-    @operation.end_user_ids.length.should == 0
+    @operation.count.should == 7
+    @operation.end_user_ids.length.should == 1
+    @operation.to_a.should == ['not', @field1.to_h, @field2.to_h]
   end
 end