named_scope and nested order clauses #603

Closed
lighthouse-import opened this Issue May 16, 2011 · 9 comments

Comments

Projects
None yet
2 participants

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/2253
Created by Erik Andrejko - 2011-01-17 08:17:54 UTC

named_scope currently has some unexpected behavior when nesting :order clauses. This problem also occurs in the underlying with_scope implementation.

This test does not pass:

def test_scoping_with_multiple_order
  expected = Monk.find(:all, :order => "birth_year, first_name").map{|m| m.full_name}
  options = {:order => "birth_year"}
  Monk.with_scope(:find => options) do
    assert_equal expected, Monk.find(:all, :order => "first_name").map{|m| m.full_name}
  end
end

This test passes, and has the expected behavior:

def test_single_named_scope_overrides_default_scope_order
  expected = Monk.find(:all, :order => "last_name asc").collect {|m| m.id}
  received = Monk.by_last_name.collect {|m| m.id}
  assert_equal expected, received
end

This test does not pass:

def test_nested_scopes_orders_combined
  expected = Monk.find(:all, :order => "last_name asc, first_name asc").collect {|m| m.last_name + ", " + m.first_name}
  received = Monk.by_last_name.by_first_name.collect {|m| m.last_name + ", " + m.first_name}
  assert_equal expected, received
end

I have included a patch containing these tests and the Monk model.

For more discussion see this conversation on Rails Core.

Imported from Lighthouse.
Comment by Peter Wagenet - 2009-03-19 14:05:24 UTC

This may be fixed in my patch here: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1812-default_scope-cant-take-procs

Imported from Lighthouse.
Comment by Maurício Linhares - 2009-05-28 21:20:16 UTC

On Rails 2.3.2 this isn't fixed yet.

+1

Would be really nice to have this fixed.

                if key == :conditions && merge
                  if params[key].is_a?(Hash) && hash[method][key].is_a?(Hash)
                    hash[method][key] = merge_conditions(hash[method][key].deep_merge(params[key]))
                  else
                    hash[method][key] = merge_conditions(params[key], hash[method][key])
                  end
                elsif key == :include && merge
                  hash[method][key] = merge_includes(hash[method][key], params[key]).uniq
                elsif key == :joins && merge
                  hash[method][key] = merge_joins(params[key], hash[method][key])
                elsif key == :order && merge
                  hash[method][key] = [params[key], hash[method][key]].join(' , ')
                else
                  hash[method][key] = hash[method][key] || params[key]
                end

Imported from Lighthouse.
Comment by Erik Andrejko - 2009-06-06 23:23:51 UTC

A patch that includes tests for the desired behavior is attached.

Imported from Lighthouse.
Comment by Jon - 2009-06-25 20:50:38 UTC

Still having this problem as you can see in this gist: http://gist.github.com/136140

Probably related to this ticket: #2346

Imported from Lighthouse.
Comment by Pratik - 2009-07-03 11:49:56 UTC

Hey,

Can we have a patch without new tables please ?

Thanks!

Imported from Lighthouse.
Comment by Emilio Tagua - 2009-07-07 20:43:09 UTC

You may want to take a look to this ticket/patch, i think that solves this problem:

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2810-with_scope-should-accept-and-use-order-option

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-05-04 17:48:42 UTC

[bulk edit]

Attachments saved to Gist: http://gist.github.com/971620

Contributor

josevalim commented May 16, 2011

This has been fixed on master.

@josevalim josevalim closed this May 16, 2011

hisas pushed a commit to hisas/rails that referenced this issue May 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment