CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-4
sscirrus
sscirrus

Reputation: 56759

The Rails way to validate a web of classes

My app has many interrelationships like:

# Company
has_many :programs
has_many :projects
has_many :users

# Project
has_many :users
has_many :programs
belongs_to :company

# User
belongs_to :project
has_many :programs
belongs_to :company

# Program
belongs_to :project
belongs_to :user
belongs_to :company

Every program must belong to a project and user, BOTH OF WHICH belong to current_user.company.

Approach 1 - controller upon create/update

@program = Program.new(program_params)
@program.company = current_user.company
@allowed_projects = current_user.company.projects
unless @allowed_projects.include? @program.project
  raise Exception
end

Approach 2 - model-based validation

before_save :ensure_all_allowed
def ensure_all_allowed
  current_user = ???
  self.company_id = current_user.company_id
  # Then a similar validation to above for self.project_id
end

I feel these are both awkward and not 'the Rails way'.

I assume Approach 2 is the better method because it'll save all this awkward controller code and hold better to the MVC standard.

How can I validate these items correctly?

Upvotes: 0

Views: 31

Answers (2)

max
max

Reputation: 102250

Since Program has a belongs to relation to both user a and project you can setup some simple validations without worrying about the current_user. This is desirable from a MVC standpoint models should not be aware of the session or the request.

class Program < ActiveRecord::Base
  # ...

  validates_presence_of :user, :company, :project
  # the unless conditions are there avoid the program blowing
  # up with nil errors - but the presence validation above covers 
  # those scenarios
  validate :user_must_belong_to_company, 
     unless: -> { company.nil? || user.nil? }

  validate :project_must_belong_to_company, 
     unless: -> { company.nil? || project.nil? }

  def user_must_belong_to_company
    unless self.company == self.user.company
      errors.add(:user, "must belong to same company as user.")
    end
  end

  def project_must_belong_to_company
    unless self.company == self.project.company
      errors.add(:company, "must belong to same company as project.")
    end
  end
end

But I'm thinking that this is just a symtom of some bad relation design choices.

What you probably need is a series of many to many relations - it does not seem very realistic that a project can only have one user or a program either for that part.

class Company
  has_many :users
  has_many :projects
  has_many :assignments, through :projects
  has_many :programs, through :projects
end

class User
  belongs_to :company 
  has_many :projects, through: :assignments
end

class Project
  has_many :assignments, class_name: 'ProjectAssignment'
  has_many :users, through: :assignments
  belongs_to :company  
end

# you can really call this whatever floats you boat
class ProjectAssignment
  belongs_to :user
  belongs_to :project
end

class Program
  belongs_to :project
  has_one :company, through: :project
  has_many :assignments, class_name: 'ProgramAssignment'
  has_many :users, through: :assignments
end

# you can really call this whatever floats you boat
class ProgramAssignment
  belongs_to :user
  belongs_to :program
end

That would automatically eliminate the problem with the company since it gets it through a parent relation.

The second problem that a user should not be able to create programs in a project he / she is not a member of sounds like something which should instead be handled on the authorization level - not in a validation.

Pundit example:

class ProgramPolicy < ApplicationPolicy

  # ...
  def create?
    record.project.users.include?(user)
  end
end

CanCanCan example:

class Ability
  include CanCan::Ability

  def initialize(user)
    user ||= User.new # guest user (not logged in)

    can :create, Program do |p|
      p.project.users.include?(user)
    end
  end
end

Upvotes: 1

SteveTurczyn
SteveTurczyn

Reputation: 36860

It's actually somewhat problematic to access current user in a model. It's not impossible, but it requires an around_action that will load the current user in the model class in a thread safe way.

Better would be to assign the current user in the controller

@program.user = current_user
@program.company = @program.user.company

Then do the validation in the model

validate :project_must_be_allowed

def project_must_be_allowed
  unless company.projects include project
    errors.add(:project, "Project is not valid for company.")
  end
end

However, it would be a more normalized setup if you did through relationships

class Company
  has_many :users
  has_many :projects, through: :users

That way your 'projects' table doesn't need a company_id

You could still do the validation as I described but you'd have to add one method to the model...

def company
  user.company
end

or more simply...

delegate :company, to: :user

Upvotes: 2

Related Questions