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

Paper_trail creates a first version with the custom meta attribute approved set to false even when it should be true #385

Closed
duarme opened this issue Jun 20, 2014 · 9 comments

Comments

@duarme
Copy link

duarme commented Jun 20, 2014

The Content model has a bunch of attributes, but one of them is approved defined by this code in its migration:

add_column :contents, :approved, :boolean, null: false, default: false

And in its ContentVersion migration:

class CreateContentVersions < ActiveRecord::Migration
  def self.up
    create_table :content_versions do |t|
      t.string   :item_type, :null => false
      t.integer  :item_id,   :null => false
      t.string   :event,     :null => false
      t.boolean  :approved, null: false, default: false # <= this one
      t.string   :whodunnit
      t.text     :object
      t.text     :object_changes
      t.datetime :created_at
    end
    add_index :content_versions, [:item_type, :item_id]
  end
# ...

Of course, Content also has paper_trail:

# content.rb
has_paper_trail class_name: 'ContentVersion', meta: { approved: :approved }

This code creates and saves a content object:

content = Content.create!({
      author_id: user.id,
      content: Lorem.sentence,
      approved: true
    })

I expect it's first version to have approved = true, but, checking it via rails console it's easy to find out that content.versions.first.approved is false, while it should be true:

#!
- !ruby/object:ContentVersion
  attributes:
    id: 447
    item_type: Measure
    item_id: 42
    event: create
    approved: false
    whodunnit: '1'
    object: 
    object_changes: "--- !ruby/hash:[...]"
    created_at: 2014-06-20 10:08:59.263012000 Z

Why is that?

@duarme
Copy link
Author

duarme commented Jun 20, 2014

By the way, I'm riding rails 3.2.14 and paper_trail 2.7.2.

@duarme
Copy link
Author

duarme commented Jun 20, 2014

I found a temporary fix:

# content.rb
# ...
  after_save :fix_first_version, on: :create
#...
private

def fix_first_version
  first_version = self.versions.first
  first_version.approved = self.approved
  first_version.save!
end

@batter
Copy link
Collaborator

batter commented Jun 20, 2014

Please see #185. The metadata columns get filled with the attribute as it is before state persistence, so in this case it would be nil before the object is created, however, since you have the column value defaulting to false in the database, it is getting set to false instead of nil as it normally would.

@batter batter closed this as completed Jun 20, 2014
@duarme
Copy link
Author

duarme commented Jun 20, 2014

Ok thank you!
So I guess I have to change the has_paper_trail declaration like this:

has_paper_trail class_name: 'ContentVersion', meta: {approved: Proc.new { |content| content.approved }

@batter
Copy link
Collaborator

batter commented Jun 20, 2014

Yes, but just realize that the value being stored in that column (if it changes frequently) may not actually represent the value for the state of the object represented in that corresponding version. For instance:

content = Content.create!(:approved => true)
content.update_attribute(:approved, false)
content.versions.last.approved # => false
content.versions.last.reify # => #<Content id: 1, approved: true>

As stated in the README:

PaperTrail stores the pre-change version of the model, unlike some other auditing/versioning plugins, so you can retrieve the original version. This is useful when you start keeping a paper trail for models that already have records in the database.

The reason why PaperTrail doesn't record about the state of the current object is because it is redundant, since you can access that data simply by accessing the live object's record.

@duarme
Copy link
Author

duarme commented Jun 23, 2014

Uhm, I'm not following you here, because in the README you also state:

Why would you do this? In this example, author_id is an attribute of Article and PaperTrail will store it anyway in a serialized form in the object column of the version record. But let's say you wanted to pull out all versions for a particular author; without the metadata you would have to deserialize (reify) each version object to see if belonged to the author in question. Clearly this is inefficient. Using the metadata you can find just those versions you want:

This is the use I intended for the approved metadata: I need the search for "approved" versions to be efficient, so I need its value to represent the same value stored in the serialized object.

So I guess I'll have to stick with the above mentioned temporary fix.

@batter
Copy link
Collaborator

batter commented Jun 23, 2014

That's what I'm trying to say, the metadata columns, by default, if pointed at an attribute, will be populated by the value of that attribute at the state it was at before the modification was made.

I guess you could fix this by either removing the default value for the column, or having the default value be true, since create events are the only events where that value will not be set yet.

@duarme
Copy link
Author

duarme commented Jun 23, 2014

I can't set the approved default to true because unapproved content may be created, anyway I got you and I think I found an acceptable solution, thank you!

@jobinthepast
Copy link

jobinthepast commented Jul 29, 2019

I got confused too. At first I thought it was a bug that made the meta value always 1 step behind the current state. But now I understand thats truly how it works. Thanks.

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

3 participants