gocardless / statesman

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

How do you name, and where do you put your state machine and transition files in a Rails app? #523

Closed aaronkelton closed 10 months ago

aaronkelton commented 12 months ago

I ran into some errors and gotchas while setting up Statesman for the first time. I followed the readme in a top-down approach: first creating the OrderStateMachine class in a file named order_state_machine.rb. I just put it under the "models" directory, right next to order.rb. After linking it to my model, I created the OrderTransition class, but made the mistake of naming my file order_transitions.rb. At this point in time I was told I could start working with my state machine, but because I made transitions plural, I got an error that took longer than I care to admit discovering.

uninitialized constant OrderTransition (NameError)

machine = OrderStateMachine.new(order, transition_class: OrderTransition)
                                                         ^^^^^^^^^^^^^^^
Did you mean?  OrderTransitions

Putting these two new files right next to my model felt wrong, like I should have put it in a directory structure under models > statesman > order or similar.

Back to the error above. After renaming the file to the singular _ordertransition.rb I was indeed able to start working with my state machine... until Order.in_state(:cancelled)

An error occurred when inspecting the object: #<ActiveRecord::StatementInvalid:"PG::UndefinedTable: ERROR:  relation \"order_transitions\" does not exist\nLINE 1: SELECT \"orders\".* FROM \"orders\" LEFT OUTER JOIN order_tran...\n

I glanced back at the transitions file and noticed the class inherits from ActiveRecord::Base, so of course I needed a migration. But it isn't until the Persistence section later on that we generate the transition model and the migration.

Ok, so what's your point?

Sharing my experience, I feel like the readme should start with the migration and model generation. This would answer the question to the title of this issue.

One place I've seen this done well is rspec-rails, where they mention Installation and Usage first before the DSL.

Sorry I don't have a PR with a suggested change. I just wanted to share my first impression going through the setup.

aaronkelton commented 12 months ago

Here's another gotcha I came across in the readme:

Your transition class should include Statesman::Adapters::ActiveRecordTransition if you're using the ActiveRecord adapter.

But I'm also on Rails 7, so it sounds like the best way to go for metadata is using t.json :metadata, default: {}. However, then the second bullet point says to remove it:

Remove the include Statesman::Adapters::ActiveRecordTransition statement from your transition model.

Does having the json/jsonb column type necessitate we remove this module's inclusion, which is otherwise required with the AR adapter?

aaronkelton commented 12 months ago

And perhaps since the statesman-events gem is unmaintained and advised against using, then the section on events should be removed from this readme?

ingemar commented 10 months ago

Hi Aaron,

I hope that helps you.