Is it good practices to copy value from parent to child using after_save callback?

Schema.rb

  create_table "bills", force: :cascade do |t|
    t.bigint "freight_id"
    t.decimal "vat", default: "0.0", null: false
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
  end
  create_table "bill_entries", force: :cascade do |t|
    t.bigint "bill_id", null: false
    t.decimal "quantity"
    t.decimal "price", default: "0.0", null: false
    t.decimal "amount", null: false
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["bill_id"], name: "index_bill_entries_on_bill_id"
  end

Parent class

class Bill < ApplicationRecord
  has_many :bill_entries, autosave: true
  before_validation :calculate_bill_entries_total
  
  def calculcate_bill_entries_total
    bill_entries.each do |bill_entry|
      bill_entry.amount = bill_entry.price * bill_entry.quantity + ( bill_entry.price * bill_entry.quantity * self.vat / 100)
    end
  end
end

Child

class BillEntry < ApplicationRecord
  belongs_to :bill
end

the problem with this code snippet is sometimes the bill_entry amount is not updated even though the vat from parents is changed

I would make amount a method and not an attribute

Nothing stands out as wrong with this code so there must be something else at play here. Is there a particular scenario where it doesn’t update? Can you reproduce the problem? If so then you can trace through it with the debugger. Also is there any sort of transaction at play where there might be a partial rollback?

You mentioned in your subject that you are using after_save even though the code uses before_validation. I think either would work or also before_save. I would probably choose either before_save or after_save just so if validation of the parent fails you didn’t spend the time loading the child objects.

The docs do mention that it’s not a good idea to modify the association before save but I haven’t used the autosave module before. I think you’d either want to move that calculation into the BillEntry model or turn off autosave and actually update the BillEntry records with your save.

You should avoid modifying the association content before autosave callbacks are executed. Placing your callbacks after associations is usually a good practice.

Or you could use update!(amount: ...) instead of amount =