Improving the test suite, refactoring the form and Checkout controller to use data injection instead of haxxx

app/assets/javascripts/darkswarm/controllers/checkout_controller.js.coffee

@@ -1,11 +1,15 @@
-angular.module("Checkout").controller "CheckoutCtrl", ($scope, $rootScope) ->
+angular.module("Checkout").controller "CheckoutCtrl", ($scope, $rootScope, order) ->
   $scope.require_ship_address = false
   $scope.shipping_method = -1
   $scope.payment_method = -1
   $scope.same_as_billing = true
+  $scope.order = order
 
+  $scope.require_ship_address = $scope.order.shipping_methods[$scope.order.shipping_method_id]
+ 
   $scope.shippingMethodChanged = ->
-    $scope.require_ship_address = $("#order_shipping_method_id_" + $scope.shipping_method).attr("data-require-ship-address")
+    $scope.require_ship_address = $scope.order.shipping_methods[$scope.order.shipping_method_id] 
+
 
   $scope.purchase = (event)->
     event.preventDefault()

app/views/shop/checkout/_form.html.haml

@@ -1,63 +1,63 @@
 %checkout{"ng-controller" => "CheckoutCtrl"}
   = f_form_for current_order, url: main_app.shop_update_checkout_path, html: {name: "checkout", id: "checkout_form"} do |f|
+
+    :javascript
+      angular.module('Checkout').value('order', #{render "shop/checkout/order"})
+    
     .large-12.columns
       %fieldset#details
         %legend Customer Details
         .row
           .large-6.columns
             = f.text_field :email 
-          = f.fields_for :bill_address do |ba|
+          = f.fields_for :bill_address, @order.bill_address do |ba|
             .large-6.columns
               = ba.text_field :phone 
-        = f.fields_for :bill_address do |ba|
+        = f.fields_for :bill_address, @order.bill_address do |ba|
           .row
             .large-6.columns
-              = ba.text_field :firstname
+              = ba.text_field :firstname, "ng-model" => "order.bill_address.firstname"
             .large-6.columns
-              = ba.text_field :lastname
+              = ba.text_field :lastname, "ng-model" => "order.bill_address.lastname"
 
         %fieldset
           %legend Billing Address
-          = f.fields_for :bill_address do |ba|
+          = f.fields_for :bill_address, @order.bill_address do |ba|
             .row
               .large-12.columns
                 = ba.text_field :address1, label: "Billing Address",
-                  "ng-model" => "billing_address1"
+                  "ng-model" => "order.bill_address.address1"
             .row
               .large-12.columns
                 = ba.text_field :address2,
-                  "ng-model" => "billing_address2"
+                  "ng-model" => "order.bill_address.address2"
             .row
               .large-6.columns
                 = ba.text_field :city,
-                  "ng-model" => "billing_city"
+                  "ng-model" => "order.bill_address.city"
 
               .large-6.columns
                 = ba.select :country_id, Spree::Country.order(:name).select([:id, :name]).map{|c| [c.name, c.id]},
-                  "ng-model" => "billing_country_id"
+                  {}, "ng-model" => "order.bill_address.country_id"
             .row
               .large-6.columns
                 = ba.select :state_id, Spree::State.order(:name).select([:id, :name]).map{|c| [c.name, c.id]},
-                  "ng-model" => "billing_state_id"
+                  "ng-model" => "order.bill_address.state_id"
               .large-6.columns.right
                 = ba.text_field :zipcode,
-                  "ng-model" => "billing_zipcode"
+                  "ng-model" => "order.bill_address.zipcode"
 
       %fieldset#shipping
-
         %legend Shipping
-        - checked_id = @order.shipping_method_id || current_distributor.shipping_methods.first.andand.id 
         - for ship_method, i in current_distributor.shipping_methods.uniq
           .row
             .large-12.columns
               = f.radio_button :shipping_method_id, ship_method.id,
                 text: ship_method.name,
-                "ng-init" => "shipping_method = #{checked_id}; shippingMethodChanged()",
-                "ng-model" => "shipping_method",
                 "ng-change" => "shippingMethodChanged()",
-                "data-require-ship-address" => ship_method.require_ship_address
+                "ng-model" => "order.shipping_method_id"
 
-        = f.fields_for :ship_address do |sa|
+        = f.fields_for :ship_address, @order.ship_address do |sa|
           #ship_address{"ng-show" => "require_ship_address"}
             %label
               = check_box_tag :same_as_billing, true, true, 
@@ -87,13 +87,13 @@
                   = sa.text_field :lastname
 
             #ship_address_hidden{"ng-show" => "same_as_billing"}
-              = sa.hidden_field :address1, "ng-value" => "billing_address1"
-              = sa.hidden_field :address2, "ng-value" => "billing_address2"
-              = sa.hidden_field :city, "ng-value" => "billing_city"
-              = sa.hidden_field :country_id, "ng-value" => "billing_country_id"
-              = sa.hidden_field :zipcode, "ng-value" => "billing_zipcode"
-              = sa.hidden_field :firstname, "ng-value" => "billing_firstname"
-              = sa.hidden_field :lastname, "ng-value" => "billing_lastname"
+              = sa.hidden_field :address1, "ng-value" => "order.bill_address.address1"
+              = sa.hidden_field :address2, "ng-value" => "order.bill_address.address2"
+              = sa.hidden_field :city, "ng-value" => "order.bill_address.city"
+              = sa.hidden_field :country_id, "ng-value" => "order.bill_address.country_id"
+              = sa.hidden_field :zipcode, "ng-value" => "order.bill_address.zipcode"
+              = sa.hidden_field :firstname, "ng-value" => "order.bill_address.firstname"
+              = sa.hidden_field :lastname, "ng-value" => "order.bill_address.lastname"
 
       %fieldset#payment
         %legend Payment Details
@@ -107,3 +107,4 @@
           .row{"ng-show" => "payment_method == #{method.id}"}
             .large-12.columns
               = render partial: "spree/checkout/payment/#{method.method_type}", :locals => { :payment_method => method }
+

app/views/shop/checkout/_order.rabl

@@ -0,0 +1,23 @@
+object current_order
+attributes :id, :email
+
+# Default to the first shipping method if none selected
+# We don't want to do this on the order, BUT
+# We can't set checked="checked" as Angular ignores it
+# So we default the value in the JSON representation of the order
+node :shipping_method_id do
+  current_order.shipping_method_id || current_order.distributor.shipping_methods.first.andand.id
+end
+
+child current_order.bill_address => :bill_address do
+  attributes :phone, :firstname, :lastname, :address1, :address2, :city, :country_id, :state_id, :zipcode
+end
+
+child current_order.ship_address => :ship_address do
+  attributes :phone, :firstname, :lastname, :address1, :address2, :city, :country_id, :state_id, :zipcode
+end
+
+# Format here is {id: require_ship_address}
+node :shipping_methods do
+  Hash[current_order.distributor.shipping_methods.collect { |method| [method.id, method.require_ship_address] }]
+end

spec/features/consumer/shopping/checkout_spec.rb

@@ -111,104 +111,129 @@ feature "As a consumer I want to check out my cart", js: true do
     end
   end
 
-  describe "logged in, distributor selected, order cycle selected, product in cart" do
-    let(:user) { create_enterprise_user }
-    before do
-      login_to_consumer_section
-      select_distributor
-      select_order_cycle
-      add_product_to_cart
-    end
-
-    describe "with shipping methods" do
-      let(:sm1) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow") }
-      let(:sm2) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue") }
+  # Run these tests both logged in and logged out!
+  [:in, :out].each do |auth_state|
+    describe "logged #{auth_state.to_s}, distributor selected, order cycle selected, product in cart" do
+      let(:user) { create_enterprise_user }
       before do
-        distributor.shipping_methods << sm1 
-        distributor.shipping_methods << sm2 
-        visit "/shop/checkout"
-      end
-      it "shows all shipping methods" do
-        page.should have_content "Frogs"
-        page.should have_content "Donkeys"
+        if auth_state == :in
+          login_to_consumer_section
+        end
+        select_distributor
+        select_order_cycle
+        add_product_to_cart
       end
 
-      it "doesn't show ship address forms when a shipping method wants no address" do
-        choose(sm2.name)
-        find("#ship_address").visible?.should be_false
-      end
-
-      context "When shipping method requires an address" do
+      describe "with shipping methods" do
+        let(:sm1) { create(:shipping_method, require_ship_address: true, name: "Frogs", description: "yellow") }
+        let(:sm2) { create(:shipping_method, require_ship_address: false, name: "Donkeys", description: "blue") }
         before do
-          choose(sm1.name)
-        end
-        it "shows the hidden ship address fields by default" do
-          check "Shipping address same as billing address?"
-          find("#ship_address_hidden").visible?.should be_true
-          find("#ship_address > div.visible").visible?.should be_false
-        end
-
-        it "shows ship address forms when 'same as billing address' is unchecked" do
-          uncheck "Shipping address same as billing address?"
-          find("#ship_address_hidden").visible?.should be_false
-          find("#ship_address > div.visible").visible?.should be_true
-        end
-      end
-
-      it "copies billing address to hidden shipping address fields" do
-        choose(sm1.name)
-        check "Shipping address same as billing address?"
-        fill_in "Billing Address", with: "testy"
-        within "#ship_address_hidden" do
-          find("#order_ship_address_attributes_address1").value.should == "testy"
-        end
-      end
-
-      describe "with payment methods" do
-        let(:pm1) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") }
-        let(:pm2) { create(:payment_method, distributors: [distributor]) }
-
-        before do
-          pm1 # Lazy evaluation of ze create()s
-          pm2
+          distributor.shipping_methods << sm1 
+          distributor.shipping_methods << sm2 
           visit "/shop/checkout"
         end
-
-        it "shows all available payment methods" do
-          page.should have_content pm1.name
-          page.should have_content pm2.name
+        it "shows all shipping methods" do
+          page.should have_content "Frogs"
+          page.should have_content "Donkeys"
         end
 
-        describe "Purchasing" do
-          it "re-renders with errors when we submit the incomplete form" do
-            choose sm2.name
-            click_button "Purchase"
-            current_path.should == "/shop/checkout"
-            page.should have_content "can't be blank"
+        it "doesn't show ship address forms when a shipping method wants no address" do
+          choose(sm2.name)
+          find("#ship_address").visible?.should be_false
+        end
+
+        context "When shipping method requires an address" do
+          before do
+            choose(sm1.name)
+          end
+          it "shows the hidden ship address fields by default" do
+            check "Shipping address same as billing address?"
+            find("#ship_address_hidden").visible?.should be_true
+            find("#ship_address > div.visible").visible?.should be_false
           end
 
-          it "renders errors on the shipping method where appropriate"
+          it "shows ship address forms when 'same as billing address' is unchecked" do
+            uncheck "Shipping address same as billing address?"
+            find("#ship_address_hidden").visible?.should be_false
+            find("#ship_address > div.visible").visible?.should be_true
+          end
+        end
 
-          it "takes us to the order confirmation page when we submit a complete form" do
-            choose sm2.name
-            choose pm1.name
-            within "#details" do
-              fill_in "First Name", with: "Will"
-              fill_in "Last Name", with: "Marshall"
-              fill_in "Billing Address", with: "123 Your Face"
-              select "Australia", from: "Country"
-              select "Victoria", from: "State"
-              fill_in "Customer E-Mail", with: "test@test.com"
-              fill_in "Phone", with: "0468363090"
-              fill_in "City", with: "Melbourne"
-              fill_in "Zip Code", with: "3066"
+        it "copies billing address to hidden shipping address fields" do
+          choose(sm1.name)
+          check "Shipping address same as billing address?"
+          fill_in "Billing Address", with: "testy"
+          within "#ship_address_hidden" do
+            find("#order_ship_address_attributes_address1").value.should == "testy"
+          end
+        end
+
+        describe "with payment methods" do
+          let(:pm1) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::PaymentMethod::Check") }
+          let(:pm2) { create(:payment_method, distributors: [distributor]) }
+
+          before do
+            pm1 # Lazy evaluation of ze create()s
+            pm2
+            visit "/shop/checkout"
+          end
+
+          it "shows all available payment methods" do
+            page.should have_content pm1.name
+            page.should have_content pm2.name
+          end
+
+          describe "Purchasing" do
+            it "re-renders with errors when we submit the incomplete form" do
+              choose sm2.name
+              click_button "Purchase"
+              current_path.should == "/shop/checkout"
+              page.should have_content "can't be blank"
+            end
+
+            it "renders errors on the shipping method where appropriate"
+
+            it "takes us to the order confirmation page when we submit a complete form" do
+              choose sm2.name
+              choose pm1.name
+              within "#details" do
+                fill_in "First Name", with: "Will"
+                fill_in "Last Name", with: "Marshall"
+                fill_in "Billing Address", with: "123 Your Face"
+                select "Australia", from: "Country"
+                select "Victoria", from: "State"
+                fill_in "Customer E-Mail", with: "test@test.com"
+                fill_in "Phone", with: "0468363090"
+                fill_in "City", with: "Melbourne"
+                fill_in "Zip Code", with: "3066"
+              end
+              click_button "Purchase"
+              page.should have_content "Your order has been processed successfully"
+            end
+
+            it "takes us to the order confirmation page when submitted with 'same as billing address' checked" do
+              choose sm1.name
+              choose pm1.name
+              within "#details" do
+                fill_in "First Name", with: "Will"
+                fill_in "Last Name", with: "Marshall"
+                fill_in "Billing Address", with: "123 Your Face"
+                select "Australia", from: "Country"
+                select "Victoria", from: "State"
+                fill_in "Customer E-Mail", with: "test@test.com"
+                fill_in "Phone", with: "0468363090"
+                fill_in "City", with: "Melbourne"
+                fill_in "Zip Code", with: "3066"
+              end
+              check "Shipping address same as billing address?"
+              click_button "Purchase"
+              page.should have_content "Your order has been processed successfully"
             end
-            click_button "Purchase"
-            page.should have_content "Your order has been processed successfully"
           end
         end
       end
     end
+    
   end
 end