acts_as_paranoid: Conflict with ActiveRecord destroyed? method

ActiveRecord to have method name destroyed? (http://apidock.com/rails/ActiveRecord/Persistence/destroyed%3F). Can’t we avoid alias_method :destroyed?, :deleted?.

Due to overriding of destroyed? method by the gem we cannot use update_column or update_columns method on records which are soft deleted. (http://apidock.com/rails/ActiveRecord/Persistence/update_columns)

This line throws exception raise ActiveRecordError, "cannot update a destroyed record" if destroyed? while using update_columns.

Is it intentional?

About this issue

  • Original URL
  • State: open
  • Created 8 years ago
  • Reactions: 7
  • Comments: 15 (6 by maintainers)

Most upvoted comments

I faced the same problem. Not sure what is the best approach, record.destroyed? is true after we call record.destroy, but rails checks that now in update_column. Workaround is to clear paranoid_value before updating the column.

self.paranoid_value = nil
update_column self.class.paranoid_column, nil

In general, you should not depend on a Rails app’s real models when migrating.

@mvz It’s not about depending on the model in a migration. What if I add a non-nullable field and want to backfill records with calculated values?

That is indeed a valid use-case. When I wrote that comment I was only thinking of actions inside Rails migrations, and also I was of the opinion that all data migrations like backfilling should happen inside Rails migrations. In practice, this is not always possible, especially when there are many existing records.

The question is: what is the reason for making this library opinionated about this?

It is kind of an accident: ActsAsParanoid first implemented #destroyed? to return true for soft-deleted records, and then later Rails made destroyed records immutable.

I also don’t see any connection between soft deletion and immutability. Two completely different topics and they shouldn’t be confused.

There is a tension here: On the one hand we want soft-deleted records to behave for the most part like actually destroyed objects, and Rails considers destroyed objects immutable. On the other hand we want to allow things that are not possible for actually destroyed records:

  1. We should be able to restore a record
  2. We should be able to update a soft-deleted record, for example to keep it valid, or to satisfy some new database constraint

I see two ways to facilitate item 2:

  1. Make #destroyed? return false, and #deleted? return true for soft-deleted records. This is a simple solution but is a breaking change. Also, there may be unexpected changes in behavior in Rails because it only looks at #destroyed?.
  2. Provide some kind of context where soft-deleted records can be updated, for example:
ActsAsParanoid.allow_updating_soft_deleted_records do
   soft_deleted.update(new_column: some_value)
end

Yes, I meant immutable apart from recovery. So the soft-deleted record is immutable until it is recovered (or fully destroyed).

What about cases where schema changes or other data migrations need to happen? Certainly, you can get around validations/callbacks while doing those if necessary but there are also times where you’d want validations/callbacks to run while also being able to update/adjust soft-deleted records.

What about cases where schema changes or other data migrations need to happen?

In general, you should not depend on a Rails app’s real models when migrating.

@mvz It’s not about depending on the model in a migration. What if I add a non-nullable field and want to backfill records with calculated values? There are all sorts of reasons to update soft-deleted records.

The question is: what is the reason for making this library opinionated about this? Shouldn’t it be as general-purpose as possible to serve as many users as possible? I also don’t see any connection between soft deletion and immutability. Two completely different topics and they shouldn’t be confused.

What about cases where schema changes or other data migrations need to happen?

In general, you should not depend on a Rails app’s real models when migrating. If you need some model, you should create a nested class inside the migration for it, like so:

class DoSomething < ActiveRecord::Migration[6.1]
  class Foo < ActiveRecord::Base
    # You can put any validations here, but leave out acts_as_paranoid if that makes sense.
  end

  def up
    Foo.find_each do |foo|
      Foo.update(...)
    end
  end

  # ...
end