EugZol / where_exists

Adds the power of SQL Exists to ActiveRecord
MIT License
110 stars 18 forks source link

Rails 7 #21

Closed yosiat closed 2 years ago

yosiat commented 3 years ago

Hi!

Rails 7 alpha 2 is out, and I am currently working on getting my application to pass tests with it.

Tested locally with rails 7 alpha 2 and all tests pass, there is only one deprecation notice from test helper on rails 7.1 -

DEPRECATION WARNING: ActiveRecord::Base.default_timezone= is deprecated and will be removed in Rails 7.1.
Use `ActiveRecord.default_timezone=` instead.
 (called from <top (required)> at /Users/yosi/code/where_exists/test/test_helper.rb:8)
EugZol commented 2 years ago

@yosiat Hi, thanks!

Does the new suggested ActiveRecord.default_timezone= work in Rails 5.2? If so, we can fix it and drop Rails 4.x support.

yosiat commented 2 years ago

@EugZol all tests are passing with rails 5.2 :)

If you want, I can submit PR for adding CI with Github Actions.

EugZol commented 2 years ago

@yosiat That would be very much appreciated. I admit I worked only with GitLab CI. Would be interesting to integrate Github Actions here.

Regarding this PR:

yosiat commented 2 years ago

Hi @EugZol

Added this to PR - Github Actions (see results here - https://github.com/yosiat/where_exists/actions/runs/1534771257)

I didn't fixed the deprecation warning since it's only on Rails 7 and other versions throw this error -

❯ bundle exec appraisal 6.1 rake test
>> BUNDLE_GEMFILE=/Users/yosi/code/where_exists/gemfiles/6.1.gemfile bundle exec rake test
/Users/yosi/.rvm/rubies/ruby-2.7.2/bin/ruby -w -I"lib:lib:test" -I"/Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib" "/Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb" "test/belongs_to_polymorphic_test.rb" "test/belongs_to_test.rb" "test/documentation_test.rb" "test/has_many_polymorphic_test.rb" "test/has_many_test.rb" "test/has_many_through_test.rb"
Traceback (most recent call last):
        6: from /Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb:5:in `<main>'
        5: from /Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb:5:in `select'
        4: from /Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb:17:in `block in <main>'
        3: from /Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb:17:in `require'
        2: from /Users/yosi/code/where_exists/test/belongs_to_polymorphic_test.rb:1:in `<top (required)>'
        1: from /Users/yosi/code/where_exists/test/belongs_to_polymorphic_test.rb:1:in `require'
/Users/yosi/code/where_exists/test/test_helper.rb:8:in `<top (required)>': undefined method `default_timezone=' for ActiveRecord:Module (NoMethodError)
rake aborted!
Command failed with status (1): [ruby -w -I"lib:lib:test" -I"/Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib" "/Users/yosi/.rvm/gems/ruby-2.7.2/gems/rake-12.3.3/lib/rake/rake_test_loader.rb" "test/belongs_to_polymorphic_test.rb" "test/belongs_to_test.rb" "test/documentation_test.rb" "test/has_many_polymorphic_test.rb" "test/has_many_test.rb" "test/has_many_through_test.rb" ]
yosiat commented 2 years ago

@EugZol hey... I added CI, updated to test with Rails 7.. what is more left for this PR to get merged?

EugZol commented 2 years ago

@yosiat Please remove ActiveRecord::Base.default_timezone= from the code – which you mentioned in the start of this thread. I will deal with this PR on the weekends. Sorry for the delays.

Update: Ah, I see, it works only in Rails 7.

yosiat commented 2 years ago

@EugZol thanks for merge !

And delays - are ok and acceptable, it's open source project :)

EugZol commented 2 years ago

@yosiat Pushed 2.0.0. Thanks for the interesting PR: enjoyed the workflows :)

yosiat commented 2 years ago

Thanks! going to update now :)

About workflows - yes liked the outcome as well, it's simpler than panko workflow I have.. I'll replace in the near future to have same setup as here :)