Add to_key and to_param methods to ActiveModel::Conversion.

5 files changed, 70 insertions, 27 deletions

diff --git a/activemodel/lib/active_model/conversion.rb b/activemodel/lib/active_model/conversion.rb
index c14a07c7dc..78dc34197b 100644
--- a/activemodel/lib/active_model/conversion.rb
+++ b/activemodel/lib/active_model/conversion.rb
@@ -1,19 +1,44 @@
 module ActiveModel
-  # If your object is already designed to implement all of the Active Model featurs
-  # include this module in your Class.
-  # 
-  #   class MyClass
+  # Handle default conversions: to_model, to_key and to_param.
+  #
+  # == Example
+  #
+  # Let's take for example this non persisted object.
+  #
+  #   class ContactMessage
   #     include ActiveModel::Conversion
+  #
+  #     # Always a new record, since it's not persisted in the DB.
+  #     def new_record?
+  #       true
+  #     end
   #   end
-  # 
-  # Returns self to the <tt>:to_model</tt> method.
-  # 
-  # If your model does not act like an Active Model object, then you should define
-  # <tt>:to_model</tt> yourself returning a proxy object that wraps your object
-  # with Active Model compliant methods.
+  #
+  #   cm = ContactMessage.new
+  #   cm.to_model == self #=> true
+  #   cm.to_key           #=> nil
+  #   cm.to_param         #=> nil
+  #
   module Conversion
+    # If your object is already designed to implement all of the Active Model you can use
+    # the default to_model implementation, which simply returns self.
+    # 
+    # If your model does not act like an Active Model object, then you should define
+    # <tt>:to_model</tt> yourself returning a proxy object that wraps your object
+    # with Active Model compliant methods.
     def to_model
       self
     end
+
+    # Returns an Enumerable of all (primary) key attributes or nil if new_record? is true
+    def to_key
+      new_record? ? nil : [id]
+    end
+
+    # Returns a string representing the object's key suitable for use in URLs,
+    # or nil if new_record? is true
+    def to_param
+      to_key ? to_key.join('-') : nil
+    end
   end
 end
diff --git a/activemodel/test/cases/conversion_test.rb b/activemodel/test/cases/conversion_test.rb
new file mode 100644
index 0000000000..373424df2f
--- /dev/null
+++ b/activemodel/test/cases/conversion_test.rb
@@ -0,0 +1,25 @@
+require 'cases/helper'
+require 'models/contact'
+
+class ConversionTest < ActiveModel::TestCase
+  test "to_model default implementation returns self" do
+    contact = Contact.new
+    assert_equal contact, contact.to_model
+  end
+
+  test "to_key default implementation returns nil for new records" do
+    assert_nil Contact.new.to_key
+  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
+  end
+
+  test "to_param default implementation returns nil for new records" do
+    assert_nil Contact.new.to_param
+  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
+  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 99f8eff5d8..e4c069e1ab 100644
--- a/activemodel/test/cases/lint_test.rb
+++ b/activemodel/test/cases/lint_test.rb
@@ -1,24 +1,11 @@
-require "cases/helper"
+require 'cases/helper'
 
 class LintTest < ActiveModel::TestCase
   include ActiveModel::Lint::Tests
 
   class CompliantModel
     extend ActiveModel::Naming
-
-    def to_model
-      self
-    end
-
-    def to_key
-      new_record? ? nil : [id]
-    end
-
-    def to_param
-      return nil if to_key.nil?
-      # some default for CPKs, real implementations will differ
-      to_key.length > 1 ? to_key.join('-') : to_key.first.to_s
-    end
+    include ActiveModel::Conversion
 
     def valid?()      true end
     def new_record?() true end
diff --git a/activemodel/test/models/contact.rb b/activemodel/test/models/contact.rb
index f9fb0af027..dbdb8539b7 100644
--- a/activemodel/test/models/contact.rb
+++ b/activemodel/test/models/contact.rb
@@ -1,7 +1,13 @@
 class Contact
-  attr_accessor :name, :age, :created_at, :awesome, :preferences
+  include ActiveModel::Conversion
+
+  attr_accessor :id, :name, :age, :created_at, :awesome, :preferences, :new_record
 
   def initialize(options = {})
     options.each { |name, value| send("#{name}=", value) }
   end
+
+  def new_record?
+    defined?(@new_record) ? @new_record : true
+  end
 end
diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb
index 8549414a12..095814b635 100644
--- a/activerecord/lib/active_record/attribute_methods/primary_key.rb
+++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb
@@ -50,7 +50,7 @@ module ActiveRecord
         # This method also takes custom primary keys specified via
         # the +set_primary_key+ into account.
         def to_key
-          new_record? ? nil : [ self.send(self.class.primary_key) ]
+          new_record? ? nil : [ self.primary_key ]
         end
 
       end