Skip to content

changed rails dependency "~>4.2" to ">= 4.2" - #3

Merged
mwlang merged 2 commits into
mwlang:masterfrom
brijeshror:develop
Jul 3, 2017
Merged

mwlang merged 2 commits into
mwlang:masterfrom
brijeshror:develop

Conversation

@brijeshror

@brijeshror brijeshror commented Jun 23, 2017

Copy link
Copy Markdown
Contributor
  1. Updated rails dependencies to ">= 4.2"
  2. Used simplecov in rspec instead of codeclimate-test-reporter, because Usage of Code Climate Test Reporter is deprecated in version +1.0

W, [2017-06-23T14:10:21.546973 #31050] WARN -- : This usage of the Code Climate Test Reporter is now deprecated. Since version
1.0, we now require you to run SimpleCov in your test/spec helper, and then
run the provided codeclimate-test-reporter binary separately to report your
results to Code Climate.
More information here: https://github.com/codeclimate/ruby-test-reporter/blob/master/README.md

Look https://docs.codeclimate.com/v1.0/docs/test-coverage-ruby to use codeclimate now.

  1. Removed Active Record call back config config.active_record.raise_in_transactional_callbacks. No longer required.
    Reference : http://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#error-handling-in-transaction-callbacks

  2. Added railtie to set active_record configbelongs_to_required_by_default to false, Because to resolve validation errors,

    • Validation failed: Post must exist
    • Validation failed: Parent must exist

Error rises when trying to create record without defining association belongs_to field value in which default value specified at DB level.
Either we can use optional or required options in belongs_to association. Or we can add active_record config belongs_to_required_by_default to resolve this issue.
References :
http://blog.bigbinary.com/2016/02/15/rails-5-makes-belong-to-association-required-by-default.html
https://stackoverflow.com/questions/38983666/validation-failed-class-must-exist/38986580#38986580
http://edgeguides.rubyonrails.org/5_0_release_notes.html#active-record-notable-changes

@brijeshror brijeshror changed the title changed rails dependency 4.2 to 5.1 changed rails dependency "~>4.2" to ">= 4.2" Jul 3, 2017
@mwlang
mwlang merged commit 9bf3b49 into mwlang:master Jul 3, 2017
@flackou

flackou commented Apr 18, 2018

Copy link
Copy Markdown

Hi @brijeshror,

First of all, thanks for this PR. Unfortunately it doesn't seem to be compatible with Rails 5.2 : if you try to launch the gem tests after upgrading Rails, you'll get this error :

% bundle exec rake
...
55 examples, 0 failures, 1 pending
% bundle update rails
...
Using rails 5.2.0 (was 4.2.9)
Using rails_wordpress 0.2.1 from source at `.`
Bundle updated!
% bundle exec rake   
rake aborted!
NoMethodError: undefined method `sqlite3=' for #<Class:0x007fbd0864a650>
/Users/jlestavel/Work/github/rails_wordpress/lib/rails_wordpress/railtie.rb:9:in `block (3 levels) in <class:Railtie>'
/Users/jlestavel/Work/github/rails_wordpress/lib/rails_wordpress/railtie.rb:9:in `each'
/Users/jlestavel/Work/github/rails_wordpress/lib/rails_wordpress/railtie.rb:9:in `block (2 levels) in <class:Railtie>'

It seems related to https://github.com/rails/rails/pull/29699/files.

I started to look at what I can do to fix that but in fact I'm a bit embarrassed that the gem now forces belongs_to_required_by_default to false. For example, I use this gem as an engine in a larger Rails app, with another database aside, and I don't want my app setting to be globally changed by a gem ! So I can work on a fix but I would like your opinion before : is it OK if I remove the Railties and add optional: true to all the belongs_to ? Or is there something else I don't see ?

Thanks for your help 😃 ,

Julien

PS : @mwlang of course your opinion is welcome too !

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

Successfully merging this pull request may close these issues.

3 participants