Cleaner ActiveRecordQueries mixin

[isaacseymour]

Pass configuration to the mixin as arguments, and only define the three
documented public methods, moving the other private methods into a
separate, inaccessible object.

The advantage of this is:

• we can check the presence of the right associations when the module is included (i.e. app boot), rather than at first load
• only the documented methods get defined, the others (although private), are still defined as private methods on the model in the old approach

Disadvantages are:

• the code is in some sense less clear (I'd say marginally but...subjective)
• the include must be executed after the has_many - which is kinda icky style-wise (and potentially a breaking change)? Could argue that instead we should have an interface like

Statesman::Adapter::ActiveRecordQueries.define(self, **options)

(although that adds another indirection which hides the include more)

In the README I've defined this as a minor-level change because the old-style will still work. I think this is probably wrong now I'm writing about it, given that the check for the definition of the transition_class and initial_state class methods now runs when it's included, so if you had

class Order < ApplicationRecord
  include Statesman::Adapter::ActiveRecordQueries
  has_many :order_transitions
  def self.transition_class
    OrderTransition
  end

  def self.initial_state
    :initial
  end
end

it'll break.

Can either:

• change the code to not check for those two methods until in_state runs
• make this totally non-backwards compatible and add it in v5.

Interested to hear thoughts from @lawrencejones and whoever else is maintaining this these days 😄

[lawrencejones]

I like this, and think the advantages you list are pretty compelling. I'm not going to be able to properly review this now- grabbing a flight to Thailand for 2 weeks tomorrow morning!- but wanted to drop an early favourable opinion before I vanish, in case anyone reviews it in my absence.

[isaacseymour]

@lawrencejones I've rebased, and you're back from 🇹🇭. Wanna get this merged? 😄

[danwakefield]

Code looks ok, though not sure that I find the benefits meaningful.
Releasing with a Major bump makes most sense to me given it can break existing code.

[danwakefield]

You can remove .map(&:to_s) here since you know the define_ methods above have already done that.

[lawrencejones]

Looks like @isaacseymour might be busy now- going to leave that cleanup of .map(&:to_s) for him to follow-up in his own time.