Skip to content
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

Add Rails 6 Support #124

Merged
merged 30 commits into from Aug 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e0ee1ad
Add Rails 6 to Travis Config
danielricecodes Aug 13, 2019
1fcf62e
Test against MRI 2.6.3
danielricecodes Aug 13, 2019
d70375f
Ignore unsupported Rails 6 Rubies
danielricecodes Aug 13, 2019
90ba8fc
Add bundler setup for Travis, supports older rails/ruby combinations …
danielricecodes Aug 13, 2019
bd13b83
Map Rails 6 to "V5" validator class
danielricecodes Aug 13, 2019
8cbe82b
Try new Travis Config
danielricecodes Aug 13, 2019
add128a
Revert "Map Rails 6 to "V5" validator class"
danielricecodes Aug 13, 2019
00089f4
Ignore ruby version dot file
danielricecodes Aug 13, 2019
d10791d
Use released gem's, not a github branch
danielricecodes Aug 13, 2019
53eef30
Add Rails 6 test task
danielricecodes Aug 13, 2019
2ab706b
Create explicit V6 class
danielricecodes Aug 13, 2019
63905dd
Read Association scope directly from ActiveRecord
danielricecodes Aug 13, 2019
f5d4fba
Fix AssociationScope public API change
danielricecodes Aug 13, 2019
363c230
Whoops
danielricecodes Aug 13, 2019
03de215
Speed up build by using gem's instead of fetching the entire Rails pr…
danielricecodes Aug 13, 2019
f167583
Remove constraints on gems to work with 6.0.0
Aug 18, 2019
7b95093
Allow ActiveRecord and ActiveSupport 6.0
danielricecodes Aug 19, 2019
3230adb
Better comparison of gems
Aug 19, 2019
3845921
Open to 7.0
Aug 19, 2019
dbedde4
Merge pull request #1 from jbryant92/rails-6-support
danielricecodes Aug 19, 2019
610cb85
Using unscoped when retrieving the belongs_to association with delete…
RomainAlexandre Aug 20, 2019
97c060e
Merge pull request #2 from RomainAlexandre/rails-6-support
danielricecodes Aug 20, 2019
04e7af2
Undo change from 2ab706be55f2363de96e1c4704ffda9b23e83e22
danielricecodes Aug 20, 2019
980fce2
Remove unnecessary variable
danielricecodes Aug 20, 2019
97fc483
Add Rails 6 Contributors to Acknowledgements
danielricecodes Aug 20, 2019
dc333e3
Clarify Rails version support
danielricecodes Aug 20, 2019
0c9b243
Ensure we test against new Rails 6.0 patches also.
danielricecodes Aug 20, 2019
8d8c98f
Infer test versions from the files in the gemfiles directory
danielricecodes Aug 14, 2019
7d857f2
Remove duplicate key
danielricecodes Aug 21, 2019
c8f333e
Remove unneeded escape character, clean whitespace
danielricecodes Aug 21, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -4,3 +4,4 @@ pkg
Gemfile.lock
gemfiles/*.lock
.idea/
.ruby-version
25 changes: 20 additions & 5 deletions .travis.yml
@@ -1,12 +1,15 @@
language: ruby

script: "bundle exec rake test"
cache:
- bundler
- directories:
- vendor/bundle
rvm:
- 2.2.10
- 2.3.8
- 2.4.5
- 2.5.3
- 2.6.0
- 2.6.3
- ruby-head
- jruby
- jruby-9.1.8.0
Expand All @@ -16,6 +19,12 @@ gemfile:
- gemfiles/active_record_50.gemfile
- gemfiles/active_record_51.gemfile
- gemfiles/active_record_52.gemfile
- gemfiles/active_record_60.gemfile
before_install:
- gem install bundler --version 1.11.2
- export BUNDLE_PATH="${TRAVIS_BUILD_DIR}/vendor/bundle"
before_cache:
- rm -f ${BUNDLE_PATH}/**/extensions/**/{gem_make.out,mkmf.log}
matrix:
allow_failures:
- rvm: ruby-head
Expand All @@ -27,9 +36,15 @@ matrix:
- gemfile: gemfiles/active_record_edge.gemfile
rvm: 2.5.3
- gemfile: gemfiles/active_record_edge.gemfile
rvm: 2.6.0
rvm: 2.6.3
exclude:
- rvm: 2.2.10
gemfile: gemfiles/active_record_60.gemfile
- rvm: 2.3.8
gemfile: gemfiles/active_record_60.gemfile
- rvm: 2.4.5
gemfile: gemfiles/active_record_60.gemfile
fast_finish: true

branches:
only:
- master
- master
7 changes: 4 additions & 3 deletions README.md
Expand Up @@ -9,12 +9,12 @@ recoverable later.

## Support

**This branch targets Rails 4.2, 5.0 and 5.1, with experimental support for 5.2**
**This branch targets Rails 4.2, 5.0 and 5.1, with experimental support for 5.2+**

If you're working with another version, switch to the corresponding branch, or
If you're working with Rails 4.1-, switch to the corresponding branch, or
require an older version of the `acts_as_paranoid` gem.

### Known issues with Rails 5.2
### Known issues with Rails 5.2+

* Using acts_as_paranoid and ActiveStorage on the same model
[leads to a SystemStackError](https://github.com/ActsAsParanoid/acts_as_paranoid/issues/103).
Expand Down Expand Up @@ -287,5 +287,6 @@ Watch out for these caveats:
* To [Jean Boussier](https://github.com/byroot) for initial Rails 4.0.0 support
* To [Matijs van Zuijlen](https://github.com/mvz) for Rails 4.1 and 4.2 support
* To [Andrey Ponomarenko](https://github.com/sjke) for Rails 5 support
* To [Daniel Rice](https://github.com/danielricecodes), [Josh Bryant](https://github.com/jbryant92), and [Romain Alexandre](https://github.com/RomainAlexandre) for Rails 6.0 support.

See `LICENSE`.
2 changes: 1 addition & 1 deletion Rakefile
Expand Up @@ -9,7 +9,7 @@ desc 'Default: run unit tests.'
task :default => "test:all"

namespace :test do
versions = %w(active_record_42 active_record_50 active_record_51 active_record_52 active_record_edge)
versions = Dir["gemfiles/*.gemfile"].map {|gemfile_path| gemfile_path.split(/\/|\./)[1]}

versions.each do |version|
desc "Test acts_as_paranoid against #{version}"
Expand Down
4 changes: 2 additions & 2 deletions acts_as_paranoid.gemspec
Expand Up @@ -17,8 +17,8 @@ Gem::Specification.new do |spec|
spec.test_files = Dir["test/*.rb"]
spec.require_paths = ["lib"]

spec.add_dependency "activerecord", ">= 4.2", "< 6.0"
spec.add_dependency "activesupport", ">= 4.2", "< 6.0"
spec.add_dependency "activerecord", ">= 4.2", "< 7.0"
spec.add_dependency "activesupport", ">= 4.2", "< 7.0"

spec.add_development_dependency "bundler", ">= 1.5", "< 3.0"
spec.add_development_dependency "rake"
Expand Down
6 changes: 2 additions & 4 deletions gemfiles/active_record_42.gemfile
@@ -1,9 +1,7 @@
source 'https://rubygems.org'

github 'rails/rails', branch: '4-2-stable' do
gem 'activerecord', require: 'active_record'
gem 'activesupport', require: 'active_support'
end
gem 'activerecord', '~> 4.2.0', require: 'active_record'
gem 'activesupport', '~> 4.2.0', require: 'active_support'

# Development dependencies
group :development do
Expand Down
6 changes: 2 additions & 4 deletions gemfiles/active_record_50.gemfile
@@ -1,9 +1,7 @@
source 'https://rubygems.org'

github 'rails/rails', branch: '5-0-stable' do
gem 'activerecord', require: 'active_record'
gem 'activesupport', require: 'active_support'
end
gem 'activerecord', '~> 5.0.0', require: 'active_record'
gem 'activesupport', '~> 5.0.0', require: 'active_support'

# Development dependencies
group :development do
Expand Down
6 changes: 2 additions & 4 deletions gemfiles/active_record_51.gemfile
@@ -1,9 +1,7 @@
source 'https://rubygems.org'

github 'rails/rails', branch: '5-1-stable' do
gem 'activerecord', require: 'active_record'
gem 'activesupport', require: 'active_support'
end
gem 'activerecord', '~> 5.1.0', require: 'active_record'
gem 'activesupport', '~> 5.1.0', require: 'active_support'

# Development dependencies
group :development do
Expand Down
6 changes: 2 additions & 4 deletions gemfiles/active_record_52.gemfile
@@ -1,9 +1,7 @@
source 'https://rubygems.org'

github 'rails/rails', branch: '5-2-stable' do
gem 'activerecord', require: 'active_record'
gem 'activesupport', require: 'active_support'
end
gem 'activerecord', '~> 5.2.0', require: 'active_record'
gem 'activesupport', '~> 5.2.0', require: 'active_support'

# Development dependencies
group :development do
Expand Down
12 changes: 12 additions & 0 deletions gemfiles/active_record_60.gemfile
@@ -0,0 +1,12 @@
source 'https://rubygems.org'

gem 'activerecord', '~> 6.0.0', require: 'active_record'
gem 'activesupport', '~> 6.0.0', require: 'active_support'

# Development dependencies
group :development do
gem "sqlite3", :platforms => [:ruby]
gem "activerecord-jdbcsqlite3-adapter", :platforms => [:jruby]
end

gemspec :path => '../'
2 changes: 1 addition & 1 deletion lib/acts_as_paranoid/associations.rb
Expand Up @@ -31,7 +31,7 @@ def #{target}_with_unscoped(*args)
association = association(:#{target})
return nil if association.options[:polymorphic] && association.klass.nil?
return #{target}_without_unscoped(*args) unless association.klass.paranoid?
association.klass.with_deleted.scoping { #{target}_without_unscoped(*args) }
association.klass.with_deleted.scoping { association.klass.unscoped { #{target}_without_unscoped(*args) } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to check if this doesn't have any unwanted side effects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to explain here why I made that change, if it helps railsagency#2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I created some explicit tests for this and it looks like everything is fine 👍. I'll merge this in and then create another PR to add those tests.

end
alias_method :#{target}_without_unscoped, :#{target}
alias_method :#{target}, :#{target}_with_unscoped
Expand Down
12 changes: 10 additions & 2 deletions lib/acts_as_paranoid/core.rb
Expand Up @@ -184,7 +184,11 @@ def recover_dependent_associations(window, options)
scope = klass.only_deleted

# Merge in the association's scope
scope = scope.merge(association(reflection.name).association_scope)
scope = if ActiveRecord::VERSION::MAJOR >= 6
scope.merge(ActiveRecord::Associations::AssociationScope.scope(association(reflection.name)))
else
scope.merge(association(reflection.name).association_scope)
end

# We can only recover by window if both parent and dependant have a
# paranoid column type of :time.
Expand All @@ -205,7 +209,11 @@ def destroy_dependent_associations!
scope = klass.only_deleted

# Merge in the association's scope
scope = scope.merge(association(reflection.name).association_scope)
scope = if ActiveRecord::VERSION::MAJOR >= 6
scope.merge(ActiveRecord::Associations::AssociationScope.scope(association(reflection.name)))
else
scope.merge(association(reflection.name).association_scope)
end

scope.each do |object|
object.destroy!
Expand Down
15 changes: 10 additions & 5 deletions lib/acts_as_paranoid/validations.rb
Expand Up @@ -8,10 +8,9 @@ def self.included(base)

class UniquenessWithoutDeletedValidator
def self.[](version)
version = version.to_s
name = "V#{version.tr('.', '_')}"
name = "V#{version.to_s.tr('.', '_')}"
unless constants.include? name.to_sym
raise "Unknown validator version #{version.inspect}; expected one of #{constants.sort.join(', ')}"
raise "Unknown validator version #{name.inspect}; expected one of #{constants.sort.join(', ')}"
end
const_get name
end
Expand All @@ -38,8 +37,11 @@ def validate_each(record, attribute, value)
protected

def build_relation(klass, attribute, value)
return super(klass, klass.arel_table, attribute, value) if ActiveRecord::VERSION::MINOR == 0
super
if ActiveRecord::VERSION::MINOR == 0 && ActiveRecord::VERSION::MAJOR == 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit odd because you would think this class would only be used with Rails 5 (except it isn't because it's subclassed to V6). I'm not sure how to resolve this without duplicating all of V5 (except build_relation) to V6, so 🤷‍♂️.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I wasn't sure what the best way to code this would be since most options seemed to involve just copying and pasting, so I opted for the fewest lines of code: simply making V6 an empty subclass of V5.

return super(klass, klass.arel_table, attribute, value)
else
super
end
end
end

Expand Down Expand Up @@ -75,6 +77,9 @@ def validate_each(record, attribute, value)
end
end
end

class V6 < V5
end
end

module ClassMethods
Expand Down
2 changes: 1 addition & 1 deletion test/test_associations.rb
Expand Up @@ -228,7 +228,7 @@ def test_double_belongs_to_with_deleted
end

def test_mass_assignment_of_paranoid_column_enabled
if ActiveRecord::VERSION::MAJOR > 4 && ActiveRecord::VERSION::MINOR > 1
if Gem.loaded_specs['activerecord'].version >= Gem::Version.new('5.2.0')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvz - there are several places in the code that check the current ActiveRecord version. Since Rails 6 is out now, it might be worthwhile to release a new major version of this gem where old versions of Rails/Ruby are no longer supported. A new gem version that supports only Rails 5.2+ w/ Ruby 2.5+ only would be nice clean slate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's definitely a good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind working on that after we get this PR merged.

skip 'Creation as deleted is not supported with Rails >= 5.2'
end
now = Time.now
Expand Down