This project is archived and is in readonly mode.

#1331 ✓wontfix
James Le Cuirot

Allow arbitrary named_scope instances to be merged

Reported by James Le Cuirot | November 5th, 2008 @ 10:23 AM | in 2.x

I was rather surprised to find that given you can do this...


x = Shirt.red
y = x.dry_clean_only
y.all

You cannot also do something like this...


x = Shirt.red
y = Shirt.dry_clean_only
z = x.merge(y)
z.all

The usefulness of this might not be obvious from this example but it has helped make my code a bit less complicated. I have attached a patch with a testcase.

My friend said that merge was a confusing method name to use but I disagree. It is at least not defined by Array or Enumerable so it should be safe to use. If you'd rather use something else, that's fine, I don't care as long as it works. :)

It is important to note which scope takes precedence for named scope extensions. When doing x.y, it is y that applies. For x.merge(y), I felt it was more intuitive that x applies. I'm not using extensions (yet) so again, I don't really mind either way.

Comments and changes to this ticket

  • James Le Cuirot

    James Le Cuirot November 5th, 2008 @ 10:24 AM

    Here's the same patch for 2.1 if anyone wants it.

  • James Le Cuirot
  • CancelProfileIsBroken

    CancelProfileIsBroken August 5th, 2009 @ 02:10 PM

    • Tag changed from activerecord, edge, enhancement, named_scope, patch to activerecord, bugmash, edge, enhancement, named_scope, patch
  • Dan Pickett

    Dan Pickett August 8th, 2009 @ 11:07 PM

    +1 for the feature - I've definitely run into use cases where I've wanted this, but I'm not sure I agree with the implementation

    Also, I could not merge this into 2-3-stable cleanly

    I'm not wowed by the implementation - maybe we need a way to chain them ie. Shirt.red.and.dry_clean_only or Shirt.red_and_dry_clean_only

  • James Le Cuirot

    James Le Cuirot August 9th, 2009 @ 10:29 AM

    I'll try and post a new patch today. I know that this works with 2.3. I'll have a think about your suggestion.

  • James Le Cuirot

    James Le Cuirot August 9th, 2009 @ 03:08 PM

    Here's the patch against 2-3-stable.

  • James Le Cuirot

    James Le Cuirot August 9th, 2009 @ 03:19 PM

    Dan, had a think about your suggestion but it doesn't really make sense. It's already possible to do stuff like Shirt.red.dry_clean_only. What I'm asking for is the ability to merge two already-existing scope instances. In my example, you can't just do "z = x.y" because that would be treating y as a method name, not a local variable name.

  • Elad Meidar

    Elad Meidar August 9th, 2009 @ 04:20 PM

    @james, Not clear what you are after.

    You can chain named_scopes for a while now, but you want AND on the return record set ?

  • John Trupiano

    John Trupiano August 9th, 2009 @ 04:24 PM

    @Elad, I think he's looking for the union of two named scopes. So if we had a named scope to return all green M&M's and a named scope to return all red M&M's, he'd want to be able to quickly get the collection of M&M's that are either red or green (the union of the two sets).

  • James Le Cuirot

    James Le Cuirot August 9th, 2009 @ 04:40 PM

    I'm not sure I'd use the term "union" because conditions in one scope might remove records included by the other scope. I just want to merge two scopes together, like you do with chaining (it would give the same result), but for scopes that have already been assigned to variables. It's difficult to give a useful example.

  • Matt Jones

    Matt Jones August 9th, 2009 @ 06:36 PM

    The name doesn't make a whole lot of sense - I was really hoping you'd written an implementation of scope union (joining with OR) rather than this.

    This is also going (I think) to fail oddly if you try to merge in a previously chained scope:

    x = Shirt.red.cotton
    y = Shirt.dry_clean_only
    
    z = y.merge(x)
    

    Because of that, I'm not sure what the advantage of merge over just chaining the scopes properly in the first place is.

    Note that the most common case for this (search scopes and such) can work like this:

    proxy = Shirt
    [:red, :cotton, :dry_clean_only].each do |s|
      proxy = proxy.send(s) unless params[s].blank?
    end
    @shirts = proxy.all # or paginate - whatever
    

    Maybe you could give a more convincing use case where chaining doesn't work?

  • David Trasbo

    David Trasbo August 9th, 2009 @ 08:03 PM

    verified

    Patch applies cleanly to 2-3-stable. I assume the tests are passing, I'm having some problems getting Ruby 1.9 and 1.8 play nicely. +1 on the idea, it may need more tests, though.

  • Elad Meidar

    Elad Meidar August 9th, 2009 @ 09:27 PM

    I agree with Matt, i don't see where this #merge is going to be any different from chaining scopes.

  • José Valim

    José Valim August 9th, 2009 @ 11:01 PM

    • Tag changed from activerecord, bugmash, edge, enhancement, named_scope, patch to activerecord, edge, enhancement, named_scope, patch
    • State changed from “new” to “wontfix”

    This is a new feature that won't be added to Rails 2.3. For Rails 3.0 (master), Emilio (miloops) is already working on something similar in his GSoC project, since he's adding relations to ActiveRecord.

  • James Le Cuirot

    James Le Cuirot August 9th, 2009 @ 11:09 PM

    That's fair enough. Thanks for the info.

Create your profile

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

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages