Require persisted? in ActiveModel::Lint and remove new_record? and destroyed? methods. ActionPack does not care if the resource is new or if it was destroyed, it cares only if it's persisted somewhere or not.

actionpack/lib/action_controller/polymorphic_routes.rb

@@ -92,8 +92,7 @@ module ActionController
       inflection = if options[:action].to_s == "new"
         args.pop
         :singular
-      elsif (record.respond_to?(:new_record?) && record.new_record?) ||
+      elsif (record.respond_to?(:persisted?) && !record.persisted?)
-            (record.respond_to?(:destroyed?) && record.destroyed?)
         args.pop
         :plural
       elsif record.is_a?(Class)

actionpack/lib/action_view/helpers/active_model_helper.rb

@@ -80,13 +80,13 @@ module ActionView
         record = convert_to_model(record)
 
         options = options.symbolize_keys
-        options[:action] ||= record.new_record? ? "create" : "update"
+        options[:action] ||= record.persisted? ? "update" : "create"
         action = url_for(:action => options[:action], :id => record)
 
         submit_value = options[:submit_value] || options[:action].gsub(/[^\w]/, '').capitalize
 
         contents = form_tag({:action => action}, :method =>(options[:method] || 'post'), :enctype => options[:multipart] ? 'multipart/form-data': nil)
-        contents.safe_concat hidden_field(record_name, :id) unless record.new_record?
+        contents.safe_concat hidden_field(record_name, :id) if record.persisted?
         contents.safe_concat all_input_tags(record, record_name, options)
         yield contents if block_given?
         contents.safe_concat submit_tag(submit_value)

actionpack/lib/action_view/helpers/form_helper.rb

@@ -316,14 +316,13 @@ module ActionView
 
       def apply_form_for_options!(object_or_array, options) #:nodoc:
         object = object_or_array.is_a?(Array) ? object_or_array.last : object_or_array
-
         object = convert_to_model(object)
 
         html_options =
-          if object.respond_to?(:new_record?) && object.new_record?
+          if object.respond_to?(:persisted?) && object.persisted?
-            { :class  => dom_class(object, :new),  :id => dom_id(object), :method => :post }
-          else
             { :class  => dom_class(object, :edit), :id => dom_id(object, :edit), :method => :put }
+          else
+            { :class  => dom_class(object, :new),  :id => dom_id(object), :method => :post }
           end
 
         options[:html] ||= {}
@@ -1150,7 +1149,7 @@ module ActionView
 
         def submit_default_value
           object = @object.respond_to?(:to_model) ? @object.to_model : @object
-          key    = object ? (object.new_record? ? :create : :update) : :submit
+          key    = object ? (object.persisted? ? :update : :create) : :submit
 
           model = if object.class.respond_to?(:model_name)
             object.class.model_name.human
@@ -1176,7 +1175,7 @@ module ActionView
           association = args.shift
           association = association.to_model if association.respond_to?(:to_model)
 
-          if association.respond_to?(:new_record?)
+          if association.respond_to?(:persisted?)
             association = [association] if @object.send(association_name).is_a?(Array)
           elsif !association.is_a?(Array)
             association = @object.send(association_name)
@@ -1195,13 +1194,13 @@ module ActionView
         def fields_for_nested_model(name, object, options, block)
           object = object.to_model if object.respond_to?(:to_model)
 
-          if object.new_record?
+          if object.persisted?
-            @template.fields_for(name, object, options, &block)
-          else
             @template.fields_for(name, object, options) do |builder|
               block.call(builder)
               @template.concat builder.hidden_field(:id) unless builder.emitted_hidden_id?
             end
+          else
+            @template.fields_for(name, object, options, &block)
           end
         end
 

actionpack/lib/action_view/helpers/url_helper.rb

@@ -63,7 +63,7 @@ module ActionView
       #   # => /testing/jump/#tax&ship
       #
       #   <%= url_for(Workshop.new) %>
-      #   # relies on Workshop answering a new_record? call (and in this case returning true)
+      #   # relies on Workshop answering a persisted? call (and in this case returning false)
       #   # => /workshops
       #
       #   <%= url_for(@workshop) %>

actionpack/test/controller/mime_responds_test.rb

@@ -513,7 +513,7 @@ class RespondWithController < ActionController::Base
 protected
 
   def resource
-    Customer.new("david", 13)
+    Customer.new("david", request.delete? ? nil : 13)
   end
 
   def _render_js(js, options)
@@ -717,7 +717,7 @@ class RespondWithControllerTest < ActionController::TestCase
       delete :using_resource
       assert_equal "text/html", @response.content_type
       assert_equal 302, @response.status
-      assert_equal "http://www.example.com/customers/13", @response.location
+      assert_equal "http://www.example.com/customers", @response.location
     end
   end
 

actionpack/test/controller/redirect_test.rb

@@ -6,14 +6,14 @@ end
 class Workshop
   extend ActiveModel::Naming
   include ActiveModel::Conversion
-  attr_accessor :id, :new_record
+  attr_accessor :id
 
-  def initialize(id, new_record)
+  def initialize(id)
-    @id, @new_record = id, new_record
+    @id = id
   end
 
-  def new_record?
+  def persisted?
-    @new_record
+    id.present?
   end
 
   def to_s
@@ -88,11 +88,11 @@ class RedirectController < ActionController::Base
   end
 
   def redirect_to_existing_record
-    redirect_to Workshop.new(5, false)
+    redirect_to Workshop.new(5)
   end
 
   def redirect_to_new_record
-    redirect_to Workshop.new(5, true)
+    redirect_to Workshop.new(nil)
   end
 
   def redirect_to_nil
@@ -239,11 +239,11 @@ class RedirectTest < ActionController::TestCase
 
       get :redirect_to_existing_record
       assert_equal "http://test.host/workshops/5", redirect_to_url
-      assert_redirected_to Workshop.new(5, false)
+      assert_redirected_to Workshop.new(5)
 
       get :redirect_to_new_record
       assert_equal "http://test.host/workshops", redirect_to_url
-      assert_redirected_to Workshop.new(5, true)
+      assert_redirected_to Workshop.new(nil)
     end
   end
 

actionpack/test/lib/controller/fake_models.rb

@@ -6,14 +6,6 @@ class Customer < Struct.new(:name, :id)
 
   undef_method :to_json
 
-  def to_key
-    id ? [id] : nil
-  end
-
-  def to_param
-    id.to_s
-  end
-
   def to_xml(options={})
     if options[:builder]
       options[:builder].name name
@@ -31,8 +23,8 @@ class Customer < Struct.new(:name, :id)
     []
   end
 
-  def destroyed?
+  def persisted?
-    false
+    id.present?
   end
 end
 
@@ -47,12 +39,8 @@ module Quiz
     extend ActiveModel::Naming
     include ActiveModel::Conversion
 
-    def to_key
+    def persisted?
-      id ? [id] : nil
+      id.present?
-    end
-
-    def to_param
-      id.to_s
     end
   end
 
@@ -67,16 +55,12 @@ class Post < Struct.new(:title, :author_name, :body, :secret, :written_on, :cost
 
   alias_method :secret?, :secret
 
-  def to_key
+  def persisted=(boolean)
-    id ? [id] : nil
+    @persisted = boolean
   end
 
-  def new_record=(boolean)
+  def persisted?
-    @new_record = boolean
+    @persisted
-  end
-
-  def new_record?
-    @new_record
   end
 
   attr_accessor :author
@@ -98,7 +82,7 @@ class Comment
   def initialize(id = nil, post_id = nil); @id, @post_id = id, post_id end
   def to_key; id ? [id] : nil end
   def save; @id = 1; @post_id = 1 end
-  def new_record?; @id.nil? end
+  def persisted?; @id.present? end
   def to_param; @id; end
   def name
     @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}"
@@ -118,7 +102,7 @@ class Tag
   def initialize(id = nil, post_id = nil); @id, @post_id = id, post_id end
   def to_key; id ? [id] : nil end
   def save; @id = 1; @post_id = 1 end
-  def new_record?; @id.nil? end
+  def persisted?; @id.present? end
   def to_param; @id; end
   def value
     @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}"
@@ -138,7 +122,7 @@ class CommentRelevance
   def initialize(id = nil, comment_id = nil); @id, @comment_id = id, comment_id end
   def to_key; id ? [id] : nil end
   def save; @id = 1; @comment_id = 1 end
-  def new_record?; @id.nil? end
+  def persisted?; @id.present? end
   def to_param; @id; end
   def value
     @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}"
@@ -154,7 +138,7 @@ class TagRelevance
   def initialize(id = nil, tag_id = nil); @id, @tag_id = id, tag_id end
   def to_key; id ? [id] : nil end
   def save; @id = 1; @tag_id = 1 end
-  def new_record?; @id.nil? end
+  def persisted?; @id.present? end
   def to_param; @id; end
   def value
     @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}"

actionpack/test/template/active_model_helper_test.rb

@@ -64,7 +64,7 @@ class ActiveModelHelperTest < ActionView::TestCase
       }.new
     end
 
-    def @post.new_record?() true end
+    def @post.persisted?() false end
     def @post.to_param() nil end
 
     def @post.column_for_attribute(attr_name)
@@ -155,7 +155,7 @@ class ActiveModelHelperTest < ActionView::TestCase
 
     silence_warnings do
       class << @post
-        def new_record?() false end
+        def persisted?() true end
         def to_param() id end
         def id() 1 end
       end

actionpack/test/template/atom_feed_helper_test.rb

@@ -4,8 +4,8 @@ class Scroll < Struct.new(:id, :to_param, :title, :body, :updated_at, :created_a
   extend ActiveModel::Naming
   include ActiveModel::Conversion
 
-  def new_record?
+  def persisted?
-    true
+    false
   end
 end
 
@@ -34,7 +34,7 @@ class ScrollsController < ActionController::Base
           feed.updated((@scrolls.first.created_at))
 
           for scroll in @scrolls
-            feed.entry(scroll, :url => "/otherstuff/" + scroll.to_param, :updated => Time.utc(2007, 1, scroll.id)) do |entry|
+            feed.entry(scroll, :url => "/otherstuff/" + scroll.to_param.to_s, :updated => Time.utc(2007, 1, scroll.id)) do |entry|
               entry.title(scroll.title)
               entry.content(scroll.body, :type => 'html')
 
@@ -55,7 +55,7 @@ class ScrollsController < ActionController::Base
           end
 
           for scroll in @scrolls
-            feed.entry(scroll, :url => "/otherstuff/" + scroll.to_param, :updated => Time.utc(2007, 1, scroll.id)) do |entry|
+            feed.entry(scroll, :url => "/otherstuff/" + scroll.to_param.to_s, :updated => Time.utc(2007, 1, scroll.id)) do |entry|
               entry.title(scroll.title)
               entry.content(scroll.body, :type => 'html')
             end

actionpack/test/template/form_helper_test.rb

@@ -53,6 +53,7 @@ class FormHelperTest < ActionView::TestCase
     def @post.id_before_type_cast; 123; end
     def @post.to_param; '123'; end
 
+    @post.persisted   = true
     @post.title       = "Hello World"
     @post.author_name = ""
     @post.body        = "Back to the hill and over it again!"
@@ -529,7 +530,7 @@ class FormHelperTest < ActionView::TestCase
   def test_submit_with_object_as_new_record_and_locale_strings
     old_locale, I18n.locale = I18n.locale, :submit
 
-    def @post.new_record?() true; end
+    @post.persisted = false
     form_for(:post, @post) do |f|
       concat f.submit
     end
@@ -1363,7 +1364,7 @@ class FormHelperTest < ActionView::TestCase
 
   def test_form_for_with_new_object
     post = Post.new
-    post.new_record = true
+    post.persisted = false
     def post.id() nil end
 
     form_for(post) do |f| end
@@ -1373,9 +1374,7 @@ class FormHelperTest < ActionView::TestCase
   end
 
   def test_form_for_with_existing_object_in_list
-    @post.new_record = false
     @comment.save
-
     form_for([@post, @comment]) {}
 
     expected = %(<form action="#{comment_path(@post, @comment)}" class="edit_comment" id="edit_comment_1" method="post"><div style="margin:0;padding:0;display:inline"><input name="_method" type="hidden" value="put" /></div></form>)
@@ -1383,8 +1382,6 @@ class FormHelperTest < ActionView::TestCase
   end
 
   def test_form_for_with_new_object_in_list
-    @post.new_record = false
-
     form_for([@post, @comment]) {}
 
     expected = %(<form action="#{comments_path(@post)}" class="new_comment" id="new_comment" method="post"></form>)
@@ -1392,9 +1389,7 @@ class FormHelperTest < ActionView::TestCase
   end
 
   def test_form_for_with_existing_object_and_namespace_in_list
-    @post.new_record = false
     @comment.save
-
     form_for([:admin, @post, @comment]) {}
 
     expected = %(<form action="#{admin_comment_path(@post, @comment)}" class="edit_comment" id="edit_comment_1" method="post"><div style="margin:0;padding:0;display:inline"><input name="_method" type="hidden" value="put" /></div></form>)
@@ -1402,8 +1397,6 @@ class FormHelperTest < ActionView::TestCase
   end
 
   def test_form_for_with_new_object_and_namespace_in_list
-    @post.new_record = false
-
     form_for([:admin, @post, @comment]) {}
 
     expected = %(<form action="#{admin_comments_path(@post)}" class="new_comment" id="new_comment" method="post"></form>)

actionpack/test/template/record_tag_helper_test.rb

@@ -18,6 +18,7 @@ class RecordTagHelperTest < ActionView::TestCase
   def setup
     super
     @post = Post.new
+    @post.persisted = true
   end
 
   def test_content_tag_for

actionpack/test/template/url_helper_test.rb

@@ -499,14 +499,14 @@ end
 class Workshop
   extend ActiveModel::Naming
   include ActiveModel::Conversion
-  attr_accessor :id, :new_record
+  attr_accessor :id
 
-  def initialize(id, new_record)
+  def initialize(id)
-    @id, @new_record = id, new_record
+    @id = id
   end
 
-  def new_record?
+  def persisted?
-    @new_record
+    id.present?
   end
 
   def to_s
@@ -517,14 +517,14 @@ end
 class Session
   extend ActiveModel::Naming
   include ActiveModel::Conversion
-  attr_accessor :id, :workshop_id, :new_record
+  attr_accessor :id, :workshop_id
 
-  def initialize(id, new_record)
+  def initialize(id)
-    @id, @new_record = id, new_record
+    @id = id
   end
 
-  def new_record?
+  def persisted?
-    @new_record
+    id.present?
   end
 
   def to_s
@@ -534,12 +534,12 @@ end
 
 class WorkshopsController < ActionController::Base
   def index
-    @workshop = Workshop.new(1, true)
+    @workshop = Workshop.new(nil)
     render :inline => "<%= url_for(@workshop) %>\n<%= link_to('Workshop', @workshop) %>"
   end
 
   def show
-    @workshop = Workshop.new(params[:id], false)
+    @workshop = Workshop.new(params[:id])
     render :inline => "<%= url_for(@workshop) %>\n<%= link_to('Workshop', @workshop) %>"
   end
 
@@ -548,14 +548,14 @@ end
 
 class SessionsController < ActionController::Base
   def index
-    @workshop = Workshop.new(params[:workshop_id], false)
+    @workshop = Workshop.new(params[:workshop_id])
-    @session = Session.new(1, true)
+    @session = Session.new(nil)
     render :inline => "<%= url_for([@workshop, @session]) %>\n<%= link_to('Session', [@workshop, @session]) %>"
   end
 
   def show
-    @workshop = Workshop.new(params[:workshop_id], false)
+    @workshop = Workshop.new(params[:workshop_id])
-    @session = Session.new(params[:id], false)
+    @session = Session.new(params[:id])
     render :inline => "<%= url_for([@workshop, @session]) %>\n<%= link_to('Session', [@workshop, @session]) %>"
   end
 
@@ -565,8 +565,8 @@ end
 class PolymorphicControllerTest < ActionController::TestCase
   def setup
     super
-    @request    = ActionController::TestRequest.new
+    @request  = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
+    @response = ActionController::TestResponse.new
   end
 
   def test_new_resource

activemodel/examples/validations.rb

@@ -16,7 +16,7 @@ class Person
     @persisted = true
   end
 
-  def new_record?
+  def persisted?
     @persisted
   end
 end

activemodel/lib/active_model/conversion.rb

@@ -8,9 +8,9 @@ module ActiveModel
   #   class ContactMessage
   #     include ActiveModel::Conversion
   #
-  #     # Always a new record, since it's not persisted in the DB.
+  #     # ContactMessage are never persisted in the DB
-  #     def new_record?
+  #     def persisted?
-  #       true
+  #       false
   #     end
   #   end
   #
@@ -30,13 +30,13 @@ module ActiveModel
       self
     end
 
-    # Returns an Enumerable of all (primary) key attributes or nil if new_record? is true
+    # Returns an Enumerable of all (primary) key attributes or nil if persisted? is fakse
     def to_key
-      new_record? ? nil : [id]
+      persisted? ? [id] : nil
     end
 
     # Returns a string representing the object's key suitable for use in URLs,
-    # or nil if new_record? is true
+    # or nil if persisted? is false
     def to_param
       to_key ? to_key.join('-') : nil
     end

activemodel/lib/active_model/lint.rb

@@ -17,28 +17,28 @@ module ActiveModel
       # == Responds to <tt>to_key</tt>
       #
       # Returns an Enumerable of all (primary) key attributes
-      # or nil if model.new_record? is true
+      # or nil if model.persisted? is false
       def test_to_key
         assert model.respond_to?(:to_key), "The model should respond to to_key"
-        def model.new_record?() true end
+        def model.persisted?() false end
         assert model.to_key.nil?
-        def model.new_record?() false end
+        def model.persisted?() true end
         assert model.to_key.respond_to?(:each)
       end
 
       # == Responds to <tt>to_param</tt>
       #
       # Returns a string representing the object's key suitable for use in URLs
-      # or nil if model.new_record? is true.
+      # or nil if model.persisted? is false.
       #
       # Implementers can decide to either raise an exception or provide a default
       # in case the record uses a composite primary key. There are no tests for this
       # behavior in lint because it doesn't make sense to force any of the possible
       # implementation strategies on the implementer. However, if the resource is
-      # a new_record?, then to_param should always return nil.
+      # not persisted?, then to_param should always return nil.
       def test_to_param
         assert model.respond_to?(:to_param), "The model should respond to to_param"
-        def model.new_record?() true end
+        def model.persisted?() false end
         assert model.to_param.nil?
       end
 
@@ -51,21 +51,16 @@ module ActiveModel
         assert_boolean model.valid?, "valid?"
       end
 
-      # == Responds to <tt>new_record?</tt>
+      # == Responds to <tt>persisted?</tt>
       #
       # Returns a boolean that specifies whether the object has been persisted yet.
       # This is used when calculating the URL for an object. If the object is
       # not persisted, a form for that object, for instance, will be POSTed to the
       # collection. If it is persisted, a form for the object will put PUTed to the
       # URL for the object.
-      def test_new_record?
+      def test_persisted?
-        assert model.respond_to?(:new_record?), "The model should respond to new_record?"
+        assert model.respond_to?(:persisted?), "The model should respond to persisted?"
-        assert_boolean model.new_record?, "new_record?"
+        assert_boolean model.persisted?, "persisted?"
-      end
-
-      def test_destroyed?
-        assert model.respond_to?(:destroyed?), "The model should respond to destroyed?"
-        assert_boolean model.destroyed?, "destroyed?"
       end
 
       # == Naming

activemodel/test/cases/conversion_test.rb

@@ -12,7 +12,7 @@ class ConversionTest < ActiveModel::TestCase
   end
 
   test "to_key default implementation returns the id in an array for persisted records" do
-    assert_equal [1], Contact.new(:new_record => false, :id => 1).to_key
+    assert_equal [1], Contact.new(:id => 1).to_key
   end
 
   test "to_param default implementation returns nil for new records" do
@@ -20,6 +20,6 @@ class ConversionTest < ActiveModel::TestCase
   end
 
   test "to_param default implementation returns a string of ids for persisted records" do
-    assert_equal "1", Contact.new(:new_record => false, :id => 1).to_param
+    assert_equal "1", Contact.new(:id => 1).to_param
   end
 end

activemodel/test/cases/lint_test.rb

@@ -8,8 +8,7 @@ class LintTest < ActiveModel::TestCase
     include ActiveModel::Conversion
 
     def valid?()      true end
-    def new_record?() true end
+    def persisted?() false end
-    def destroyed?()  true end
 
     def errors
       obj = Object.new

activemodel/test/models/contact.rb

@@ -1,13 +1,13 @@
 class Contact
   include ActiveModel::Conversion
 
-  attr_accessor :id, :name, :age, :created_at, :awesome, :preferences, :new_record
+  attr_accessor :id, :name, :age, :created_at, :awesome, :preferences
 
   def initialize(options = {})
     options.each { |name, value| send("#{name}=", value) }
   end
 
-  def new_record?
+  def persisted?
-    defined?(@new_record) ? @new_record : true
+    id.present?
   end
 end

activerecord/lib/active_record/base.rb

@@ -1767,6 +1767,11 @@ module ActiveRecord #:nodoc:
         @destroyed || false
       end
 
+      # Returns if the record is persisted, i.e. it's not a new record and it was not destroyed.
+      def persisted?
+        !(new_record? || destroyed?)
+      end
+
       # :call-seq:
       #   save(options)
       #
@@ -1816,7 +1821,7 @@ module ActiveRecord #:nodoc:
       # callbacks, Observer methods, or any <tt>:dependent</tt> association
       # options, use <tt>#destroy</tt>.
       def delete
-        self.class.delete(id) unless new_record?
+        self.class.delete(id) if persisted?
         @destroyed = true
         freeze
       end
@@ -1824,7 +1829,7 @@ module ActiveRecord #:nodoc:
       # Deletes the record in the database and freezes this instance to reflect that no changes should
       # be made (since they can't be persisted).
       def destroy
-        unless new_record?
+        if persisted?
           self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).delete_all
         end
 
@@ -1844,6 +1849,7 @@ module ActiveRecord #:nodoc:
         became.instance_variable_set("@attributes", @attributes)
         became.instance_variable_set("@attributes_cache", @attributes_cache)
         became.instance_variable_set("@new_record", new_record?)
+        became.instance_variable_set("@destroyed", destroyed?)
         became
       end
 
@@ -2042,8 +2048,7 @@ module ActiveRecord #:nodoc:
       def ==(comparison_object)
         comparison_object.equal?(self) ||
           (comparison_object.instance_of?(self.class) &&
-            comparison_object.id == id &&
+            comparison_object.id == id && !comparison_object.new_record?)
-            !comparison_object.new_record?)
       end
 
       # Delegates to ==

activerecord/test/cases/base_test.rb

@@ -1330,11 +1330,6 @@ class BasicsTest < ActiveRecord::TestCase
   end
 
   def test_destroyed_returns_boolean
-    developer = Developer.new
-    assert_equal developer.destroyed?, false
-    developer.destroy
-    assert_equal developer.destroyed?, true
-
     developer = Developer.first
     assert_equal developer.destroyed?, false
     developer.destroy
@@ -1346,6 +1341,23 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal developer.destroyed?, true
   end
 
+  def test_persisted_returns_boolean
+    developer = Developer.new(:name => "Jose")
+    assert_equal developer.persisted?, false
+    developer.save!
+    assert_equal developer.persisted?, true
+
+    developer = Developer.first
+    assert_equal developer.persisted?, true
+    developer.destroy
+    assert_equal developer.persisted?, false
+
+    developer = Developer.last
+    assert_equal developer.persisted?, true
+    developer.delete
+    assert_equal developer.persisted?, false
+  end
+
   def test_clone
     topic = Topic.find(1)
     cloned_topic = nil