views:

450

answers:

1

I am trying to implement before_destroy on a model. While I am following this before_destroy example I either receive a NameError or end up with something which doesn't do the before_destroy. What's wrong with my syntax?

class Person < ActiveRecord::Base

  has_many :book_loans, :order => 'return_date desc'
  has_many :books, :through => :book_loans

  before_destroy
    errors.add_to_base "Cannot delete borrower with loans" unless
                                                           book_loans.count == 0
end

Doesn't Compile

  before_destroy
    errors.add_to_base "Cannot delete borrower with loans" unless
                                                           book_loans.count == 0

Doesn't Work

def before_destroy
  errors.add_to_base "Cannot delete borrower with loans" unless
                                                         book_loans.count == 0
end
+5  A: 

You can't add errors before_destroy, because they won't impact the validation. You have to add the errors before the validation occurs, with e.g. before_validation.

There are some additional callbacks, before_validation_on_create and before_validation_on_update. There is no on_destroy version, though, because save and valid? is never called, so adding validation errors when you try to destroy something would be pointless.

In other words: your before_destroy runs, but it doesn't affect anything, because nothing checks for validation errors when you destroy records.

Here's one take on implementing something that prevents record saving based on certain conditions.

before_destroy :require_no_book_loans

private

def require_no_book_loans
  raise ActiveRecord::RecordInvalid unless book_loans.count == 0
end

You could also create your own error class and raise that.

FYI: You are supposed to pass a block to before_destroy in your syntax error example.

before_destroy {|r|
  r.errors.add_to_base "Foo" # ...
}
August Lilleaas
So how would I prevent the destruction of a person who has loans?
ahsteele
I'll update the answer, sec :)
August Lilleaas
Sweet so this will raise an error. Considering that exceptions should only occur in exceptional circumstances would I be better off preventing the destroy link from even showing up when the user has loans?
ahsteele
Yeah, that sounds sensible. People could still forge it by making a request to destroy it by hand, but you're raising an error so it won't be destroyed anyway.
August Lilleaas
Cool thanks for the help.
ahsteele