From 250c8092461f5e6bf62751b313f6605a37fd1b2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Sun, 21 Feb 2010 11:09:21 +0100
Subject: 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.

---
 .../lib/action_controller/polymorphic_routes.rb    |  3 +-
 .../lib/action_view/helpers/active_model_helper.rb |  4 +--
 actionpack/lib/action_view/helpers/form_helper.rb  | 17 +++++----
 actionpack/lib/action_view/helpers/url_helper.rb   |  2 +-
 actionpack/test/controller/mime_responds_test.rb   |  4 +--
 actionpack/test/controller/redirect_test.rb        | 18 +++++-----
 actionpack/test/lib/controller/fake_models.rb      | 40 +++++++---------------
 .../test/template/active_model_helper_test.rb      |  4 +--
 actionpack/test/template/atom_feed_helper_test.rb  |  8 ++---
 actionpack/test/template/form_helper_test.rb       | 13 ++-----
 actionpack/test/template/record_tag_helper_test.rb |  1 +
 actionpack/test/template/url_helper_test.rb        | 36 +++++++++----------
 12 files changed, 63 insertions(+), 87 deletions(-)

(limited to 'actionpack')

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
-  end
-
-  def new_record=(boolean)
-    @new_record = boolean
+  def persisted=(boolean)
+    @persisted = 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 = %(<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>)
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
-- 
cgit v1.2.3