This project is archived and is in readonly mode.
named_scope with bang
-
Steven SorokaThis would make search forms a lot simpler.
I like the idea.
-
Alex MacCaw+1
-
Rodrigo Urubatan
+1
Really liked the idea
-
Jeremy KemperI agree that this looks nice, but I think scopes should be treated as immutable. The semantics are clearer, it's easier to reason about them, and it allows nice things like caching a sql statement for a scope without worrying about it being changed out from under you.
-
Ryan BatesIf you would like to keep scopes immutable, is there a better alternative to building them up conditionally? I really don't like having to reset a local variable each time like this:
topics = Topic.approved topics = topics.replied if only_show_replied? topics = topics.scoped :limit => limit if limitThis is a pattern I run into frequently and would love to see it addressed.
-
Chris WadeSomething that seems to have been lost in the translation from the Railscast to this ticket is the usage that inspired this patch.
In the Railscast Ryan used this technique to build a dynamic/anonymous scope based on the request parameters. Given that by using the technique he did Ryan was expecting the query to change from request to request it seems perfectly reasonable to allow such usage of named_scope (although it seems less of a named_scope now).
Additionally, isn't a bang method normally expected to change the object it's being called on? So wouldn't the developer choosing to call the bang version be expecting the query to mutate?
Sure there are ramifications to using this technique but, to me at least, it should be the developer choosing to use the technique or not as opposed to the framework simply not allowing it. (Yes I know Rails is opinionated, but so are the developers using it!)
-
Zyclops
+1
-
JJ
+1. Using Rails 2.1 for only 2 weeks, I've already encountered a situation where this bang syntax would have been more intuitive and tidier.
-
Ryan Bates
There seems to be a lot of interest in this patch, any chance of it being accepted?
Regarding keeping scopes immutable, I fail to see what big advantages this brings. It's easy enough to clear any caches in the "replace" method, and if the developer needs it to be immutable he can call "freeze" on it. Is there a case where this would not be sufficient?
-
Lawrence Pit- Tag set to activerecord, named_scope, patch, tested
This patch is flawed. Given the examples and the tests in the patch:
topics = Topic.approved
topics.replied!
topics.scoped!
I don't see why anyone would want this, as this would execute 3 queries. I think the whole idea is to mix the scopes you want, and then execute 1 query.
When you do this:
scope = Product.scoped({})
you effectively executed a query to fetch all the products. Every time you assign a new scope a query is executed. So building up your conditions this way is highly inefficient.
Given railscast 112 (which end result does 6 queries), instead of assigning a new scope each time, what you'd need to do is build up your conditions as a hash, and then simply call Product.all(conditions) once.
The example given in Railscast 113 doesn't work at all, even with this patch is applied. It simply returns all products. The block argument |scope| is useless as yield doesn't pass any value to it. A block given to named_scope only allows to extend it (the block is passed into Module.new, that's all).
I do see what Ryan was trying to do, and I think the idea is very nice. But it needs a different implementation.
As a suggestion, possibly allow this:
def find_topics
scope = Topic.approved_scope
scope.replied_scope!
scope.assigned_scope!(current_user) if current_user
scope.price_scope!(price) if price
scope.all
end
May not be nicest, but I think this is what Ryan was on about. You want to work with the scopes without each executing the find method while your manipulating them. To make Jeremy happy, when you request a *_scope you could return a copy of the scope, not the original scope definition.
-
Lawrence Pit
Sorry for the above, it does work, though not entirely as I expected. I was testing in script/console, and then it doesn't work, because the output of scope would trigger the find method. When you chain scopes within a method, and then call the method you're fine though.
wrt railscasts 113, the code should look like:
def find_products returning (Product.scoped({})) do |scope| #etc. end endNow the block is yielded by the returning method instead of the scoped method.
-
Ryan Bates
I decided to implement this functionality as a gem. It's slightly different, so instead of altering Scope, it uses a separate scope builder class.
http://github.com/ryanb/scope-bu...
Would this implementation more likely be accepted into Rails core? If so I can write up a patch. :)
-
Magnus BergmarkWhat's the status of this? I think this could be a valuable addition to core.
-
Pratik- State changed from new to wontfix
I agree with what Jeremy said. Apart from that, I'm not really a fan of building scopes conditionally on the fly. It feels like we're missing a Condition Builder and named scopes are what comes the closest. Closing this ticket for now.
-
Zyclops
A condition builder would be a really nice feature, something where you can easily access the various parts that make up a query and at any point and add, edit or manipulate.
At the moment named_scope is the easiest way of generating various conditions from large forms. There is no way of course to inspect and manipulate the attached sql once you've already scoped it (unless your using .send(:construct_finder_sql, {}) and than manipulate the sql string.
Any thoughts on how a condition builder would work? I've not seen any good examples in the other languages i've used. So far named_scope is still the winner.