Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Dynamic state machine proposal #1743

Closed
wants to merge 12 commits into from

4 participants

@radar
Collaborator

This issue should address #1418.

See the code in this gist to see what it looks like. As I explain there:

The code works by calling the checkout_flow method which clears all current transitions. The go_to_state method then defines transitions for each step of the process. The first call will transition from cart, and subsequent calls will transition from the state before it.

If a state has a condition on it, then a record of all previous states are kept until there is a state that has no condition. Once this happens, the code will define state transitions for all state paths for the states in between. For instance, the follow transitions will be defined between delivery and complete:

  • Delivery to payment
  • Delivery to confirm
  • Delivery to complete
  • Payment to confirm
  • Payment to complete
  • Confirm to complete

The remove method will remove a transition that has previously been either automatically or manually defined. In the case of this state machine, the delivery to confirm transition has been automatically defined, but we don't want it. So we get rid of it. As in the above example, the "delivery to confirm" transition will be removed.

What this will allow for is for dynamic state machine definitions for Order objects by re-defining the checkout_flow block on them and using the new state definition API.

This issue is just opening that implementation up for consideration. Does it look sensible? Does it fit the use cases? That kind of thing.

Thoughts?

@radar radar closed this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@radar radar closed this in fb59527
@radar radar referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@radar radar reopened this
@radar radar commented on the diff
core/app/models/spree/order.rb
@@ -450,6 +413,14 @@ def payment_method
end
end
+ def process_payments!
@radar Collaborator
radar added a note

This just moves the begin/rescue logic out of the state machine and into this method, which I think is neater.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
core/app/models/spree/order.rb
@@ -576,13 +547,13 @@ def require_email
end
def has_available_shipment
- return unless :address == state_name.to_sym
+ return unless address?
@radar Collaborator
radar added a note

mmm, query methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@radar radar commented on the diff
core/app/models/spree/order/checkout.rb
((3 lines not shown))
+ module Checkout
+ def self.included(klass)
+ klass.class_eval do
+ cattr_accessor :next_event_transitions
+ cattr_accessor :previous_states
+ cattr_accessor :checkout_flow
+
+ def self.checkout_flow(&block)
+ if block_given?
+ @checkout_flow = block
+ else
+ @checkout_flow
+ end
+ end
+
+ def self.define_state_machine!
@radar Collaborator
radar added a note

This method is the magic. This is called when Core::Engine does a to_prepare. This means that you can re-define the next state transitions as many times as you wish and every time to_prepare is called, the state machine will re-define the transitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
core/app/models/spree/order/checkout.rb
((13 lines not shown))
+ else
+ @checkout_flow
+ end
+ end
+
+ def self.define_state_machine!
+ @machine ||= begin
+ self.previous_states = [:cart]
+ instance_eval(&checkout_flow)
+ klass = self
+
+ state_machine :state, :initial => :cart do
+ klass.next_event_transitions.each { |t| transition(t.merge(:on => :next)) }
+
+ # Persist the state on the order
+ after_transition do |order|
@radar Collaborator
radar added a note

I am not sure if this is actually required any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
core/app/models/spree/order/checkout.rb
((55 lines not shown))
+ end
+
+ before_transition :to => :delivery, :do => :remove_invalid_shipments!
+
+ after_transition :to => :complete, :do => :finalize!
+ after_transition :to => :delivery, :do => :create_tax_charge!
+ after_transition :to => :payment, :do => :create_shipment!
+ after_transition :to => :resumed, :do => :after_resume
+ after_transition :to => :canceled, :do => :after_cancel
+ end
+ end
+ end
+
+ def self.go_to_state(name, options={})
+ if options[:if]
+ previous_states.each do |state|
@radar Collaborator
radar added a note

I thought about moving this out into its own method but decided against it because I am lazy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@radar radar commented on the diff
core/app/models/spree/order/checkout.rb
((67 lines not shown))
+
+ def self.go_to_state(name, options={})
+ if options[:if]
+ previous_states.each do |state|
+ add_transition({:from => state, :to => name}.merge(options))
+ end
+ self.previous_states << name
+ else
+ previous_states.each do |state|
+ add_transition({:from => state, :to => name}.merge(options))
+ end
+ self.previous_states = [name]
+ end
+ end
+
+ def self.remove_transition(options={})
@radar Collaborator
radar added a note

This was previously called remove, but I've renamed it to remove_transition so that there isn't a "mysterious" remove method on the Order class, and so that it fits more into the same theme as find_transition and add_transition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@radar radar commented on the diff
core/app/models/spree/order/checkout.rb
((79 lines not shown))
+ end
+ end
+
+ def self.remove_transition(options={})
+ if transition = find_transition(options)
+ self.next_event_transitions.delete(transition)
+ end
+ end
+
+ def self.find_transition(options={})
+ self.next_event_transitions.detect do |transition|
+ transition[options[:from].to_sym] == options[:to].to_sym
+ end
+ end
+
+ def self.next_event_transitions
@radar Collaborator
radar added a note

This used to be called transitions. Same reasons for renaming the remove method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@radar
Collaborator

Just a note: this doesn't actually change the checkout breadcrumbs yet. I've not yet begun work on that, but will probably knock it off tomorrow.

radar added some commits
@radar radar Implemented checkout_steps method
This replaces the code duplication in checkout_helper to determine what steps to show for an order during the checkout process
256d28f
@radar radar checkout_steps should always return a string b5b4e54
@radar
Collaborator

Checkout breadcrumbs are in @256d28f. The build is passing on Travis.

@radar
Collaborator

Kicking the tyres some more on this today.

@radar
Collaborator

I can't think of anything more to do on this. Can @BDQ, @cmar, @LBRapid, @joneslee85 and/or @schof look this over and see if they can break it?

@joneslee85
Collaborator

@radar Will spend sometime on this this weekend, see if I could break it

@schof
Owner

I'm ok to merge this as long as we update the documentation and release notes adequately (plus resolve conflicts of course.)

@radar radar referenced this pull request from a commit
@radar radar Introduced customizable checkout flow
Merges #1743
fa1d66c
@radar
Collaborator

Merged to master with @fa1d66c. Tests are running on master now. When they're good, I'll update the Checkout Customization guide appropriately.

@radar radar closed this
@gsnarawat

I want to remove the Shipping page validations.. I used this code but still there is validations exist. plz help.

checkout_flow do
go_to_state :address, :if => lambda { |order| order.payment_required? }
go_to_state :payment, :if => lambda { |order| order.payment_required? }
go_to_state :confirm, :if => lambda { |order| order.confirmation_required? }
go_to_state :complete
remove_transition :from => :delivery, :to => :confirm
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 10, 2012
  1. @radar
  2. @radar
  3. @radar

    Implemented checkout_steps method

    radar authored
    This replaces the code duplication in checkout_helper to determine what steps to show for an order during the checkout process
  4. @radar
Commits on Jul 13, 2012
  1. @radar
  2. @radar

    define_state_machine! needs to be called after to_prepare hooks have …

    radar authored
    …fired so that state machine overrides work
  3. @radar
  4. @radar

    Call create_shipment! when order transitions from delivery, rather th…

    radar authored
    …an into payment
    
    The latter assumes that the delivery step immediately precedes the payment step, which may not be true.
  5. @radar
  6. @radar
Commits on Jul 31, 2012
  1. @radar
  2. @radar

    Make core redefine the state machine on every reload

    radar authored
    This prevents state methods from magically disappearing
This page is out of date. Refresh to see the latest.
View
3  .travis.yml
@@ -23,8 +23,9 @@ notifications:
- "irc.freenode.org#spree"
branches:
only:
+ - flexible-checkout-flow
- 1-0-stable
- - auth-take-two
+ - 1-1-stable
- master
rvm:
- 1.8.7
View
10 core/app/controllers/spree/checkout_controller.rb
@@ -6,6 +6,7 @@ class CheckoutController < BaseController
ssl_required
before_filter :load_order
+ before_filter :ensure_valid_state
before_filter :associate_user
rescue_from Spree::Core::GatewayError, :with => :rescue_from_spree_gateway_error
@@ -37,6 +38,15 @@ def update
end
private
+ def ensure_valid_state
+ if params[:state] && params[:state] != 'cart' &&
+ !@order.checkout_steps.include?(params[:state])
+ params[:state] == 'cart'
+ @order.state = 'cart'
+ redirect_to checkout_path
+ end
+ end
+
def load_order
@order = current_order
redirect_to cart_path and return unless @order and @order.checkout_allowed?
View
6 core/app/helpers/spree/checkout_helper.rb
@@ -1,11 +1,7 @@
module Spree
module CheckoutHelper
def checkout_states
- if @order.payment and @order.payment.payment_method.payment_profiles_supported?
- %w(address delivery payment confirm complete)
- else
- %w(address delivery payment complete)
- end
+ @order.checkout_steps
end
def checkout_progress
View
85 core/app/models/spree/order.rb
@@ -2,6 +2,16 @@
module Spree
class Order < ActiveRecord::Base
+ include Checkout
+ checkout_flow do
+ go_to_state :address
+ go_to_state :delivery
+ go_to_state :payment, :if => lambda { |order| order.payment_required? }
+ go_to_state :confirm, :if => lambda { |order| order.confirmation_required? }
+ go_to_state :complete
+ remove_transition :from => :delivery, :to => :confirm
+ end
+
token_resource
attr_accessible :line_items, :bill_address_attributes, :ship_address_attributes, :payments_attributes,
@@ -54,54 +64,6 @@ class Order < ActiveRecord::Base
class_attribute :update_hooks
self.update_hooks = Set.new
- # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
- state_machine :initial => 'cart', :use_transactions => false do
-
- event :next do
- transition :from => 'cart', :to => 'address'
- transition :from => 'address', :to => 'delivery'
- transition :from => 'delivery', :to => 'payment', :if => :payment_required?
- transition :from => 'delivery', :to => 'complete'
- transition :from => 'confirm', :to => 'complete'
-
- # note: some payment methods will not support a confirm step
- transition :from => 'payment', :to => 'confirm',
- :if => Proc.new { |order| order.payment_method && order.payment_method.payment_profiles_supported? }
-
- transition :from => 'payment', :to => 'complete'
- end
-
- event :cancel do
- transition :to => 'canceled', :if => :allow_cancel?
- end
- event :return do
- transition :to => 'returned', :from => 'awaiting_return'
- end
- event :resume do
- transition :to => 'resumed', :from => 'canceled', :if => :allow_resume?
- end
- event :authorize_return do
- transition :to => 'awaiting_return'
- end
-
- before_transition :to => 'complete' do |order|
- begin
- order.process_payments!
- rescue Core::GatewayError
- !!Spree::Config[:allow_checkout_on_gateway_error]
- end
- end
-
- before_transition :to => 'delivery', :do => :remove_invalid_shipments!
-
- after_transition :to => 'complete', :do => :finalize!
- after_transition :to => 'delivery', :do => :create_tax_charge!
- after_transition :to => 'payment', :do => :create_shipment!
- after_transition :to => 'resumed', :do => :after_resume
- after_transition :to => 'canceled', :do => :after_cancel
-
- end
-
def self.by_number(number)
where(:number => number)
end
@@ -156,6 +118,11 @@ def payment_required?
total.to_f > 0.0
end
+ # If true, causes the confirmation step to happen during the checkout process
+ def confirmation_required?
+ payment_method && payment_method.payment_profiles_supported?
+ end
+
# Indicates the number of items in the order
def item_count
line_items.sum(:quantity)
@@ -361,7 +328,6 @@ def create_shipment!
:address => self.ship_address,
:inventory_units => self.inventory_units}, :without_protection => true)
end
-
end
def outstanding_balance
@@ -383,10 +349,6 @@ def credit_cards
CreditCard.scoped(:conditions => { :id => credit_card_ids })
end
- def process_payments!
- ret = payments.each(&:process!)
- end
-
# Finalizes an in progress order after checkout is complete.
# Called after transition to complete state when payments will have been processed
def finalize!
@@ -450,6 +412,14 @@ def payment_method
end
end
+ def process_payments!
@radar Collaborator
radar added a note

This just moves the begin/rescue logic out of the state machine and into this method, which I think is neater.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ begin
+ payments.each(&:process!)
+ rescue Core::GatewayError
+ !!Spree::Config[:allow_checkout_on_gateway_error]
+ end
+ end
+
def billing_firstname
bill_address.try(:firstname)
end
@@ -473,6 +443,10 @@ def merge!(order)
order.destroy
end
+ def has_step?(step)
+ checkout_steps.include?(step)
+ end
+
private
def link_by_email
self.email = user.email if self.user and not user.anonymous?
@@ -576,13 +550,14 @@ def require_email
end
def has_available_shipment
- return unless :address == state_name.to_sym
+ return unless has_step?("delivery")
+ return unless address?
return unless ship_address && ship_address.valid?
errors.add(:base, :no_shipping_methods_available) if available_shipping_methods.empty?
end
def has_available_payment
- return unless :delivery == state_name.to_sym
+ return unless delivery?
errors.add(:base, :no_payment_methods_available) if available_payment_methods.empty?
end
View
124 core/app/models/spree/order/checkout.rb
@@ -0,0 +1,124 @@
+module Spree
+ class Order < ActiveRecord::Base
+ module Checkout
+ def self.included(klass)
+ klass.class_eval do
+ cattr_accessor :next_event_transitions
+ cattr_accessor :previous_states
+ cattr_accessor :checkout_flow
+ cattr_accessor :checkout_steps
+
+ def self.checkout_flow(&block)
+ if block_given?
+ @checkout_flow = block
+ else
+ @checkout_flow
+ end
+ end
+
+ def self.define_state_machine!
@radar Collaborator
radar added a note

This method is the magic. This is called when Core::Engine does a to_prepare. This means that you can re-define the next state transitions as many times as you wish and every time to_prepare is called, the state machine will re-define the transitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.checkout_steps = []
+ self.next_event_transitions = []
+ self.previous_states = [:cart]
+ instance_eval(&checkout_flow)
+ klass = self
+
+ state_machine :state, :initial => :cart do
+ klass.next_event_transitions.each { |t| transition(t.merge(:on => :next)) }
+
+ # Persist the state on the order
+ after_transition do |order|
+ order.state = order.state
+ order.save
+ end
+
+ event :cancel do
+ transition :to => :canceled, :if => :allow_cancel?
+ end
+
+ event :return do
+ transition :to => :returned, :from => :awaiting_return
+ end
+
+ event :resume do
+ transition :to => :resumed, :from => :canceled, :if => :allow_resume?
+ end
+
+ event :authorize_return do
+ transition :to => :awaiting_return
+ end
+
+ before_transition :to => :complete do |order|
+ begin
+ order.process_payments!
+ rescue Spree::Core::GatewayError
+ !!Spree::Config[:allow_checkout_on_gateway_error]
+ end
+ end
+
+ before_transition :to => :delivery, :do => :remove_invalid_shipments!
+
+ after_transition :to => :complete, :do => :finalize!
+ after_transition :to => :delivery, :do => :create_tax_charge!
+ after_transition :to => :resumed, :do => :after_resume
+ after_transition :to => :canceled, :do => :after_cancel
+
+ after_transition :from => :delivery, :do => :create_shipment!
+ end
+ end
+
+ def self.go_to_state(name, options={})
+ self.checkout_steps[name] = options
+ if options[:if]
+ previous_states.each do |state|
+ add_transition({:from => state, :to => name}.merge(options))
+ end
+ self.previous_states << name
+ else
+ previous_states.each do |state|
+ add_transition({:from => state, :to => name}.merge(options))
+ end
+ self.previous_states = [name]
+ end
+ end
+
+ def self.remove_transition(options={})
@radar Collaborator
radar added a note

This was previously called remove, but I've renamed it to remove_transition so that there isn't a "mysterious" remove method on the Order class, and so that it fits more into the same theme as find_transition and add_transition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if transition = find_transition(options)
+ self.next_event_transitions.delete(transition)
+ end
+ end
+
+ def self.find_transition(options={})
+ self.next_event_transitions.detect do |transition|
+ transition[options[:from].to_sym] == options[:to].to_sym
+ end
+ end
+
+ def self.next_event_transitions
@radar Collaborator
radar added a note

This used to be called transitions. Same reasons for renaming the remove method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ @next_event_transitions ||= []
+ end
+
+ def self.checkout_steps
+ @checkout_steps ||= ActiveSupport::OrderedHash.new
+ end
+
+ def self.add_transition(options)
+ self.next_event_transitions << { options.delete(:from) => options.delete(:to) }.merge(options)
+ end
+
+ def checkout_steps
+ checkout_steps = []
+ # TODO: replace this with each_with_object once Ruby 1.9 is standard
+ self.class.checkout_steps.each do |step, options|
+ if options[:if]
+ next unless options[:if].call(self)
+ end
+ checkout_steps << step
+ end
+ checkout_steps.map(&:to_s)
+ end
+ end
+ end
+ end
+ end
+end
View
14 core/app/views/spree/shared/_order_details.html.erb
@@ -14,12 +14,14 @@
</div>
</div>
- <div class="columns alpha four">
- <h6><%= t(:shipping_method) %> <%= link_to "(#{t(:edit)})", checkout_state_path(:delivery) unless @order.completed? %></h6>
- <div class="delivery">
- <%= order.shipping_method.name %>
+ <% if @order.has_step?("delivery") %>
+ <div class="columns alpha four">
+ <h6><%= t(:shipping_method) %> <%= link_to "(#{t(:edit)})", checkout_state_path(:delivery) unless @order.completed? %></h6>
+ <div class="delivery">
+ <%= order.shipping_method.name %>
+ </div>
</div>
- </div>
+ <% end %>
<div class="columns omega four">
<h6><%= t(:payment_information) %> <%= link_to "(#{t(:edit)})", checkout_state_path(:payment) unless @order.completed? %></h6>
@@ -110,4 +112,4 @@
</tr>
<% end %>
</tfoot>
-</table>
+</table>
View
4 core/lib/spree/core/engine.rb
@@ -10,11 +10,14 @@ class Engine < ::Rails::Engine
config.autoload_paths += %W(#{config.root}/lib)
def self.activate
+ Spree::Order.define_state_machine!
end
config.to_prepare &method(:activate).to_proc
config.after_initialize do
+ Spree::Order.define_state_machine!
+
ActiveSupport::Notifications.subscribe(/^spree\./) do |*args|
event_name, start_time, end_time, id, payload = args
Activator.active.event_name_starts_with(event_name).each do |activator|
@@ -22,7 +25,6 @@ def self.activate
activator.activate(payload)
end
end
-
end
# We need to reload the routes here due to how Spree sets them up.
View
141 core/spec/models/order/checkout_spec.rb
@@ -0,0 +1,141 @@
+require 'spec_helper'
+
+describe Spree::Order do
+ let(:order) { Spree::Order.new }
+ context "with default state machine" do
+ it "has the following transitions" do
+ transitions = [
+ { :address => :delivery },
+ { :delivery => :payment },
+ { :payment => :confirm },
+ { :confirm => :complete },
+ { :payment => :complete },
+ { :delivery => :complete }
+ ]
+ transitions.each do |transition|
+ transition = Spree::Order.find_transition(:from => transition.keys.first, :to => transition.values.first)
+ transition.should_not be_nil
+ end
+ end
+
+ it "does not have a transition from delivery to confirm" do
+ transition = Spree::Order.find_transition(:from => :delivery, :to => :confirm)
+ transition.should be_nil
+ end
+
+ context "#checkout_steps" do
+ context "when confirmation not required" do
+ before do
+ order.stub :confirmation_required? => false
+ order.stub :payment_required? => true
+ end
+
+ specify do
+ order.checkout_steps.should == %w(address delivery payment complete)
+ end
+ end
+
+ context "when confirmation required" do
+ before do
+ order.stub :confirmation_required? => true
+ order.stub :payment_required? => true
+ end
+
+ specify do
+ order.checkout_steps.should == %w(address delivery payment confirm complete)
+ end
+ end
+
+ context "when payment not required" do
+ before { order.stub :payment_required? => false }
+ specify do
+ order.checkout_steps.should == %w(address delivery complete)
+ end
+ end
+
+ context "when payment required" do
+ before { order.stub :payment_required? => true }
+ specify do
+ order.checkout_steps.should == %w(address delivery payment complete)
+ end
+ end
+ end
+
+ it "starts out at cart" do
+ order.state.should == "cart"
+ end
+
+ it "transitions to address" do
+ order.next!
+ order.state.should == "address"
+ end
+
+ context "from address" do
+ before do
+ order.state = 'address'
+ end
+
+ it "transitions to delivery" do
+ order.stub(:has_available_payment)
+ order.next!
+ order.state.should == "delivery"
+ end
+ end
+
+ context "from delivery" do
+ before do
+ order.state = 'delivery'
+ end
+
+ context "with payment required" do
+ before do
+ order.stub :payment_required? => true
+ end
+
+ it "transitions to payment" do
+ order.next!
+ order.state.should == 'payment'
+ end
+ end
+
+ context "without payment required" do
+ before do
+ order.stub :payment_required? => false
+ end
+
+ it "transitions to complete" do
+ order.next!
+ order.state.should == "complete"
+ end
+ end
+ end
+
+ context "from payment" do
+ before do
+ order.state = 'payment'
+ end
+
+ context "with confirmation required" do
+ before do
+ order.stub :confirmation_required? => true
+ end
+
+ it "transitions to confirm" do
+ order.next!
+ order.state.should == "confirm"
+ end
+ end
+
+ context "without confirmation required" do
+ before do
+ order.stub :confirmation_required? => false
+ end
+
+ it "transitions to complete" do
+ order.next!
+ order.state.should == "complete"
+ end
+ end
+ end
+ end
+end
View
12 core/spec/models/order_spec.rb
@@ -134,6 +134,7 @@ def compute(computable)
end
context "#next!" do
+ before { order.stub(:require_email => false) }
context "when current state is confirm" do
before { order.state = "confirm" }
it "should finalize order when transitioning to complete state" do
@@ -164,13 +165,10 @@ def compute(computable)
end
it "should complete the order" do
- pending
- order.next
- order.state.should == "complete"
- end
-
+ order.next
+ order.state.should == "complete"
+ end
end
-
end
end
context "when current state is address" do
@@ -1006,7 +1004,7 @@ def compute(computable)
context "#add_variant" do
it "should update order totals" do
- order = Spree::Order.create!
+ order = Spree::Order.create
order.item_total.to_f.should == 0.00
order.total.to_f.should == 0.00
Something went wrong with that request. Please try again.