rubysherpas / paranoia

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

Calling with_deleted unscopes all tables with has_many through #534

Open scsmith opened 1 year ago

scsmith commented 1 year ago

Assuming you have three models such as User, Posts, Comments all with acts_as_paranoid. If you have associations such as:

# User.rb
has_many :posts
has_many :comments, through: :posts

Calling user.comments would result in SQL with something like the following:

SELECT comments.* FROM comments
INNER JOIN posts ON comments.post_id = posts.id
INNER JOIN users on posts.user_id = users.id
WHERE comments.deleted_at IS NULL AND posts.deleted_at IS NULL AND users.deleted_at IS NULL
AND user.id = 1

calling users.comments.with_deleted will in turn call

unscope(where: :deleted_at)

this will result in all of the WHERE x.deleted_at IS NULL being removed.

I think that unscope should actually be restricted to the relationship that's being called, for example with_deleted here should actually call

unscope(where: { comments: :with_deleted})

Before I submit a pull request I'd like to guage interest in this as it would be quite a breaking change? Any thoughts / comments about how this could be achieved with the least upset to backwards compatibility?

janko commented 3 months ago

This is a huge issue for us, we've been bitten by this behavior many times, and we cannot seem to avoid making the same mistake. Why does with_deleted get applied to all tables in a joined association dataset, is it intended behavior? Because it sure does seem like a bug to me.

scsmith commented 3 months ago

This is a huge issue for us, we've been bitten by this behavior many times, and we cannot seem to avoid making the same mistake. Why does with_deleted get applied to all tables in a joined association dataset, is it intended behavior? Because it sure does seem like a bug to me.

For what it's worth we added this to our application record and just renamed all of the with_deleted occurances:

  # This works similar to with_deleted from the paranoia gem.
  # However, this will restrict the query to only omit deleted_at
  # for the current table_name.
  def self.with_omitted(table_name = self.table_name)
    unscope(where: { table_name => :deleted_at })
  end
janko commented 2 months ago

Yeah, we did something similar after I posted this comment, we monkey-patched Paranoia's with_deleted:

class Paranoia::Query
  def with_deleted
    unscope where: { table_name.to_sym => paranoia_column }
  end
end