From 1c44fe5ab4a7cdfb9b0bdfd7b96b249c05db8e53 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Thu, 9 Mar 2023 11:08:57 -0500 Subject: [PATCH 1/6] First pass at fixing `includes` to properly build and set the association. --- lib/active_force/active_query.rb | 6 ++++-- lib/active_force/association.rb | 5 +---- lib/active_force/association/has_many_association.rb | 4 ++++ lib/active_force/sobject.rb | 7 +++++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/active_force/active_query.rb b/lib/active_force/active_query.rb index cbf7555..b0325dc 100644 --- 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,7 @@ def includes(*relations) relations.each do |relation| association = sobject.associations[relation] fields Association::EagerLoadProjectionBuilder.build association + association_mapping[association.sfdc_association_field] = association.relation_name end self end diff --git a/lib/active_force/association.rb b/lib/active_force/association.rb index 59504ca..b09a5d9 100644 --- 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/has_many_association.rb b/lib/active_force/association/has_many_association.rb index fc9edfb..f40eff5 100644 --- a/lib/active_force/association/has_many_association.rb +++ b/lib/active_force/association/has_many_association.rb @@ -1,6 +1,10 @@ module ActiveForce module Association class HasManyAssociation < Association + def sfdc_association_field + relationship_name.gsub /__c\z/, 's__r' + end + private def default_foreign_key diff --git a/lib/active_force/sobject.rb b/lib/active_force/sobject.rb index 7ceb416..51c071f 100644 --- 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) + column = association_mapping[column] + 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) From b73a9407af2de344e7630b6179d68195cf338f9e Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Thu, 9 Mar 2023 16:19:12 -0500 Subject: [PATCH 2/6] 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. --- .../association/eager_load_projection_builder.rb | 4 +--- lib/active_force/association/has_many_association.rb | 5 ++++- spec/active_force/sobject/includes_spec.rb | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/active_force/association/eager_load_projection_builder.rb b/lib/active_force/association/eager_load_projection_builder.rb index 16fd4b6..3d815fe 100644 --- 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 index f40eff5..35cdc63 100644 --- a/lib/active_force/association/has_many_association.rb +++ b/lib/active_force/association/has_many_association.rb @@ -2,7 +2,10 @@ module ActiveForce module Association class HasManyAssociation < Association def sfdc_association_field - relationship_name.gsub /__c\z/, 's__r' + 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 diff --git a/spec/active_force/sobject/includes_spec.rb b/spec/active_force/sobject/includes_spec.rb index b1d4464..fafd2ab 100644 --- a/spec/active_force/sobject/includes_spec.rb +++ b/spec/active_force/sobject/includes_spec.rb @@ -278,7 +278,7 @@ module ActiveForce } })] 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 From 88f13744dc98f0d748462b2f51745df2aa9c943d Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Fri, 10 Mar 2023 09:47:15 -0500 Subject: [PATCH 3/6] Bump gem version and update CHANGELOG. --- CHANGELOG.md | 7 +++++-- lib/active_force/version.rb | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2562d33..c3b4070 100644 --- 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/version.rb b/lib/active_force/version.rb index c95ec49..a6e95d1 100644 --- 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 From 12e623b95799a66f57ae831d9979b024f59f4097 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Fri, 10 Mar 2023 11:02:08 -0500 Subject: [PATCH 4/6] Adding `downcase` call when doing the `association_mapping` comparison because our `table_name` may not match the capitalization of the SF relationship field. --- lib/active_force/active_query.rb | 3 ++- lib/active_force/sobject.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/active_force/active_query.rb b/lib/active_force/active_query.rb index b0325dc..630e348 100644 --- a/lib/active_force/active_query.rb +++ b/lib/active_force/active_query.rb @@ -66,7 +66,8 @@ def includes(*relations) relations.each do |relation| association = sobject.associations[relation] fields Association::EagerLoadProjectionBuilder.build association - association_mapping[association.sfdc_association_field] = association.relation_name + # 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/sobject.rb b/lib/active_force/sobject.rb index 51c071f..404d76b 100644 --- a/lib/active_force/sobject.rb +++ b/lib/active_force/sobject.rb @@ -59,8 +59,8 @@ def self.build mash, association_mapping={} sobject.build_attributes = mash[:build_attributes] || mash sobject.run_callbacks(:build) do mash.each do |column, value| - if association_mapping.has_key?(column) - column = association_mapping[column] + if association_mapping.has_key?(column.downcase) + column = association_mapping[column.downcase] end sobject.write_value column, value end From 0c7b8ad62f62eaba7ea5728fce9ff2591681cbb0 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Fri, 10 Mar 2023 11:15:34 -0500 Subject: [PATCH 5/6] 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. --- lib/active_force/association/has_one_association.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/active_force/association/has_one_association.rb b/lib/active_force/association/has_one_association.rb index e824bbe..1f4c56b 100644 --- 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 From 2629c5c57a997e61af58ceeb390ed149e6ec63a0 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Fri, 10 Mar 2023 14:24:06 -0500 Subject: [PATCH 6/6] Fix eager loading for `belongs_to`. --- .../association/belongs_to_association.rb | 9 +++++++++ spec/active_force/sobject/includes_spec.rb | 18 +++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/active_force/association/belongs_to_association.rb b/lib/active_force/association/belongs_to_association.rb index 707adff..81cc36d 100644 --- 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/spec/active_force/sobject/includes_spec.rb b/spec/active_force/sobject/includes_spec.rb index fafd2ab..a7f47c5 100644 --- a/spec/active_force/sobject/includes_spec.rb +++ b/spec/active_force/sobject/includes_spec.rb @@ -46,7 +46,7 @@ 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 @@ -54,7 +54,7 @@ module ActiveForce "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,7 +115,7 @@ 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 @@ -123,10 +123,10 @@ module ActiveForce "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,7 +273,7 @@ module ActiveForce {'Id' => '213', 'AccountId' => '123'}, {'Id' => '214', 'AccountId' => '123'} ]), - 'Owner__r' => { + 'OwnerId' => { 'Id' => '321' } })]