From 56923c4e00c610e87d61d4f87fa15fa616a39512 Mon Sep 17 00:00:00 2001
From: Charlie Massry <charlesdmassry@gmail.com>
Date: Mon, 20 Nov 2023 08:58:19 -0500
Subject: [PATCH] Don't query salesforce for a `.first` call if query already
 has records

---
 .gitignore                             |  1 +
 CHANGELOG.md                           |  2 ++
 lib/active_force/active_query.rb       |  4 ++++
 lib/active_force/query.rb              | 16 ++++++++++++----
 spec/active_force/active_query_spec.rb | 18 ++++++++++++++++++
 spec/active_force/query_spec.rb        | 16 ++++++++++++++++
 6 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index d87d4be..2de3904 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,3 +15,4 @@ spec/reports
 test/tmp
 test/version_tmp
 tmp
+.idea
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 758d66f..f12a585 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Not released
 
+- Change `.first` to not query the API if records have already been retrieved (https://github.com/Beyond-Finance/active_force/pull/77)
+
 ## 0.20.1
 - Revert "ActiveForce .first performance enhancement (#73)" (https://github.com/Beyond-Finance/active_force/pull/76)
 
diff --git a/lib/active_force/active_query.rb b/lib/active_force/active_query.rb
index 54e36b7..74d5775 100644
--- a/lib/active_force/active_query.rb
+++ b/lib/active_force/active_query.rb
@@ -56,6 +56,10 @@ def limit limit
       limit == 1 ? super.to_a.first : super
     end
 
+    def first
+      super.to_a.first
+    end
+
     def not args=nil, *rest
       return self if args.nil?
 
diff --git a/lib/active_force/query.rb b/lib/active_force/query.rb
index 2afb02a..aa0072e 100644
--- a/lib/active_force/query.rb
+++ b/lib/active_force/query.rb
@@ -78,7 +78,15 @@ def find id
     end
 
     def first
-      limit 1
+      if @records
+        clone_and_set_instance_variables(
+          size: 1,
+          records: [@records.first],
+          decorated_records: [@decorated_records&.first]
+        )
+      else
+        clone_and_set_instance_variables(size: 1)
+      end
     end
 
     def last(limit = 1)
@@ -126,9 +134,9 @@ def build_offset
 
       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) }
+        { decorated_records: nil, records: nil }
+          .merge(instance_variable_hash)
+          .each { |k,v| clone.instance_variable_set("@#{k.to_s}", v) }
         clone
       end
   end
diff --git a/spec/active_force/active_query_spec.rb b/spec/active_force/active_query_spec.rb
index 2b0055f..003fabd 100644
--- a/spec/active_force/active_query_spec.rb
+++ b/spec/active_force/active_query_spec.rb
@@ -463,4 +463,22 @@
    end
     
   end
+
+  describe '#first' do
+    before do
+      allow(client).to receive(:query).and_return(api_result)
+      api_result.each do |instance|
+        allow(active_query).to receive(:build).with(instance, {}).and_return(double(:sobject, id: instance['Id']))
+      end
+    end
+
+    it 'returns a single record when the api was already queried' do
+      active_query.to_a
+      expect(active_query.first.id).to eq("0000000000AAAAABBB")
+    end
+
+    it 'returns a single record when the api was not already queried' do
+      expect(active_query.first.id).to eq("0000000000AAAAABBB")
+    end
+  end
 end
diff --git a/spec/active_force/query_spec.rb b/spec/active_force/query_spec.rb
index b3c187d..0345ab7 100644
--- a/spec/active_force/query_spec.rb
+++ b/spec/active_force/query_spec.rb
@@ -165,6 +165,22 @@
       expect(query.to_s).to eq "SELECT Id, name, etc FROM table_name"
       expect(new_query.to_s).to eq 'SELECT Id, name, etc FROM table_name LIMIT 1'
     end
+
+    it "does not query if records have already been fetched" do
+      query = ActiveForce::Query.new 'table_name'
+      query.instance_variable_set(:@records, %w[foo bar])
+      query.instance_variable_set(:@decorated_records, %w[foo bar])
+      expect(query).not_to receive(:clone_and_set_instance_variables).with(size: 1)
+      expect(query).to receive(:clone_and_set_instance_variables).with(size: 1, records: ['foo'], decorated_records: ['foo'])
+      query.first
+    end
+
+    it 'queries the api if it has not been queried yet' do
+      query = ActiveForce::Query.new 'table_name'
+      query.instance_variable_set(:@records, nil)
+      expect(query).to receive(:clone_and_set_instance_variables).with(size: 1)
+      query.first
+    end
   end
 
   describe '.last' do