destroy w/ params and :dependent => destroy

rubyonrails-talk
1

Hello,

I'm having trouble debugging an override of destroy. I have an object that in some cases I need to send a parameter when I destroy it. Usually not though. So I've defined destroy in the model class like this:

class Look < ActiveRecord::Base   belongs_to :palette

  has_many :colorings, :dependent => :destroy   has_many :colors, :class_name => 'SiteColor', :dependent => :destroy   has_one :active_regatta, :class_name => 'Regatta'   belongs_to :regatta

  def destroy(options = {})     # special destroy so we don't accidentally default look     protected_ids = [1]

    super unless (protected_ids.include?(self.id) || options[:override] )   end end

The problem is that when I try to destroy one of these objects, I get an error on the call to super, "wrong number of arguments (1 for 0)" :

l=Look.find(62)

=> #<Look:0x23d2a28 @attributes={"name"=>nil, "id"=>"62", "palette_id"=>nil, "regatta_id"=>nil}>

l.destroy

ArgumentError: wrong number of arguments (1 for 0)   from ./script/../config/../config/../app/models/look.rb:19:in `destroy'   from ./script/../config/../config/../app/models/look.rb:19:in `destroy'   from (irb):2

Note that there is a :has_many :dependent => :destroy relationship.

It seems that my call to super is getting arguments I don't know about, but I don't understand how to dig any deeper into this. I tried stepping into the call to super itself, and I immediately end up in the rescue clause for ActiveRecord's transaction method:

(rdb:1) where --> #0 /usr/local/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/connection_adapters/abstract/database_statements.rb:62 in 'transaction' (rdb:1) l = [57, 66] in /usr/local/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/connection_adapters/abstract/database_statements.rb    57 transaction_open = true    58 end    59 yield    60 end    61 rescue Exception => database_transaction_rollback => 62 if transaction_open    63 transaction_open = false    64 rollback_db_transaction    65 end    66 raise

FWIW, I considered the possibility that it was one of the dependent relationships' objects who's destroy method was getting sent a parameter it wasn't expecting - so I defined both of those to accept a parameter as well, and then *their* call to super caused this problem.

2

Maybe you are over-complicating this.

Rails has a "before_destroy" filter.

Just use that, and if the ID of the object being destroyed is 1, abort the process (by just returning false).

So maybe something as simple as this (just off the top of my head, so you'll wanna test it of course)

class Look < ActiveRecord::Base   belongs_to :palette

  has_many :colorings, :dependent => :destroy   has_many :colors, :class_name => 'SiteColor', :dependent => :destroy   has_one :active_regatta, :class_name => 'Regatta'   belongs_to :regatta

  before_destroy { |record| return false if record.id == 1 }

end

Note: it's such a simple check that you don't even need to call a method... it can be handled right in the Proc.

Also note: the filter chain halts if you return false, which is probably what you want in this case, but again: test it to be sure.

HTH!

-Danimal

3

Daniel,

I appreciate the suggestion. I don't think it will cover my case, but I must admit I had forgoten all about before_destroy. I'll have to put some more thought into this.

The problem is that I want to be able to force the destroy if necessary. I'm not sure how I would articulate an override into before_destroy. But I'll look into it.

Part of the reason for my question is that this seems to be a case where what I thought I knew doesn't seem to be correct, so I'm looking to improving my understanding of Rails.

-Avram

4

Avram,

I'm confused. What do you mean by "want to be able to force the destroy"? If you have @look as the instance of the Look object you want to destroy, you just call @look.destroy, right? The before_destroy hook is just to make sure that you don't destroy the Look object with ID=1 (or whatever ID you want to protect).

Some other things to consider:

You could use before_destroy but have it call a method instead of an inline proc. I.e.:

before_destroy :skip_delete_if_not_one

def skip_delete_if_not_one   return false if id == 1 end

Then, you could add other logic there if need be.

Another thought that just occurred to me: If the default destroy method takes an options hash, maybe you need to pass that to super as well? Maybe that's the error you are getting.

Also, I vaguely remember something in the Agile book about this... i.e. not deleting the user with ID=1. You might see how that was done.

I don't think the problem is that your thinking is off... you are thinking about this in the right way. But like so many things in the Rails world, there are lots of ways to do something. So, you can call a before filter (as I suggested). Or you can override the destroy method (as you suggested). Both are "the right way"... it's more a matter of taste, style and what you specifically want to accomplish.

-Danimal

5

Danimal,

Here's what I want:

Object.find(2).destroy # object 2 is destroyed Object.find(1).destroy # has no effect Object.find(1).destroy :override => true # object 1 is destroyed.

I want to prevent someone from being able to delete certain Look objects by mistake, but I don't want to completely block my ability to destroy the special ones.

The super destroy method does not take any parameters. The error is "1 for 0" not "0 for 1". It only comes up when I add parameters to my destroy method.

If I simply change my destroy method to   def destroy     super if self.id != 1   end

I don't have any problems.

But now I can't destroy object 1 intetionally without changing the code.

6

I wonder if by declaring it as:

def destroy(options = {}) ...

if you are not actually overriding the default destroy method? (I've got a reasonable amount of Ruby and Rails experience, but I'm by no means an expert, so I may be misunderstanding something here).

What if you make a second method called: "destroy_override". So instead of calling: Look.find(1).destroy :override => true you'd call: Look.find(1).destroy_override

Then, you do your original code but without the options parameter. That overrides destroy() as a method and does the ID check. Then, in destroy_override it just calls destroy, probably through an alias for the original destroy so it doesn't call your overridden method.

I hope that rambling made sense. Let me know if it didn't.

-Danimal

7

It made sense, it just doesn't prevent accidental destruction of the object. It requires someone to be aware that they can't indiscriminately call destroy on objects of this class. It isn't really any different than requiring people to always say "object.destroy if !object.protected?.