Implement a state lock version to prevent race conditions in checkout…

… controller. refs spree#4190

Fix broken specs.

Add regression test for state lock version. Fixes spree#4190

core/config/locales/en.yml

@@ -920,6 +920,7 @@ en:
     or_over_price: ! '%{price} or over'
     order: Order
     order_adjustments: Order adjustments
+    order_already_updated: The order has already been updated.
     order_approved: Order approved
     order_canceled: Order canceled
     order_details: Order Details

core/db/migrate/20141021194502_add_state_lock_version_to_order.rb

@@ -0,0 +1,5 @@
+class AddStateLockVersionToOrder < ActiveRecord::Migration
+  def change
+    add_column :spree_orders, :state_lock_version, :integer, default: 0, null: false
+  end
+end

core/lib/spree/permitted_attributes.rb

@@ -36,7 +36,10 @@ module PermittedAttributes
       state: [:name, :abbr]
     ]
 
-    @@checkout_attributes = [:email, :use_billing, :shipping_method_id, :coupon_code, :special_instructions]
+    @@checkout_attributes = [
+      :coupon_code, :email, :shipping_method_id, :special_instructions,
+      :state_lock_version, :use_billing
+    ]
 
     @@customer_return_attributes = [:stock_location_id, return_items_attributes: [:id, :inventory_unit_id, :return_authorization_id, :returned, :pre_tax_amount, :acceptance_status, :exchange_variant_id]]
 

frontend/app/controllers/spree/checkout_controller.rb

@@ -73,6 +73,16 @@ def load_order_with_lock
         @order = current_order(lock: true)
         redirect_to spree.cart_path and return unless @order
 
+        @order.with_lock do
+          if params[:order] && params[:order][:state_lock_version]
+            unless @order.state_lock_version == params[:order][:state_lock_version].to_i
+              flash[:error] = Spree.t(:order_already_updated)
+              redirect_to checkout_state_path(@order.state) and return
+            end
+            @order.increment!(:state_lock_version)
+          end
+        end
+
         if params[:state]
           redirect_to checkout_state_path(@order.state) if @order.can_go_to_state?(params[:state]) && !skip_state_validation?
           @order.state = params[:state]

frontend/app/views/spree/checkout/edit.html.erb

@@ -1,11 +1,11 @@
-<div id="checkout" data-hook>  
+<div id="checkout" data-hook>
   <%= render :partial => 'spree/shared/error_messages', :locals => { :target => @order } %>
-  
+
   <div class="row" data-hook="checkout_header">
     <h1 class="columns three alpha" data-hook="checkout_title"><%= Spree.t(:checkout) %></h1>
     <div class="columns thirteen omega" data-hook="checkout_progress"><%= checkout_progress %></div>
   </div>
-  
+
   <div class="row" data-hook="checkout_content">
     <div class="columns <%= if @order.state != 'confirm' then 'alpha twelve' else 'alpha omega sixteen' end %>" data-hook="checkout_form_wrapper">
       <%= form_for @order, :url => update_checkout_path(@order.state), :html => { :id => "checkout_form_#{@order.state}" } do |form| %>
@@ -15,9 +15,10 @@
             <%= form.text_field :email %>
           </p>
         <% end %>
+        <%= form.hidden_field :state_lock_version %>
         <%= render @order.state, :form => form %>
       <% end %>
-    </div>    
+    </div>
     <% if @order.state != 'confirm' %>
       <div id="checkout-summary" data-hook="checkout_summary_box" class="columns omega four">
         <%= render :partial => 'summary', :locals => { :order => @order } %>

frontend/spec/controllers/spree/checkout_controller_spec.rb

@@ -56,9 +56,9 @@
       end
 
       it "should associate the order with a user" do
-        order.user = nil
+        order.update_column :user_id, nil
         order.should_receive(:associate_user!).with(user)
-        spree_get :edit, {}, :order_id => 1
+        spree_get :edit, {}, order_id: 1
       end
     end
   end
@@ -142,14 +142,12 @@ def spree_post_address
 
       context "with the order in the address state" do
         before do
-          order.update_column(:state, "address")
+          order.update_columns(ship_address_id: create(:address).id, state: "address")
           order.stub :user => user
         end
 
         context "with a billing and shipping address" do
           before do
-            order.ship_address = FactoryGirl.create(:address)
-
             @expected_bill_address_id = order.bill_address.id
             @expected_ship_address_id = order.ship_address.id
 
@@ -223,6 +221,26 @@ def spree_post_address
       end
     end
 
+    # Regression test for #4190
+    context "state_lock_version incorrect" do
+      before do
+        order.update_columns(state_lock_version: 2, state: "address")
+      end
+
+      it "redirects back to current state" do
+        spree_post :update, {
+            state: "address",
+            order: {
+              bill_address_attributes: order.bill_address.attributes.except("created_at", "updated_at"),
+              state_lock_version: 1,
+              use_billing: true
+            }
+        }
+        expect(response).to redirect_to spree.checkout_state_path('address')
+        expect(flash[:error]).to eq "The order has already been updated."
+      end
+    end
+
     context "when current_order is nil" do
       before { controller.stub :current_order => nil }