1

I am writing a simple accounting system for managing costs. The structure is like this:

Invoice  - can have many products
  Product - can have many costs, also can act_as_tree 
      LineItem  - 
    Product
        LineItem
        LineItem
      Product
      LineItem
  Product
      LineItem
  LineItem

I had this set up as a has_many and belongs_to for the three classes but think that the following is more appropriate (based upon reading Rails 3 Way - any shortcomings are my lack of understanding; just trying to give context)

Class Invoice < ActiveRecord::Base
    has_many :products
    has_many :line_items, :through => :products
end

 Class Product < ActiveRecord::Base
    belongs_to :invoice
    belongs_to :line_item
 end

class LineItem < ActiveRecord::Base
    has_many :products
    has_many :invoices, :through => :invoices
end

But I don't this is working correctly.

if I do the following:

>@i=Invoice.find(1)
>@i.products # this works
>@i.line_items # doesn't work, unidentified method line_items

This is the first time I'm using has_many :through. Is this set up correctly for my data model? Also, is it possible to use it in conjunction with acts_as_tree - I'd like to be able to say:

>@i.line_items 

and get back all the line items for that specific invoice. Possible?

thx for help

timpone
  • 19,235
  • 36
  • 121
  • 211

3 Answers3

1

First a question: What is your relation between Product and LineItem: Has 1 product many line items or is 1 and the same line item referenced in many products? The rest of this answer is based on the assumption that every product should have multiple line items.

I think your models should be defined like that:

# No change
Class Invoice < ActiveRecord::Base
  has_many :products
  has_many :line_items, :through => :products
end

# Changed the relation to :line_items
Class Product < ActiveRecord::Base
  belongs_to :invoice
  has_many :line_items
end

class LineItem < ActiveRecord::Base
  belongs_to :products
  # The following then does not make sense
  ##has_many :invoices, :through => :invoices
end
mliebelt
  • 15,345
  • 7
  • 55
  • 92
  • Your right, it is the first one - should have added that in original question. But ... this seems to work! I'll have to play around with the acts_as_tree some to make sure it's working well. How would I load everything in 1 swoop like including all associcated items? >@i=Invoice.find(1, :include => [:products, :line_items]) seems to do it – timpone Sep 23 '11 at 05:54
  • Don't know that, sounds like another question :-). What is your real question: reading with each invoice the products and line items? Or reading the whole tree of products? – mliebelt Sep 23 '11 at 05:59
  • Agreed prob another question. thx for answer. Would like to do things like query all line_items for each Invoice, there is also associated locations etc... thx for answer – timpone Sep 23 '11 at 06:07
1

why did you choose this structure? in such cases i usually do

Class Invoice < ActiveRecord::Base
  has_many :line_items
  has_many :products, :through => :line_items
end

Class Product < ActiveRecord::Base
  has_many :line_items
  # if you need
  has_many :products, :through => :line_items
end

class LineItem < ActiveRecord::Base
  belongs_to :products
  belongs_to :invoice
end

this fullfills following requirements:

Invoice and Product have a Many2Many relationship The relationship between an Invoice and a Product is a LineItem, which provides further information like price, amount, applied taxes, etc. You can create a LineItem by adding a Product:

invoice = Invoice.create
invoice.products << Product.find_by_name('awesome')
Marian Theisen
  • 6,100
  • 29
  • 39
  • its a little bit of a unique situation. every product is unique (imagine a used car lot) so we aren't dealing with 100 products of exactly the same type. Always one product of one unique type. However, there are some common elements that might be changing. And, also are using acts_as_tree for handling products that are essentially the sum of prodcuts. ugh... a little ugly. Let me think a little bit more about whether should change the modeling though. thx – timpone Sep 23 '11 at 20:22
0

Your Invoice should only have line items! These can be a tree structure but you should not have products directly referenced from your invoice (what if a product price changes: this would affect existing invoices!)

So, to fix your structure:

invoice
  line_item   # references a product, but has its own fields (price!)
  line_item
    line_item

Each line_item should reference one product using belong_to. It's your join model between invoices and products:

class Invoice
  has_many :line_items
end

class LineItem
  belongs_to :invoice
  belongs_to :product
  acts_as_tree # (implies has_many :line_items with the parent invoice_id, etc.)
end

class Product
  has_many :line_items
end

This is basically all you need to build an invoice. You can think of the tree structure as being independent from the Invoice:LineItem:Product relationship. It can work as a flat list, or enhanced to become a tree.

If your products normally contain other products and you need to know which children to add to the invoice when the parent product is added (as line items), you'll need to tree your products too:

class Product
  has_many :line_items
  acts_as_tree
end

This tree structure is independent of the tree structure in line items. Remember: products can change, and this shouldn't affect existing line items in your invoices.

Andrew Vit
  • 18,961
  • 6
  • 77
  • 84
  • it's a little bit of a unique domain (and I changed the names to try them make sense more). each product is a unique instance (like used equipment where costs on the item are being managed). I have also changed names and probably didn't use the best choice. thx – timpone Sep 26 '11 at 17:08
  • Ok. I hope I made a point though... the important thing is to keep a separation between the product itself (catalogue item) and how it appeared on the invoice when sold (sales item). – Andrew Vit Sep 26 '11 at 18:06
  • yes, thx Andrew. it's a little bit of a screwy app and I probably made it more screwy by using common accounting terms incorrectly (prob should be 'commission reconciliation' rather than invoice. My apologies for poor writing on my end and I appreciate your attention to detail. – timpone Sep 27 '11 at 18:30