Project

General

Profile

Bug #12011

Replace .includes(table).where(table) by .eager_load

Added by Daniel Lobato Garcia almost 4 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Category:
Rails
Target version:
Difficulty:
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

On Rails 3, .includes(:users).where("users.name = 'daniel'") works because Rails would be smart enough to figure out the where query uses the includes table.
In this case it'd internally resolve it to eager_load to use the table users in the query. However on Rails 4 they decided they didn't want to maintain a parser for
SQL, so you have to append .references(:table) after all .includes(:table).where("SQL that uses table"). This is not retrocompatible, but we can directly use
eager_load on Rails 3 and 4 to avoid using .references.


Related issues

Related to Foreman - Tracker #3157: Rails 4.1 upgrade tasksClosed2013-09-27

Associated revisions

Revision 7fdb705b (diff)
Added by Daniel Lobato Garcia almost 4 years ago

Fixes #12011 - Replace .includes(table).where(table) by .eager_load

On Rails 3, .includes(:users).where("users.name = 'daniel'") works because
Rails would be smart enough to figure out the where query uses the includes
table. In this case it'd internally resolve it to eager_load to use the table
users in the query.

However on Rails 4 they decided they didn't want to maintain a parser for SQL,
so we have to append .references(:table) after every
.includes(:table).where("SQL that uses table").
This is not retrocompatible, but we can directly use eager_load on Rails 3 and
4 to avoid using .references.

History

#1 Updated by Daniel Lobato Garcia almost 4 years ago

  • Related to Bug #3517: Help text for 'domain create' does not line up correctly added

#2 Updated by Daniel Lobato Garcia almost 4 years ago

#3 Updated by Daniel Lobato Garcia almost 4 years ago

  • Related to deleted (Bug #3517: Help text for 'domain create' does not line up correctly)

#4 Updated by The Foreman Bot almost 4 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2768 added
  • Pull request deleted ()

#5 Updated by Dominic Cleal almost 4 years ago

  • Legacy Backlogs Release (now unused) set to 71

#6 Updated by Daniel Lobato Garcia almost 4 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF