Callback after_rollback on :destroy seems not be working correclty

[marcosgz]

Apparently using the callback :after_rollback on :destroy action is calling the update method instead of destroy. Example:

Table

mysql> CREATE TABLE models (
    ->   id INT NOT NULL AUTO_INCREMENT PRIMARY KEY,
    ->   name VARCHAR(255),
    ->   value VARCHAR(255)
    -> );
Query OK, 0 rows affected (0.07 sec)
mysql> INSERT INTO models(name, value) VALUES('First', 'First');
Query OK, 1 row affected (0.00 sec)

Model

class Model < ActiveRecord::Base
  before_create  :raise_rollback!
  before_destroy :raise_rollback!
  before_update  :raise_rollback!
  after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)}
  after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)}
  after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)}

private
  def raise_rollback!
    raise ActiveRecord::Rollback
  end

  def do_after_rollback(on)
    puts "after_rollback_on_#{on}"
  end
end

Rails Console

Loading development environment (Rails 3.2.8)
[1] pry(main)> Model.create(name:"Name", value: "Value")
   (0.1ms)  BEGIN
   (0.1ms)  ROLLBACK
after_rollback_on_create
=> #<Model id: nil, name: "Name", value: "Value">
[2] pry(main)> Model.first.update_attribute(:name, 'New')
  Model Load (0.4ms)  SELECT `models`.* FROM `models` LIMIT 1
   (0.2ms)  BEGIN
   (0.2ms)  ROLLBACK
after_rollback_on_update
=> nil

# Here is the issue
# Should be something like after_rollback_on_destroy instead of after_rollback_on_update right
[3] pry(main)> Model.first.destroy
  Model Load (0.4ms)  SELECT `models`.* FROM `models` LIMIT 1
   (0.2ms)  BEGIN
   (0.2ms)  ROLLBACK
after_rollback_on_update
=> nil

or this behavior is correct?

[al2o3cr]

Looks like a chunk of the offending behavior is here:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/transactions.rb#L372

The issue is that, by the time after_rollback is fired, everything has been restored - so destroyed? won't return true. The update case fires here only because it's essentially a catchall - anything that wouldn't fire an :on => :create or an :on => :destroy will instead fire the :on => :update callbacks.

I'm not sure if this behavior is correct, but I'm also not sure what could be done to make it work correctly. Almost by definition, the records are supposed to be back in their previous state after the rollback...

[neerajsingh0101]

As @al2o3cr pointed out after_rollback(:on => :destroy) code could never be executed because destroyed? would always return false because of rollback in the below code.

    def transaction_include_any_action?(actions) #:nodoc:
      actions.any? do |action|
        case action
        when :create
          transaction_record_state(:new_record)
        when :destroy
          destroyed?
        when :update
          !(transaction_record_state(:new_record) || destroyed?)
        end
      end
    end

How about raising an exception that after_rollback does not support on: :destroy ?

[rafaelfranca]

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

[rails-bot]

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

[EverybodyKurts]

Ran in to the same problem today :(. Does anybody have a workaround?

If I ran the after_rollback callback without on: :destroy, is there a way to see that destroy was called for the record?

[EverybodyKurts]

Here's my current workaround:

class Enrollment < ActiveRecord::Base
  # Callbacks
  # ---------------------------------------------------------

  before_destroy :cancel_destroy
  after_rollback :set_as_inactive

  private

  def set_as_inactive
    if @destroy_attempted
      write_attribute(:active, false)
      @destroy_attempted = false
      save!
    end
  end


  # Enrollments are never actually deleted - they are just set to inactive.
  def cancel_destroy
    @destroy_attempted = true

    false
  end
end

By the way, I would love it if somebody pointed out anything wrong or potentially dangerous about this.

[al2o3cr]

@KurtRMueller - a solution like the one paranoia uses is a little less circuitous:

https://github.com/radar/paranoia/blob/rails4/lib/paranoia.rb#L58

[EverybodyKurts]

@al2o3cr - oh, nice! Thanks for heads up.