Added Rails 5.0 support

[mvastola]

Resolves #249.

Note: As rails-observers has not yet released a version with Rails 5.0 compatibility (the appraisals Gemfile in this PR points to a commit in rails-observers Github), but can still be merged without issue. To be used with Rails 5.0, any application containing a version of audited in its Gemfile with this PR merged will also need the following entry:

gem 'rails-observers', :github => 'rails/rails-observers', :branch => 'master'

This rails50 appraisal in this repository should be updated to point to rails-observers on RubyGems once a version is released that is compatible with Rails 5.0.

[mvastola]

@danielmorrison Please note that, according it its README, protected_attributes is no longer maintained as of the time of the release of Rails 5, so I have updated the test suite with the assumption that -- at the very least -- audited will be not support this feature for Rails 5 apps. (This gem also broke functionality when it was enabled under Rails 5 in ways it shouldn't have.)

A question that the owners of this gem should answer, however, is whether audited should abandon all support for protected_attributes whatsoever, since it is soon to be abandonware.

[mvastola]

All tests are now passing and this is ready to merge.

[mvastola]

@danielmorrison Could you also clarify the current support status for Rails 4.0 and 4.1? The README.md currently says:

Audited currently (4.x) works with Rails 4.2. For Rails 3, use gem version 3.0 or see the 3.0-stable branch.

But is silent on 4.0 and 4.1, which are still tested by Travis CI in the master branch. I took care to make sure the tests for 4.0 and 4.1 continue to pass when making my edits, but I'm not sure if you plan on abandoning support for these versions when 5.0 is released, have already done so, or only quasi-support them. In any case I think it's probably advisable to state the current policy in README.md (as well as the future policy, if it is known and will change).

If audited is no longer supporting 4.0 and/or 4.1, please let me know if I should be removing those tests from .travis.yml.

For now I changed the README.md to state that the gem works in 4.2 and 5.0 and that the gem may work in 4.0 and 4.1 but that functionality in the later two isn't guaranteed. I consider this change totally tentative though, pending comment from audited's owners.

[mvastola]

Update: I'm marking this PR as ready to merge since there are no known regressions and audited's runtime dependencies cause it to obtain only stable releases of rails-observers from RubyGems. Therefore, merging this request (and even pushing a new release of this gem) should have no adverse effects whatsoever.

Until a new release of rails-observers is pushed to RubyGems, audited will continue to not work on Rails 5.0, with the exception that advanced users would be able to ensure functionality if they manually added the following entry to their Gemfiles:

gem 'rails-observers', :github => 'rails/rails-observers', :branch => 'master'

[mvastola]

@danielmorrison FYI, going through the changelogs, I just came across definitive proof here that protected_attributes is no longer supported in Rails 5.0.

Also, @danielmorrison, do you happen to have any ETA on reviewing of this PR? I only ask because you expressed willingness to personally write this entire thing yourself nearly two weeks ago, so I was hoping you had time..

Please let me know what, if any, changes/clarifications you'd like made or any questions I can answer.

[mvastola]

@danielmorrison PS: I know I had been editing it endlessly, so apologies if you were waiting on me to stop, but I swear I'm done now (barring new, as-of-yet undiscovered issues with the PR). :-p

[mvastola]

@danielmorrison would it be possible to get this PR reviewed?

I understand if you may be hesitant to merge it just quite yet because it has to point to the master branch of rails-observers (pending the next release of that gem, which I am following up on as it was promised almost a month ago).

That said, considering the tentative dates for the RC(s) and subsequent initial release for Rails 5 are rapidly approaching (both scheduled for later in February), it would be ideal to get a jump start on any changes/revisions you might want made to this PR before you consider it worthy to be merged. Ideally, it is obviously preferable to allow as much time as possible between the introduction of Rails 5 compatibility in audited and the final release of Rails 5.0.0.

[danielmorrison]

why the throw :abort here?

[mvastola]

See this comment to the PR.

[danielmorrison]

@mvastola great work. Sorry for the delay, my free time has been limited lately.

I had one question, otherwise this looks good to me!

[mvastola]

From activerecord/CHANGELOG.md for Rails 5:

Change the way in which callback chains can be halted.

The preferred method to halt a callback chain from now on is to explicitly throw(:abort). In the past, returning false in an Active Record before_ callback had the side effect of halting the callback chain. This is not recommended anymore and, depending on the value of the ActiveSupport.halt_callback_chains_on_return_false option, will either not work at all or display a deprecation warning.

[danielmorrison]

Cool. I learned something new!

[mvastola]

Thanks for reviewing. I actually hadn't meant for you to merge just yet since I'm running into hurdles getting a version of rails-observers on Ruby Gems that supports Rails 5, but I suppose this can't hurt. I would caution against a new release of this gem at this time though.

[mvastola]

PS: I understand your time is super limited, but if you have any insight such that you could lend at rails/rails-observers#41 it would be greatly appreciated.

Barring that, a comment from you on the PR as the owner of this gem might facilitate my receiving some assistance from the maintainers (whose most recent response time is 29 days and counting).

Thanks!

[danielmorrison]

Ha, well master can be wonky for a few days. I'll take a peek later today.

[mvastola]

Thanks. The only "wonkiness" is rependencies won't resolve correctly if someone is pulling audited from its github master branch but plain vanilla RubyGems to fetch rails-observers, all whIle running a beta release of rails.

Which is to say, their app would have to inherently be rather "wonky" before we got involved in order for this edge-case to apply and cause this issue.

[ericboehs]

Now that Rails 5 is out, will a new release be cut? Or are we waiting on rails-observers?

[nynhex]

👍

[danielmorrison]

I plan to look into it this afternoon and see what (if anything) is still missing.

[danielmorrison]

I cut a new PR #276 for a release, but rails-observers needs a new version cut.

If you want to try with Rails 5 today, use:

gem 'audited', github: 'collectiveidea/audited', branch: 'rails5'
gem 'rails-observers', github: 'rails/rails-observers', branch: 'master'

and let me know on the PR if you have any problems!

[ericboehs]

Great @danielmorrison! @rafaelfranca is planning to release a new version of rails-observers in "the next days". rails/rails-observers@876c522

[rafaelfranca]

👍 rails-observers is on the top of my list since we use in Shopify.

[mvastola]

rails-observers has rails 5 support merged in, but there was a minor regression in my PR to provide that support (basically you have to manually add an extra line of code to enable each observer). I submitted a new PR to fix this regression a while ago, but it sat dormant for a bit, and since then merge conflicts have arisen.

When this got bumped last night, I tried to fix the merge conflicts, but doing so in the most straightforward manner seems to have unfixed this bug. I'm working on updating the PR and then it should be good on my end for @rafaelfranca to push.