gocardless / statesman

A statesmanlike state machine library.
https://gocardless.com/blog/statesman/
MIT License
1.78k stars 163 forks source link

Namespaced Models #130

Closed DanielWright closed 9 years ago

DanielWright commented 9 years ago

Hi there,

I'm having some difficulty implementing Statesman with namespaced ActiveRecord models. The root cause, I've discovered, is the Stateman ActiveRecord adapter relies on the transition model's table_name exactly matching its has_many association to the parent model.

In the following example, this falls apart:

module MyNamespace
  class Foo < AR::Base
    def self.table_name_prefix
      "my_namespace_"
    end

    has_many :foo_transitions, class_name: "MyNamespace::FooTransition"

   def state_machine
     @_machine ||= Statesman::Machine.new(self, transition_class: MyNamespace::FooTransition)
   end
  end

  class FooTransition < AR::Base
    include Statesman::Adapters::ActiveRecordTransition

    def self.table_name_prefix
      "my_namespace_"
    end

    belongs_to :foo, class_name: 'MyNamespace::Foo',
      inverse_of: :foo_transitions
  end
end

$ bundle exec rails console
2.2.0 > MyNamespace::Foo.last.state_machine.history
NoMethodError: undefined method `my_namespace_foo_transitions' for #<MyNamespace::Foo id: 7>

I considered proposing a pull request that reflects on the parent model's associations and tries to infer the transitions association via tortured comparison with @transition_class.model_name. However, I realized even that inference would break down if the transition-class's table-name doesn't match the association for whatever reason.

I propose instead adding another configuration option to Statesman::Machine's initializer, something like association_name, which can be passed down to the adapter and sent to the parent model to retrieve the transitions collection. I'm working on a PR, but I'd appreciate any feedback on the idea before I go too far down that road.

Thanks!

isaacseymour commented 9 years ago

Hi @DanielWright, the second approach sounds sensible. I'd be in favour of keeping the current behaviour as default (it's correct 99% of the time), but having the association_name available as an override. Look forward to the PR!

greysteil commented 9 years ago

Closing now #131 is merged and included in a release. Thanks again @DanielWright !