Merging "order" clauses from default_scope and regular scope

Hi,

I'd like to help by writing some failing tests, but I don't really know what I have to test.
If I knew what is the expected behavior, it'd be easier.

I've (quickly) scanned the test suite of ActiveRecord and I've seen that there is indeed a "reorder" method to override a previous order clause, so the 2n part of my issue is irrelevant.

For the 1st part, I need to know if the default_scope is expected to have a higher priority when merging the 2 order clauses, or the "named" scope has the highest priority. This way I could write a test that verifies that the 2 syntaxes are right

I'll read the test suite with more attention to see if I can figure this out by myself, but if someone has a straight answer, that'll save me some time.

Thanks.

I've found where I have to make the tests.

Basically, I've duplicated the DeveloperOrderedBySalary model and changed the syntax for the order clauses (in the default_scope and the named scope). Then I duplicated the test named "test_named_scope_overwrites_default" in relation_scoping_test.rb.

I'll send you a patch for those changes as soon as possible.

Here is my patch. I contains only a failing case. I'll try later to make it pass, but I'm not sure I'll have the time in the next few days.

Neeraj, if we change the order "order" arguments are added, couldn't we solve this issue?

http://github.com/rails/rails/blob/master/activerecord/lib/active_r...

I may be way out of my league here, but isn't it just a matter of when the default_scope's "order" is applied ?

As I mentioned default_scope issue can easily be fixed by changing the order in apply_finder_options. But that will break with_scope and that's the issue.

Jérémy can you take a look with the latest 3-0-stable code?

The point 1) you mentioned should be fixed by Neeraj now.
On the point 2 now default_scope is always concatenated to the order clauses and the other clauses too. If you want to "unscope" the order you have the following ways to do that.
unscoped method which unscope all the clauses and only and except methods which can unscope partially applying unscope over some clauses only.

Please Jérémy, Neeraj confirm this so we can close the issue.

The 1st point is OK for me with the latest 3-0-stable branch.

But the 2nd point is not entirely OK.

If I want to reset the order, I can use "reorder" if I specify the new order clause, but I can't simply ask to reset the order and then give another scope.
A similar situation is when I unscope a model, then add a scope, the original default_scope is here again.

For example, with this simple class :

class Article < ActiveRecord::Base
  default_scope :order => 'title'
  scope :by_position, :order => 'position desc'
end

In the Rails console, I get these results :

$ Article.scoped.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  title" 
$ Article.unscoped.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"" 
$ Article.unscoped.by_position.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  title, position desc" 
$ Article.reorder("created_at desc").to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  created_at desc" 
$ Article.reorder.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  title" 
$ Article.unscoped.reorder.by_position.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  title, position desc"

• Line 1 is expected, I get the default_scope
• Line 2 is expected, I get the unscoped model
• Line 3 is not expected the default_scope shouldn't be here
• Line 4 is expected, the "manual" reorder method reset the ordering
• Line 5 is not expected, the order clause from the default_scope is not reset (or it is reapplied)
• Line 6 is not expected but it makes sense regarding the result of the line 5

This is tricky because any scope is built on top of the default scope.
So when you apply a scope, the default scope cones together. You can
easily notice it working by moving the default scope declaration after
the scope one.

Not sure if there is an easy fix, but this surely should be documented.

On Sep 7, 2010, at 16:38, "Lighthouse" <no-reply@lighthouseapp.com>
wrote:

This is a bug. There is no easy way to put it.

 Article.unscoped.by_position.to_sql
 => "SELECT     \"articles\".* FROM       \"articles\"  ORDER BY  title, position desc"

This issue is already in LH as ticket #5386 AR::Base#unscoped inconsistency.

Jeremy: I have already updated the doc suggesting people to use block form of unscoped. Above code could be fixed by writing

 Article.unscoped { User.by_position.to_sql }

Neeraj: OK, I understand the block syntax.

In the current version of the file (http://github.com/rails/rails/blob/master/activerecord/lib/active_r...) the documentation suggest to do this

Post.unscoped {
  limit(10) # Fires "SELECT * FROM posts LIMIT 10"
}

But I get a "NoMethodError: undefined method limit' for #<Object:0x1001c8288>".<br/> If I use a scope instead of a limit method, I get a "NameError: undefined local variable or methodby_position' for #<Object:0x1001c8288>" (and I've obviously verified that the scope 'by_position' exists and is working).

If I change the call for this, it is working :

Post.unscoped {
  Post.limit(10)
}

It's also working if I use a scope.

It might just be a matter of documentation though, not a Rails bug.

Sorry for the bad formatting, I didn't use the preview to make sure it was OK.
It seems I can't edit my own comments, strange.

@Jeremy Feel free to update the doc at http://github.com/lifo/docrails . Anyone can update it. Sorry about that omission.

The example in the documentation works if you are inside the model
class level. Still it would be nice to change the docs!

On Sep 7, 2010, at 18:00, "Lighthouse" <no-reply@lighthouseapp.com>
wrote:

I have a feeling that a lot of people would run into this issue. Wish I could make unscoped code only support block usage. :-)