The before_destroy callback prevents child records from being deleted in Ruby on Rails

Because RoR does not offer a validate_on_destroy, I am essentially implementing one by using the before_destroy callback.

Using before_destory works and prevents a project that has had effort_logged? from being deleted. The below implementation does not work because when no has been logged I want to delete the project and all of its dependents. As long as before_destroy is implemented as it is below I am unable to do so.

If I understand how :dependent => :destroy works in relation to before_destroy the dependent children are deleted before the parent's before_destroy method is called. If my assumption is correct is accessing the children in the effort_logged? method somehow causing them to not be deleted? Is there a better means to check to see if a parent can be deleted based on its children?

Aside from curiosity on how RoR works my goal is to pass the following two tests:

• when no effort logged project deletion deletes dependents (this test fails)开发者_运维知识库
• cannot delete project with effort logged (this test passes)

Given everything outlined below I would expect both of these tests to pass.

Project Model

class Project < ActiveRecord::Base
  has_many :project_phases, :dependent => :destroy

  def before_destroy
     if effort_logged?
        errors.add_to_base("A project with effort logged cannot be deleted")
        false
     else
        true
     end
  end

  def effort_logged?
     project_phases.each do |project_phase|
        project_phase.deliverables.each do |deliverable|
           if (deliverable.effort_logged?)
              return true
           end
        end
     end
  end
end

Project Phase Model

class ProjectPhase < ActiveRecord::Base
  belongs_to :project
  has_many :deliverables, :dependent => :destroy
end

Deliverable Model

class Deliverable < ActiveRecord::Base
  has_many :effort_logs, :dependent => :destroy

  def effort_logged?
    total_effort_logged != 0
  end

  def total_effort_logged
    effort_logs.to_a.sum {|log| log.duration}
  end
end

Effort Log Model

class EffortLog < ActiveRecord::Base
  belongs_to :deliverable
end

Test cannot delete project with effort logged

test "cannot delete project with effort logged" do
   project = projects(:ProjectOne)

   assert !project.destroy, "#{project.errors.full_messages.to_sentence}"
end

Test when no effort logged project deletion deletes dependents

test "when no effort logged project deletion deletes dependents" do
   project = projects(:ProjectNoEffort)

   # all phases of the project
   project_phases = project.project_phases

   # all deliverables of all phases of the project
   project_phases_deliverables = {}

   # all effort logs of all deliverables of the project
   deliverables_effort_logs = {}

   project_phases.each do |project_phase|
      project_phases_deliverables[project_phase.name + "-" + project_phase.id.to_s] =
         project_phase.deliverables
   end

   project_phases_deliverables.each { |project_phase, project_phase_deliverables|
      project_phase_deliverables.each do |deliverable|
         deliverables_effort_logs[deliverable.name + "-" + deliverable.id.to_s] =
            deliverable.effort_logs
      end
   }

   project.destroy

   assert_equal(0, project_phases.count,
                "Project phases still exist for the deleted project")

   project_phases_deliverables.each { |project_phase, project_phases_deliverables|
      assert_equal(0, project_phases_deliverables.count,
      "Deliverables still exist for the project phase \"" + project_phase + "\"")
   }

   deliverables_effort_logs.each { |deliverable, deliverables_effort_logs|
      assert_equal(0, deliverables_effort_logs.count,
      "Effort logs still exist for the deliverable \"" + deliverable + "\"")
   }
end

I found this question because I was encountering the same problem. It turns out that the ordering of callbacks matters. When you define a relationship in Rails, the :dependent option actually creates a callback behind the scenes. If you define a before_destroy callback after the relationship, then your callback isn't called until after the relationships are destroyed.

The solution is to change the order of callbacks so that you first define whatever callback depends on the relationships still existing. The relationship definition should come afterwards.

Your code should look more like this:

class Project < ActiveRecord::Base
  # this must come BEFORE the call to has_many
  before_destroy :ensure_no_effort_logged

  # this must come AFTER the call to before_destroy
  has_many :project_phases, :dependent => :destroy

  # this can be placed anywhere in the class
  def ensure_no_effort_logged
    if effort_logged?
      errors.add_to_base("A project with effort logged cannot be deleted")
      false
    else
      true
    end
  end
end

You are correct, the children are wiped out before you get to before_destroy. Its not elegant but could you do something like this? : (btw, sorry i haven't tested this. it is more a thought than anything else).

in EffortLog, have a before_destroy :ready_to_die?

ready_to_die? would check if it has a zero effort value. If yes, destroy itself. If No, raise an exception (my example is EffortLogError). note: if you wanted to manually destroy something, you would need to zero it out yourself first.

then on your Project have a method with a more descriptive name:

def carefully_destroy
  Begin
    Project.transaction do
       self.destroy
    end 
  rescue EffortLogError
     self.errors.add_to_base("A Project with effort can't be deleted")
     #do some sort of redirect to the right spot.
  end
end

try adding raise ActiveRecord::Rollback right after you add the errors to base on the before_destroy filter.

After debugging the tests and keeping a close eye on the values of my methods and variables I was able to determine that the effort_logged? method was having issues. When there was logged effort it would return true. However, when there was no logged effort it would return the array of the project_phases. I modified effort_logged? to utilize a retval and that fixed the issue. The below method could stand for a refactoring.

  def effort_logged?
    retval = false

    project_phases.each do |project_phase|
      project_phase.deliverables.each do |deliverable|
         if (deliverable.effort_logged?)
            retval = true
         end
      end
    end

    return retval
  end