Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deleted? and paranoia_destroyed? returning different results #542

Closed
ranieseah opened this issue Jun 9, 2023 · 4 comments
Closed

deleted? and paranoia_destroyed? returning different results #542

ranieseah opened this issue Jun 9, 2023 · 4 comments

Comments

@ranieseah
Copy link

ranieseah commented Jun 9, 2023

  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
Copy link
Collaborator

interesting ! any idea what the solution should be ?

@ranieseah
Copy link
Author

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
Copy link
Collaborator

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

@mathieujobin
Copy link
Collaborator

this appears similar to #545

please test v2.6.3 and reopen if its not fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants