From 5f571aad2f88a42a5274d618bcf6f19ab5847f4d Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sun, 24 Feb 2019 18:24:29 +0800
Subject: [PATCH 01/18] Set required ship address in order in spec

The ship address of the order is properly set in
ProxyOrder#initialise_order! through OrderFactory.

The source of these spec failures seem to be a matter of how the records
are set up for the tests.
---
 spec/models/proxy_order_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb
index 55687c264e..e8b4199816 100644
--- a/spec/models/proxy_order_spec.rb
+++ b/spec/models/proxy_order_spec.rb
@@ -78,7 +78,7 @@ xdescribe ProxyOrder, type: :model do
   describe "resume" do
     let!(:payment_method) { create(:payment_method) }
     let!(:shipment) { create(:shipment) }
-    let(:order) { create(:order_with_totals, shipments: [shipment]) }
+    let(:order) { create(:order_with_totals, ship_address: create(:address), shipments: [shipment]) }
     let(:proxy_order) { create(:proxy_order, order: order, canceled_at: Time.zone.now) }
     let(:order_cycle) { proxy_order.order_cycle }
 

From 0306cc7ca7d0b28261af2e6519ffc952d9b22eae Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 26 Feb 2019 15:49:38 +0800
Subject: [PATCH 02/18] Set shipping method when creating order for
 subscription

---
 app/services/order_factory.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/services/order_factory.rb b/app/services/order_factory.rb
index 85c887411a..9aa5c9fb05 100644
--- a/app/services/order_factory.rb
+++ b/app/services/order_factory.rb
@@ -15,6 +15,7 @@ class OrderFactory
     set_user
     build_line_items
     set_addresses
+    set_shipping_method
     create_payment
     @order
   end
@@ -65,6 +66,10 @@ class OrderFactory
     @order.update_attributes(attrs.slice(:bill_address_attributes, :ship_address_attributes))
   end
 
+  def set_shipping_method
+    @order.select_shipping_method(attrs[:shipping_method_id])
+  end
+
   def create_payment
     @order.update_distribution_charge!
     @order.payments.create(payment_method_id: attrs[:payment_method_id], amount: @order.reload.total)

From f42eb3dab1cbc72c97e89a0d69de9a86f2b1c7cf Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 26 Feb 2019 15:51:59 +0800
Subject: [PATCH 03/18] Create shipment when creating a order for a
 subscription

A spec has been added to check that the attributes for the order states
after "cart" ("address", "delivery", "payment") are retained if the
order is transitioned to completion, BUT currently this is passing
because it is the only shipping method available.
---
 app/services/order_factory.rb       |  6 ++++++
 spec/services/order_factory_spec.rb | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/app/services/order_factory.rb b/app/services/order_factory.rb
index 9aa5c9fb05..fc4bd5f424 100644
--- a/app/services/order_factory.rb
+++ b/app/services/order_factory.rb
@@ -15,8 +15,10 @@ class OrderFactory
     set_user
     build_line_items
     set_addresses
+    create_shipment
     set_shipping_method
     create_payment
+
     @order
   end
 
@@ -66,6 +68,10 @@ class OrderFactory
     @order.update_attributes(attrs.slice(:bill_address_attributes, :ship_address_attributes))
   end
 
+  def create_shipment
+    @order.create_proposed_shipments
+  end
+
   def set_shipping_method
     @order.select_shipping_method(attrs[:shipping_method_id])
   end
diff --git a/spec/services/order_factory_spec.rb b/spec/services/order_factory_spec.rb
index 672ac0a780..1aa28486bd 100644
--- a/spec/services/order_factory_spec.rb
+++ b/spec/services/order_factory_spec.rb
@@ -42,6 +42,19 @@ describe OrderFactory do
       expect(order.complete?).to be false
     end
 
+    it "retains address, delivery, and payment attributes until completion of the order" do
+      while !order.completed? do break unless order.next! end
+
+      order.reload
+
+      expect(order.customer).to eq customer
+      expect(order.shipping_method).to eq shipping_method
+      expect(order.payments.first.payment_method).to eq payment_method
+      expect(order.bill_address).to eq bill_address
+      expect(order.ship_address).to eq ship_address
+      expect(order.total).to eq 38.0
+    end
+
     context "when the customer does not have a user associated with it" do
       before { customer.update_attribute(:user_id, nil) }
 

From ce5691d08079234e9f3fa5e52c4857065b9e58b1 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 12 Mar 2019 13:06:16 +0800
Subject: [PATCH 04/18] Copy admin/orders#edit from Spree controller

This method needs to be updated to use custom order advance logic in
service.
---
 .../spree/admin/orders_controller_decorator.rb       | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb
index 4e727d98f2..930058dd3d 100644
--- a/app/controllers/spree/admin/orders_controller_decorator.rb
+++ b/app/controllers/spree/admin/orders_controller_decorator.rb
@@ -27,6 +27,18 @@ Spree::Admin::OrdersController.class_eval do
     # within the page then fetches the data it needs from Api::OrdersController
   end
 
+  def edit
+    @order.shipments.map &:refresh_rates
+
+    # Transition as far as we can go
+    while @order.next; end
+
+    # The payment step shows an error of 'No pending payments'
+    # Clearing the errors from the order object will stop this error
+    # appearing on the edit page where we don't want it to.
+    @order.errors.clear
+  end
+
   # Re-implement spree method so that it redirects to edit instead of rendering edit
   #   This allows page reloads while adding variants to the order (/edit), without being redirected to customer details page (/update)
   def update

From 7a8bf76123461ed747768f0f405f54a15736c163 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 12 Mar 2019 13:29:09 +0800
Subject: [PATCH 05/18] Move transitioning of order in admin/orders#edit to
 service

---
 .../spree/admin/orders_controller_decorator.rb  |  3 +--
 app/services/advance_order_service.rb           | 11 +++++++++++
 .../spree/admin/orders_controller_spec.rb       | 12 ++++++++++++
 spec/services/advance_order_service_spec.rb     | 17 +++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 app/services/advance_order_service.rb
 create mode 100644 spec/services/advance_order_service_spec.rb

diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb
index 930058dd3d..8ae465e05d 100644
--- a/app/controllers/spree/admin/orders_controller_decorator.rb
+++ b/app/controllers/spree/admin/orders_controller_decorator.rb
@@ -30,8 +30,7 @@ Spree::Admin::OrdersController.class_eval do
   def edit
     @order.shipments.map &:refresh_rates
 
-    # Transition as far as we can go
-    while @order.next; end
+    AdvanceOrderService.new(@order).call
 
     # The payment step shows an error of 'No pending payments'
     # Clearing the errors from the order object will stop this error
diff --git a/app/services/advance_order_service.rb b/app/services/advance_order_service.rb
new file mode 100644
index 0000000000..c9a96e090e
--- /dev/null
+++ b/app/services/advance_order_service.rb
@@ -0,0 +1,11 @@
+class AdvanceOrderService
+  attr_reader :order
+
+  def initialize(order)
+    @order = order
+  end
+
+  def call
+    while order.next; end
+  end
+end
diff --git a/spec/controllers/spree/admin/orders_controller_spec.rb b/spec/controllers/spree/admin/orders_controller_spec.rb
index ef7bff3e62..b827f3df0b 100644
--- a/spec/controllers/spree/admin/orders_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders_controller_spec.rb
@@ -4,6 +4,18 @@ describe Spree::Admin::OrdersController, type: :controller do
   include AuthenticationWorkflow
   include OpenFoodNetwork::EmailHelper
 
+  describe "#edit" do
+    let!(:order) { create(:order_with_totals_and_distribution, ship_address: create(:address)) }
+
+    before { login_as_admin }
+
+    it "advances the order state" do
+      expect {
+        spree_get :edit, id: order
+      }.to change { order.reload.state }.from("cart").to("complete")
+    end
+  end
+
   context "#update" do
     let(:params) do
       { id: order,
diff --git a/spec/services/advance_order_service_spec.rb b/spec/services/advance_order_service_spec.rb
new file mode 100644
index 0000000000..b51f293af5
--- /dev/null
+++ b/spec/services/advance_order_service_spec.rb
@@ -0,0 +1,17 @@
+require "spec_helper"
+
+describe AdvanceOrderService do
+  let!(:order) do
+    create(:order_with_totals_and_distribution, bill_address: create(:address),
+                                                ship_address: create(:address))
+  end
+
+  let(:service) { described_class.new(order) }
+
+  it "transitions the order multiple steps" do
+    expect(order.state).to eq("cart")
+    service.call
+    order.reload
+    expect(order.state).to eq("complete")
+  end
+end

From 54991f819188aacfd76beb99dcb6bf93a52970a0 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 12 Mar 2019 15:44:53 +0800
Subject: [PATCH 06/18] Retain shipping method when transitioning order

---
 app/services/advance_order_service.rb       | 10 +++++++++-
 spec/services/advance_order_service_spec.rb | 22 ++++++++++++++++++++-
 spec/services/order_factory_spec.rb         |  5 ++++-
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/app/services/advance_order_service.rb b/app/services/advance_order_service.rb
index c9a96e090e..7d1ddc6208 100644
--- a/app/services/advance_order_service.rb
+++ b/app/services/advance_order_service.rb
@@ -6,6 +6,14 @@ class AdvanceOrderService
   end
 
   def call
-    while order.next; end
+    shipping_method_id = @order.shipping_method.id if @order.shipping_method.present?
+
+    while @order.state != "complete"
+      break unless @order.next
+
+      if @order.state == "delivery"
+        @order.select_shipping_method(shipping_method_id) if shipping_method_id.present?
+      end
+    end
   end
 end
diff --git a/spec/services/advance_order_service_spec.rb b/spec/services/advance_order_service_spec.rb
index b51f293af5..72c31c5a2e 100644
--- a/spec/services/advance_order_service_spec.rb
+++ b/spec/services/advance_order_service_spec.rb
@@ -1,8 +1,10 @@
 require "spec_helper"
 
 describe AdvanceOrderService do
+  let!(:distributor) { create(:distributor_enterprise) }
   let!(:order) do
-    create(:order_with_totals_and_distribution, bill_address: create(:address),
+    create(:order_with_totals_and_distribution, distributor: distributor,
+                                                bill_address: create(:address),
                                                 ship_address: create(:address))
   end
 
@@ -14,4 +16,22 @@ describe AdvanceOrderService do
     order.reload
     expect(order.state).to eq("complete")
   end
+
+  describe "transition from delivery" do
+    let!(:shipping_method_a) { create(:shipping_method, distributors: [ distributor ]) }
+    let!(:shipping_method_b) { create(:shipping_method, distributors: [ distributor ]) }
+    let!(:shipping_method_c) { create(:shipping_method, distributors: [ distributor ]) }
+
+    before do
+      # Create shipping rates for available shipping methods.
+      order.shipments.each(&:refresh_rates)
+    end
+
+    it "retains delivery method of the order" do
+      order.select_shipping_method(shipping_method_b.id)
+      service.call
+      order.reload
+      expect(order.shipping_method).to eq(shipping_method_b)
+    end
+  end
 end
diff --git a/spec/services/order_factory_spec.rb b/spec/services/order_factory_spec.rb
index 1aa28486bd..68b72bd1da 100644
--- a/spec/services/order_factory_spec.rb
+++ b/spec/services/order_factory_spec.rb
@@ -7,6 +7,9 @@ describe OrderFactory do
   let(:customer) { create(:customer, user: user) }
   let(:shop) { create(:distributor_enterprise) }
   let(:order_cycle) { create(:simple_order_cycle) }
+  let!(:other_shipping_method_a) { create(:shipping_method) }
+  let!(:shipping_method) { create(:shipping_method) }
+  let!(:other_shipping_method_b) { create(:shipping_method) }
   let(:payment_method) { create(:payment_method) }
   let(:ship_address) { create(:address) }
   let(:bill_address) { create(:address) }
@@ -43,7 +46,7 @@ describe OrderFactory do
     end
 
     it "retains address, delivery, and payment attributes until completion of the order" do
-      while !order.completed? do break unless order.next! end
+      AdvanceOrderService.new(order).call
 
       order.reload
 

From 5b5d1967fd8cf2baba41b47b4bafe3e103620ea1 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 26 Feb 2019 17:55:13 +0800
Subject: [PATCH 07/18] Fix queries for pending order shipment in OrderSyncer

---
 app/services/order_syncer.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb
index de7aa44790..663073fadd 100644
--- a/app/services/order_syncer.rb
+++ b/app/services/order_syncer.rb
@@ -66,12 +66,13 @@ class OrderSyncer
   end
 
   def update_shipment_for(order)
-    shipment = order.shipments.with_state('pending').where(shipping_method_id: shipping_method_id_was).last
-    if shipment
+    shipment = order.shipment
+
+    if shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id_was
       shipment.update_attributes(shipping_method_id: shipping_method_id)
       order.update_attribute(:shipping_method_id, shipping_method_id)
     else
-      unless order.shipments.with_state('pending').where(shipping_method_id: shipping_method_id).any?
+      unless shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id
         order_update_issues.add(order, I18n.t('admin.shipping_method'))
       end
     end

From 9316096d4d08b9ef9cddb72d3de0e79a4b35ed23 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 26 Feb 2019 17:58:21 +0800
Subject: [PATCH 08/18] Fix updating of the shipping method for an order

---
 app/services/order_syncer.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb
index 663073fadd..2aea46686b 100644
--- a/app/services/order_syncer.rb
+++ b/app/services/order_syncer.rb
@@ -69,8 +69,7 @@ class OrderSyncer
     shipment = order.shipment
 
     if shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id_was
-      shipment.update_attributes(shipping_method_id: shipping_method_id)
-      order.update_attribute(:shipping_method_id, shipping_method_id)
+      order.select_shipping_method(shipping_method_id)
     else
       unless shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id
         order_update_issues.add(order, I18n.t('admin.shipping_method'))

From a261d912ea49889d5ed11fc9fd792dfaee28ee80 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Tue, 12 Mar 2019 16:08:17 +0800
Subject: [PATCH 09/18] Simplify order sync logic for shipping methods

---
 app/services/order_syncer.rb | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb
index 2aea46686b..830b00a459 100644
--- a/app/services/order_syncer.rb
+++ b/app/services/order_syncer.rb
@@ -67,13 +67,12 @@ class OrderSyncer
 
   def update_shipment_for(order)
     shipment = order.shipment
+    return track_shipping_method_issue(order) if shipment.blank?
 
-    if shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id_was
+    if shipment.state == "pending" && shipment.shipping_method.id == shipping_method_id_was
       order.select_shipping_method(shipping_method_id)
-    else
-      unless shipment.andand.state == "pending" && shipment.shipping_method.id == shipping_method_id
-        order_update_issues.add(order, I18n.t('admin.shipping_method'))
-      end
+    elsif !(shipment.state == "pending" && shipment.shipping_method.id == shipping_method_id)
+      track_shipping_method_issue(order)
     end
   end
 
@@ -106,4 +105,8 @@ class OrderSyncer
       order.ship_address[attr] == distributor_address[attr]
     end
   end
+
+  def track_shipping_method_issue(order)
+    order_update_issues.add(order, I18n.t('admin.shipping_method'))
+  end
 end

From 182fde58e540e88514ac2cf7e4edc731ffa5d654 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Sun, 24 Feb 2019 18:22:45 +0800
Subject: [PATCH 10/18] Uncomment now passing tests related to subscriptions

---
 .../admin/proxy_orders_controller_spec.rb          |  4 ++--
 spec/features/admin/subscriptions_spec.rb          |  2 +-
 spec/models/proxy_order_spec.rb                    |  2 +-
 spec/services/order_syncer_spec.rb                 | 14 +++++++-------
 spec/services/subscription_form_spec.rb            |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/spec/controllers/admin/proxy_orders_controller_spec.rb b/spec/controllers/admin/proxy_orders_controller_spec.rb
index befde1500e..b86f9f414c 100644
--- a/spec/controllers/admin/proxy_orders_controller_spec.rb
+++ b/spec/controllers/admin/proxy_orders_controller_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-xdescribe Admin::ProxyOrdersController, type: :controller do
+describe Admin::ProxyOrdersController, type: :controller do
   include AuthenticationWorkflow
 
   describe 'cancel' do
@@ -107,7 +107,7 @@ xdescribe Admin::ProxyOrdersController, type: :controller do
           before { shop.update_attributes(owner: user) }
 
           context "when resuming succeeds" do
-            it 'renders the resumed proxy_order as json' do
+            xit 'renders the resumed proxy_order as json' do
               spree_get :resume, params
               json_response = JSON.parse(response.body)
               expect(json_response['state']).to eq "resumed"
diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb
index 3f8dca93fe..bbb5293079 100644
--- a/spec/features/admin/subscriptions_spec.rb
+++ b/spec/features/admin/subscriptions_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-xfeature 'Subscriptions' do
+feature 'Subscriptions' do
   include AdminHelper
   include AuthenticationWorkflow
   include WebHelper
diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb
index e8b4199816..d7c0a1ba8d 100644
--- a/spec/models/proxy_order_spec.rb
+++ b/spec/models/proxy_order_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-xdescribe ProxyOrder, type: :model do
+describe ProxyOrder, type: :model do
   describe "cancel" do
     let(:order_cycle) { create(:simple_order_cycle) }
     let(:subscription) { create(:subscription) }
diff --git a/spec/services/order_syncer_spec.rb b/spec/services/order_syncer_spec.rb
index 8645eb674e..6d44862ab8 100644
--- a/spec/services/order_syncer_spec.rb
+++ b/spec/services/order_syncer_spec.rb
@@ -11,7 +11,7 @@ describe OrderSyncer do
     context "when the shipping method on an order is the same as the subscription" do
       let(:params) { { shipping_method_id: new_shipping_method.id } }
 
-      xit "updates the shipping_method on the order and on shipments" do
+      it "updates the shipping_method on the order and on shipments" do
         expect(order.shipments.first.shipping_method_id_was).to eq shipping_method.id
         subscription.assign_attributes(params)
         expect(syncer.sync!).to be true
@@ -34,7 +34,7 @@ describe OrderSyncer do
           expect(syncer.sync!).to be true
         end
 
-        xit "does not update the shipping_method on the subscription or on the pre-altered shipment" do
+        it "does not update the shipping_method on the subscription or on the pre-altered shipment" do
           expect(order.reload.shipping_method).to eq new_shipping_method
           expect(order.reload.shipments.first.shipping_method).to eq new_shipping_method
           expect(syncer.order_update_issues[order.id]).to be nil
@@ -54,7 +54,7 @@ describe OrderSyncer do
           expect(syncer.sync!).to be true
         end
 
-        xit "does not update the shipping_method on the subscription or on the pre-altered shipment" do
+        it "does not update the shipping_method on the subscription or on the pre-altered shipment" do
           expect(order.reload.shipping_method).to eq changed_shipping_method
           expect(order.reload.shipments.first.shipping_method).to eq changed_shipping_method
           expect(syncer.order_update_issues[order.id]).to include "Shipping Method"
@@ -226,7 +226,7 @@ describe OrderSyncer do
     context "when a ship address is not required" do
       before { shipping_method.update_attributes(require_ship_address: false) }
 
-      xit "does not change the ship address" do
+      it "does not change the ship address" do
         subscription.assign_attributes(params)
         expect(syncer.sync!).to be true
         expect(syncer.order_update_issues.keys).to_not include order.id
@@ -241,7 +241,7 @@ describe OrderSyncer do
         let(:new_shipping_method) { create(:shipping_method, require_ship_address: true) }
         before { params.merge!(shipping_method_id: new_shipping_method.id) }
 
-        xit "updates ship_address attrs" do
+        it "updates ship_address attrs" do
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true
           expect(syncer.order_update_issues.keys).to_not include order.id
@@ -272,7 +272,7 @@ describe OrderSyncer do
 
       context "when the ship address on the order doesn't match that on the subscription" do
         before { order.ship_address.update_attributes(firstname: "Jane") }
-        xit "does not update ship_address on the order" do
+        it "does not update ship_address on the order" do
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true
           expect(syncer.order_update_issues.keys).to include order.id
@@ -312,7 +312,7 @@ describe OrderSyncer do
       let(:params) { { subscription_line_items_attributes: [{ id: sli.id, quantity: 3}] } }
       let(:syncer) { OrderSyncer.new(subscription) }
 
-      xit "updates the line_item quantities and totals on all orders" do
+      it "updates the line_item quantities and totals on all orders" do
         expect(order.reload.total.to_f).to eq 59.97
         subscription.assign_attributes(params)
         expect(syncer.sync!).to be true
diff --git a/spec/services/subscription_form_spec.rb b/spec/services/subscription_form_spec.rb
index 21ab30f285..8d9d0b879d 100644
--- a/spec/services/subscription_form_spec.rb
+++ b/spec/services/subscription_form_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-xdescribe SubscriptionForm do
+describe SubscriptionForm do
   describe "creating a new subscription" do
     let!(:shop) { create(:distributor_enterprise) }
     let!(:customer) { create(:customer, enterprise: shop) }

From 7c3433425fe7cea941a3daf418535aa12a25f534 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 27 Feb 2019 13:16:54 +1100
Subject: [PATCH 11/18] Fix setup records for OrderFactory specs

---
 spec/services/order_factory_spec.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/spec/services/order_factory_spec.rb b/spec/services/order_factory_spec.rb
index 68b72bd1da..a1b47c0282 100644
--- a/spec/services/order_factory_spec.rb
+++ b/spec/services/order_factory_spec.rb
@@ -24,6 +24,7 @@ describe OrderFactory do
       attrs[:customer_id] = customer.id
       attrs[:distributor_id] = shop.id
       attrs[:order_cycle_id] = order_cycle.id
+      attrs[:shipping_method_id] = shipping_method.id
       attrs[:payment_method_id] = payment_method.id
       attrs[:bill_address_attributes] = bill_address.attributes.except("id")
       attrs[:ship_address_attributes] = ship_address.attributes.except("id")
@@ -38,6 +39,7 @@ describe OrderFactory do
       expect(order.user).to eq user
       expect(order.distributor).to eq shop
       expect(order.order_cycle).to eq order_cycle
+      expect(order.shipments.first.shipping_method).to eq shipping_method
       expect(order.payments.first.payment_method).to eq payment_method
       expect(order.bill_address).to eq bill_address
       expect(order.ship_address).to eq ship_address

From 0cfee3756731b1fd37dbdcd426a3fe998c5a89b7 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 27 Feb 2019 21:37:25 +0800
Subject: [PATCH 12/18] Fix setup records for SubscriptionPlacementJob specs

---
 spec/jobs/subscription_placement_job_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/jobs/subscription_placement_job_spec.rb b/spec/jobs/subscription_placement_job_spec.rb
index a55bce0d00..9b9ff1f4f7 100644
--- a/spec/jobs/subscription_placement_job_spec.rb
+++ b/spec/jobs/subscription_placement_job_spec.rb
@@ -40,7 +40,8 @@ describe SubscriptionPlacementJob do
 
   describe "performing the job" do
     context "when unplaced proxy_orders exist" do
-      let!(:proxy_order) { create(:proxy_order) }
+      let!(:subscription) { create(:subscription, with_items: true) }
+      let!(:proxy_order) { create(:proxy_order, subscription: subscription) }
 
       before do
         allow(job).to receive(:proxy_orders) { ProxyOrder.where(id: proxy_order.id) }

From 56e2ee5366af326c67ba6baec5b5ba3676a72372 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 27 Feb 2019 23:50:21 +0800
Subject: [PATCH 13/18] Fix specs for OrderSyncer

---
 spec/services/order_syncer_spec.rb | 32 ++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/spec/services/order_syncer_spec.rb b/spec/services/order_syncer_spec.rb
index 6d44862ab8..5d88dcd03a 100644
--- a/spec/services/order_syncer_spec.rb
+++ b/spec/services/order_syncer_spec.rb
@@ -2,17 +2,23 @@ require 'spec_helper'
 
 describe OrderSyncer do
   describe "updating the shipping method" do
-    let(:subscription) { create(:subscription, with_items: true, with_proxy_orders: true) }
-    let(:order) { subscription.proxy_orders.first.initialise_order! }
-    let(:shipping_method) { subscription.shipping_method }
-    let(:new_shipping_method) { create(:shipping_method, distributors: [subscription.shop]) }
+    let!(:subscription) { create(:subscription, with_items: true, with_proxy_orders: true) }
+    let!(:order) { subscription.proxy_orders.first.initialise_order! }
+    let!(:shipping_method) { subscription.shipping_method }
+    let!(:new_shipping_method) { create(:shipping_method, distributors: [subscription.shop]) }
+
     let(:syncer) { OrderSyncer.new(subscription) }
 
     context "when the shipping method on an order is the same as the subscription" do
       let(:params) { { shipping_method_id: new_shipping_method.id } }
 
+      before do
+        # Create shipping rates for available shipping methods.
+        order.shipments.each(&:refresh_rates)
+      end
+
       it "updates the shipping_method on the order and on shipments" do
-        expect(order.shipments.first.shipping_method_id_was).to eq shipping_method.id
+        expect(order.shipments.first.shipping_method).to eq shipping_method
         subscription.assign_attributes(params)
         expect(syncer.sync!).to be true
         expect(order.reload.shipping_method).to eq new_shipping_method
@@ -25,11 +31,13 @@ describe OrderSyncer do
 
       context "when the shipping method on a shipment is the same as the new shipping method on the subscription" do
         before do
+          # Create shipping rates for available shipping methods.
+          order.shipments.each(&:refresh_rates)
+
           # Updating the shipping method on a shipment updates the shipping method on the order,
           # and vice-versa via logic in Spree's shipments controller. So updating both here mimics that
           # behaviour.
-          order.shipments.first.update_attributes(shipping_method_id: new_shipping_method.id)
-          order.update_attributes(shipping_method_id: new_shipping_method.id)
+          order.select_shipping_method(new_shipping_method.id)
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true
         end
@@ -42,15 +50,18 @@ describe OrderSyncer do
       end
 
       context "when the shipping method on a shipment is not the same as the new shipping method on the subscription" do
-        let(:changed_shipping_method) { create(:shipping_method) }
+        let!(:changed_shipping_method) { create(:shipping_method) }
 
         before do
+          # Create shipping rates for available shipping methods.
+          order.shipments.each(&:refresh_rates)
+
           # Updating the shipping method on a shipment updates the shipping method on the order,
           # and vice-versa via logic in Spree's shipments controller. So updating both here mimics that
           # behaviour.
-          order.shipments.first.update_attributes(shipping_method_id: changed_shipping_method.id)
-          order.update_attributes(shipping_method_id: changed_shipping_method.id)
+          order.select_shipping_method(changed_shipping_method.id)
           subscription.assign_attributes(params)
+
           expect(syncer.sync!).to be true
         end
 
@@ -239,6 +250,7 @@ describe OrderSyncer do
 
       context "but the shipping method is being changed to one that requires a ship_address" do
         let(:new_shipping_method) { create(:shipping_method, require_ship_address: true) }
+
         before { params.merge!(shipping_method_id: new_shipping_method.id) }
 
         it "updates ship_address attrs" do

From abaf90fc4188f44894ee685035334e85fec31bcb Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 13 Mar 2019 10:15:37 +0800
Subject: [PATCH 14/18] Copy more actions advancing order from Spree

This copies the following:

* admin/orders/customer_details#update
* admin/payments#create
---
 .../customer_details_controller_decorator.rb  | 15 +++++++++
 .../admin/payments_controller_decorator.rb    | 31 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb b/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
index a8ce4dad59..38bd15142a 100644
--- a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
+++ b/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
@@ -1,6 +1,21 @@
 Spree::Admin::Orders::CustomerDetailsController.class_eval do
   before_filter :set_guest_checkout_status, only: :update
 
+  def update
+    if @order.update_attributes(params[:order])
+      if params[:guest_checkout] == "false"
+        @order.associate_user!(Spree.user_class.find_by_email(@order.email))
+      end
+      while @order.next; end
+
+      @order.shipments.map &:refresh_rates
+      flash[:success] = Spree.t('customer_details_updated')
+      redirect_to admin_order_customer_path(@order)
+    else
+      render :action => :edit
+    end
+  end
+
   # Inherit CanCan permissions for the current order
   def model_class
     load_order unless @order
diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb
index ccbdef25d5..fb3c1c6865 100644
--- a/app/controllers/spree/admin/payments_controller_decorator.rb
+++ b/app/controllers/spree/admin/payments_controller_decorator.rb
@@ -1,6 +1,37 @@
 Spree::Admin::PaymentsController.class_eval do
   append_before_filter :filter_payment_methods
 
+  def create
+    @payment = @order.payments.build(object_params)
+    if @payment.payment_method.is_a?(Spree::Gateway) && @payment.payment_method.payment_profiles_supported? && params[:card].present? and params[:card] != 'new'
+      @payment.source = CreditCard.find_by_id(params[:card])
+    end
+
+    begin
+      unless @payment.save
+        redirect_to admin_order_payments_path(@order)
+        return
+      end
+
+      if @order.completed?
+        @payment.process!
+        flash[:success] = flash_message_for(@payment, :successfully_created)
+
+         redirect_to admin_order_payments_path(@order)
+      else
+        #This is the first payment (admin created order)
+        until @order.completed?
+          @order.next!
+        end
+        flash[:success] = Spree.t(:new_order_completed)
+        redirect_to edit_admin_order_url(@order)
+      end
+
+    rescue Spree::Core::GatewayError => e
+      flash[:error] = "#{e.message}"
+      redirect_to new_admin_order_payment_path(@order)
+    end
+  end
 
   # When a user fires an event, take them back to where they came from
   # (we can't use respond_override because Spree no longer uses respond_with)

From 0e691815eaaea57f6d24ceb3d792fb4f975117b0 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 13 Mar 2019 14:52:33 +1100
Subject: [PATCH 15/18] Use service in admin/orders/customer_details#update

---
 .../admin/orders/customer_details_controller_decorator.rb | 3 ++-
 .../admin/orders/customer_details_controller_spec.rb      | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb b/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
index 38bd15142a..7f22b4808d 100644
--- a/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
+++ b/app/controllers/spree/admin/orders/customer_details_controller_decorator.rb
@@ -6,7 +6,8 @@ Spree::Admin::Orders::CustomerDetailsController.class_eval do
       if params[:guest_checkout] == "false"
         @order.associate_user!(Spree.user_class.find_by_email(@order.email))
       end
-      while @order.next; end
+
+      AdvanceOrderService.new(@order).call
 
       @order.shipments.map &:refresh_rates
       flash[:success] = Spree.t('customer_details_updated')
diff --git a/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb b/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb
index 437d7f8d4e..a8b193458d 100644
--- a/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb
+++ b/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb
@@ -39,6 +39,14 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do
         login_as_enterprise_user [order.distributor]
       end
 
+      it "advances the order state" do
+        expect {
+          spree_post :update, order: { email: user.email, bill_address_attributes: address_params,
+                                       ship_address_attributes: address_params },
+                              order_id: order.number
+        }.to change { order.reload.state }.from("cart").to("complete")
+      end
+
       context "when adding details of a registered user" do
         it "redirects to shipments on success" do
           spree_post :update, order: { email: user.email, bill_address_attributes: address_params, ship_address_attributes: address_params }, order_id: order.number

From 8782f2087cb1db0f3925c06edff174e276b86def Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Wed, 13 Mar 2019 12:37:15 +0800
Subject: [PATCH 16/18] Use service in admin/payments#create

This separates logic for bang and non-bang versions of
Spree::Order#next.

The different conditions used in both methods (state == "completed" vs
order.completed?) have implications in whether a transition is attempted
or not.
---
 .../admin/payments_controller_decorator.rb    |  6 ++--
 app/services/advance_order_service.rb         | 30 ++++++++++++++-----
 .../spree/admin/payments_controller_spec.rb   | 20 ++++++++++++-
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb
index fb3c1c6865..9f42935790 100644
--- a/app/controllers/spree/admin/payments_controller_decorator.rb
+++ b/app/controllers/spree/admin/payments_controller_decorator.rb
@@ -19,10 +19,8 @@ Spree::Admin::PaymentsController.class_eval do
 
          redirect_to admin_order_payments_path(@order)
       else
-        #This is the first payment (admin created order)
-        until @order.completed?
-          @order.next!
-        end
+        AdvanceOrderService.new(@order).call(true)
+
         flash[:success] = Spree.t(:new_order_completed)
         redirect_to edit_admin_order_url(@order)
       end
diff --git a/app/services/advance_order_service.rb b/app/services/advance_order_service.rb
index 7d1ddc6208..9b2f1876c9 100644
--- a/app/services/advance_order_service.rb
+++ b/app/services/advance_order_service.rb
@@ -5,15 +5,31 @@ class AdvanceOrderService
     @order = order
   end
 
-  def call
-    shipping_method_id = @order.shipping_method.id if @order.shipping_method.present?
+  def call(raise_on_error = false)
+    shipping_method_id = order.shipping_method.id if order.shipping_method.present?
+    options = { shipping_method_id: shipping_method_id }
+    raise_on_error ? advance_order!(options) : advance_order(options)
+  end
 
-    while @order.state != "complete"
-      break unless @order.next
+  private
 
-      if @order.state == "delivery"
-        @order.select_shipping_method(shipping_method_id) if shipping_method_id.present?
-      end
+  def advance_order(options)
+    until order.state == "complete"
+      break unless order.next
+      after_transition_hook(options)
+    end
+  end
+
+  def advance_order!(options)
+    until order.completed?
+      order.next!
+      after_transition_hook(options)
+    end
+  end
+
+  def after_transition_hook(options)
+    if order.state == "delivery"
+      order.select_shipping_method(options[:shipping_method_id]) if options[:shipping_method_id]
     end
   end
 end
diff --git a/spec/controllers/spree/admin/payments_controller_spec.rb b/spec/controllers/spree/admin/payments_controller_spec.rb
index 534c49c14e..367e6c6f6b 100644
--- a/spec/controllers/spree/admin/payments_controller_spec.rb
+++ b/spec/controllers/spree/admin/payments_controller_spec.rb
@@ -6,9 +6,27 @@ describe Spree::Admin::PaymentsController, type: :controller do
   let!(:order) { create(:order, distributor: shop, state: 'complete') }
   let!(:line_item) { create(:line_item, order: order, price: 5.0) }
 
+  before do
+    allow(controller).to receive(:spree_current_user) { user }
+  end
+
+  context "#create" do
+    let!(:payment_method) { create(:payment_method, distributors: [ shop ]) }
+    let!(:order) do
+      create(:order_with_totals_and_distribution, distributor: shop, state: "payment")
+    end
+
+    let(:params) { { amount: order.total, payment_method_id: payment_method.id } }
+
+    it "advances the order state" do
+      expect {
+        spree_post :create, payment: params, order_id: order.number
+      }.to change { order.reload.state }.from("payment").to("complete")
+    end
+  end
+
   context "as an enterprise user" do
     before do
-      allow(controller).to receive(:spree_current_user) { user }
       order.reload.update_totals
     end
 

From 7505dd0410043d51065122c7db65c2032ce32226 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Fri, 15 Mar 2019 07:58:58 +1100
Subject: [PATCH 17/18] Make order sync for shipping methods readable

---
 app/services/order_syncer.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/services/order_syncer.rb b/app/services/order_syncer.rb
index 830b00a459..cbe43d8bd2 100644
--- a/app/services/order_syncer.rb
+++ b/app/services/order_syncer.rb
@@ -66,13 +66,12 @@ class OrderSyncer
   end
 
   def update_shipment_for(order)
-    shipment = order.shipment
-    return track_shipping_method_issue(order) if shipment.blank?
+    return if pending_shipment_with?(order, shipping_method_id) # No need to do anything.
 
-    if shipment.state == "pending" && shipment.shipping_method.id == shipping_method_id_was
+    if pending_shipment_with?(order, shipping_method_id_was)
       order.select_shipping_method(shipping_method_id)
-    elsif !(shipment.state == "pending" && shipment.shipping_method.id == shipping_method_id)
-      track_shipping_method_issue(order)
+    else
+      order_update_issues.add(order, I18n.t('admin.shipping_method'))
     end
   end
 
@@ -106,7 +105,8 @@ class OrderSyncer
     end
   end
 
-  def track_shipping_method_issue(order)
-    order_update_issues.add(order, I18n.t('admin.shipping_method'))
+  def pending_shipment_with?(order, shipping_method_id)
+    return false unless order.shipment.present? && order.shipment.state == "pending"
+    order.shipping_method.id == shipping_method_id
   end
 end

From 94606025c9c881c61fe52c07dc9adf4f86f35962 Mon Sep 17 00:00:00 2001
From: Kristina Lim <kristinalim.ph@gmail.com>
Date: Thu, 21 Mar 2019 20:33:22 +0800
Subject: [PATCH 18/18] Separate call and call! for AdvanceOrderService

---
 .../admin/payments_controller_decorator.rb     |  2 +-
 app/services/advance_order_service.rb          | 15 +++++++++++----
 spec/services/advance_order_service_spec.rb    | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb
index 9f42935790..2ccefab819 100644
--- a/app/controllers/spree/admin/payments_controller_decorator.rb
+++ b/app/controllers/spree/admin/payments_controller_decorator.rb
@@ -19,7 +19,7 @@ Spree::Admin::PaymentsController.class_eval do
 
          redirect_to admin_order_payments_path(@order)
       else
-        AdvanceOrderService.new(@order).call(true)
+        AdvanceOrderService.new(@order).call!
 
         flash[:success] = Spree.t(:new_order_completed)
         redirect_to edit_admin_order_url(@order)
diff --git a/app/services/advance_order_service.rb b/app/services/advance_order_service.rb
index 9b2f1876c9..08113bd862 100644
--- a/app/services/advance_order_service.rb
+++ b/app/services/advance_order_service.rb
@@ -5,14 +5,21 @@ class AdvanceOrderService
     @order = order
   end
 
-  def call(raise_on_error = false)
-    shipping_method_id = order.shipping_method.id if order.shipping_method.present?
-    options = { shipping_method_id: shipping_method_id }
-    raise_on_error ? advance_order!(options) : advance_order(options)
+  def call
+    advance_order(advance_order_options)
+  end
+
+  def call!
+    advance_order!(advance_order_options)
   end
 
   private
 
+  def advance_order_options
+    shipping_method_id = order.shipping_method.id if order.shipping_method.present?
+    { shipping_method_id: shipping_method_id }
+  end
+
   def advance_order(options)
     until order.state == "complete"
       break unless order.next
diff --git a/spec/services/advance_order_service_spec.rb b/spec/services/advance_order_service_spec.rb
index 72c31c5a2e..60bcec75df 100644
--- a/spec/services/advance_order_service_spec.rb
+++ b/spec/services/advance_order_service_spec.rb
@@ -34,4 +34,22 @@ describe AdvanceOrderService do
       expect(order.shipping_method).to eq(shipping_method_b)
     end
   end
+
+  context "when raising on error" do
+    it "transitions the order multiple steps" do
+      service.call!
+      order.reload
+      expect(order.state).to eq("complete")
+    end
+
+    context "when order cannot advance to the next state" do
+      let!(:order) do
+        create(:order, distributor: distributor)
+      end
+
+      it "raises error" do
+        expect { service.call! }.to raise_error(StateMachine::InvalidTransition)
+      end
+    end
+  end
 end