Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Moved a few methods from RecordIdentifier to ActiveModel::Naming

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 6807b080996ee4bd6b80abb4f5e9964632c421c8 1 parent fa8b290
@drogus drogus authored josevalim committed
View
31 actionpack/lib/action_controller/record_identifier.rb
@@ -46,7 +46,7 @@ module RecordIdentifier
# dom_class(post, :edit) # => "edit_post"
# dom_class(Person, :edit) # => "edit_person"
def dom_class(record_or_class, prefix = nil)
- singular = singular_class_name(record_or_class)
+ singular = ActiveModel::Naming.singular(record_or_class)
prefix ? "#{prefix}#{JOIN}#{singular}" : singular
end
@@ -85,34 +85,5 @@ def record_key_for_dom_id(record)
def sanitize_dom_id(candidate_id)
candidate_id # TODO implement conversion to valid DOM id values
end
-
- # Returns the plural class name of a record or class. Examples:
- #
- # plural_class_name(post) # => "posts"
- # plural_class_name(Highrise::Person) # => "highrise_people"
- def plural_class_name(record_or_class)
- model_name_from_record_or_class(record_or_class).plural
- end
-
- # Returns the singular class name of a record or class. Examples:
- #
- # singular_class_name(post) # => "post"
- # singular_class_name(Highrise::Person) # => "highrise_person"
- def singular_class_name(record_or_class)
- model_name_from_record_or_class(record_or_class).singular
- end
-
- # Identifies whether the class name of a record or class is uncountable. Examples:
- #
- # uncountable?(Sheep) # => true
- # uncountable?(Post) => false
- def uncountable?(record_or_class)
- plural_class_name(record_or_class) == singular_class_name(record_or_class)
- end
-
- private
- def model_name_from_record_or_class(record_or_class)
- (record_or_class.is_a?(Class) ? record_or_class : record_or_class.class).model_name
- end
end
end
View
6 actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
@@ -155,7 +155,7 @@ def build_named_route_call(records, inflection, options = {})
if parent.is_a?(Symbol) || parent.is_a?(String)
string << "#{parent}_"
else
- string << ActionController::RecordIdentifier.plural_class_name(parent).singularize
+ string << ActiveModel::Naming.plural(parent).singularize
string << "_"
end
end
@@ -164,10 +164,10 @@ def build_named_route_call(records, inflection, options = {})
if record.is_a?(Symbol) || record.is_a?(String)
route << "#{record}_"
else
- route << ActionController::RecordIdentifier.plural_class_name(record)
+ route << ActiveModel::Naming.plural(record)
route = route.singularize if inflection == :singular
route << "_"
- route << "index_" if ActionController::RecordIdentifier.uncountable?(record) && inflection == :plural
+ route << "index_" if ActiveModel::Naming.uncountable?(record) && inflection == :plural
end
action_prefix(options) + route + routing_type(options).to_s
View
10 actionpack/lib/action_view/helpers/form_helper.rb
@@ -302,12 +302,12 @@ def form_for(record_or_name_or_array, *args, &proc)
object_name = record_or_name_or_array
when Array
object = record_or_name_or_array.last
- object_name = options[:as] || ActionController::RecordIdentifier.singular_class_name(object)
+ object_name = options[:as] || ActiveModel::Naming.singular(object)
apply_form_for_options!(record_or_name_or_array, options)
args.unshift object
else
object = record_or_name_or_array
- object_name = options[:as] || ActionController::RecordIdentifier.singular_class_name(object)
+ object_name = options[:as] || ActiveModel::Naming.singular(object)
apply_form_for_options!([object], options)
args.unshift object
end
@@ -533,7 +533,7 @@ def fields_for(record_or_name_or_array, *args, &block)
object = args.first
else
object = record_or_name_or_array
- object_name = ActionController::RecordIdentifier.singular_class_name(object)
+ object_name = ActiveModel::Naming.singular(object)
end
builder = options[:builder] || ActionView::Base.default_form_builder
@@ -1156,11 +1156,11 @@ def fields_for(record_or_name_or_array, *args, &block)
end
when Array
object = record_or_name_or_array.last
- name = "#{object_name}#{index}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
+ name = "#{object_name}#{index}[#{ActiveModel::Naming.singular(object)}]"
args.unshift(object)
else
object = record_or_name_or_array
- name = "#{object_name}#{index}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
+ name = "#{object_name}#{index}[#{ActiveModel::Naming.singular(object)}]"
args.unshift(object)
end
View
40 actionpack/test/controller/record_identifier_test.rb
@@ -26,20 +26,6 @@ def name
end
end
-class Comment::Nested < Comment; end
-
-class Test::Unit::TestCase
- protected
- def comments_url
- 'http://www.example.com/comments'
- end
-
- def comment_url(comment)
- "http://www.example.com/comments/#{comment.id}"
- end
-end
-
-
class RecordIdentifierTest < Test::Unit::TestCase
include ActionController::RecordIdentifier
@@ -76,30 +62,4 @@ def test_dom_class
def test_dom_class_with_prefix
assert_equal "custom_prefix_#{@singular}", dom_class(@record, :custom_prefix)
end
-
- def test_singular_class_name
- assert_equal @singular, singular_class_name(@record)
- end
-
- def test_singular_class_name_for_class
- assert_equal @singular, singular_class_name(@klass)
- end
-
- def test_plural_class_name
- assert_equal @plural, plural_class_name(@record)
- end
-
- def test_plural_class_name_for_class
- assert_equal @plural, plural_class_name(@klass)
- end
-
- def test_uncountable
- assert_equal true, uncountable?(@uncountable)
- assert_equal false, uncountable?(@klass)
- end
-
- private
- def method_missing(method, *args)
- RecordIdentifier.send(method, *args)
- end
end
View
29 activemodel/lib/active_model/naming.rb
@@ -57,6 +57,35 @@ module Naming
def model_name
@_model_name ||= ActiveModel::Name.new(self)
end
+
+ # Returns the plural class name of a record or class. Examples:
+ #
+ # ActiveModel::Naming.plural(post) # => "posts"
+ # ActiveModel::Naming.plural(Highrise::Person) # => "highrise_people"
+ def self.plural(record_or_class)
+ model_name_from_record_or_class(record_or_class).plural
+ end
+
+ # Returns the singular class name of a record or class. Examples:
+ #
+ # ActiveModel::Naming.singular(post) # => "post"
+ # ActiveModel::Naming.singular(Highrise::Person) # => "highrise_person"
+ def self.singular(record_or_class)
+ model_name_from_record_or_class(record_or_class).singular
+ end
+
+ # Identifies whether the class name of a record or class is uncountable. Examples:
+ #
+ # ActiveModel::Naming.uncountable?(Sheep) # => true
+ # ActiveModel::Naming.uncountable?(Post) => false
+ def self.uncountable?(record_or_class)
+ plural(record_or_class) == singular(record_or_class)
+ end
+
+ private
+ def self.model_name_from_record_or_class(record_or_class)
+ (record_or_class.is_a?(Class) ? record_or_class : record_or_class.class).model_name
@ernie
ernie added a note

Shouldn't this call to_model if the object's not already a class? Example: a class whose objects wrap another model. The wrapped model will be the one for which we want the name. This can be partially worked around by overriding "class" on the wrapper, but that seems awfully evil.

@josevalim Owner

Right now we are assuming that the object passed in was already manipulated and converted to_model.

@ernie
ernie added a note

Hmmm. Maybe it's just me, but MetaSearch tests are failing when I count on that being true.

Could you have a look at http://github.com/rails/rails/blob/master/actionpack/lib/action_view/helpers/form_helper.rb#L308 ? Seems like that's a case where to_model hasn't been called, but we end up asking it for its param_key.

@josevalim Owner

Yeah, it is missing there. So I agree with you, it is better to handle such scenarios in this method itself.

@ernie
ernie added a note

Actually the more I look at it, with the reliance on converting to class, it seems I'd have to resort to metaprogramming to get a model_name object that contains info derived partially from the wrapped model, anyway... since I need to set up a class method to return a specific name. Something tells me that with these changes, I'm probably taking the wrong approach altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
end
end
View
63 activemodel/test/cases/naming_helpers_test.rb
@@ -0,0 +1,63 @@
+require 'cases/helper'
+
+class Comment
+ extend ActiveModel::Naming
+ include ActiveModel::Conversion
+
+ attr_reader :id
+ def to_key; id ? [id] : nil end
+ def save; @id = 1 end
+ def new_record?; @id.nil? end
+ def name
+ @id.nil? ? 'new comment' : "comment ##{@id}"
+ end
+end
+
+class Sheep
+ extend ActiveModel::Naming
+ include ActiveModel::Conversion
+
+ attr_reader :id
+ def to_key; id ? [id] : nil end
+ def save; @id = 1 end
+ def new_record?; @id.nil? end
+ def name
+ @id.nil? ? 'new sheep' : "sheep ##{@id}"
+ end
+end
+
+class NamingHelpersTest < Test::Unit::TestCase
+ def setup
+ @klass = Comment
+ @record = @klass.new
+ @singular = 'comment'
+ @plural = 'comments'
+ @uncountable = Sheep
+ end
+
+ def test_singular
+ assert_equal @singular, singular(@record)
+ end
+
+ def test_singular_for_class
+ assert_equal @singular, singular(@klass)
+ end
+
+ def test_plural
+ assert_equal @plural, plural(@record)
+ end
+
+ def test_plural_for_class
+ assert_equal @plural, plural(@klass)
+ end
+
+ def test_uncountable
+ assert_equal true, uncountable?(@uncountable)
+ assert_equal false, uncountable?(@klass)
+ end
+
+ private
+ def method_missing(method, *args)
+ ActiveModel::Naming.send(method, *args)
+ end
+end
@ernie

Shouldn't this call to_model if the object's not already a class? Example: a class whose objects wrap another model. The wrapped model will be the one for which we want the name. This can be partially worked around by overriding "class" on the wrapper, but that seems awfully evil.

@josevalim

Right now we are assuming that the object passed in was already manipulated and converted to_model.

@ernie

Hmmm. Maybe it's just me, but MetaSearch tests are failing when I count on that being true.

Could you have a look at http://github.com/rails/rails/blob/master/actionpack/lib/action_view/helpers/form_helper.rb#L308 ? Seems like that's a case where to_model hasn't been called, but we end up asking it for its param_key.

@josevalim

Yeah, it is missing there. So I agree with you, it is better to handle such scenarios in this method itself.

@ernie

Actually the more I look at it, with the reliance on converting to class, it seems I'd have to resort to metaprogramming to get a model_name object that contains info derived partially from the wrapped model, anyway... since I need to set up a class method to return a specific name. Something tells me that with these changes, I'm probably taking the wrong approach altogether.

Please sign in to comment.
Something went wrong with that request. Please try again.