0

I have Invoices with many Invoice Line Items. Invoice line items point to a specific item. When creating or updating an Invoice, I'd like to validate that there is not more than 1 invoice line item with the same Item (Item ID). I am using accepts nested attributes and nested forms.

I know about validates_uniqueness_of item_id: {scope: invoice_id}

However, I cannot for the life of me get it to work properly. Here is my code:

Invoice Line Item

belongs_to :item

validates_uniqueness_of :item_id, scope: :invoice_id

Invoice

has_many :invoice_line_items, dependent: :destroy
accepts_nested_attributes_for :invoice_line_items, allow_destroy: true

Invoice Controller

  // strong params
  params.require(:invoice).permit(
    :id,
    :description, 
    :company_id, 
    invoice_line_items_attributes: [
      :id,
      :invoice_id,
      :item_id,
      :quantity,
      :_destroy
    ]
  )
  // ...
  // create action
  def create
    @invoice = Invoice.new(invoice_params)

    respond_to do |format|
      if @invoice.save
         
        format.html { redirect_to @invoice }
      else
        format.html { render action: 'new' }
      end
    end
  end

The controller code is pretty standard (what rails scaffold creates).

UPDATE - NOTE that after more diagnosing, I find that on create it always lets me create multiple line items with the same item when first creating an invoice and when editing an invoice without modifying the line items, but NOT when editing an invoice and trying to add another line item with the same item or modifying an attribute of one of the line items. It seems to be something I'm not understanding with how rails handles nested validations.

UPDATE 2 If I add validates_associated :invoice_line_items, it only resolves the problem when editing an already created invoice without modifying attributes. It seems to force validation check regardless of what was modified. It presents an issues when using _destroy, however.

UPDATE 3 Added controller code.

Question - how can I validate an attribute on a models has many records using nested form and accepts nested attributes?

karns
  • 5,391
  • 8
  • 35
  • 57
  • The issue is new records don't have IDs yet. Hence why it's caught on edit but not create. – Chiperific Nov 11 '22 at 21:33
  • Is there a typo in your Invoices Controller? `invoice_line_items_attributes` should have `:id, :item_id, :invoice_id`, not `:invoice_line_item_id` – Chiperific Nov 11 '22 at 21:45
  • Please post your controller code for creating a new invoice. I can assume you are using `.build` but maybe you aren't? You want a `before_create` method that compares the `item_id` of each of the line times for uniqueness. Then have a `before_save` method that compares any new line items' `item_id` to the existing line items. I'm also not clear on your DB relationships. Are items something that exists and an invoice has many items through invoice line items? i.e. like a catalog where the items are set by the seller, and the invoice is a record of what a buyer is buying? – Beartech Nov 13 '22 at 05:11
  • Can you sketch out that data relationship? It's confusing that `invoice_line_item` would belong to `item`. It feels like `invoice_line_item` would be a join table between invoices and items. – Beartech Nov 13 '22 at 05:14
  • @Chiperific I thought of this. I would like to know what best practice is to achieve what I outlined in spite of this fact. Yes, there was a typo, but actually that was an extraneous attribute declaration since rails can infer this since its accepts nested attributes for. – karns Nov 14 '22 at 14:44
  • @Beartech An invoice has many invoice_line_items, invoice line items have one item. It is indeed a join table -> theoretically an invoice has many items, yes. – karns Nov 14 '22 at 14:46
  • @Beartech I added relevant controller code, thanks. – karns Nov 14 '22 at 14:57
  • The code you've posted so far looks correct, so the issue has to be coming from elsewhere. Can you post your full controllers and models? – Chiperific Nov 15 '22 at 23:29
  • Well if the data relationship is actually as you say above, then your invoice model needs `has_many :items, through: :invoice_line_items` and your invoice_line_item model needs `belongs_to :items` AND `belongs_to :invoice` . – Beartech Nov 15 '22 at 23:46

2 Answers2

0

I know this isn't directly answering your qestion, but I would do things a bit differently.

The only reason InvoiceLineItem exists is to associate one Invoice to many Items.

Instead of having a bunch of database records for InvoiceLineItem, I would consider a field (e.g. HSTORE or JSONB) that stores the Items directly to the Invoice:

> @invoice.item_hash
> { #item1: #quantity1, #item2: #quantity2, #item3: #quantity3, ... }

Using the :item_id as a key in the hash will prevent duplicate values by default.

A simple implementation is to use ActiveRecord::Store which involves using a text field and letting Rails handle serialization of the data.

Rails also supports JSON and JSONB and Hstore data types in Postgresql and JSON in MySQL 5.7+

Lookups will be faster as you don't need to traverse through InvoiceLineItem to get between Invoice and Item. And there are lots of great resources about interacting with JSONB columns.

# invoice.rb
...
def items
  Item.find( item_hash.keys)
end

It's a bit less intuitive to get "invoices that reference this item", but still very possible (and fast):

# item.rb
...
# using a Postgres JSON query operator:
# jsonb ? text → boolean (Does the text string exist as a top-level key or array element within the JSON value?)
# https://www.postgresql.org/docs/current/functions-json.html#FUNCTIONS-JSONB-OP-TABLE
def invoices
  Invoice.where('item_hash ? :key', key: id)
end
Chiperific
  • 4,428
  • 3
  • 21
  • 41
  • I appreciate the creativity. Would you have suggestions on what to do without resorting to different database methodologies? What would the caveats to this approach be? – karns Nov 15 '22 at 13:38
  • If you want to use the InvoiceLineItems join table, you're going to have to keep wrestling with the association. My suggestion is to start fresh with more unique names that help you reason through things easier. Invoice has_many Lines; Line belongs_to Invoice; Line belongs_to Item; Item has_many Lines. – Chiperific Nov 15 '22 at 16:13
  • Stick with the guide as close as possible: https://guides.rubyonrails.org/association_basics.html#the-has-many-through-association – Chiperific Nov 15 '22 at 16:14
  • Caveats for a Store field: you have to write methods to handle the associations instead of leaning on Rails to do it automagically. – Chiperific Nov 15 '22 at 16:16
  • It appears you just defined the schema as I already have it. I'm not concerned with my associations/schema, I know it to be what I need and reasonable. I'm also leaning away from JSON in my DB, I feel there is no need. I'd like to know the proper rails way to accomplish what I asked using relational database concepts. Thanks! – karns Nov 15 '22 at 19:44
0

After reading this through a few times I think see it as:

An invoice has many invoice_line_items

An invoice has many items through invoice_line_items

Item_id needs to be unique to every invoice, thus no repeating of items in the line_item list.

Items are a catalogue of things that can show up in multiple invoices. i.e. items are things like widget_one and widget_two, and an invoice can only contain one line with widget one, but many invoices could contain this same item. If this is not true and an item will only ever show up in one invoice, let me know and I will change my code.

So I think your validation should not be in Items, as items know nothing about invoices. You want to make sure your join table has no entries where a given invoice_id has duplicate item_id entries.

item.rb:

has_many :invoice_line_items
has_many :invoices, through: :invoice_line_items

invoice.rb:

has_many :invoice_line_items
has_many :items, through: :invoice_line_items

invoice_line_item.rb:

belongs_to :item
belongs_to :invoice
validates_uniqueness_of :item_id, :scope => :invoice_id

This SHOULD give you what you need. When you create a new invoice and save it, Rails should try to save each of the line items and the join table. When it hits the duplicate item_id it should fail and throw an error.

If you are going to have unique constraints in your code I would alway back it up with a constraint in your database so that if you do something that makes an end run around this code it will still fail. So a migration should be added to do something like:

add_uniq_constraint_to_invoice_line_items.rb:

def change
  add_index :invoice_line_items, [:invoice_id, :item_id], unique: true    
end

This index will prevent creation of a record with those two columns the same.

Beartech
  • 6,173
  • 1
  • 18
  • 41
  • Thank you, I went over all of my code and it appears I have all of these relationships set up already. What could be causing rails to skip/fail validating the uniqueness of the invoice_id and item_id? – karns Nov 23 '22 at 15:25
  • So are you saying you already had code that matches this? Because this is different from what you posted in the question. Without adding more info to your question (code, logs, etc) you probably wont find more help since we are just guessing at this point. – Beartech Nov 23 '22 at 16:32