Fixing includes to properly build and set the association. (#12)

* First pass at fixing `includes` to properly build and set the association. * Fix overriding of `sfdc_association_field` so that it properly pluralizes the `relationship_name`. Also fixed a test that was expecting a thing to be cached but was not ever added to the `includes`... curious how that test was passing before. * Bump gem version and update CHANGELOG. * Adding `downcase` call when doing the `association_mapping` comparison because our `table_name` may not match the capitalization of the SF relationship field. * If the relationship at Salesforce appears to be pluralized but the business rules say it is a `has_one` we will end up with an array of 1. This will guard against that unlikely scenario. * Fix eager loading for `belongs_to`.
Beyond-Finance · Mar 14, 2023 · f1430fb · f1430fb
1 parent d7c783a
commit f1430fb
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,11 @@
 # Changelog
 
 ## Not released
-* Fix `where` chaining on `ActiveQuery` (https://github.com/Beyond-Finance/active_force/pull/7)
-* Add `find_by!` which raises `ActiveForce::RecordNotFound` if nothing is found. (https://github.com/Beyond-Finance/active_force/pull/8)
+
+## 0.10.0
+* Fix `#where` chaining on `ActiveQuery` (https://github.com/Beyond-Finance/active_force/pull/7)
+* Add `#find_by!` which raises `ActiveForce::RecordNotFound` if nothing is found. (https://github.com/Beyond-Finance/active_force/pull/8)
+* Fix `#includes` to find, build, and set the association. (https://github.com/Beyond-Finance/active_force/pull/12)
 
 ## 0.9.1
 * Fix invalid error class (https://github.com/Beyond-Finance/active_force/pull/6)

diff --git a/lib/active_force/active_query.rb b/lib/active_force/active_query.rb
@@ -8,13 +8,14 @@ class RecordNotFound < StandardError; end
   class ActiveQuery < Query
     extend Forwardable
 
-    attr_reader :sobject
+    attr_reader :sobject, :association_mapping
 
     def_delegators :sobject, :sfdc_client, :build, :table_name, :mappings
     def_delegators :to_a, :each, :map, :inspect, :pluck, :each_with_object
 
     def initialize sobject
       @sobject = sobject
+      @association_mapping = {}
       super table_name
       fields sobject.fields
     end
@@ -24,7 +25,7 @@ def to_a
     end
 
     private def records
-      @records ||= result.to_a.map { |mash| build mash }
+      @records ||= result.to_a.map { |mash| build mash, association_mapping }
     end
 
     alias_method :all, :to_a
@@ -65,6 +66,8 @@ def includes(*relations)
       relations.each do |relation|
         association = sobject.associations[relation]
         fields Association::EagerLoadProjectionBuilder.build association
+        # downcase the key and downcase when we do the comparison so we don't do any more crazy string manipulation
+        association_mapping[association.sfdc_association_field.downcase] = association.relation_name
       end
       self
     end

diff --git a/lib/active_force/association.rb b/lib/active_force/association.rb
@@ -11,11 +11,8 @@ def associations
       @associations ||= {}
     end
 
-    # i.e name = 'Quota__r'
     def find_association name
-      associations.values.detect do |association|
-        association.represents_sfdc_table? name
-      end
+      associations[name.to_sym]
     end
 
     def has_many relation_name, options = {}

diff --git a/lib/active_force/association/belongs_to_association.rb b/lib/active_force/association/belongs_to_association.rb
@@ -1,8 +1,17 @@
 module ActiveForce
   module Association
     class BelongsToAssociation < Association
+
+      def relationship_name
+        options[:relationship_name] || default_relationship_name
+      end
+
       private
 
+      def default_relationship_name
+        @parent.mappings[foreign_key].gsub /__c\z/, '__r'
+      end
+
       def default_foreign_key
         infer_foreign_key_from_model relation_model
       end

diff --git a/lib/active_force/association/eager_load_projection_builder.rb b/lib/active_force/association/eager_load_projection_builder.rb
@@ -40,9 +40,7 @@ class HasManyAssociationProjectionBuilder < AbstractProjectionBuilder
       # relationship name. Per SFDC convention, the name needs
       # to be pluralized
       def projections
-        match = association.sfdc_association_field.match /__r\z/
-        # pluralize the table name, and append '__r' if it was there to begin with
-        relationship_name = association.sfdc_association_field.sub(match.to_s, '').pluralize + match.to_s
+        relationship_name = association.sfdc_association_field
         query = Query.new relationship_name
         query.fields association.relation_model.fields
         ["(#{query.to_s})"]

diff --git a/lib/active_force/association/has_many_association.rb b/lib/active_force/association/has_many_association.rb
@@ -1,6 +1,13 @@
 module ActiveForce
   module Association
     class HasManyAssociation < Association
+      def sfdc_association_field
+        name = relationship_name.gsub /__c\z/, '__r'
+        match = name.match /__r\z/
+        # pluralize the table name, and append '__r' if it was there to begin with
+        name.sub(match.to_s, '').pluralize + match.to_s
+      end
+
       private
 
       def default_foreign_key

diff --git a/lib/active_force/association/has_one_association.rb b/lib/active_force/association/has_one_association.rb
@@ -18,6 +18,7 @@ def define_relation_method
 
         @parent.send :define_method, "#{_method}=" do |other|
           value_to_set = other.nil? ? nil : self.id
+          other = other.first if other.is_a?(Array)
           # Do we change the object that was passed in or do we modify the already associated object?
           obj_to_change = value_to_set ? other : self.send(association.relation_name)
           obj_to_change.send "#{ association.foreign_key }=", value_to_set

diff --git a/lib/active_force/sobject.rb b/lib/active_force/sobject.rb
@@ -53,12 +53,15 @@ def self.query
     end
 
     attr_accessor :build_attributes
-    def self.build mash
+    def self.build mash, association_mapping={}
       return unless mash
       sobject = new
       sobject.build_attributes = mash[:build_attributes] || mash
       sobject.run_callbacks(:build) do
         mash.each do |column, value|
+          if association_mapping.has_key?(column.downcase)
+            column = association_mapping[column.downcase]
+          end
           sobject.write_value column, value
         end
       end
@@ -164,7 +167,7 @@ def reload
     end
 
     def write_value key, value
-      if association = self.class.find_association(key.to_s)
+      if association = self.class.find_association(key.to_sym)
         field = association.relation_name
         value = Association::RelationModelBuilder.build(association, value)
       elsif key.to_sym.in?(mappings.keys)

diff --git a/lib/active_force/version.rb b/lib/active_force/version.rb
@@ -1,3 +1,3 @@
 module ActiveForce
-  VERSION = "0.9.1"
+  VERSION = "0.10.0"
 end
diff --git a/spec/active_force/sobject/includes_spec.rb b/spec/active_force/sobject/includes_spec.rb
@@ -46,15 +46,15 @@ module ActiveForce
         context 'with namespaced SObjects' do
           it 'queries the API for the associated record' do
             soql = Salesforce::Territory.includes(:quota).where(id: '123').to_s
-            expect(soql).to eq "SELECT Id, QuotaId, WidgetId, Quota__r.Id FROM Territory WHERE (Id = '123')"
+            expect(soql).to eq "SELECT Id, QuotaId, WidgetId, QuotaId.Id FROM Territory WHERE (Id = '123')"
           end
 
           it "queries the API once to retrieve the object and its related one" do
             response = [build_restforce_sobject({
               "Id"       => "123",
               "QuotaId"  => "321",
               "WidgetId" => "321",
-              "Quota__r" => {
+              "QuotaId" => {
                 "Id" => "321"
               }
             })]
@@ -88,7 +88,7 @@ module ActiveForce
 
           context 'when the class name does not match the SFDC entity name' do
             let(:expected_soql) do
-              "SELECT Id, QuotaId, WidgetId, Tegdiw__r.Id FROM Territory WHERE (Id = '123')"
+              "SELECT Id, QuotaId, WidgetId, WidgetId.Id FROM Territory WHERE (Id = '123')"
             end
 
             it 'queries the API for the associated record' do
@@ -100,7 +100,7 @@ module ActiveForce
               response = [build_restforce_sobject({
                 "Id"        => "123",
                 "WidgetId"  => "321",
-                "Tegdiw__r" => {
+                "WidgetId" => {
                   "Id" => "321"
                 }
               })]
@@ -115,18 +115,18 @@ module ActiveForce
           context 'child to several parents' do
             it 'queries the API for associated records' do
               soql = Salesforce::Territory.includes(:quota, :widget).where(id: '123').to_s
-              expect(soql).to eq "SELECT Id, QuotaId, WidgetId, Quota__r.Id, Tegdiw__r.Id FROM Territory WHERE (Id = '123')"
+              expect(soql).to eq "SELECT Id, QuotaId, WidgetId, QuotaId.Id, WidgetId.Id FROM Territory WHERE (Id = '123')"
             end
 
             it "queries the API once to retrieve the object and its assocations" do
               response = [build_restforce_sobject({
                 "Id"       => "123",
                 "QuotaId"  => "321",
                 "WidgetId" => "321",
-                "Quota__r" => {
+                "QuotaId" => {
                   "Id" => "321"
                 },
-                "Tegdiw__r" => {
+                "WidgetId" => {
                   "Id" => "321"
                 }
               })]
@@ -263,7 +263,7 @@ module ActiveForce
       describe 'mixing belongs_to and has_many' do
         it 'formulates the correct SOQL query' do
           soql = Account.includes(:opportunities, :owner).where(id: '123').to_s
-          expect(soql).to eq "SELECT Id, OwnerId, (SELECT Id, AccountId FROM Opportunities), Owner__r.Id FROM Account WHERE (Id = '123')"
+          expect(soql).to eq "SELECT Id, OwnerId, (SELECT Id, AccountId FROM Opportunities), OwnerId.Id FROM Account WHERE (Id = '123')"
         end
 
         it 'builds the associated objects and caches them' do
@@ -273,12 +273,12 @@ module ActiveForce
               {'Id' => '213', 'AccountId' => '123'},
               {'Id' => '214', 'AccountId' => '123'}
             ]),
-            'Owner__r' => {
+            'OwnerId' => {
               'Id' => '321'
             }
           })]
           allow(client).to receive(:query).once.and_return response
-          account = Account.includes(:opportunities).find '123'
+          account = Account.includes(:opportunities, :owner).find '123'
           expect(account.opportunities).to be_an Array
           expect(account.opportunities.all? { |o| o.is_a? Opportunity }).to eq true
           expect(account.owner).to be_a Owner