-
Notifications
You must be signed in to change notification settings - Fork 21.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scope without lambda ignores condition chain #10864
Comments
Our, temporary, solution can be found here: galdomedia/activerecord-deprecated_finders@dabc9bc |
/cc @jonleighton |
Not sure if this is by design but while upgrading to rails 4 I was adding the lambda or -> (i thought the same) to the scopes but I get this error when using this syntax: scope :example, -> { |*args| where('attr = ?', (args.first)) } scope :example, lambda { |*args| where('attr = ?', (args.first)) } not sure why this happens exactly |
@jeremyrichardson88 : This is Ruby's design. This is more or less the same thing but the syntax is not exactly the same: -> *args { ... }
# Or
lambda {|*args| ... } |
This issue has been automatically marked as stale because it has not been commented on for at least The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the Thank you for all your contributions. |
This is the downside of using scopes without lambda. With Rails 4.1 this is not possible anymore. On Rails 4.0 we already show the deprecation warning and it can be changed to raise an exception if you configure the deprecation behavior to |
Consider following models
Calling following
will result in following sql code:
I consider this important issue, as it's quite easy just to forget using lambda syntax and results are far from expected. Lambda syntax is quite big change in Rails4 and I suspect, that many people will forget about using it. In my opinion exception should be raised in such situations.
This is somehow connected to not yet merged #10528 and issue #10421 - so this ticket just aims to show how serious situation is.
Due to not merged #10528, it's still an issue on branch 4-0-stable.
The text was updated successfully, but these errors were encountered: