Fix rails 7.2 loading

[jdlubrano]

Fixes #1518

Problem

In a typical Rails app, the Ransack gem gets loaded before ActiveRecord gets loaded. Rails recommends that gems avoid loading Rails frameworks when they themselves are loaded. Ransack accomplishes that (as far as I can tell); however, Ransack initializes certain parts of its code within blocks like

if defined?(::ActiveRecord)
  ...set up some stuff
end

If Ransack is loaded before ActiveRecord is loaded, those blocks of code are never reached. I think these blocks of code should be wrapped with ActiveSupport.on_load(:active_record) instead of if defined?(::ActiveRecord). The blocks of code should run after ActiveRecord is loaded, which is what on_load(:active_record) is meant to accomplish.

Test coverage

I wasn't quite sure how to cover this in the specs, so I adjusted the load order in spec/spec_helper.rb to require "ransack" first. That did expose the load order issue(s) within the gem, so maybe that is good enough. It might be ideal to set up a test that spins up a Rails app and runs a basic query just to check that the basic functionality of the gem works within Rails 🤷‍♂️ . For local development, I used a single file Rails app like this:

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'debug'
  gem 'irb'
  gem 'rails', '>= 7.1'
  gem 'sqlite3', '~> 1.4'
  gem 'ransack' # , path: "./ransack"
end

require 'debug'
require 'rails/all'
database = 'development.sqlite3'

ENV['DATABASE_URL'] = "sqlite3:#{database}"

class App < Rails::Application
  config.load_defaults 7.2
  config.eager_load = false
end

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: database)
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.string :title, null: false
  end
end

App.initialize!

class Post < ActiveRecord::Base

  def self.ransackable_associations(_auth_object = nil) = []

  def self.ransackable_attributes(_auth_object = nil)
    %w[title]
  end

end

1.upto(10) { |i| Post.create!(title: "Title #{i}") }

IRB.start

Post.ransack(title_eq: "Title 1").result.to_sql
=> SELECT * FROM posts where title = 'Title 1'

[jdlubrano]

Just requiring this explicitly since on_load does get used further down in this file (and I suppose should be considered in any other files that get required from here).

[jdlubrano]

I'm not sure that this was ever truly dependent on ActiveRecord. At one point, this was a version check. I agree with the comment that it would be nice to avoid this patch to ActionView, but I also think that the patch is necessary regardless of whether ActiveRecord is defined or not. Moreover, the code within doesn't depend on ActiveRecord as we are just calling send on an object 🤔 .

[jdlubrano]

cc: @scottbarrow

[jdlubrano]

machinist/active_record requires active_record, which made all of the code wrapped in defined?(::ActiveRecord) execute. Requiring machinist/active_record after required ransack, I think, defines a load order that better imitates a Rails app.

[jdlubrano]

Hello 👋 . I was just checking whether there was any interest in this pull request? I was able to get all of the different DBs and Rails versions passing locally. Sorry for those oversights initially.