-
Notifications
You must be signed in to change notification settings - Fork 903
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
Fix deprecation warning for paper_trail_on_destroy(:after) #813
Fix deprecation warning for paper_trail_on_destroy(:after) #813
Conversation
I added a spec to ensure that the deprecation warning is happening and it's not passing for some reason. In the process, I discovered that |
allow(::ActiveRecord::Base).to receive(:belongs_to_required_by_default). | ||
and_return(true) | ||
expect(::ActiveSupport::Deprecation).to receive(:warn). | ||
with(/paper_trail_on_destroy(:after) is incompatible with ActiveRecord/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mike, I think your test doesn't pass because your regex doesn't match the actual output.
pattern = /paper_trail_on_destroy(:after) is incompatible with ActiveRecord/
output = "paper_trail_on_destroy(:after) is incompatible with ActiveRecord"
pattern.match(output) # => nil
Parentheses have a special meaning in regular expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run the AR5 tests locally with bundle exec appraisal ar5 rake
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the parenthesis thing! I should have noticed that previously and I've fixed in an amended commit: 0bf252e
However, that doesn't seem to be the actual issue. If that were the problem, I would have seen the method called 1 time with a mismatched argument, but instead I'm seeing it called 0 times...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the real issue is that paper_trail_on_destroy
is a class method that is called as soon as the AfterDestroyModifier
class is loaded, so the warning is happening before the example is run in this spec.
Makes sense. We can live without the spec, thanks. |
* Accept either a String or a Symbol * Correctly warn on beta versions of ActiveRecord 5
@jaredbeck Dropped the useless spec 👍 |
Thanks Mike! |
👍 |
"after"
or:after