CODEWITHSUNDEEP
ruby-on-railsactiverecord
Chazu
Chazu

Reputation: 1018

ActiveRecord query returns an incorrect model

I have been scratching my head over this one for a little while, and though I'm sure its a stupid mistake, I've reached the point where I must consult SO if I am to preserve the hair follicles I have left.

I've written a function in Rails (3.1.2) which should return an array populated with ActiveRecord model objects (users, in this case) which meet a certain criterion. The criterion is that the user's current list (denoted by the field active_list_id) must not be nil. The code follows:

def build_list_array
   @lists = Array.new
   User.all.each do |user|
     @active_list_id = user.active_list_id
     @lists<< List.find(@active_list_id) if @active_list_id != nil #TODO WHAT?!? WHY IS  THIS RETURNING USERS?
   end
end

As you can see, I'm initializing an empty array, cycling through all users and adding their active list to the array if the relevant reference on the user record is not nil. The problem is, this is returning user objects, not list objects.

Here are the associations from the user and list models:

user model:
  has_many :lists
  has_many :tasks

list model:
  belongs_to :user

A brief word about the reference to active_list: A user can have many lists, but only one is active at any time. Therefore, I need to reference that list on the user record. The active list is not a foreign key in the typical sense, then.

I appreciate any help you can give me...Thanks =)

Upvotes: 0

Views: 135

Answers (4)

Bradley Priest
Bradley Priest

Reputation: 7458

Extending Steven's answer, to get the Lists

class User
  belongs_to :active_list, :class_name => "List"

def build_list_array
  @lists = User.where('active_list_id is not null').map(&:active_list).compact

Upvotes: 1

BaronVonBraun
BaronVonBraun

Reputation: 4293

As it stands, your build_list_array will return an array of User because of the behavior of each. When iterating over a collection using each, the call to each returns the original collection.

For example,

list = []
# returns => []
[1,2,3,4,5].each { |number| list << number * 10 }
# returns => [1, 2, 3, 4, 5]
list
# returns => [10, 20, 30, 40, 50]

In your code, the last statement in your build_list_array method is the each call, meaning the return value of each is what is returned by the method. If you simply added a return statement at the end of the method you would be good to go.

def build_list_array
  @lists = Array.new
  User.all.each do |user|
    @active_list_id = user.active_list_id
    @lists<< List.find(@active_list_id) if @active_list_id
  end
  return @lists  # Actually return @lists
end

That being said, you should probably use something like Bradley's answer as a basis for more "correct" Rails code.

Upvotes: 2

Frederick Cheung
Frederick Cheung

Reputation: 84114

each always returns the collection it iterates on (no matter what happens inside the block). Sounds like you want to return @lists at the end of your method.

You seem to be making a curious use of instance variables. You could also fetch this in one query via a join, something along the lines of

List.joins('inner join users on active_list_id =lists.id')

Upvotes: 2

Steven Garcia
Steven Garcia

Reputation: 2834

Activerecord's Arel is your friend here:

User.where(:active_list_id.not_eq => nil)

Upvotes: 1

Related Questions