Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Make collection and collection_from_object methods return an array

This transforms for instance scoped objects into arrays and avoid unneeded queries

[#5958 state:committed]
  • Loading branch information...
commit 27f4ffd11a91b534fde9b484cb7c4e515ec0fe77 1 parent 7cc371a
@spastorino spastorino authored
Showing with 4 additions and 3 deletions.
  1. +4 −3 actionpack/lib/action_view/renderer/partial_renderer.rb
View
7 actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -90,13 +90,14 @@ def render_partial
def collection
if @options.key?(:collection)
- @options[:collection] || []
+ collection = @options[:collection]
+ collection.respond_to?(:to_ary) ? collection.to_ary : []
@parndt
parndt added a note

Could this not be like this:

collection = @options[:collection]
collection = collection.to_ary if collection.respond_to?(:to_ary)
collection

This would mean that whatever the developer passes will get rendered and you'd still get the desired optimisation for things rendered from DB collections?

Yeah, I know this is from 2010, but it cost some time when I couldn't figure out why my collection (plain ruby object that includes Enumerable) wouldn't render.

❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️

@simi
simi added a note

I think it should warn you if to_ary method is not found on collection or check for map method also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
def collection_from_object
if @object.respond_to?(:to_ary)
- @object
+ @object.to_ary
end
end
@@ -163,4 +164,4 @@ def retrieve_variable(path)
[variable, variable_counter]
end
end
-end
+end

3 comments on commit 27f4ffd

@cyrille

Some people were using ActiveSupport::OrderedHash as a collection for exemple in a Nested Set

Imagine this structure (taken as an example from the Ancestry plugin)

{ #
=> { #
=> { #
=> {}
}
}
}

With this commit render partial with collection will render nothing or if converted before to an array will only render first level of the tree...

@josevalim
Owner

Can you please provide a patch with tests? Or maybe even a fix?

@sorentwo

This type of structure comes up often when grouping things, but in those instances it never feels right to pass a more custom data structure to a partial anyhow. I would vote to keep the more streamlined approach and handle less straight forward data structures with looping or helpers.

As usage of 1.9 grows I don't see continued use of OrderedHash anyhow. Checking for OrderedHash specifically would be unnecessary overhead down the road.

@parndt

Could this not be like this:

collection = @options[:collection]
collection = collection.to_ary if collection.respond_to?(:to_ary)
collection

This would mean that whatever the developer passes will get rendered and you'd still get the desired optimisation for things rendered from DB collections?

Yeah, I know this is from 2010, but it cost some time when I couldn't figure out why my collection (plain ruby object that includes Enumerable) wouldn't render.

❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️❤️💛💙💜💚💜💙💛❤️💛💙💜💚💜💙💛❤️

@simi

I think it should warn you if to_ary method is not found on collection or check for map method also.

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