-
Notifications
You must be signed in to change notification settings - Fork 213
Comment on Clarifying Business Logic and Writing Clean Code
Having got completed frustrated trying to format this reply in both Disqus and then Slack, I have resorted to adding a wiki page on the Localsupport wiki :-]
When I was reading Sam's article I had role/permissioning ringing in my ears too, but on second thoughts I wonder whether we've managed to conflate the workflow "how a user becomes an admin" with permissioning ? And then there's also the transitions (what happens during state change)
I played a little with the workflow gem to describe the state machine.
class User
include Workflow
workflow do
state :new do
event :email_confirmed, :transitions_to => :site_user
end
state :site_user do
event :apply_org_admin :transition_to => :awaiting_org_admin_review
end
state :awaiting_org_admin_review do
event :review, :transitions_to => :being_reviewed
end
state :being_reviewed do
event :accept, :transitions_to => :org_admin
event :reject, :transitions_to => :site_user
event :banned, :transitions_to => :banned
end
state :org_admin
state :banned
end
end
Providing,
user = User.new
user.site_user? # => false
user.awaiting_org_admin_review? # => false
user.new? # => true
user.email_confirmed!
user.site_user? # => true
user.review!
puts user.current_state # => being_reviewed
user.accept! # or user.reject! or user.banned!
With model changes,
class User
def new
puts "send confirmation email"
end
def email_confirmed
puts "send welcome as site user email"
end
def awaiting_org_admin_review?(org='')
# check state && org
end
def apply_org_admin(org= '')
# store org somewhere for later
puts "send email to user"
puts "sned email to site_admin"
end
def reject
puts "send email to the author here explaining the reason for the rejection"
end
end
Does looking at it like this make the tests easier ? rspec tests the the workflow and permissioning separately.
Cumcumber, tests the combinations of of both ?
"As a site user when I apply for org_admin I should get an email telling me so"
"As site_admin when site_user applies for org_admin I should get the application email"
As a site_user awaiting_org_admin_review I shouldn't be offered further request buttons
...
It moves the workflow into the model, but maybe this is where it should sit?
I don't think there is such an array of different resource type and users to warrant using cancan at the moment (I may be wrong, and I'm not really certain of the requirements), but workflow does make it easy for me to understand the state machine.
It has already lead me to ask myself, Can a site user, apply for admin_org status on several organisations simultaneously?, How do I stop a persistent user from re-applying ? (I slipped in a new state banned)., I additionally introduced a level 1 state Site_user, so that new users (users who's email address has not yet been confirmed) and therefore should not be able to apply for org_admin status.
The ultimate solution would be to be able to describe the business logic and get it agreed first in Cucumber, and then auto-magically generate the state machine and permissioning ruby code, from that. Maybe then would be a time to have an html permissioning matrix for the customer to edit, however I'm not sure that the business logic can be entirely soft, since even using Cucumber, we're inventing our own DSL. Maybe a useful half-way-measure would be to reveal the rules (described in the DSL) somewhere for the site admin to read.
What would the business logic look like ?
The public can,
- read the public content
- register and become a new user
A New user can,
- login and logout
- edit own profile
- confirm own email address and become a Site User
- read site content
- cancel own membership
A Site user can,
- apply as volunteer for job
- apply for one organisation admin status and become Site user awaiting organisation admin review
The Site admin can,
- edit all content
- review users awaiting organisation admin review by,
- approving request
- rejecting request
- cancelling review
- banning user
...