Add rspec-support; Use ::Rails::Engine for model and migration

[belt]

Adds rspec helper functions: with_versioning()

Adds (additional) specs for Version

Uses rails-engine for the model and one of the two migrations

Introduces a behavior change (could be re-coded for original behavior)

• :item_id defaults to a string to cover string-based IDs e.g. UID

[belt]

Forgot to push two files.

[belt]

Oh... and README.md became ugly when I updated the chunk on migrations. Sorry, don't know that markdown syntax. Please fix it.

[batter]

Hi, thanks for the pull request. Just to clarify, is the purpose of this pull request to provide helpers for people that are testing their PaperTrail-enabled rails applications with RSpec? Or is it to switch the test suite from Test::Unit to RSpec for this gem? Or both?

I'd like to provide extra helpers for RSpec test suites if possible, however, I think the test suite for this gem should probably remain implemented via Test::Unit at this point.

Also, what is the purpose of the PaperTrail::Engine class you have provided?

[belt]

:: Intent
My apologies for not making my intent clear. I use these helpers to test my PaperTrail enabled applications in rspec and cucumber.

I have no interest in engaging in yet-another flame-war of this-vs-that technologies… especially in not-my-project. Use the right tool for the job.

However, my projects use Rspec, cucumber and some of those use PaperTrail. Therefore, I need these helpers in some form. As I'm not the only one in this… predicament, I figured it would be good of me to give-back.

:: PaperTrail::Engine purpose
As stated, "Uses rails-engine for the model and one of the two migrations"

This pulls the Version model "into" the rails-app that uses PaperTrail… as if it's native. No "require 'version'" required… anywhere.

It also allows one of the two migrations to be generated via a rake paper_trail:install:migrations … the other migration is generated via rails g paper_trail:install (by processing ERB).

On 29 Apr 2013, at 08:42 , Ben Atkins notifications@github.com wrote:

Hi, thanks for the pull request. Just to clarify, is the purpose of this pull request to provide helpers for people that are testing their PaperTrail-enabled rails applications with RSpec? Or is it to switch the test suite from Test::Unit to RSpec for this gem? Or both?

I'd like to provide extra helpers for RSpec test suites if possible, however, I think the test suite for this gem should probably remain implemented via Test::Unit at this point.

Also, what is the purpose of the PaperTrail::Engine class you have provided?

—
Reply to this email directly or view it on GitHub.

[batter]

Ok cool, I thought that was what you were going for, just wanted to make sure.

I understand what you are saying that the PaperTrail::Engine class does, however, I'd like to understand what if any advantages this offers over the traditional migration generation? Right now the install generator has an optional --with-changes flag that can be passed in, in order to generate a migration for adding the 'object_changes' migration, so I'm wondering if we should keep this. That being said, I like the idea of a Native pull in, considering the Version class is going to need to get namespaced for Rails 4 compatibility. Can you try to explain what advantages the Engine provides over the traditional migration generation?

Also, was wondering what the purpose of the 'Platform' gem that you added into the .gemspec in this comit was and whether it is really necessary? I'd like to try to keep the .gemspec as minimal as possible.

I'd also prefer to keep the rails dependency requirements at ~> 3.0 to keep compatibility for all versions of Rails 3.x (at least until the next major release).

[belt]

Side note: current migration generation generates an integer column for :item_id … which is a clone of the :id column for the "audited" model. BUT… rails can, does, and should probably use-by-default, non-integer ID's e.g. UIDs. Please make this an option, at the very least, in the current master branch.

What are the use cases for-and-against making with-changes?

I believe I confused Platform for rbconfig (standard ruby lib) to determine if I should use ruby-debug, ruby-debug19, or debugger.

As far as the engine, I do not believe asking for a comparison of the advantages is the best way to ask that question. Why use ERB if there are no variables to evaluate? I believe the reason the "traditional" way of migration generation i.e. using ERB generation from a template, is due to a stack overflow article and one of its respondents ignorance of railsties / engines and-or outdated knowledge i.e. when we didn't have rails-engines… and no one has out-voted what I perceive as the "right" was to integrate migrations, models, controllers etc from a gem into the rails app.

On 29 Apr 2013, at 17:30 , Ben Atkins notifications@github.com wrote:

Ok cool, I thought that was what you were going for, just wanted to make sure.

I understand what you are saying that the PaperTrail::Engine class does, however, I'd like to understand what if any advantages this offers over the traditional migration generation? Right now the install generator has an optional --with-changes flag that can be passed in, in order to generate a migration for adding the 'object_changes' migration, so I'm wondering if we should keep this. That being said, I like the idea of a Native pull in, considering the Version class is going to need to get namespaced for Rails 4 compatibility. Can you try to explain what advantages the Engine provides over the traditional migration generation?

Also, was wondering what the purpose of the 'Platform' gem that you added into the .gemspec in this comit was and whether it is really necessary? I'd like to try to keep the .gemspec as minimal as possible.

—
Reply to this email directly or view it on GitHub.

[batter]

@belt - Thank you for the very useful pull request, apologies for the delayed response, but it seemed appropriate to try to put this into the impending 3.0.0 release. I've merged in the RSpec and Cucumber helper extensions you wrote and then made some modifications.

With the RSpec helper I simplified it and tried to make it so that it is compatible with RSpec outside of the rails context (so it can be used with Sinatra or other types of applications), because the 3.0.0 release decouples PaperTrail from Rails. See e3a039b and 642c296.

With the Cucumber helper I added a before hook to try to mimic the behavior of the RSpec helper. I'm actually not entirely sure this will work the way I have it written, as I have very little experience working with Cucumber, but I gave it a crack, and from reading the documentation this seemed to be the correct syntax. See 272203d.

I refrained from pulling in the Rails::Engine piece because it seemed unnecessary, and didn't seem to provide anything additional in terms of functionality as far as I could tell. If you still feel that the Engine piece provides something that's really useful that I'm not seeing feel free to respond and/or make another pull request. If you have a link to some documentation or an example of code that uses the Rails::Engine syntax for migration generation I would look forward to reading it and learning more about the advantages.