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

Ignore or Skip attributes dynamically is not working #1240

Closed
3 tasks done
pgonzaga opened this issue Mar 31, 2020 · 5 comments · Fixed by #1256
Closed
3 tasks done

Ignore or Skip attributes dynamically is not working #1240

pgonzaga opened this issue Mar 31, 2020 · 5 comments · Fixed by #1256

Comments

@pgonzaga
Copy link

  • 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
# frozen_string_literal: true

# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gem 'paper_trail', '10.3.1'
gem 'rails', '5.2.3'

ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  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.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail"

# STEP FOUR: Define your AR models here.
class User < ActiveRecord::Base
  has_paper_trail on: %i[update destroy], ignore: { first_name: Proc.new { |obj|  true } }
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_1
    User.create(first_name: "Jane")
    assert_difference(-> { PaperTrail::Version.count },  0) {
      User.update(first_name: "Jane 2")
    }
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`
@tlynam
Copy link
Contributor

tlynam commented Apr 26, 2020

Thank you for creating this bug report! I was able to reproduce the issue with your script and investigating.

@tlynam
Copy link
Contributor

tlynam commented Apr 26, 2020

Hi @pgonzaga, it looks like a version is being created during an update because updated_at is being recorded as a change. Can you confirm this is the case for you?

user = User.create(first_name: 'jane')
user.update(first_name: 'jane 2')
user.versions.size
=> 1
user.versions.first.changeset
=> {"updated_at"=>[Sun, 26 Apr 2020 19:31:39 UTC +00:00, Sun, 26 Apr 2020 19:31:41 UTC +00:00]}

@tlynam
Copy link
Contributor

tlynam commented Jun 1, 2020

Hi @pgonzaga, were you able to see if a version is being created during an update because updated_at is being recorded as a change?

@guigs
Copy link

guigs commented Jun 9, 2020

I also encountered this issue. When using ignore: %w[foo] it also ignore changes in updated_at, but when using with a proc like ignore: { foo: Proc.new { |obj| true } } it does not ignore changes in updated_at.

I think this is a surprising behavior, and a bug. I think the cause and possible fix is in the method ignored_attr_has_changed? which doesn't consider ignore attributes with a Proc and it is called by changed_notably? method.

tlynam added a commit to tlynam/paper_trail that referenced this issue Jun 14, 2020
- It was only ignoring attributes defined as an array.
- This commit now ignores when attributes are either defined as an array or defined as a hash of procs.
- Consolidates calculation to also be used when determining if changed and not ignored.

Resolves paper-trail-gem#1240
@tlynam
Copy link
Contributor

tlynam commented Jun 14, 2020

Thank you @guigs! Now I understand the issue. Thank you also for pointing out where to fix this. I opened #1247 for a fix.

tlynam added a commit to tlynam/paper_trail that referenced this issue Jul 27, 2020
- It was only ignoring attributes defined as an array.
- This commit now ignores when attributes are either defined as an array or defined as a hash of procs.
- Consolidates calculation to also be used when determining if changed and not ignored.

Resolves paper-trail-gem#1240
tlynam added a commit to tlynam/paper_trail that referenced this issue Jul 27, 2020
- It was only ignoring attributes defined as an array.
- This commit now ignores when attributes are either defined as an array or defined as a hash of procs.
- Consolidates calculation to also be used when determining if changed and not ignored.

Resolves paper-trail-gem#1240
tlynam added a commit to tlynam/paper_trail that referenced this issue Jul 27, 2020
- It was only ignoring attributes defined as an array.
- This commit now ignores when attributes are either defined as an array or defined as a hash of procs.
- Consolidates calculation to also be used when determining if changed and not ignored.

Resolves paper-trail-gem#1240
tlynam added a commit to tlynam/paper_trail that referenced this issue Aug 9, 2020
- It was only ignoring attributes defined as an array.
- This commit now ignores when attributes are either defined as an array or defined as a hash of procs.
- Consolidates calculation to also be used when determining if changed and not ignored.

Resolves paper-trail-gem#1240
tlynam added a commit to tlynam/paper_trail that referenced this issue Aug 16, 2020
…d via Hash

- It was only ignoring attributes defined as symbols.
- It now ignores when attributes are either defined as symbols or Hashes.
- Consolidates calculation to be shared when determining if changed and not ignored.

Resolves paper-trail-gem#1240
tlynam added a commit to tlynam/paper_trail that referenced this issue Aug 16, 2020
…d via Hash

- It was only ignoring attributes defined as symbols.
- It now ignores when attributes are either defined as symbols or Hashes.
- Consolidates calculation to be shared when determining if changed and not ignored.

Resolves paper-trail-gem#1240
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.

3 participants