Is there a better way to get this data?

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 in the same model with a field called "role".

+2


source to share


2 answers


From a db point of view, you should load all events at once and not have an 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

      

+2


source


Such logic, IMHO, will be more correctly configured at the model level.

You can create a new model method like current_events in 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 can simply add

def index
  @events = @current_user.current_events
end

      

This way your logic is encapsulated on your model (and may improve later on me, add with more complexity, check), and your controller doesn't have to know (and care) what it is, just calls and shows the user's current_events.

0


source







All Articles