Fix for 'Sequel model instances respond_to :each' #227

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

kdstew commented Apr 19, 2012

As mentioned in Issue #175.

When using Sequel to replace Active Record in rails. rabl was considering all Sequel objects as collections, since each Sequel model instance has a method each.

Kevin Stewart This commit fixes #175. Changed collection test methods to check if t…
…he module Enumerable is included in the object instead of checking for the existence of a method each.
aee215c
Owner

nesquena commented Apr 19, 2012

Do you know if this works with named scopes in active record that need to be kicked. Do named scopes include enumerable? I am just trying to think of potential edge cases. Let me play around with this and see if it checks out.

@nesquena nesquena commented on an outdated diff Apr 19, 2012

lib/rabl/helpers.rb
@@ -50,12 +50,12 @@ def determine_object_root(data, include_root=true)
# is_object?([]) => false
# is_object?({}) => false
def is_object?(obj)
- obj && !data_object(obj).respond_to?(:each)
+ obj && !obj.class.included_modules.include?(Enumerable)
@nesquena

nesquena Apr 19, 2012

Owner

You should keep data_object(obj) around here and check the class of that. Same with below.

kdstew commented Apr 19, 2012

Good thinking

I just tested to see if a scope, order_by_fullname, includes Enumerable:

Patient.order_by_fullname.class.included_modules.include?(Enumerable)
=> false 

Unfortunately ActiveRecord::Relation (class of the scope) does not include Enumerable.

I'll keep thinking about another possible way to make this check that works with Sequel and ActiveRecord

Owner

nesquena commented Apr 19, 2012

Yeah the edge cases make this frustrating to work out. I guess we could check respond_to? of another enumerable method like map that maybe sequel doesn't define. Feels hacky though.

Collaborator

databyte commented Apr 26, 2012

Not the route to go - will continue the discussion in Issue #175

databyte closed this Apr 26, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment