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

Manually creating a version saves the object with nil attributes #1047

Closed
abhinav-shrivastava opened this issue Feb 14, 2018 · 6 comments
Closed

Comments

@abhinav-shrivastava
Copy link

abhinav-shrivastava commented Feb 14, 2018

I'm using rails 5.1 with postgres database.

My use case involves creating versions when I explicitly want to instead of on callbacks like on: [:update] or something.

I did so by adding on: [] in my my_model.rb and using paper_trail.touch_with_version on my model instance. The problem is whenever I do this the first time the version is saved with nil attributes. Weird enough, the next time call paper_trail.touch_with_version it saves it correctly with all attributes correctly initialised.

Sample logs from my rails console:

document = Document.first
 #<Document:0x0000123456
id: 1,
name: "Sample document">

document.paper_trail.touch_with_version

document.versions.last.reify.name
=> nil

document.paper_trail.touch_with_version

document.versions.last.reify.name
=> "Sample document"
@jaredbeck
Copy link
Member

jaredbeck commented Feb 14, 2018

Thanks for the bug report. Per our contributing guide please use
our bug report template.

@jaredbeck
Copy link
Member

Are you still working on this? Want me to keep it open?

@abhinav-shrivastava
Copy link
Author

abhinav-shrivastava commented Feb 22, 2018

I am able to reproduce the issue, I was wondering if this is intentional like the issue #43.

require "bundler/inline"

gemfile(true) do
  ruby "2.4.2"
  source "https://rubygems.org"
  gem "activerecord", "5.1.4"
  gem "minitest", "5.10.3"
  gem "paper_trail", "8.0.0", require: false
  gem "sqlite3"
  gem "byebug", "9.1.0"
end

require "active_record"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.text :first_name, null: false
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.text :object, limit: 1_073_741_823
    t.text :object_changes, limit: 1_073_741_823
    t.integer :transaction_id
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
  add_index :versions, [:transaction_id]
end

ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail/config"

PaperTrail::Config.instance.track_associations = false

require "paper_trail"

class User < ActiveRecord::Base
  has_paper_trail on: []
end

class BugTest < ActiveSupport::TestCase
  def test_1
    user = User.create(first_name: "Jane")
    user.paper_trail.touch_with_version
    user_from_paper_trail = PaperTrail::Version.first.reify
    assert_equal("Jane", user_from_paper_trail.first_name)
  end
end

@jaredbeck
Copy link
Member

Hi Abhinav,

Interesting. The documentation for touch_with_version says:

Mimics the touch method from ActiveRecord::Persistence,
but also creates a version. A version is created regardless of
options such as :on, :if, or :unless.

It doesn't say anything about what goes in the version record, so we expect it to work the same as any other "update" event. So, we expect it to record the user's first_name. Looks like a bug to me.

As a workaround, you could install the update callback,

-has_paper_trail on: []
+has_paper_trail on: [:update]

I'm not sure what the real fix for this problem should be. Any ideas?

@jaredbeck
Copy link
Member

jaredbeck commented Feb 23, 2018

I'm not sure what the real fix for this problem should be. Any ideas?

Ah. I see that #1048 is your suggestion.

jaredbeck added a commit that referenced this issue Mar 4, 2018
Fixes #1047

Fixes touch_with_version when the update callback is not installed,
eg. `has_paper_trail(on: [])`
jaredbeck added a commit that referenced this issue Mar 4, 2018
Fixes #1047

Fixes touch_with_version when the update callback is not installed,
eg. `has_paper_trail(on: [])`
@jaredbeck
Copy link
Member

Fixed by #1060, will release as 9.0.0.

westonganger pushed a commit to westonganger/paper_trail that referenced this issue May 18, 2018
Fixes paper-trail-gem#1047

Fixes touch_with_version when the update callback is not installed,
eg. `has_paper_trail(on: [])`
aried3r pushed a commit to aried3r/paper_trail that referenced this issue Dec 14, 2020
Fixes paper-trail-gem#1047

Fixes touch_with_version when the update callback is not installed,
eg. `has_paper_trail(on: [])`
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