Raise UnknownAttributeError if .where is given invalid attribute …

…names (#80)

* include key in where clause when no matching field

* update changelog

* raise UnknownFieldError when given invalid attribute key

* update changelog

* fix decorate spec for ActiveQuery

---------

Co-authored-by: Sean Edge <sedge@beyondfinance.com>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Not released
 
+- Raise `UnknownFieldError` if `.where` is given non-existent attribute names (https://github.com/Beyond-Finance/active_force/pull/80) 
 - Fix `.update` and `.update!`: include given `nil` valued attributes in request (https://github.com/Beyond-Finance/active_force/pull/79)
 - Change `.first` to not query the API if records have already been retrieved (https://github.com/Beyond-Finance/active_force/pull/77)
 

lib/active_force/active_query.rb

@@ -4,6 +4,13 @@
 
 module ActiveForce
   class PreparedStatementInvalid < ArgumentError; end
+
+  class UnknownFieldError < StandardError
+    def initialize(object, field)
+      super("unknown field '#{field}' for #{object.name}")
+    end
+  end
+
   class RecordNotFound < StandardError
     attr_reader :table_name, :conditions
 
@@ -45,9 +52,9 @@ def count
       sfdc_client.query(super.to_s).first.expr0
     end
 
-    def sum field
+    def sum(field)
       raise ArgumentError, 'field is required' if field.blank?
-      raise ArgumentError, "field '#{field}' does not exist on #{sobject}" unless mappings.key?(field.to_sym)
+      raise UnknownFieldError.new(sobject, field) unless mappings.key?(field.to_sym)
 
       sfdc_client.query(super(mappings.fetch(field.to_sym)).to_s).first.expr0
     end
@@ -167,7 +174,10 @@ def raise_if_bind_arity_mismatch(expected_var_count, actual_var_count)
 
     def build_conditions_from_hash(hash)
       hash.map do |key, value|
-        applicable_predicate mappings[key], value
+        field = mappings[key]
+        raise UnknownFieldError.new(sobject, key) if field.blank?
+
+        applicable_predicate(field, value)
       end
     end
 

spec/active_force/active_query_spec.rb

@@ -1,12 +1,17 @@
 require 'spec_helper'
 
+class TestSObject < ActiveForce::SObject
+  def self.decorate(records)
+    records
+  end
+end
+
 describe ActiveForce::ActiveQuery do
   let(:sobject) do
-    double("sobject", {
-      table_name: "table_name",
-      fields: [],
-      mappings: mappings
-    })
+    class_double(
+      TestSObject,
+      { table_name: 'table_name', fields: [], mappings: mappings, name: 'TableName' }
+    )
   end
   let(:mappings){ { id: "Id", field: "Field__c", other_field: "Other_Field" } }
   let(:client) { double('client', query: nil) }
@@ -296,6 +301,13 @@
         end
       end
     end
+
+    context 'when given attributes Hash with fields that do not exist on the SObject' do
+      it 'raises UnknownFieldError' do
+        expect { active_query.where(xyz: 1) }
+          .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'xyz' for #{sobject.name}/i)
+      end
+    end
   end
 
   describe '#not' do
@@ -329,6 +341,11 @@
       new_query = active_query.find_by field: 123
       expect(new_query).to be_nil
     end
+
+    it 'should raise UnknownFieldError if given invalid field' do
+      expect { active_query.find_by(invalid: true) }
+        .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'invalid' for #{sobject.name}/i)
+    end
   end
 
   describe '#find_by!' do
@@ -337,6 +354,11 @@
       expect { active_query.find_by!(field: 123) }
         .to raise_error(ActiveForce::RecordNotFound, "Couldn't find #{sobject.table_name} with {:field=>123}")
     end
+
+    it 'should raise UnknownFieldError if given invalid field' do
+      expect { active_query.find_by!(invalid: true) }
+        .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'invalid' for #{sobject.name}/i)
+    end
   end
 
   describe '#find!' do

spec/active_force/sobject_spec.rb

@@ -386,9 +386,9 @@ class IceCream < ActiveForce::SObject
       expect { Whizbang.sum(nil) }.to raise_error(ArgumentError, 'field is required')
     end
 
-    it 'raises ArgumentError if given invalid field' do
+    it 'raises UnknownFieldError if given invalid field' do
       expect { Whizbang.sum(:invalid) }
-        .to raise_error(ArgumentError, /field 'invalid' does not exist on Whizbang/i)
+        .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'invalid' for Whizbang/i)
     end
 
     it 'sends the correct query to the client' do
@@ -419,17 +419,28 @@ class IceCream < ActiveForce::SObject
       expect(client).to receive(:query).with("SELECT #{Whizbang.fields.join ', '} FROM Whizbang__c WHERE (Id = 123) AND (Text_Label = 'foo') LIMIT 1")
       Whizbang.find_by id: 123, text: "foo"
     end
+
+    it 'raises UnknownFieldError if given invalid field' do
+      expect { Whizbang.find_by(xyz: 1) }
+        .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'xyz' for Whizbang/)
+    end
   end
 
   describe "#find_by!" do
     it "queries the client, with the SFDC field names and correctly enclosed values" do
       expect(client).to receive(:query).with("SELECT #{Whizbang.fields.join ', '} FROM Whizbang__c WHERE (Id = 123) AND (Text_Label = 'foo') LIMIT 1").and_return([Restforce::Mash.new(Id: 123, text: 'foo')])
       Whizbang.find_by! id: 123, text: "foo"
     end
+
     it "raises if nothing found" do
       expect(client).to receive(:query).with("SELECT #{Whizbang.fields.join ', '} FROM Whizbang__c WHERE (Id = 123) AND (Text_Label = 'foo') LIMIT 1")
       expect { Whizbang.find_by! id: 123, text: "foo" }.to raise_error(ActiveForce::RecordNotFound)
     end
+
+    it 'raises UnknownFieldError if given invalid field' do
+      expect { Whizbang.find_by!(xyz: 1) }
+        .to raise_error(ActiveForce::UnknownFieldError, /unknown field 'xyz' for Whizbang/)
+    end
   end
 
   describe '.find!' do