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

When using papertrail with mock models papertrail takes the wrong item_type #1474

Open
3 tasks done
domingo2000 opened this issue May 16, 2024 · 1 comment
Open
3 tasks done

Comments

@domingo2000
Copy link

Check the following boxes:

  • This is not a usage question, this is a bug report
  • This bug can be reproduced with the script I provide below
  • This bug can be reproduced in the latest release of the paper_trail gem

This issue is about using paper_trail with rails and good migrations, so the template does not apply for this case, but i will put as much information as possible to reproduce the issue.

The bug is the following. When running a migration with good migrations the application does not autoload all the models. So a mock class needs to be created if it's necessary to use the model in the migration.

The problem is that when given a mock class in the migration, PaperTrail instead of taking class Foo it takes the class BackfillFooNames::Foo and the item_type takes the value item_type: BackfillFooNames::Foo instead of item_type: Foo. This is problematic because then the versions created by papertrail during the migrations are inconsistent with the ones in the application and cannot be used.

Here is the information and code to reproduce:

Migration code:

class BackfillFooNames < ActiveRecord::Migration[6.1]

  class Foo < ApplicationRecord
    self.table_name = "foos"
    has_paper_trail meta: { item_type: 'Foo' }
  end


  def up
    foo = Foo.first
    foo.bar = 'baz'
    foo.save!
  end

  def down
    foo = Foo.first
    foo.bar = nil
    foo.save!
  end
end

Our solution in papertrail < 14.0.0 is to also mock the item_type using the meta option as shown in the following code:

class BackfillFooNames < ActiveRecord::Migration[6.1]

  class Foo < ApplicationRecord
    self.table_name = "foos"
    has_paper_trail meta: { item_type: 'Foo' }
  end


  def up
    foo = Foo.first
    foo.bar = 'baz'
    foo.save!
  end

  def down
    foo = Foo.first
    foo.bar = nil
    foo.save!
  end
end

The problem is that in PaperTrail 14 introduced a breaking change that makes that meta attribute forbidden, so now the code above can not run in versions >= 14.0.0

Here is more data to reproduce the problem

Papertrail version: 15.1.0
Rails version: 6.1.7.7

Gemfile:

source 'https://rubygems.org'
git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby '3.2.2'

gem 'rails', '~> 6.1.7', '>= 6.1.7.7'
gem 'sqlite3', '~> 1.4'
gem 'puma', '~> 5.0'
gem 'sass-rails', '>= 6'
gem 'webpacker', '~> 5.0'
gem 'turbolinks', '~> 5'
gem 'jbuilder', '~> 2.7'
gem 'bootsnap', '>= 1.4.4', require: false

group :development, :test do
  gem 'byebug', platforms: [:mri, :mingw, :x64_mingw]
end

group :development do
  gem 'web-console', '>= 4.1.0'
  gem 'rack-mini-profiler', '~> 2.0'
  gem 'listen', '~> 3.3'
  gem 'spring'
end

group :test do
  gem 'capybara', '>= 3.26'
  gem 'selenium-webdriver', '>= 4.0.0.rc1'
  gem 'webdrivers'
end

gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]

gem 'paper_trail', '~> 15.1.0'

Gemfile.lock dependencies:

DEPENDENCIES
  bootsnap (>= 1.4.4)
  byebug
  capybara (>= 3.26)
  jbuilder (~> 2.7)
  listen (~> 3.3)
  paper_trail (~> 15.1.0)
  puma (~> 5.0)
  rack-mini-profiler (~> 2.0)
  rails (~> 6.1.7, >= 6.1.7.7)
  sass-rails (>= 6)
  selenium-webdriver (>= 4.0.0.rc1)
  spring
  sqlite3 (~> 1.4)
  turbolinks (~> 5)
  tzinfo-data
  web-console (>= 4.1.0)
  webdrivers
  webpacker (~> 5.0)

As i see maybe you could rollback the forbid of has_paper_trail meta: { item_type: 'Foo' } at the own user risk and document that is risky to change that parameter as was before. Or maybe you have a better solution passing anothe parameter to the has_papertrail that could be used in the mock classes.

Im willing to open a PR to fix this issue.

@MarcelEeken
Copy link

MarcelEeken commented Jun 4, 2024

We are running into a similar situation where we have a weird setup with a shared module that defines logic between two ActiveRecord classes that write to the same database table and are treated as the same entity.

We want to be able to track the history as a single entity, doesn't matter which of the two classes triggered the change. This looks like:

class Shared
  extend ActiveSupport::Concern

  included do
    self.table_name "shared_model"

    has_paper_trail(
      meta: {
        item_type: "SharedModel"
      }
    )
  end
end

class Foo
  include Shared
end

class Bar
  include Shared
end

After the upgrade to PaperTrail 14 this is now also throwing an error. And if we would remove the item_type setting it would result in the versions table having entries for Foo and Bar instead of SharedModel.

If possible we would also like to have an option to configure this somewhere.

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