Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Jun 10, 2012

Conversation

Projects
None yet
6 participants
Contributor

kennyj commented Jun 10, 2012

If You've the following model

class Post < ActiveRecord::Base
  scope :since, Proc.new { where('written_on >= ?', Time.now - 1.day) }
  scope :to,    Proc.new { where('written_on <= ?', Time.now) }
end

and you call Post.since.to, we get an exception.

Because since method return ActiveRecord::Relation instance, and this class's method_missing call Array's method firstly.
Thus ActiveRecord::Relation#to is same to Array#to, we get an exception.

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 10, 2012

@rafaelfranca rafaelfranca Merge pull request #6695 from kennyj/fix_6635
Fix #6635. We should call Scoping methods, before calling Array methods.
4da1af7

@rafaelfranca rafaelfranca merged commit 4da1af7 into rails:master Jun 10, 2012

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 10, 2012

@rafaelfranca rafaelfranca Merge pull request #6695 from kennyj/fix_6635
Fix #6635. We should call Scoping methods, before calling Array methods.
5dfb01e

ActiveModel::Serializer is effected by this change.

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

Member

steveklabnik replied Jun 15, 2012

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

Like josevalim/active_model_serializers#81

Contributor

sferik replied 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.

Owner

rafaelfranca replied Jul 2, 2012

@sferik pull requests are always welcome.

Owner

rafaelfranca replied Jul 2, 2012

@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.

Contributor

sferik replied Jul 2, 2012

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

Owner

rafaelfranca replied Jul 2, 2012

@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?

Contributor

kennyj replied Jul 3, 2012

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

Owner

rafaelfranca replied Jul 3, 2012

@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.

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.

Owner

rafaelfranca replied Jul 3, 2012

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

Contributor

kennyj replied 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 :-)

Contributor

kennyj replied Jul 4, 2012

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

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