rubysherpas / paranoia

acts_as_paranoid for Rails 5, 6 and 7
Other
2.87k stars 524 forks source link

deleted? and paranoia_destroyed? returning different results #542

Closed ranieseah closed 9 months ago

ranieseah commented 1 year ago
  def paranoia_destroyed?
    paranoia_column_value != paranoia_sentinel_value
  end
  alias :deleted? :paranoia_destroyed?

In certain edge cases, self.deleted? and self.paranoia_destroyed? returns two different results, thus rolling back the destroy_all transactions even though self has been successfully soft deleted as expected. This is because alias is a copy of the method, and will produce different results when values are being redefined. reference: https://www.rubyguides.com/2018/11/ruby-alias-keyword/ (see last section: Aliasing Makes a Copy Of The Method)

This is because of the change added in v2.6.0: raise ActiveRecord::Rollback, "Not destroyed" unless self.deleted? in method paranoia_destroy. Can this be changed to raise ActiveRecord::Rollback, "Not destroyed" unless self.paranoia_destroyed? instead of using the alias method name self.deleted??

mathieujobin commented 1 year ago

interesting ! any idea what the solution should be ?

ranieseah commented 1 year ago

im suggesting to do away with the alias method? It does seem like most of the code is already using .paranoia_destroyed? directly, with only 1 place left using the .deleted?.

There might be other parts of the codes using alias method name .deleted? but at least within paranoia_destroy method, it should be OK for it to not use the alias method name, essentially change it from raise ActiveRecord::Rollback, "Not destroyed" unless self.deleted? to raise ActiveRecord::Rollback, "Not destroyed" unless self.paranoia_destroyed??

mathieujobin commented 10 months ago

Can you make a PR with your suggestion ? it will be easier to review and see if it breaks anything.

mathieujobin commented 9 months ago

this appears similar to #545

please test v2.6.3 and reopen if its not fixed