[RFC] Use a state machine to manage ApplicationDraft states

[cypher]

Previously, the state of an ApplicationDraft was controlled by setting
*_by attributes. While this works as long as there is a small number
of states, this gets complicated quickly if the possible states
increases, as well as managing transitions between states.

When an ApplicationDraft transitions from the 'draft' state to the
'applied' state by calling submit_application, it sets the
applied_at column to either a supplied DateTime, or the current
DateTime.

Missing/ToDo: The ready? method seems to be intended as a
guard to make sure an ApplicationDraft is ready to be submitted,
but it currently always returns false, and so it can't be used as a
transition guard.

[carpodaster]

I think the new default column is now aasm_state (which also should also remove the need for the column: :state option in the model). Can you change it for the less generic name? It could also be a string column instead of a text one, but I guess that makes little difference

[cypher]

I decided to go with state because a few of the controllers/helpers depend on a state method/attribute being available, and aasm doesn't care either way.

But sure, I can rename it, and add a state that delegates to aasm_state. Or would you prefer to rename the calls as well?

[cypher]

Also, in my experience, varchar offers no gains over text in PostgreSQL, but text has the upside that you'll never have to recast the column in case you need something more than e.g. 255 chars.

[carpodaster]

I decided to go with state because a few of the controllers/helpers depend on a state method/attribute being available

Ah, you're right. I thought the calls to state were all changed in favor of the aasm predicates. But in that case, it would even count as another argument against adding aasm as adding it doesn't fully hide away the underlying state implementation or state-holding data attributes.

[carpodaster]

Also, in my experience, varchar offers no gains over text in PostgreSQL

Ok cool. Yes, I have had my fair share of truncated strings on varchar cols, too, and while I doubt we will ever have states > 255 chars, let's stick with text!

[cypher]

Sorry, saw your comment too late. I'll revert 8f6eefe.

Actually, I was able to get rid of most of the state calls, instead calling e.g. applied? directly on a draft. The only place where state is still used is when it's displayed in a template (application_drafts_helper.rb, line 12)

[carpodaster]

Hey @cypher, thanks! I've made a few comments but on a more general level:

While this works as long as there is a small number of states, this gets complicated quickly if the possible states increases, as well as managing transitions between states.

The thing is: it's very unlikely that there will be more states than draft and applied. applied is the final state as the ApplicationDraft will have been transformed into an Application record by then.

With that in mind, I'm not really in favor of adding the external dependency of aasm while also increasing complexity (the submit_application(*) adds some magic for beginners / people not familiar with aasm)

Re: your presentation »it's never too late to add a finite state machine«: do you see any additional states for an ApplicationDraft heading our way?

[cypher]

TL;DR: A state machine adds little overhead, and provides stability and future extensibility.

I've started working on #144, and the way I'm currently implementing it is actually an additional state in ApplicationDraft. That is, I've added a mentor_signoff event, that transitions an ApplicationDraft from applied to signed_off, which in turn could then trigger the conversion of an ApplicationDraft into an Application. This also ensures that a signed off ApplicationDraft cannot return to a previous state due to a bug.

Additionally, something like a feedback loop might also be desirable at some point, that is, mentors could "reject" a draft, and return it to the applicants for additional editing. This could easily be implemented via the state machine, especially when it comes to notifying all the people involved.

In theory, you could even stop using an extra model for the draft, and keep everything in the Application model, and then simply use a state machine to manage the different validations at each stage (but that would most likely be quite an undertaking, and probably not worth it).

[carpodaster]

That is, I've added a mentor_signoff event, that transitions an ApplicationDraft from applied to signed_off

I buy it! Good stuff 😄

[alicetragedy]

yaaaaayyyyy 🎈