Feature/fix chaining methods v2 (#10)

* fix chaining on where clauses, but still need to fix other chainable methods

* create clone_and_set_instance_variables method and write specs

* remove pry statement

* fix associations

* reset records and decorated records on on cloned instance

* (WIP) fix none so that specs pass

* add tests to check for has_many mutation

* fix count, sum, not, and or

* add tests for active query not method

* update version an changelog

* fix sum ArgumentError message for invalid field

---------

Co-authored-by: rferg <rmferguson77@gmail.com>

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Not released
 
+- Fix bug with has_many queries due to query method chaining mutating in-place (https://github.com/Beyond-Finance/active_force/pull/10)
+
 ## 0.16.0
 
 - Fix `default` in models when default value is overridden by the same value, it is still sent to salesforce (https://github.com/Beyond-Finance/active_force/pull/61)

lib/active_force/active_query.rb

@@ -42,36 +42,34 @@ def to_a
     alias_method :all, :to_a
 
     def count
-      super
-      sfdc_client.query(to_s).first.expr0
+      sfdc_client.query(super.to_s).first.expr0
     end
 
     def sum field
-      super(mappings[field])
-      sfdc_client.query(to_s).first.expr0
+      raise ArgumentError, 'field is required' if field.blank?
+      raise ArgumentError, "field '#{field}' does not exist on #{sobject}" unless mappings.key?(field.to_sym)
+
+      sfdc_client.query(super(mappings.fetch(field.to_sym)).to_s).first.expr0
     end
 
     def limit limit
-      super
-      limit == 1 ? to_a.first : self
+      limit == 1 ? super.to_a.first : super
     end
 
     def not args=nil, *rest
       return self if args.nil?
+
       super build_condition args, rest
-      self
     end
 
     def where args=nil, *rest
       return self if args.nil?
-      return clone_self_and_clear_cache.where(args, *rest) if @decorated_records.present?
       super build_condition args, rest
-      self
     end
 
-    def select *fields
-      fields.map! { |field| mappings[field] }
-      super *fields
+    def select *selected_fields
+      selected_fields.map! { |field| mappings[field] }
+      super *selected_fields
     end
 
     def find!(id)
@@ -100,8 +98,10 @@ def includes(*relations)
     end
 
     def none
-      @records = []
-      where(id: '1'*18).where(id: '0'*18)
+      clone_and_set_instance_variables(
+        records: [],
+        conditions: [build_condition(id: '1' * 18), build_condition(id: '0' * 18)]
+      )
     end
 
     def loaded?
@@ -205,13 +205,6 @@ def result
       sfdc_client.query(self.to_s)
     end
 
-    def clone_self_and_clear_cache
-      new_query = self.clone
-      new_query.instance_variable_set(:@decorated_records, nil)
-      new_query.instance_variable_set(:@records, nil)
-      new_query
-    end
-
     def build_order_by(args)
       args.map do |arg|
         case arg
@@ -228,6 +221,5 @@ def build_order_by(args)
     def order_type(type)
       type == :desc ? 'DESC' : 'ASC'
     end
-
   end
 end

lib/active_force/query.rb

@@ -31,51 +31,50 @@ def to_s
     end
 
     def select *columns
-      @query_fields = columns
-      self
+      clone_and_set_instance_variables(query_fields: columns)
+    end
+
+    def where condition = nil
+      new_conditions = @conditions | [condition]
+      if new_conditions != @conditions
+        clone_and_set_instance_variables({conditions: new_conditions})
+      else
+        self
+      end
     end
 
     def not condition
-      @conditions << "NOT ((#{ condition.join(') AND (') }))"
-      self
+      condition ? where("NOT ((#{condition.join(') AND (')}))") : self
     end
 
     def or query
-      @conditions = ["(#{ and_conditions }) OR (#{ query.and_conditions })"]
-      self
-    end
+      return self unless query
 
-    def where condition = nil
-      @conditions << condition if condition
-      self
+      clone_and_set_instance_variables(conditions: ["(#{and_conditions}) OR (#{query.and_conditions})"])
     end
 
     def order order
-      @order = order if order
-      self
+      order ? clone_and_set_instance_variables(order: order) : self
     end
 
     def limit size
-      @size = size if size
-      self
+      size ? clone_and_set_instance_variables(size: size) : self
     end
 
     def limit_value
       @size
     end
 
     def offset offset
-      @offset = offset
-      self
+      clone_and_set_instance_variables(offset: offset)
     end
 
     def offset_value
       @offset
     end
 
     def find id
-      where "#{ @table_id } = '#{ id }'"
-      limit 1
+      where("#{ @table_id } = '#{ id }'").limit 1
     end
 
     def first
@@ -87,18 +86,17 @@ def last
     end
 
     def join object_query
-      fields ["(#{ object_query.to_s })"]
-      self
+      chained_query = self.clone
+      chained_query.fields ["(#{ object_query.to_s })"]
+      chained_query
     end
 
     def count
-      @query_fields = ["count(Id)"]
-      self
+      clone_and_set_instance_variables(query_fields: ["count(Id)"])
     end
 
     def sum field
-      @query_fields = ["sum(#{field})"]
-      self
+      clone_and_set_instance_variables(query_fields: ["sum(#{field})"])
     end
 
     protected
@@ -125,5 +123,13 @@ def build_order
       def build_offset
         "OFFSET #{ @offset }" if @offset
       end
+
+      def clone_and_set_instance_variables instance_variable_hash={}
+        clone = self.clone
+        clone.instance_variable_set(:@decorated_records, nil)
+        clone.instance_variable_set(:@records, nil)
+        instance_variable_hash.each { |k,v| clone.instance_variable_set("@#{k.to_s}", v) }
+        clone
+      end
   end
 end