Order clause gets dropped on has_one through #10016

Closed
ginter opened this Issue Mar 31, 2013 · 5 comments

Comments

Projects
None yet
4 participants
@ginter

ginter commented Mar 31, 2013

In the following situation, I would expect the order clause of the has_one relation to carry over to the has_one through, but it has to be explicitly stated again for it to be included in the generated SQL.

class Suite
  has_one :tenancy, order: 'tenancies.started_at DESC'
  has_one :tenant, through: :tenancy, order: 'tenancies.started_at DESC'
end

PS I realize I'm kind of using a hack with the has_one here.

@neerajdotname

This comment has been minimized.

Show comment Hide comment
@neerajdotname

neerajdotname Mar 31, 2013

Member

Can you post a gist using the template mentioned at https://gist.github.com/neerajdotname/5187216 ?

Member

neerajdotname commented Mar 31, 2013

Can you post a gist using the template mentioned at https://gist.github.com/neerajdotname/5187216 ?

@ginter

This comment has been minimized.

Show comment Hide comment
@ginter

ginter Mar 31, 2013

Same thing happens with a has_many so just used that since I know how to get the generated sql.

gem 'activerecord', '3.2.11'
require 'active_record'
require "minitest/autorun"
require 'minitest/pride'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :suites, force: true do |t|
  end
  create_table :tenancies, force: true do |t|
    t.integer :suite_id
    t.integer :tenant_id
    t.datetime :started_at
  end
  create_table :tenants, force: true do |t|
  end
end

class Suite < ActiveRecord::Base
  has_many :tenancies, order: 'tenancies.started_at DESC'
  has_many :tenants, through: :tenancies
end

class Tenancy < ActiveRecord::Base
  belongs_to :suite
  belongs_to :tenant
end

class Tenant < ActiveRecord::Base
end

class HasManyBugTest < MiniTest::Unit::TestCase
  def setup
    @suite = Suite.new
  end

  def test_order
    assert @suite.tenancies.to_sql.include? "ORDER BY tenancies.started_at DESC"
  end

  def test_order_on_through_relation
    assert @suite.tenants.to_sql.include? "ORDER BY tenancies.started_at DESC"
  end
end

ginter commented Mar 31, 2013

Same thing happens with a has_many so just used that since I know how to get the generated sql.

gem 'activerecord', '3.2.11'
require 'active_record'
require "minitest/autorun"
require 'minitest/pride'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :suites, force: true do |t|
  end
  create_table :tenancies, force: true do |t|
    t.integer :suite_id
    t.integer :tenant_id
    t.datetime :started_at
  end
  create_table :tenants, force: true do |t|
  end
end

class Suite < ActiveRecord::Base
  has_many :tenancies, order: 'tenancies.started_at DESC'
  has_many :tenants, through: :tenancies
end

class Tenancy < ActiveRecord::Base
  belongs_to :suite
  belongs_to :tenant
end

class Tenant < ActiveRecord::Base
end

class HasManyBugTest < MiniTest::Unit::TestCase
  def setup
    @suite = Suite.new
  end

  def test_order
    assert @suite.tenancies.to_sql.include? "ORDER BY tenancies.started_at DESC"
  end

  def test_order_on_through_relation
    assert @suite.tenants.to_sql.include? "ORDER BY tenancies.started_at DESC"
  end
end
@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Apr 2, 2013

Member

I don't yet have a bigger picture about the ordering and through relations. We need to also consider the preloader when solving such issues. Also I'm not sure if the order should affect the through relation at all.

@jonleighton thoughts?

Member

senny commented Apr 2, 2013

I don't yet have a bigger picture about the ordering and through relations. We need to also consider the preloader when solving such issues. Also I'm not sure if the order should affect the through relation at all.

@jonleighton thoughts?

@jwinter

This comment has been minimized.

Show comment Hide comment
@jwinter

jwinter Apr 2, 2013

This looks like a dupe of #2083 and #1660. It doesn't look like :order ever worked this way.

jwinter commented Apr 2, 2013

This looks like a dupe of #2083 and #1660. It doesn't look like :order ever worked this way.

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Apr 2, 2013

Member

yea very similar issues have been reported before.

Member

senny commented Apr 2, 2013

yea very similar issues have been reported before.

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