Reified record cannot be persisted if associated record has been deleted

[serge1peshcoff]

Let's say I have StadiumUser model that has_one Stadium, and the dependency between them is like that:

class StadiumUser < User
    ....
    has_one :stadium, foreign_key: "user_id", dependent: :destroy, required: true
    after_create :make_stadium
end

I created StadiumUser and the Stadium with it. Then I decided to delete StadiumUser, and Stadium was deleted with it. Okay.
Then I decided to revert that model:

PaperTrail::Version.find(2594).reify.save
...
ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR:  insert or update on table "stadiums" violates foreign key constraint "fk_rails_b1bd5d1cc5"
DETAIL:  Key (user_id)=(56) is not present in table "users".
: INSERT INTO "stadiums" ("id", "user_id", "category_id", "address", "latitude", "longitude", "slug", "created_at", "areas_count", "updated_at") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING "id"

This situation also crashes rails_admin_history_rollback gem, when I am trying to revert a model that links to deleted entry, it displays the same error message.

Can something be done with that?

[jaredbeck]

Hi Sergey, good question. Thanks for testing reification of associations. This is an experimental feature and not recommended for production.

If I understand correctly, when you try to save the reified Stadium, its user_id (56) no longer exists.

Currently, Reifier#reify_has_ones simply assigns the associated record without checking if it exists.

# lib/paper_trail/reifier.rb:280
without_persisting(child) do
  model.send "#{assoc.name}=", child
end

As a workaround, you could try to manually save the child record first, then save the parent.

reified_stadium = PaperTrail::Version.find(2594).reify
reified_user = reified_stadium.user.dup # dup to clear the id
reified_user.save
reified_stadium.user = reified_user
reified_stadium.save

Of course, this is awkward for you and it would be nice if PT had this feature.

I would be happy to review a PR that adds this feature.

[jaredbeck]

However we implement this new feature, reify should never write to the database. Maybe reify should check if the associated record exists and if not, clear the id, perhaps using dup.

Unrelated, but worth mentioning: When users of PT reify and then subsequently persist a record, both operations must occur within a single transaction, to avoid phantom read errors.

[jaredbeck]

329e224 adds this to the list of known issues.

[hubertjakubiak]

Hello Jared & Sergey, what about using reify(belongs_to: true).save in such case? Please let me know what you think about it. Clearing stadium_user_id seems to be solution only when foreign key in table is nullable.

I wrote simple test to see how it works.

context "when the associated is required to create parent object" do
  setup do
    @stadium_user = StadiumUser.new name: "stadium_user_0"
    @stadium = Stadium.new name: "stadium_0"
    @stadium_user.stadium = @stadium
    @stadium_user.save
  end

  context "when parent object is destroyed" do
    setup { @stadium_user.destroy }

    should "destroy child object with parent" do
      assert_equal 0, Stadium.count
    end

    context "when reified child object" do
      setup { @stadium.versions.last.reify(belongs_to: true).save }

      should "persist child object" do
        assert_equal @stadium, @stadium.reload
        assert_equal 1, Stadium.count
      end

      should "persist parent object" do
        assert_equal @stadium_user, @stadium_user.reload
        assert_equal 1, StadiumUser.count
      end
    end
  end
end

class Stadium < ActiveRecord::Base
  has_paper_trail
  belongs_to :stadium_user
  self.table_name = "stadiums"
end

class StadiumUser < ActiveRecord::Base
  has_paper_trail
  has_one :stadium, dependent: :destroy, required: true
end

[jaredbeck]

Hi Hubert, thanks for the suggestion. I've incorporated your tests into associations_test.rb in #952. It's not exactly the same, but I think it captures the spirit of your tests.

So, for now, I guess we advise people to use belongs_to: true if they want to save the reified record.

Do y'all want to close this issue, or should we maybe discuss the reify API? For example, maybe flags like belongs_to should be true by default when track_associations is true. WDYT?

[alexevanczuk]

I talked about this with my team a while back. We were thinking of a deep_reify method that returns an array of unpersisted objects that were once destroyed), which looks at all associations, and any associated objects that were deleted as a result of the change. There are a couple of concerns with this:

1. You might want to only look at associations that are dependently destroyed; however, if this option (dependent: destroy) was added or removed since the destroy action happened, this is not dependable.
2. The array of unpersisted objects need to be returned topologically sorted so when they are saved in order, there are not dependency conflicts.

[jaredbeck]

Closing due to inactivity.

I am thinking more and more of deleting the experimental association-tracking feature. No one is stepping up to fix the known issues, and I'm not even sure some of them, like #542 can be fixed. Maybe I should deprecate it in 9.0.0 and see who steps up and volunteers to take care of it.