Fix Grumpy Cat on shop page (cart with items from a closed OC)

app/controllers/checkout_controller.rb

@@ -40,7 +40,7 @@ def edit
     # This is only required because of spree_paypal_express. If we implement
     # a version of paypal that uses this controller, and more specifically
     # the #update_failed method, then we can remove this call
-    RestartCheckout.new(@order).call
+    OrderCheckoutRestart.new(@order).call
   end
 
   def update

app/controllers/enterprises_controller.rb

@@ -65,49 +65,12 @@ def enough_stock?
   def reset_order
     order = current_order(true)
 
-    reset_distributor(order, distributor)
-
-    reset_user_and_customer(order) if try_spree_current_user
-
-    reset_order_cycle(order, distributor)
-
-    order.save!
+    OrderCartReset.new(order, params[:id], try_spree_current_user, current_customer).call
   rescue ActiveRecord::RecordNotFound
     flash[:error] = I18n.t(:enterprise_shop_show_error)
     redirect_to shops_path
   end
 
-  def distributor
-    @distributor ||= Enterprise.is_distributor.find_by_permalink(params[:id]) ||
-                     Enterprise.is_distributor.find(params[:id])
-  end
-
-  def reset_distributor(order, distributor)
-    if order.distributor && order.distributor != distributor
-      order.empty!
-      order.set_order_cycle! nil
-    end
-    order.distributor = distributor
-  end
-
-  def reset_user_and_customer(order)
-    order.associate_user!(spree_current_user) if order.user.blank? || order.email.blank?
-    order.__send__(:associate_customer) if order.customer.nil? # Only associates existing customers
-  end
-
-  def reset_order_cycle(order, distributor)
-    order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call
-    order.order_cycle = order_cycles.first if order_cycles.size == 1
-  end
-
-  def shop_order_cycles
-    if current_order_cycle
-      [current_order_cycle]
-    else
-      OrderCycle.not_closed.with_distributor(current_distributor)
-    end
-  end
-
   def set_noindex_meta_tag
     @noindex_meta_tag = true unless current_distributor.visible?
   end

app/controllers/spree/paypal_controller_decorator.rb

@@ -22,7 +22,7 @@ def reset_order_when_complete
     if current_order.complete?
       flash[:notice] = t(:order_processed_successfully)
 
-      ResetOrderService.new(self, current_order).call
+      OrderCompletionReset.new(self, current_order).call
       session[:access_token] = current_order.token
     end
   end

app/models/spree/order_decorator.rb

@@ -109,12 +109,12 @@ def empty_with_clear_shipping_and_payments!
   alias_method_chain :empty!, :clear_shipping_and_payments
 
   def set_order_cycle!(order_cycle)
-    unless self.order_cycle == order_cycle
-      self.order_cycle = order_cycle
-      self.distributor = nil unless order_cycle.nil? || order_cycle.has_distributor?(distributor)
-      empty!
-      save!
-    end
+    return if self.order_cycle == order_cycle
+
+    self.order_cycle = order_cycle
+    self.distributor = nil unless order_cycle.nil? || order_cycle.has_distributor?(distributor)
+    empty!
+    save!
   end
 
   # "Checkout" is the initial state and, for card payments, "pending" is the state after authorization

app/services/checkout/post_checkout_actions.rb

@@ -9,12 +9,12 @@ def initialize(order)
 
     def success(controller, params, current_user)
       save_order_addresses_as_user_default(params, current_user)
-      ResetOrderService.new(controller, @order).call
+      OrderCompletionReset.new(controller, @order).call
     end
 
     def failure
       @order.updater.shipping_address_from_distributor
-      RestartCheckout.new(@order).call
+      OrderCheckoutRestart.new(@order).call
     end
 
     private

app/services/order_cart_reset.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: false
+
+# Resets an order by verifying it's state and fixing any issues
+class OrderCartReset
+  def initialize(order, distributor_id, current_user, current_customer)
+    @order = order
+    @distributor ||= Enterprise.is_distributor.find_by_permalink(distributor_id) ||
+                     Enterprise.is_distributor.find(distributor_id)
+    @current_user = current_user
+    @current_customer = current_customer
+  end
+
+  def call
+    reset_distributor
+    reset_user_and_customer if current_user
+    reset_order_cycle
+    order.save!
+  end
+
+  private
+
+  attr_reader :order, :distributor, :current_user, :current_customer
+
+  def reset_distributor
+    if order.distributor && order.distributor != distributor
+      order.empty!
+      order.set_order_cycle! nil
+    end
+    order.distributor = distributor
+  end
+
+  def reset_user_and_customer
+    order.associate_user!(current_user) if order.user.blank? || order.email.blank?
+    order.__send__(:associate_customer) if order.customer.nil? # Only associates existing customers
+  end
+
+  def reset_order_cycle
+    listed_order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call
+
+    if order_cycle_not_listed?(order.order_cycle, listed_order_cycles)
+      order.order_cycle = nil
+      order.empty!
+    end
+
+    select_default_order_cycle(order, listed_order_cycles)
+  end
+
+  def order_cycle_not_listed?(order_cycle, listed_order_cycles)
+    order_cycle.present? && !listed_order_cycles.include?(order_cycle)
+  end
+
+  # If no OC is selected and there is only one in the list of OCs, selects it
+  def select_default_order_cycle(order, listed_order_cycles)
+    return unless order.order_cycle.blank? && listed_order_cycles.size == 1
+
+    order.order_cycle = listed_order_cycles.first
+  end
+end

app/services/restart_checkout.rb → app/services/order_checkout_restart.rb

@@ -1,5 +1,5 @@
 # Resets the passed order to cart state while clearing associated payments and shipments
-class RestartCheckout
+class OrderCheckoutRestart
   def initialize(order)
     @order = order
   end

app/services/reset_order_service.rb → app/services/order_completion_reset.rb

@@ -1,6 +1,8 @@
-# Builds a new order based on the one specified. This implements the "continue
-# shopping" feature once an order is completed.
-class ResetOrderService
+# frozen_string_literal: false
+
+# Resets a completed order by building a new order based on the one specified.
+# This implements the "continue shopping" feature once an order is completed.
+class OrderCompletionReset
   # Constructor
   #
   # @param controller [#expire_current_order, #current_order]

spec/controllers/checkout_controller_spec.rb

@@ -4,7 +4,7 @@
   let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
   let(:order_cycle) { create(:simple_order_cycle) }
   let(:order) { create(:order) }
-  let(:reset_order_service) { double(ResetOrderService) }
+  let(:reset_order_service) { double(OrderCompletionReset) }
 
   before do
     allow(order).to receive(:checkout_allowed?).and_return true
@@ -117,8 +117,8 @@
       let(:test_shipping_method_id) { "111" }
 
       before do
-        # stub order and resetorderservice
-        allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
+        # stub order and OrderCompletionReset
+        allow(OrderCompletionReset).to receive(:new).with(controller, order) { reset_order_service }
         allow(reset_order_service).to receive(:call)
         allow(order).to receive(:update_attributes).and_return true
         allow(controller).to receive(:current_order).and_return order
@@ -211,7 +211,7 @@
     end
 
     it "returns order confirmation url on success" do
-      allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
+      allow(OrderCompletionReset).to receive(:new).with(controller, order) { reset_order_service }
       expect(reset_order_service).to receive(:call)
 
       allow(order).to receive(:update_attributes).and_return true
@@ -232,7 +232,7 @@
 
     describe "stale object handling" do
       it "retries when a stale object error is encountered" do
-        allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
+        allow(OrderCompletionReset).to receive(:new).with(controller, order) { reset_order_service }
         expect(reset_order_service).to receive(:call)
 
         allow(order).to receive(:update_attributes).and_return true
@@ -299,11 +299,11 @@
   end
 
   describe "#update_failed" do
-    let(:restart_checkout) { instance_double(RestartCheckout, call: true) }
+    let(:restart_checkout) { instance_double(OrderCheckoutRestart, call: true) }
 
     before do
       controller.instance_variable_set(:@order, order)
-      allow(RestartCheckout).to receive(:new) { restart_checkout }
+      allow(OrderCheckoutRestart).to receive(:new) { restart_checkout }
       allow(controller).to receive(:current_order) { order }
     end
 

spec/controllers/enterprises_controller_spec.rb

@@ -3,14 +3,15 @@
 describe EnterprisesController, type: :controller do
   describe "shopping for a distributor" do
     let(:order) { controller.current_order(true) }
-
+    let(:line_item) { create(:line_item) }
     let!(:current_distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
     let!(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
-    let!(:order_cycle1) { create(:simple_order_cycle, distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 3.days.from_now ) }
+    let!(:order_cycle1) { create(:simple_order_cycle, distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 3.days.from_now, variants: [line_item.variant] ) }
     let!(:order_cycle2) { create(:simple_order_cycle, distributors: [distributor], orders_open_at: 3.days.ago, orders_close_at: 4.days.from_now ) }
 
     before do
       order.set_distributor! current_distributor
+      order.line_items << line_item
     end
 
     it "sets the shop as the distributor on the order when shopping for the distributor" do
@@ -81,21 +82,16 @@
     end
 
     it "should not empty an order if returning to the same distributor" do
-      product = create(:product)
-      create(:simple_order_cycle, distributors: [current_distributor], variants: [product.variants.first])
-      line_item = create(:line_item, variant: product.variants.first)
-      controller.current_order.line_items << line_item
-
       spree_get :shop, id: current_distributor
 
       expect(controller.current_order.distributor).to eq current_distributor
-      expect(controller.current_order.line_items.first.variant).to eq product.variants.first
+      expect(controller.current_order.line_items.first.variant).to eq line_item.variant
     end
 
     describe "when an out of stock item is in the cart" do
       let(:variant) { create(:variant, on_demand: false, on_hand: 10) }
       let(:line_item) { create(:line_item, variant: variant) }
-      let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], variants: [variant]) }
+      let(:order_cycle) { create(:simple_order_cycle, distributors: [current_distributor], variants: [variant]) }
 
       before do
         order.set_distribution! current_distributor, order_cycle
@@ -112,6 +108,19 @@
       end
     end
 
+    it "resets order if the order cycle of the current order is no longer open or visible" do
+      order.distributor = distributor
+      order.order_cycle = order_cycle1
+      order.save
+      order_cycle1.update_attribute :orders_close_at, Time.zone.now
+
+      spree_get :shop, id: distributor
+
+      expect(controller.current_order.distributor).to eq(distributor)
+      expect(controller.current_order.order_cycle).to eq(order_cycle2)
+      expect(controller.current_order.line_items).to be_empty
+    end
+
     it "sets order cycle if only one is available at the chosen distributor" do
       order_cycle2.destroy
 

spec/services/checkout/post_checkout_actions_spec.rb

@@ -11,10 +11,10 @@
     let(:params) { { order: {} } }
     let(:current_user) { order.distributor.owner }
 
-    let(:reset_order_service) { instance_double(ResetOrderService) }
+    let(:reset_order_service) { instance_double(OrderCompletionReset) }
 
     before do
-      expect(ResetOrderService).to receive(:new).
+      expect(OrderCompletionReset).to receive(:new).
         with(controller, order).and_return(reset_order_service)
       expect(reset_order_service).to receive(:call)
     end
@@ -48,10 +48,10 @@
   end
 
   describe "#failure" do
-    let(:restart_checkout_service) { instance_double(RestartCheckout) }
+    let(:restart_checkout_service) { instance_double(OrderCheckoutRestart) }
 
     it "restarts the checkout process" do
-      expect(RestartCheckout).to receive(:new).with(order).and_return(restart_checkout_service)
+      expect(OrderCheckoutRestart).to receive(:new).with(order).and_return(restart_checkout_service)
       expect(restart_checkout_service).to receive(:call)
 
       postCheckoutActions.failure

spec/services/order_cart_reset_spec.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe OrderCartReset do
+  let(:distributor) { create(:distributor_enterprise) }
+  let(:order) { create(:order, :with_line_item, distributor: distributor) }
+
+  context "if order distributor is not the requested distributor" do
+    let(:new_distributor) { create(:distributor_enterprise) }
+
+    it "empties order" do
+      OrderCartReset.new(order, new_distributor.id.to_s, nil, nil).call
+
+      expect(order.line_items).to be_empty
+    end
+  end
+
+  context "if the order's order cycle is not in the list of visible order cycles" do
+    let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) }
+    let(:order_cycle_list) { instance_double(Shop::OrderCyclesList) }
+
+    before do
+      expect(Shop::OrderCyclesList).to receive(:new).and_return(order_cycle_list)
+      order.update_attribute :order_cycle, order_cycle
+    end
+
+    it "empties order and makes order cycle nil" do
+      expect(order_cycle_list).to receive(:call).and_return([])
+
+      OrderCartReset.new(order, distributor.id.to_s, nil, nil).call
+
+      expect(order.line_items).to be_empty
+      expect(order.order_cycle).to be_nil
+    end
+
+    it "selects default Order Cycle if there's one" do
+      other_order_cycle = create(:simple_order_cycle, distributors: [distributor])
+      expect(order_cycle_list).to receive(:call).and_return([other_order_cycle])
+
+      OrderCartReset.new(order, distributor.id.to_s, nil, nil).call
+
+      expect(order.order_cycle).to eq other_order_cycle
+    end
+  end
+end