#1331 Allow arbitrary named_scope instances to be merged - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1331 ✓wontfix

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

You flagged this item as spam.
James Le Cuirot November 5th, 2008 @ 10:24 AM
Here's the same patch for 2.1 if anyone wants it.
- named_scope_merge_2_1.patch 2.1 KB
You flagged this item as spam.
James Le Cuirot February 26th, 2009 @ 01:18 AM
And again for 2.2.
- named_scope_merge_2_2.patch 2.1 KB
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 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
You flagged this item as spam.
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.
You flagged this item as spam.
James Le Cuirot August 9th, 2009 @ 03:08 PM
Here's the patch against 2-3-stable.
- named_scope_merge.patch 2.1 KB
You flagged this item as spam.
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.
You flagged this item as spam.
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 ?
You flagged this item as spam.
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).
You flagged this item as spam.
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 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 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.
You flagged this item as spam.
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 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.
You flagged this item as spam.
James Le Cuirot August 9th, 2009 @ 11:09 PM
That's fair enough. Thanks for the info.