10

I have three associated models like these:

class Product < ActiveRecord::Base
  belongs_to :user
  has_many :descriptions, {
    dependent: :destroy,
    before_add: [:add_user_id_to_description, :validate_description]
  }
  has_many :documents, through: :descriptions

  # ...

  def validate_description(d)
    unless d.valid?
      d.errors[:user_id].each do |err|
        self.errors.add(:base, "Doc error: #{err}")
      end
    end
  end
end

class Document < ActiveRecord::Base
  belongs_to :user
  has_many :descriptions, {
    dependent: :destroy,
    before_add: [:add_user_id_to_description, :validate_description]
  }
  has_many :products, through: :descriptions
end

class Description < ActiveRecord::Base
  belongs_to :user
  belongs_to :product
  belongs_to :document
end

When I do something like:

doc = user.documents.build
doc.update_attributes(:product_ids => [1,2])

And the description validation fails, then I get false and the appropriate errors on doc. This is exactly what I want.

However, if doc already exists, e.g.:

doc = user.documents.first
doc.update_attributes(:product_ids => [1,2])

And the description validation fails, then I get an ActiveRecord::RecordInvalid error.

I know exactly why this happens--the insert_record method from has_many_through_association.rb calls save! internally, which propagates the error. It exits early, skipping this call, for new records.

Is there some way I can set up my models to prevent this save!? Or am I forced to rescue from the error?

EDIT

I've tried the setup described by Carlos Drew below; I've also tried setting validates_associated :descriptions, and adding inverse_of: :whatever to the has_many :descriptions options hash. I also tried setting a before_validation callback on the Product and Document models, but apparently association callbacks get run first (?). Each attempt seemed to produce the exact same error message.

I'm pasting my error trace from the console below.

Document Load (1.8ms)  SELECT "documents".* FROM "documents" WHERE "documents"."user_id" = 19 ORDER BY "documents"."id" DESC LIMIT 1
   (1.0ms)  BEGIN
  Product Load (41.7ms)  SELECT "products".* FROM "products" WHERE "products"."id" = $1 LIMIT 1  [["id", 3640]]
  Product Load (4.1ms)  SELECT "products".* FROM "products" INNER JOIN "descriptions" ON "products"."id" = "descriptions"."product_id" WHERE "descriptions"."document_id" = 3552
  User Load (7.0ms)  SELECT "users".* FROM "users" WHERE "users"."id" = 19 LIMIT 1
  Account Load (2.0ms)  SELECT "accounts".* FROM "accounts" WHERE "accounts"."user_id" = 19 LIMIT 1
   (0.9ms)  SELECT COUNT(*) FROM "descriptions" WHERE "descriptions"."user_id" = 19
   (1.2ms)  ROLLBACK
ActiveRecord::RecordInvalid: Validation failed: User You have reached limit of 1
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/validations.rb:56:in `save!'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/attribute_methods/dirty.rb:33:in `save!'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:264:in `block in save!'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:313:in `block in with_transaction_returning_status'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:192:in `transaction'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:208:in `transaction'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:311:in `with_transaction_returning_status'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:264:in `save!'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/has_many_through_association.rb:85:in `save_through_record'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/has_many_through_association.rb:52:in `insert_record'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:496:in `block (2 levels) in concat_records'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:344:in `add_to_target'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:495:in `block in concat_records'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:493:in `each'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:493:in `concat_records'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/collection_association.rb:134:in `block in concat'
... 14 levels...
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/associations/builder/collection_association.rb:71:in `block in define_writers'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/attribute_assignment.rb:85:in `block in assign_attributes'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/attribute_assignment.rb:78:in `each'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/attribute_assignment.rb:78:in `assign_attributes'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/persistence.rb:216:in `block in update_attributes'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:313:in `block in with_transaction_returning_status'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:192:in `transaction'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:208:in `transaction'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/transactions.rb:311:in `with_transaction_returning_status'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/activerecord-3.2.13/lib/active_record/persistence.rb:215:in `update_attributes'
    from (irb):2
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/railties-3.2.13/lib/rails/commands/console.rb:47:in `start'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/railties-3.2.13/lib/rails/commands/console.rb:8:in `start'
    from /usr/local/rvm/gems/ruby-1.9.3-p125/gems/railties-3.2.13/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'
Jacob Brown
  • 7,221
  • 4
  • 30
  • 50
  • Is it choking on `validate_description` or elsewhere? If it's on `validate_description`, you could ensure that the record is a `.new_record?` before running that validation, then have similar validation for an update... – CDub Dec 15 '13 at 21:09
  • @CDub, thanks for the comment. The error is raised by the call to `:product_ids=`. `validate_description` "sees" the failed validation on the `description` object, but the error only comes when Rails later calls `save!` on the `description` object. Another solution would be to "exit early" from the transaction inside `validate_description` (since at that point `doc` will know it can't save), but I don't know of a non-hacky way to do so. – Jacob Brown Dec 15 '13 at 21:49
  • can you attach backtrace? – Fivell Dec 18 '13 at 18:06
  • Thanks for the backtrace. Is the "You have reached limit of 1" your own custom validation? Could you show the code for it? Does it always fail on that validation, or on others as well? – Carlos Drew Dec 19 '13 at 18:59
  • @CarlosDrew, thanks for your continuing assistance. Yes, it is a custom validation on my `Description` model (it validates whether a user has reached a limit for rows in the `descriptions` table). I've simplified some things and posted the full content of my models in a gist here: https://gist.github.com/kardeiz/8045365. – Jacob Brown Dec 19 '13 at 20:18
  • @kardeiz, sorry, I've not had the chance to follow up sooner. I'm not sure of the exact issue, but I continue to believe that you don't need the `before_add: validate_d`, as that sort of manual validation strategy is not anything I've ever had to use with Rails. Such is the purpose of the `validate: true` option on association definitions. My last thought is that using `update_attributes` on `product_ids` probably does some weird stuff, and is not as safe/preferred as `foo.bars << bar; foo.save`. – Carlos Drew Dec 24 '13 at 21:31
  • Oh, sorry, no, my last thought is to ask whether you're writing tests & specs, because your troubles here seem indicative of not writing tests first and code second. – Carlos Drew Dec 24 '13 at 21:33

1 Answers1

2

My intuition is that you are over-engineering the validation of the models with that before_add: :validate_description. Are you not served by standard Rails/ActiveRecord methods and conventions? Specifically, validates: true can be set for validation handling between associated models.

Still, there are some gotchas around association validations, and I would recommend reading the following:

EDIT

I got super curious about this and went and replicated the problem, as you described it, via specs (and it's in a public github project). I still think the manual before_add validations are overengineered and I didn't use them, but I am encountering the issue you describe.

So, what I'm trying to understand is whether what you're encountering is expected and desired. Rails is nothing if not opinionated, and maybe using direct setting of has_many-through associations is a sort of coder-beware use case. To be clear, what you're doing is a slightly weird thing: when you ask to set document.product_ids, what you're actually doing is setting matching document_id and product_id on certain description objects. Right? That's weird, and super unclear in intent/expected result.

What's an alternate approach, then? What you're doing is adding descriptions to a document, and those descriptions are on products. Why not, then, interact with the document products through the description interface? That should avoid the has_many-through setter weirdness, and provider a clearer, interface, I think.

Community
  • 1
  • 1
Carlos Drew
  • 1,633
  • 9
  • 17
  • thanks for the links and the suggestions. I tried the configuration above, but it didn't work. I've provided an explanation of what I tried and error trace in my question. – Jacob Brown Dec 19 '13 at 17:06
  • thanks again for your help. I've given you the bounty. Let me do some more research after the holiday break and I'll accept your answer. Also, isn't one of the points of "has_many through" the ability to kind of ignore the join model? In the context of this app, it seems natural to set `document.product_ids`. – Jacob Brown Dec 25 '13 at 20:40