fix(ActiveRecord): correctly connect to database in Rails 7.2+

[markokajzer]

ActiveRecord::Base.connection_pool.with_connection(&:active?) was used to establish a connection to the database, so that subsequently ActiveRecord::Base.connected? becomes true.

In Rails 7.2, this is not the case anymore, and ActiveRecord::Base.connection_pool.with_connection(&:active?) does not flip ActiveRecord::Base.connected?

Instead, what we can do is perform a direct check like ActiveRecord::Base.connection.database_exists?. Also see rails/rails#52946.

[dblock]

Thanks!

Don't compare versions of Ruby like this though.
https://code.dblock.org/2020/07/16/comparing-version-numbers-in-ruby.html

Let's add a test that uses >= 7.2?

[markokajzer]

1. added active_record 7.2 to the ci matrix
2. switched to using Gem::Version.new to compare versions 🙏
3. added a changelog entry

[dblock]

I assume the new CI fails without the fix?

One more nit, sorry!

[markokajzer]

added a spec for 7.2+ vs pre-7.2.

quick explanation:

DatabaseCleaner leases the connection, so ActiveRecord::Base.connected? is true when the spec starts, but we want to simulate what should happen in prod:

1. ActiveRecord::Base.connected? is false
2. ActiveRecord::Base.connection.database_exists? (or ActiveRecord::Base.connection_pool.with_connection(&:active?)) connects to the database
3. ActiveRecord::Base.connected? is true

in pre-7.2 that does not seem to work, the specs ultimately fail because DatabaseCleaner tries to operate on a closed connection. i've not been able to come up with a good spec for pre-7.2 so not sure if we should keep the spec, lmkwyt!

i added comments as well because it's maybe not obvious what is going on

---

along the way i had to do the following changes

1. pin pagy_cursor to ~> 0.6.1
   • the method pagy_cursor only supports 2 parameters on 0.8, see here
2. pin database_cleaner to a higher version to support ActiveRecord 7.2
   • ran into edge rails (future 7.1) no longer has ActiveRecord::SchemaMigration.table_name DatabaseCleaner/database_cleaner-active_record#83

i ran the full spec suite locally with all versions from the matrix and all passed, so let's hope ci does us the honor as well

[dblock]

i ran the full spec suite locally with all versions from the matrix and all passed, so let's hope ci does us the honor as well

First time contributor PRs marked as pending is a little annoying (a 1-time problem), but you can always enable CI on your fork for next time you run into this.

[markokajzer]

but you can always enable CI on your fork

thanks for the tip, i somehow overlooked this lol

[dblock]

Looks like some of the CI is failing :(

[markokajzer]

the fork is now green: https://github.com/markokajzer/slack-ruby-bot-server 😤

there were two more things to do

1. due to the upgrade in database_cleaner to support ActiveRecord v7.2, there had to be a couple more changes in how it is setup, specifically for mongoid
   • basically now you have to require the respective adapters there as well instead of just require 'database_cleaner'
   • i therefore moved some files around a bit, see 79b185d
2. mongoid-scroll recently released a v2 (coincidentally by you as well 😸) which is incompatible with the existing cursor setup
   • had to pin version to 1.0.1, see 30a2629

---

lastly, i noticed a flake in Teams oauth v1 oauth includes optional parameter, however since there's no old CI runs / logs on this repo I cannot verify if it was introduced by me or was already there...

the spec sometimes fails both for

1. activerecord [failed run] [successful re-run]
2. and for mongoid [failed] [successful]

no additional changes were made between each of those runs.
maybe you could run CI a couple of times in master to see if it fails anywhere? 🤔

sorry for the hassle by the way, this took quite a bit longer than i had hoped for myself as well 😅

[dblock]

sorry for the hassle by the way, this took quite a bit longer than i had hoped for myself as well 😅

Thank you so much for doing the work! No hassle at all, and looking forward to more :)

[dblock]

There was a failure in ruby=3.1.1, postgresql=14, active_record=~> 7.2.0, but it looked like a fluke and restarting passed. I'll watch it, it's probably a flaky test.

[dblock]

This one had a breaking change in 2.0 that's easy to fix. It's something like this (from dblock/slack-strava@88af16b#diff-633cee3ca2d39648d625e12b921138aa0a932ebe9375ba53102cbabc339e5afe).