Permalink
Browse files

Fix #6635. We should call Scoping methods, before calling Array methods.

  • Loading branch information...
1 parent 42259f6 commit cd1be1acc68221fbdfab0d13211d40bdeae630dc @kennyj kennyj committed Jun 7, 2012
@@ -32,12 +32,12 @@ def respond_to?(method, include_private = false)
protected
def method_missing(method, *args, &block)
- if Array.method_defined?(method)
- ::ActiveRecord::Delegation.delegate method, :to => :to_a
- to_a.send(method, *args, &block)
- elsif @klass.respond_to?(method)
+ if @klass.respond_to?(method)
::ActiveRecord::Delegation.delegate_to_scoped_klass(method)
scoping { @klass.send(method, *args, &block) }
+ elsif Array.method_defined?(method)
+ ::ActiveRecord::Delegation.delegate method, :to => :to_a
+ to_a.send(method, *args, &block)
elsif arel.respond_to?(method)
::ActiveRecord::Delegation.delegate method, :to => :arel
arel.send(method, *args, &block)
@@ -46,4 +46,4 @@ def method_missing(method, *args, &block)
end
end
end
-end
+end
@@ -45,6 +45,15 @@ def test_delegates_finds_and_calculations_to_the_base_class
assert_equal Topic.average(:replies_count), Topic.base.average(:replies_count)
end
+ def test_method_missing_priority_when_delegating
+ klazz = Class.new(ActiveRecord::Base) do
+ self.table_name = "topics"
+ scope :since, Proc.new { where('written_on >= ?', Time.now - 1.day) }
+ scope :to, Proc.new { where('written_on <= ?', Time.now) }
+ end
+ assert_equal klazz.to.since.all, klazz.since.to.all
+ end
+
def test_scope_should_respond_to_own_methods_and_methods_of_the_proxy
assert Topic.approved.respond_to?(:limit)
assert Topic.approved.respond_to?(:count)

13 comments on commit cd1be1a

@pivotal-ghostchips

ActiveModel::Serializer is effected by this change.

https://github.com/josevalim/active_model_serializers/issues/81

@steveklabnik
Member

As a random note, you can link to issues in other repos with user/project#number

Like josevalim/active_model_serializers#81

@sferik
Contributor
sferik commented on cd1be1a Jul 2, 2012

I'm 😞 that this shipped along with a critical security fix. I can't apply the patch until this issue is resolved.

@rafaelfranca
Member

@sferik pull requests are always welcome.

@rafaelfranca
Member

@kennyj could you check this please?

I don't have time now. But this is in my todo list. A is_a? check should solve.

@sferik
Contributor
sferik commented on cd1be1a Jul 2, 2012

@rafaelfranca Would you accept a pull request to revert this commit?

@rafaelfranca
Member

@sferik I would try to fix it since this commit is a bug fix, but we can revert it and try to fix the bug in another way. What do you thing?

@kennyj
Contributor
kennyj commented on cd1be1a Jul 3, 2012

@sferik @rafaelfranca Please tell me more details. I don't realize a problem.

@rafaelfranca
Member

@kennyj I don't know the real reason but seems like josevalim/active_model_serializers is expecting that Array methods have precedence over AR::Relation. josevalim/active_model_serializers#81 shows the error but we need to research.

@jtarchie

I believe that ActiveModel Serializers can be updated. The question is was this commit a needed and expected change for 3.2.6.

Thank god for tests! Its stop me from upgrading to 3.2.6 so far.

@rafaelfranca
Member

Yes, this is an expected change to a stable release since it is a bug fix.

@kennyj
Contributor
kennyj commented on cd1be1a Jul 3, 2012

I see... I realized that since 3.2.6 has ActiveModel Serializers problem, @sferik can't update his application from 3.2.5 (has security problem) to 3.2.6.
Yes, I'll check it :-)

@kennyj
Contributor
kennyj commented on cd1be1a Jul 4, 2012

Hi Everyone. I couldn't reproduce AM Serializers's problem ;-)
Please serve sample app or tell us the way to reproduce.

Please sign in to comment.