Add Rails 7 compatibility

[pfeiffer]

This PR adds compatibility for Rails 7.

Rails 7 has a type-check in compute_class which expects the reflected class to be a subclass of ActiveRecord. We need to monkey-patch this to allow an ActiveHash class to be the reflected klass.

Added Rails 7 to the test matrix as well.

[flavorjones]

Is there a reasonable change that could be proposed upstream in rails/rails, rather than introduce a monkeypatch here?

[kbrock]

Is there a reasonable change that could be proposed upstream in rails/rails, rather than introduce a monkeypatch here?

It is frustrating but rails specifically introduced a check that ensures that associations points to an ActiveRecord child class. The check does make sense as it could catch a potential issue.

Can someone think of a reason why Rails (without this plugin) would want to establish an association to an ActiveModel object? That seems to be one way for us to pitch removing this check from rails proper and changing it into a form that works for us.

Or is there a way to introduce associations from ActiveModel objects to another ActiveModel object?

[julianrubisch]

Tiny bit of feedback:

This almost works for me, with the only exception that I had to add a class_name: to an unrelated has_many association that references a namespaced class, i.e. User::ConnectedAccount.

Don't know what that is exactly about, just wanted to mention it.

[ohbarye]

Also, feedback as ActiveHash and tapioca (RBI generator) user:

I really appreciate it if the compatibility would resolve because a tapioca's compiler depends on AR::Reflection and it calls reflection.klass. Due to this reason, the compiler crashes if it processes any association defined by ActiveHash. ref https://github.com/Shopify/tapioca/issues/1286

But I don't think of any reasonable description for proposing upstream (rails/rails)...

[dedman]

@julianrubisch I had the same issue as you, namespaced associations didn't work, and I have a lot of them so I ended up modifying the monkey patch further. I don't have much experience monkey patching stuff so don't trust it too much, but this is what got me past this hurdle on our rails 7 upgrade :)

@pfeiffer thanks for working on the PR. I'm not sure how to fix it, but I found that when the monkey patch was calling super it was actually calling MacroReflection.compute_class, which is just a simplistic name.constantize which doesn't work for namespaced associations. By namespaced associations I mean stuff like Invoice has_many :items which should resolve to Invoice::Items. I think you might also need to set the klass variable before using it in the unless condition like I do below, but I'm not sure.

module ActiveHash
  module Associations
    module ReflectionExtensions
      if ActiveRecord::VERSION::MAJOR >= 7
        def compute_class(name)
          super
        rescue ArgumentError => e
          klass = active_record.send(:compute_type, name)
          raise unless e.message =~ /Please provide the :class_name option on the association/ && klass < ActiveHash::Base
          klass
        end
      end
    end
  end
end

class ActiveRecord::Reflection::AssociationReflection
  prepend ActiveHash::Associations::ReflectionExtensions
end

Good luck to all the other rails 7 upgraders out there!

[flavorjones]

For posterity, the upstream commit that broke associations is https://github.com/rails/rails/commit/c2fc8626d2b2f8162e757e79065d750ab0417377 related to https://github.com/rails/rails/issues/15811 and https://github.com/rails/rails/pull/40836

[flavorjones]

I've written a slightly different fix at #272 that doesn't monkeypatch active record classes. I'd love feedback on it!

[flavorjones]

272 was merged and I think that addresses Rails 7 support. I cherry-picked some of the commits from this PR there, so thank you!

[kbrock]

thanks @pfeiffer for getting this conversation going. really appreciate it