use persisted? instead of new_record? wherever possible

- persisted? is the API defined in ActiveModel
- makes it easier for extension libraries to conform to ActiveModel APIs
  without concern for whether the extended object is specifically
  ActiveRecord

[#5927 state:committed]

Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>

1 files changed, 11 insertions, 10 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 896e18af01..398c41f3e9 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -120,13 +120,13 @@ module ActiveRecord
       # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
       def <<(*records)
         result = true
-        load_target if @owner.new_record?
+        load_target unless @owner.persisted?
 
         transaction do
           flatten_deeper(records).each do |record|
             raise_on_type_mismatch(record)
             add_record_to_target_with_callbacks(record) do |r|
-              result &&= insert_record(record) unless @owner.new_record?
+              result &&= insert_record(record) if @owner.persisted?
             end
           end
         end
@@ -285,12 +285,12 @@ module ActiveRecord
       # This method is abstract in the sense that it relies on
       # +count_records+, which is a method descendants have to provide.
       def size
-        if @owner.new_record? || (loaded? && !@reflection.options[:uniq])
+        if !@owner.persisted? || (loaded? && !@reflection.options[:uniq])
           @target.size
         elsif !loaded? && @reflection.options[:group]
           load_target.size
         elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array)
-          unsaved_records = @target.select { |r| r.new_record? }
+          unsaved_records = @target.reject { |r| r.persisted? }
           unsaved_records.size + count_records
         else
           count_records
@@ -357,7 +357,7 @@ module ActiveRecord
 
       def include?(record)
         return false unless record.is_a?(@reflection.klass)
-        return include_in_memory?(record) if record.new_record?
+        return include_in_memory?(record) unless record.persisted?
         load_target if @reflection.options[:finder_sql] && !loaded?
         return @target.include?(record) if loaded?
         exists?(record)
@@ -372,7 +372,7 @@ module ActiveRecord
         end
 
         def load_target
-          if !@owner.new_record? || foreign_key_present
+          if @owner.persisted? || foreign_key_present
             begin
               if !loaded?
                 if @target.is_a?(Array) && @target.any?
@@ -513,7 +513,7 @@ module ActiveRecord
 
           transaction do
             records.each { |record| callback(:before_remove, record) }
-            old_records = records.reject { |r| r.new_record? }
+            old_records = records.select { |r| r.persisted? }
             yield(records, old_records)
             records.each { |record| callback(:after_remove, record) }
           end
@@ -538,14 +538,15 @@ module ActiveRecord
         end
 
         def ensure_owner_is_not_new
-          if @owner.new_record?
+          unless @owner.persisted?
             raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
           end
         end
 
         def fetch_first_or_last_using_find?(args)
-          args.first.kind_of?(Hash) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] ||
-                                         @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer))
+          args.first.kind_of?(Hash) || !(loaded? || !@owner.persisted? || @reflection.options[:finder_sql] ||
+                                         @target.any? { |record| !record.persisted? } || args.first.kind_of?(Integer))
+                                         # TODO - would prefer @target.none? { |r| r.persisted? }
         end
 
         def include_in_memory?(record)