8

I'm working in a legacy Rails app that has been diligently upgraded with each major version of Rails and we're currently on rails 5.1 and I can't get a before_destroy to prevent the deletion if it fails a validation

I've been reading that return false has been deprecated and we're all supposed to use throw :abort but neither are working. I'm not getting any errors, the join record just gets deleted despite the throw

user.rb:

class User < ApplicationRecord
  has_many :permission_users
  has_many :permissions, through: :permission_users, dependent: :destroy
end

join model permission_user.rb:

class PermissionUser < ApplicationRecord
  belongs_to :user
  belongs_to :permission

  before_destroy :check_before_removing!

  private

  def check_before_removing!
    if not_valid? # condition isn't important
      errors.add :base, exception.message
      throw(:abort)
    end
  end
end

my spec:

specify "cannot have their admin permissions revoked" do
  expect {
    admin.permissions.delete admin_permission
  }.to change { admin.permissions.count }.by(0)       
end

# => expected `admin.permissions.count` to have changed by 0, but was changed by -1
Sparkmasterflex
  • 1,837
  • 1
  • 20
  • 33
  • 1
    Bit off-topic, but: `expect{}.to change{}.by(0)` -> `expect{}.not_to change{}` – Greg Jun 28 '18 at 13:01
  • I actually trimmed down the spec to focus on the problem. I'm actually testing that 2 different values don't change and apparently rspec doesn't like the `.to_not change { ... }.and change { ... }` but thank you – Sparkmasterflex Jun 28 '18 at 18:45

2 Answers2

6

There's a catch with this callback:

before_destroy callbacks should be placed before dependent: :destroy associations (or use the prepend: true option), to ensure they execute before the records are deleted by dependent: :destroy.

So please try 

before_destroy :check_before_removing!, prepend: true
Greg
  • 5,862
  • 1
  • 25
  • 52
  • unless I'm doing something wrong, this still isn't working. Same failure, still getting to the `throw(:abort)` and a logging after the `throw` doesn't get printed out so I know it's throwing or whatever at that point – Sparkmasterflex Jun 28 '18 at 18:50
4

try to do something like this

before_destroy :check_before_removing, prepend: true

def check_before_removing if true && some_condition throw(:abort) end end

Hatemii
  • 39
  • 3