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

Allow to customize/extend PaperTrail::VersionAssociation #45

Open
sobrinho opened this issue Sep 18, 2023 · 2 comments · May be fixed by #46
Open

Allow to customize/extend PaperTrail::VersionAssociation #45

sobrinho opened this issue Sep 18, 2023 · 2 comments · May be fixed by #46

Comments

@sobrinho
Copy link

Hi there!

We have a separate database for PaperTrail::Version and PaperTrail::VersionAssociation but we can't find a good way of implementing this.

Currently we are doing something like this:

require 'paper_trail/version'
require 'paper_trail/version_association'

module PaperTrail
  class Version < ActiveRecord::Base
    # Rails requires a abstract class for no reason to call connects_to, but we
    # can't use one since the class will mismatch with the gem.
    self.abstract_class = true
    connects_to database: { normal: :auditing, dirty: :auditing }
    self.abstract_class = false

    self.table_name = "auditing.versions"
  end
end

module PaperTrail
  class VersionAssociation < ActiveRecord::Base
    # Rails requires a abstract class for no reason to call connects_to, but we
    # can't use one since the class will mismatch with the gem.
    self.abstract_class = true
    connects_to database: { normal: :auditing, dirty: :auditing }
    self.abstract_class = false

    self.table_name = "auditing.version_associations"
  end
end

Although, this creates an issue by creating two separate connection pools:

::PaperTrail::Version.connection.object_id
#=> 188700
::PaperTrail::VersionAssociation.connection.object_id
#=> 188720

Therefore it creates an issue with our FK between version_associations and versions.

To share the connection pool, we need to create an abstract class and inherit from it, like this:

class ApplicationVersion < ActiveRecord::Base
  self.abstract_class = true
  connects_to database: { normal: :auditing, dirty: :auditing }
end

class Version < ApplicationVersion
  include PaperTrail::VersionConcern
  self.table_name = 'auditing.versions'
end

class VersionAssociation < ApplicationVersion
  include PaperTrailAssociationTracking::VersionAssociationConcern
  self.table_name = 'auditing.version_associations'
end

While we can replace the Version class with a config option like this:

class ApplicationRecord < ActiveRecord::Base
  has_paper_trail versions: { class_name: 'Version' }
end

There's no similar option for this gem.

There are explicit class directly to PaperTrail::VersionAssociation in the gem that could be replaced to use a config option, i.e.:

Those could be replaced by something like or something:

PaperTrailAssociationTracking.model_class

In our use case, we don't need different classes per model (as supported by paper_trail) but for the application as a whole, which facilitates things here a little bit.

WDYT?

@sobrinho sobrinho linked a pull request Sep 18, 2023 that will close this issue
@westonganger
Copy link
Owner

Instead of

module PaperTrail
  class Version < ActiveRecord::Base
     ...
  end
end

module PaperTrail
  class VersionAssociation < ActiveRecord::Base
     ...
  end
end

What happens if you use

PaperTrail::Version.class_eval do
  ...
end

PaperTrail::VersionAssociation.class_eval do
  ...
end

@sobrinho
Copy link
Author

I can't use class_eval because that creates two connection pools:

::PaperTrail::Version.connection.object_id
#=> 188700
::PaperTrail::VersionAssociation.connection.object_id
#=> 188720

Rails is also tricky at that part because it requires you to be in an abstract class to replace the connection:

PaperTrail::Version.class_eval do
  connects_to database: { normal: :auditing, dirty: :auditing }
end
NotImplementedError: `connects_to` can only be called on ActiveRecord::Base or abstract classes
from /Users/sobrinho/.gem/ruby/2.7.6/gems/activerecord-6.1.7.5/lib/active_record/connection_handling.rb:82:in `connects_to'

We can workaround by doing that (very dirty):

PaperTrail::Version.class_eval do
  self.abstract_class = true
  connects_to database: { normal: :auditing, dirty: :auditing }
  self.abstract_class = false
end

But then by using the two connection pools, FKs doesn't work in a transaction because Rails will use two separate connections thinking that they are two separate DBs.

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

Successfully merging a pull request may close this issue.

2 participants