This project is archived and is in readonly mode.

#368 ✓ wontfix
Ryan Bates

named_scope with bang

Reported by Ryan Bates | June 8th, 2008 @ 12:04 AM

This patch allows you to call a named_scope with a bang (!) which will change the current scope instead of returning a new scope. This is useful for adding scopes conditionally.

topics = Topic.approved
topics.replied! if only_show_replied?
topics.scoped! :limit => limit if limit

Note: this patch is featured in Railscasts Episode 113 (releasing this Monday).

Comments and changes to this ticket

  • Steven Soroka

    Steven Soroka June 10th, 2008 @ 04:19 PM

    This would make search forms a lot simpler.

    I like the idea.

  • Alex MacCaw
  • Rodrigo Urubatan

    Rodrigo Urubatan June 10th, 2008 @ 05:56 PM

    +1

    Really liked the idea

  • Jeremy Kemper

    Jeremy Kemper June 10th, 2008 @ 09:36 PM

    I 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 Bates

    Ryan Bates June 10th, 2008 @ 09:44 PM

    If 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 limit
    

    This is a pattern I run into frequently and would love to see it addressed.

  • Chris Wade

    Chris Wade June 11th, 2008 @ 01:02 AM

    Something 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
  • JJ

    JJ June 18th, 2008 @ 11:31 AM

    +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

    Ryan Bates June 22nd, 2008 @ 06:07 AM

    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

    Lawrence Pit June 26th, 2008 @ 03:54 AM

    • 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

    Lawrence Pit June 26th, 2008 @ 05:33 AM

    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
    end
    

    Now the block is yielded by the returning method instead of the scoped method.

  • Ryan Bates

    Ryan Bates June 26th, 2008 @ 06:49 PM

    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 Bergmark

    Magnus Bergmark August 11th, 2008 @ 09:41 AM

    What's the status of this? I think this could be a valuable addition to core.

  • Pratik

    Pratik August 22nd, 2008 @ 02:09 AM

    • 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

    Zyclops August 22nd, 2008 @ 02:34 AM

    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.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Tickets have moved to Github

The new ticket tracker is available at https://github.com/rails/rails/issues

Shared Ticket Bins

Attachments

Pages