I feel like this needs to be refactored - any help? Ruby modeling

Question

So let's say you have

line_items

and line_items belong to a make and a model

a make has many models and line items

a model belongs to a make

For the bare example idea LineItem.new(:make => "Apple", :model => "Mac Book Pro")

When creating a LinteItem you want a text_field box for a make and a model. Makes and models shouldn't exist more than once.

So I used the following implementation:

before_save :find_or_create_make, :if => Proc.new {|line_item| line_item.make_title.present? }
before_save :find_or_create_model

def find_or_create_make
  make = Make.find_or_create_by_title(self.make_title)
  self.make = make
end

def find_or_create_model
  model = Model.find_or_create_by_title(self.model_title) {|u| u.make = self.make}
  self.model = model
end

However using this method means I have to run custom validations instead of a #validates_presence_of :make due to the associations happening off a virtual attribute

validate :require_make_or_make_title, :require_model_or_model_title

def require_make_or_make_title
  errors.add_to_base("Must enter a make") unless (self.make || self.make_title)
end

def require_model_or_model_title
  errors.add_to_base("Must enter a model") unless (self.model || self.model_title)
end

Meh, this is starting to suck. Now where it really sucks is editing with forms. Considering my form fields are a partial, my edit is rendering the same form as new. This means that :make_title and :model_title are blank on the form.

I'm not really sure what the best way to rectify the immediately above problem is, which was the final turning point on me thinking this needs to be refactored entirely.

If anyone can provide any feedback that would be great.

Thanks!

Answer 1

I don't think line_items should belong to a make, they should only belong to a model. And a model should have many line items. A make could have many line items through a model. You are missing a couple of methods to have your fields appear.

class LineItem
  belongs_to :model
  after_save :connect_model_and_make
  def model_title
     self.model.title
  end
  def model_title=(value)
     self.model = Model.find_or_create_by_title(value)
  end
  def make_title
     self.model.make.title
  end
  def make_title=(value)
     @make = Make.find_or_create_by_title(value)  
  end
  def connect_model_and_make
     self.model.make = @make
  end
end
class Model
  has_many :line_items
  belongs_to :make
end
class Make
  has_many :models
  has_many :line_items, :through => :models
end

It's really not that bad, there's just not super easy way to do it. I hope you put an autocomplete on those text fields at some point.