active_record finders ignores STI class

[sfcgeorge]

If single table inheritance is used on the back end, hyper-mesh won't instantiate the correct subclass, only ever the parent class.

# field.string :type
class User < ActiveRecord
  def self.meaningful
    find(42)
  end
end

class Employer < User
end

Employer.create.id # 42
Employer.meaningful.class #=> User (should be Employer)
User.meaningful.class #=> User (should also be Employer!)

The AR method becomes should also be added to allow switching class:

User.find.becomes(User)
Employer.find.becomes(User)

Basically Rails always automatically converts into the most specific STI sub-class.

Fairly sure this will fix #38 too

[sfcgeorge]

I've found 2 places in the code where the class is changing to the superclass.

Wrong self in finders

hyper-mesh/lib/reactive_record/active_record/class_methods.rb

Line 45 in 5e81185

base_class.instance_eval {ReactiveRecord::Base.find(self, primary_key, id)}

Instance eval changes self so you're not passing in the self you might think. Easy fix:

def find(id)
  base_class.instance_eval {ReactiveRecord::Base.find(self, primary_key, id)}
end

base_class used unnecessarily

hyper-mesh/lib/reactive_record/active_record/class_methods.rb

Lines 5 to 18 in 5e81185

	def base_class
	unless self < Base
	raise ActiveRecordError, "#{name} doesn't belong in a hierarchy descending from ActiveRecord"
	end
	if superclass == Base || superclass.abstract_class?
	self
	else
	superclass.base_class
	end
	end

I assume there's a reason why base_class is used all over the place. But it does what it says and gives you the superclass so you loose access to the methods in the actual model you're trying to use. I don't see why base_class is necessary at all, as with inheritance all the superclass methods are available in the child. And if you're able to call base_class then you must have inherited from Base so there's no point checking this condition.

Maybe there's another kind of AR internals object that you can end up with where it's necessary to call superclass to get something useful? But checking for those edge cases might be better than always calling base_class.

Redefining base class to return self seems to work fine. Async loading and saving on the client works in STI models.

Alternative - remove calls to base_class

hyper-mesh/lib/reactive_record/active_record/reactive_record/base.rb

Line 78 in 5e81185

model = model.base_class

Commenting out model = model.base_class has the same effect as redefining it. So if there are places when base_class is actually needed to do it's thing then they could be left in and everywhere else removed.

Alternative - use always assignment

hyper-mesh/lib/reactive_record/active_record/reactive_record/base.rb

Line 97 in 5e81185

record.ar_instance ||= infer_type_from_hash(model, record.attributes).new(record)

Without the base_class fix the model is initially memoized as User and then the server correctly returns Employer but it's never updated. Changing this to a regular assignment updates the class correctly but doesn't help with the initial page load and is probably undesirable for caching reasons.

TLDR monkeypatch

The below patch got STI models working correctly for me. The model starts of as Employer and stays that way. So far it doesn't seem to have broken anything else but probably has as I don't know what the reason behind base_class really is.

if RUBY_ENGINE == "opal"
  module ActiveRecord
    module ClassMethods
      def base_class
        unless self < Base
          raise ActiveRecordError, "#{name} doesn't descend from ActiveRecord"
        end

        if superclass == Base || superclass.abstract_class?
          self
        elsif respond_to?(:each)
          superclass.base_class
        else
          self
        end
      end

      def find(id)
        clss = self # instance_eval changes self
        base_class.instance_eval do
          ReactiveRecord::Base.find(clss, primary_key, id)
        end
      end
    end
  end
end