.
Moving the relation from one class to another feels a little off, but I’m a fan of query objects for this sort of use case as Bryan discusses in his ‘fat models’ post:
July 20, 2013 at 10:13 pm
A quick note on security with this refactoring: Without chaining through user, the query for projects will not automatically be constrained to the user, leaving this work to be done by the developer. The approach taken here is to pass current_user.id to the Project#active_projects_for method. If current_user#id is nil, this could mistakenly return all projects that are not assigned to any users. This is different from the user constrained approach where the most data would could ever mistakenly return is all of a user’s projects. Its nothing that can’t be explicitly handled, but it is something that AR associations give you for free.
July 22, 2013 at 8:35 am
@Ben,
Good point. That’s definitely a trade-off of this design. Maybe Project.active_projects_for could raise something in that exceptional situation.
July 22, 2013 at 6:51 pm
You don’t need the user_id argument. Just chain a class method on the associated class to the association:
class Project < ActiveRecord::Base
def self.active
where(active: true)
end
end
class ProjectsController < ActionController::Base
def index
@projects = current_user.projects.active
end
end
July 25, 2013 at 3:53 am
class Project < ActiveRecord::Base
…
scope :active, where(active: true)
end
is also chainable, e.g.
current_user.projects.active
and puts the "active" SQL logic in the right place
July 28, 2013 at 4:08 pm