Destroy w/ params and :dependent => destroy

Rails
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:indestroy’
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

On Apr 12, 11:12 pm, Avram D. [email protected]

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

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.

5

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

On Apr 13, 8:06 am, Avram D. [email protected]

6

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?.

7

Hey Avram,

little late, but just as info for googlers like me:
To solve your ArgumentError you described in your first post, just
call super() with brackets, instead without brackets.

Cheers,
Steffen

On 13 Apr., 12:43, Avram D. [email protected]

8

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

On Apr 13, 9:22 am, Avram D. [email protected]

9

I was having exactly the same issues as Avram and hitting exactly the
same argument error. However Steffen’s fix (of using braces) solved it
perfectly.

For reference, the reason that I (and I presume Avram also) needed to
protect against destroying normally is that I’ve basically got a
group/member setup. Any one of the members can be removed from the group
unless they’re the last one in which case I don’t want an orphaned group
with no-one around to take care of it.

I’ve overridden the destroy method of the group members to to check
whether they are the last one:

class Member
def destroy force_destroy=false
if group.members.count == 1 and unless force_destroy
raise “Member is the last on a group & can’t be destroyed. Try
destroying the group first.”
return false
end; end
super()
end
end

However in order to destroy the group itself you have to be able to
destroy the members (even the last one). When its committing Seppuku,
the group has to be able to destroy every last member hence the
over-ride requirement