ActsAsParanoid / acts_as_paranoid

ActiveRecord plugin allowing you to hide and restore records without actually deleting them.
MIT License
1.47k stars 192 forks source link

Paranoid join tables in through relationships do not work with with_deleted #337

Closed pauldruziak closed 3 months ago

pauldruziak commented 3 months ago

I added tests here to confirm it, but it failed in all versions: https://github.com/ActsAsParanoid/acts_as_paranoid/pull/336

So, then I wrote this test in my project and it failed in 7.0 and passed in 6.1:

# frozen_string_literal: true

require "spec_helper"

describe "Paranoid" do
  class ParanoidManyManyParentLeft < ActiveRecord::Base
    has_many :paranoid_many_many_children, inverse_of: :paranoid_many_many_parent_left
    has_many :paranoid_many_many_parent_rights, through: :paranoid_many_many_children
  end

  class ParanoidManyManyParentRight < ActiveRecord::Base
    acts_as_paranoid
    has_many :paranoid_many_many_children, dependent: :destroy, inverse_of: :paranoid_many_many_parent_right
    has_many :paranoid_many_many_parent_lefts, through: :paranoid_many_many_children
  end

  class ParanoidManyManyChild < ActiveRecord::Base
    acts_as_paranoid
    belongs_to :paranoid_many_many_parent_left, inverse_of: :paranoid_many_many_children
    belongs_to :paranoid_many_many_parent_right, inverse_of: :paranoid_many_many_children
  end

  before do
    ActiveRecord::Schema.define(version: 1) do
      create_table :paranoid_many_many_parent_lefts do |t|
        t.string :name
      end

      create_table :paranoid_many_many_parent_rights do |t|
        t.string :name
        t.datetime :deleted_at
      end

      create_table :paranoid_many_many_children do |t|
        t.integer :paranoid_many_many_parent_left_id
        t.integer :paranoid_many_many_parent_right_id
        t.datetime :deleted_at
      end
    end
  end

  it "should work with has_many through" do
    paranoid_left = ParanoidManyManyParentLeft.create!
    paranoid_right = paranoid_left.paranoid_many_many_parent_rights.create!
    paranoid_right.destroy

    expect(paranoid_left.paranoid_many_many_parent_rights.only_deleted).to eq([paranoid_right])
  end
end

The only difference that I see right now is that acts_as_paraniod uses SQLite for tests, while my project uses PostgreSQL. I'm going to continue the investigation. And next step I see is creating a new Rails project with SQLite to see how this will behave there.

mvz commented 3 months ago

Hi @pauldruziak I tried the test you posted in this issue and can confirm that it fails with Rails 6.1, 7.0 and 7.1.

Which version of acts_as_paranoid are you using in your project?

pauldruziak commented 3 months ago

@mvz I use v0.7.3. The problem is in the next lines:

paranoid_where_clause = ActiveRecord::Relation::WhereClause.new([paranoid_default_scope])
scope.where_clause = all.where_clause - paranoid_where_clause

The last line returns "paranoid_many_many_children"."deleted_at" IS NULL to the query because of all.where_clause

mvz commented 3 months ago

@pauldruziak then it makes sense that your tests in #336 failed: The current release is 0.10.0.

Can you confirm that version 0.10.0 also does the wrong thing in your project, both for Rails 6.1 and 7.0?

pauldruziak commented 3 months ago

@mvz yes, v0.10.0 doesn't work well for me in 6.1 and 7.0

Here is the test for the context, it passes in v0.7.3, but not in newer versions:

  it "should work with has_many through" do
    post = Post.create! name: "foo"
    tag = post.tags.create! name: "bar"
    tag.destroy
    post.reload

    expect(post.tags).to eq([])
    expect(post.tags.with_deleted).to eq([tag])
  end

  it "should work with has_many through" do
    post = Post.create! name: "foo"
    tag = post.tags.create! name: "bar"
    post.taggings.first.destroy
    post.reload

    expect(post.tags).to eq([])
    expect(post.tags.with_deleted).to eq([tag])
  end
mvz commented 3 months ago

Thanks for confirming that @pauldruziak.

mvz commented 3 months ago

@pauldruziak this should be fixed in version 0.10.1 which I just released.

pauldruziak commented 3 months ago

@mvz thanks, it works 🎉