with_versioning broken in 3.0.0

[jacobat]

The introduction of the rspec integration in 3.0.0 has broken with_versioning as this spec demonstrates:

describe "with_versioning" do
  with_versioning do
    it "should version records" do
      PaperTrail.should be_enabled
    end
  end
end

[batter]

Do you have a special with_versioning block from your test helper (such as shown in this example group on the README)? If so, you should remove it for 3.0.0 compatibility, because with_versioning is now provided to RSpec by the built-in RSpec helper (which was introduced in 3.0.0).

Please see this block in the README about the RSpec helper for more details on how it works.

[jacobat]

Actually with_versioning seems to not be provided. I just tried setting up a Rails 4.0.2 app from scratch and created a spec matching the one in the example in the README and now running rspec I get:

/Users/jacob/papertraildemo/spec/with_options_spec.rb:4:in `block in <top (required)>': undefined method `with_versioning' for #<Class:0x007fde6bbbdba8> (NoMethodError)
        from /Users/jacob/.rbenv/versions/1.9.3-p448/gemsets/bundler/gems/rspec-core-2.14.5/lib/rspec/core/example_group.rb:246:in `module_eval'
        from /Users/jacob/.rbenv/versions/1.9.3-p448/gemsets/bundler/gems/rspec-core-2.14.5/lib/rspec/core/example_group.rb:246:in `subclass'
...

I've published a minimal Rails app as an example here: https://github.com/jacobat/papertrail_error

[batter]

@jacobat - You are correct, the docs on the README are mis-stated. It appears you can only use the with_versioning block matcher within an it block on an RSpec test. I'm going to try to look into the DSL to see if there is some sort of an easy way to add custom blocks with context like that to the base set of blocks, but I'm not sure that it is possible. Hopefully I can find a way. Any suggestions you or anyone else has would be helpful.

You can also use the :versioning => true on your describe blocks as a replacement for this, and that definitely works.

[jacobat]

It used to work. Before upgrading to 3.0 we had an around hook like:

around(:each) do |example|
  with_versioning do
    example.run
  end
end

This no longer works due to the way the RSpec integration is made (because it runs it's own before-each hook, that will disable PaperTrail for all examples).

I don't think it's great to hook into the RSpec tagging system with :versioning => true as it is very implicit. If you don't know PaperTrails RSpec integration, you're very likely to miss the semantics of it. I much prefer the old with_versioning style as it is much more explicit what's going on.

Actually I would prefer to to make the whole RSpec integration optional and disabled by default. I wouldn't mind having with_versioning available - maybe just by including a module into my spec_helper or something like that. This automagic integration is too much magic for my taste. But that's a personal preference of course.

[batter]

@jacobat - you had a special with_versioning method included on your spec_helperlike the one on the README?

[jacobat]

Yes, exactly.

[batter]

@jacobat - Stepped away from this for a while but trying to wrap this up. Can you share what your helper method looked like?

[jacobat]

My helper method used to look like:

def with_versioning
  was_enabled = PaperTrail.enabled?
  PaperTrail.enabled = true
  begin
    yield
  ensure
    PaperTrail.enabled = was_enabled
  end
end

[batter]

So I've considered some options. The before(:each) { PaperTrail.enabled = false } could be changed to a before(:all), and then the I thought helper would perform the way it used to as you describe, the only issue is that the with_versioning block will get triggered before the before(:all) block, and then PaperTrail.enabled? gets set to false by the before(:all) block right before the if statement it seems, like this:

# prior: PaperTrail.enabled? == true
with_versioning do # this block call sets `PaperTrail.enabled = true`, doesn't see `before(:all)` block
  # before(:all) block gets triggered here, sets `PaperTrail.enabled = false`
  it { PaperTrail.should be_enabled } # fails!
end

I figured the before(:all) block at the Rspec.configure level would set it to false before any other blocks get evaluated, but it seems to slip into order right before before(:each) blocks get executed. Now I'm really stumped on what to do. It might make sense for PaperTrail.config class to default the enabled value depending on what environment it's being run in (in RSpec, Cucumber, or when RACK_ENV or RAILS_ENV is test, default to false instead of true, but that seems like such a hack solution). I don't understand why the before(:all) block doesn't trigger at the beginning like the terminology would suggest.

[g8d3]

This is working for me:

it 'saves each change in record' do
  with_versioning do
    PaperTrail.should be_enabled
  end
end

But when I try with an actual record, it does not work:

      described_class.all.size.should == 0
      model = create(:model)
      described_class.all.size.should == 1
      model.versions.size.should == 0
      model.update_attribute :first, 'rty'
      model.reload.versions.size.should == 1   # Fails here
      model.versions.first.changeset.should == { :first => ['qwe', 'rty'] }

[batter]

@juanpastas - this is within an 'it' block? What is the create statement on line 2 there? Is that using fixtures or a factory or something? There are a lot of things that could factor into why this is failing. Does it work if you use the :versioning => true metadata option?

[g8d3]

Yes, that's in an it block, I am using factories, I will try the versioning option, and let you know.

[g8d3]

Is working better now, but I am having this issue now:

it 'saves each change in record', :versioning => true do
    # with_versioning do
      PaperTrail.should be_enabled

      described_class.all.size.should == 0
      model = create(:model)
      described_class.all.size.should == 1
      model.versions.size.should == 1
      model.update_attribute :first, 'rty'
      # Not working in test env
      # pending 'Check https://github.com/airblade/paper_trail/issues/312'
      model.reload.versions.size.should == 2
      model.update_attribute :first, 'three'
      model.reload.versions.size.should == 3
      binding.pry
      model.versions.last.changes.should == { :first => ['rty', 'three'] }
    # end
  end

       expected: {:first=>["rty","three"]}
            got: nil (using ==)

[g8d3]

Also, see what I got from versions:

reservation.versions.map(&:changes)
=> [{}, {}, {}]
reservation.versions.map(&:changeset)
=> [nil, nil, nil]

[batter]

@juanpastas - Do you have an object_changes column on your versions table? See the Diffing Versions section of the README. You can generate one with the install generator via: rails g paper_trail:install --with_changes.

[g8d3]

That should be it, thanks a lot.

On Tue, Mar 4, 2014 at 2:56 PM, Ben Atkins notifications@github.com wrote:

@juanpastas https://github.com/juanpastas - Do you have an
object_changes column on your versions table? See the Diffing Versions
section of the READMEhttps://github.com/airblade/paper_trail#diffing-versions.
You can generate one with the install generator via: rails g
paper_trail:install --with_changes.

Reply to this email directly or view it on GitHubhttps://github.com//issues/312#issuecomment-36666733
.

[batter]

@jacobat - Just released v3.0.1, which contains the fix for this issue.