diff --git a/actionpack/lib/action_controller/polymorphic_routes.rb b/actionpack/lib/action_controller/polymorphic_routes.rb index eaed00cfb7..ae363e300c 100644 --- a/actionpack/lib/action_controller/polymorphic_routes.rb +++ b/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?) || - (record.respond_to?(:destroyed?) && record.destroyed?) + elsif (record.respond_to?(:persisted?) && !record.persisted?) args.pop :plural elsif record.is_a?(Class) diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb index 2f309fcca0..ed83b8a8b2 100644 --- a/actionpack/lib/action_view/helpers/active_model_helper.rb +++ b/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) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index ce21af9923..ace3bcfde3 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/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? - { :class => dom_class(object, :new), :id => dom_id(object), :method => :post } - else + if object.respond_to?(:persisted?) && object.persisted? { :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? - @template.fields_for(name, object, options, &block) - else + if object.persisted? @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 diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 6ca1a5d730..e121472fe3 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/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) %> diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 3bd3369242..5e34773899 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/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 diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 570ff4a41b..441bc47908 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/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) - @id, @new_record = id, new_record + def initialize(id) + @id = id end - def new_record? - @new_record + def persisted? + 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 diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index 4475d40f63..bf3e175f1f 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/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? - false + def persisted? + id.present? end end @@ -47,12 +39,8 @@ module Quiz extend ActiveModel::Naming include ActiveModel::Conversion - def to_key - id ? [id] : nil - end - - def to_param - id.to_s + def persisted? + id.present? 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 - id ? [id] : nil + def persisted=(boolean) + @persisted = boolean end - def new_record=(boolean) - @new_record = boolean - end - - def new_record? - @new_record + def persisted? + @persisted 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}" diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 3e01ae78c3..371aa53c68 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/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 diff --git a/actionpack/test/template/atom_feed_helper_test.rb b/actionpack/test/template/atom_feed_helper_test.rb index 347cb98303..869ea22469 100644 --- a/actionpack/test/template/atom_feed_helper_test.rb +++ b/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? - true + def persisted? + 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 diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 7b909fff82..d143c39c9c 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/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 = %(
) @@ -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 = %(
) @@ -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 = %(
) @@ -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 = %(
) diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index 1cd18c0692..74d7bba4fe 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/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 diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 418c050906..c917ca9d2b 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/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) - @id, @new_record = id, new_record + def initialize(id) + @id = id end - def new_record? - @new_record + def persisted? + 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) - @id, @new_record = id, new_record + def initialize(id) + @id = id end - def new_record? - @new_record + def persisted? + 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) - @session = Session.new(1, true) + @workshop = Workshop.new(params[:workshop_id]) + @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) - @session = Session.new(params[:id], false) + @workshop = Workshop.new(params[:workshop_id]) + @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 - @response = ActionController::TestResponse.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new end def test_new_resource diff --git a/activemodel/examples/validations.rb b/activemodel/examples/validations.rb index b039897ea5..0b2706076f 100644 --- a/activemodel/examples/validations.rb +++ b/activemodel/examples/validations.rb @@ -16,7 +16,7 @@ class Person @persisted = true end - def new_record? + def persisted? @persisted end end diff --git a/activemodel/lib/active_model/conversion.rb b/activemodel/lib/active_model/conversion.rb index 78dc34197b..585c20dcdf 100644 --- a/activemodel/lib/active_model/conversion.rb +++ b/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. - # def new_record? - # true + # # ContactMessage are never persisted in the DB + # def persisted? + # 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 diff --git a/activemodel/lib/active_model/lint.rb b/activemodel/lib/active_model/lint.rb index 98413ac9fb..0e62e131a3 100644 --- a/activemodel/lib/active_model/lint.rb +++ b/activemodel/lib/active_model/lint.rb @@ -17,28 +17,28 @@ module ActiveModel # == Responds to to_key # # 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 to_param # # 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 new_record? + # == Responds to persisted? # # 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? - assert model.respond_to?(:new_record?), "The model should respond to new_record?" - assert_boolean model.new_record?, "new_record?" - end - - def test_destroyed? - assert model.respond_to?(:destroyed?), "The model should respond to destroyed?" - assert_boolean model.destroyed?, "destroyed?" + def test_persisted? + assert model.respond_to?(:persisted?), "The model should respond to persisted?" + assert_boolean model.persisted?, "persisted?" end # == Naming diff --git a/activemodel/test/cases/conversion_test.rb b/activemodel/test/cases/conversion_test.rb index 373424df2f..7669bf5f65 100644 --- a/activemodel/test/cases/conversion_test.rb +++ b/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 \ No newline at end of file diff --git a/activemodel/test/cases/lint_test.rb b/activemodel/test/cases/lint_test.rb index e4c069e1ab..68372160cd 100644 --- a/activemodel/test/cases/lint_test.rb +++ b/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 destroyed?() true end + def persisted?() false end def errors obj = Object.new diff --git a/activemodel/test/models/contact.rb b/activemodel/test/models/contact.rb index dbdb8539b7..a583b89aa1 100644 --- a/activemodel/test/models/contact.rb +++ b/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? - defined?(@new_record) ? @new_record : true + def persisted? + id.present? end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index c6dde078ca..ef5a7d5787 100755 --- a/activerecord/lib/active_record/base.rb +++ b/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 :dependent association # options, use #destroy. 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.new_record?) + comparison_object.id == id && !comparison_object.new_record?) end # Delegates to == diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 1441b4278d..2c4e1a3c0f 100755 --- a/activerecord/test/cases/base_test.rb +++ b/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