Delegate to `visit` instead of calling `.id` directly

[bollard]

This is related to #348

Translation of an ActiveRecord::Base object to an integer should be defined once (i.e. in visit_ActiveRecord_base). This makes it easier for the case when a user does not want to query by id but some other field, for example persistent_id

[ernie]

:-1: on this. AR maps Base#id to whatever the primary key is.

[bollard]

And AR can still can continue to map Base#id to whatever it wants...

  def visit_ActiveRecord_Base(o, parent)
    if some_condition
      o.id # Call Base#id and retrieve primary key
    else
      # some thing else
    end
  end

But at the Squeel level I would hope this could be configurable.

Squeel is a useful abstraction away from Arel / AR which has allowed us to monkey patch bi-temporal support into Rails (for which this PR helps me continue to use). I very much hope this level of abstraction can be maintained to help support plugin developers

[ernie]

Monkey patching Squeel which monkey patches ActiveRecord which monkey patches everything hardly seems like a sustainable path forward for your app/library.

[bollard]

I agree it is certainly not an ideal situation to be in, however when making such invasive changes to Rails we were always going to have to patch a number of places in the internals.

This proposed patch is quite the opposite though - a small innocuous change which will have no noticeable impact to almost all of your users, but be incredibly useful to a small number of them and arguably make the internal API more consistent :smile: