views:

77

answers:

2

Photographers "have_many" clients.

Clients "have_many" events.

Is there a better way to assign @events here if the user is a photographer?

  def index
    if @current_user.photographer?
      @events = []
      @current_user.clients.each do |client|
        @events << client.events
      end
    else
      @events = @current_user.events
    end
  end

Edit: More code

# user.rb
class User < ActiveRecord::Base

  has_many :client_associations, 
      :foreign_key => 'photographer_id', 
      :class_name => 'Association', 
      :dependent => :destroy
  has_many :clients, :through => :client_associations

  has_one :photographer_association, 
    :foreign_key => 'client_id', 
    :class_name => 'Association', 
    :dependent => :destroy
  has_one :photographer, :through => :photographer_association

  has_many :events

  def photographer?
    self.role == 'photographer'
  end

end

# association.rb
class Association < ActiveRecord::Base
  belongs_to :client, :class_name => "User"
  belongs_to :photographer, :class_name => "User"
end

# event.rb
class Event < ActiveRecord::Base
  belongs_to :user
  has_many :images      
end

As you can see my users are all in one model with a field called "role".

+2  A: 

From the db point of view, you should load all events at once and not have the N+1 problem.

  def index
    if @current_user.photographer?
      @events = @current_user.clients.find(:all, :include => :events).map(&:events).flatten
    else
      @events = @current_user.events
    end
  end
amikazmi
Sarah Vessels
A: 

That kind of logic, IMHO, would be more properly setup on the model layer.

You could create a new model method, like current_events on the User model, and move your logic there:

def current_events
    if self.photographer?
         self.clients.find(:all, :include => :events).map(&:events).flatten
    else
         self.events
    end
end

Then, on your controller you could just add

def index
  @events = @current_user.current_events
end

Thus, your logic is encapsulated on your model (and can later me improved, added with mor e complexity, tested) and your controller doesn't needs to know (and care) about what it is, just calls and show the current_events of the user.

Yaraher
as noted from his code: "@events = @current_user.events" seems like User model has events already, and it's not going through the clients.
amikazmi
I'm not too sure of that without some code beforehand. It seems the "events" method he's calling is for the Client model. Not sure if it descends from User.
Yaraher
lemme add some more code ...
rpflo
Thanks for the tips, this actually helped me with something else. clients and photographers are both users, but there is an association model that draws the relationship between them.
rpflo