From c3f0c0ed7fdb06b29135dc9cabeb5f09e336ebc6 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 14:25:20 +0100 Subject: [PATCH 01/12] Extract method to prepare work ahead --- app/controllers/enterprises_controller.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index 717de0f29b2..e8c6794e569 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -97,7 +97,14 @@ def reset_user_and_customer(order) 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 + + select_first_order_cycle(order, order_cycles) + end + + def select_first_order_cycle(order, order_cycles) + return unless order_cycles.size == 1 + + order.order_cycle = order_cycles.first end def shop_order_cycles From 52e7ca24176cb40721625608dede3322a154baf2 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 14:26:56 +0100 Subject: [PATCH 02/12] Select first default OC only if no OC is already selected --- app/controllers/enterprises_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index e8c6794e569..ba32bb1e6a5 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -98,11 +98,11 @@ def reset_user_and_customer(order) def reset_order_cycle(order, distributor) order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call - select_first_order_cycle(order, order_cycles) + select_default_order_cycle(order, order_cycles) end - def select_first_order_cycle(order, order_cycles) - return unless order_cycles.size == 1 + def select_default_order_cycle(order, order_cycles) + return unless order.order_cycle.blank? && order_cycles.size == 1 order.order_cycle = order_cycles.first end From 94bb95861b7a443950e6c001ca80377d331979fe Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 14:29:24 +0100 Subject: [PATCH 03/12] If selected OC is not in the available OCs, empty the order --- app/controllers/enterprises_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index ba32bb1e6a5..10eab53b1b0 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -98,6 +98,11 @@ def reset_user_and_customer(order) def reset_order_cycle(order, distributor) order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call + if order.order_cycle.present? && !order_cycles.include?(order.order_cycle) + order.order_cycle = nil + order.empty! + end + select_default_order_cycle(order, order_cycles) end From 493adc8b1fd9bfb79b72a9a526539003b2f4e17c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 16:27:18 +0100 Subject: [PATCH 04/12] Fix problem in spec where wrong enterprise was being used --- spec/controllers/enterprises_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index cf6f01cca87..58515e789bf 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -95,7 +95,7 @@ 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 From 6827ce5c7f691886d859e802c393222aab307240 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 16:28:00 +0100 Subject: [PATCH 05/12] Refactor order.set_order_cycle, return early and remove indentation --- app/models/spree/order_decorator.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 8c4e7dc6c7f..08c68c61116 100644 --- a/app/models/spree/order_decorator.rb +++ b/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 From 21d1a7bc04e7432db5b2e39616265ce4bddb6a0b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 16:32:21 +0100 Subject: [PATCH 06/12] Remove dead code --- app/controllers/enterprises_controller.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index 10eab53b1b0..b64b59a545f 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -112,14 +112,6 @@ def select_default_order_cycle(order, order_cycles) order.order_cycle = order_cycles.first 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 From ba585064e1d6847a202f31ca3fcef7ab0a8aade1 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 16:45:29 +0100 Subject: [PATCH 07/12] Rename ResetOrderService to OrderCompletionReset to follow new service naming convention and also to make it more specific to completed orders --- app/controllers/spree/paypal_controller_decorator.rb | 2 +- app/services/checkout/post_checkout_actions.rb | 2 +- ...eset_order_service.rb => order_completion_reset.rb} | 8 +++++--- spec/controllers/checkout_controller_spec.rb | 10 +++++----- spec/services/checkout/post_checkout_actions_spec.rb | 4 ++-- ..._service_spec.rb => order_completion_reset_spec.rb} | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) rename app/services/{reset_order_service.rb => order_completion_reset.rb} (77%) rename spec/services/{reset_order_service_spec.rb => order_completion_reset_spec.rb} (97%) diff --git a/app/controllers/spree/paypal_controller_decorator.rb b/app/controllers/spree/paypal_controller_decorator.rb index 15122991989..0c620aeedec 100644 --- a/app/controllers/spree/paypal_controller_decorator.rb +++ b/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 diff --git a/app/services/checkout/post_checkout_actions.rb b/app/services/checkout/post_checkout_actions.rb index 63e32e91857..f9adc32f57d 100644 --- a/app/services/checkout/post_checkout_actions.rb +++ b/app/services/checkout/post_checkout_actions.rb @@ -9,7 +9,7 @@ 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 diff --git a/app/services/reset_order_service.rb b/app/services/order_completion_reset.rb similarity index 77% rename from app/services/reset_order_service.rb rename to app/services/order_completion_reset.rb index 402c377eb74..a9576c39776 100644 --- a/app/services/reset_order_service.rb +++ b/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] diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index ccbf458674c..a4adea7ba51 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/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 diff --git a/spec/services/checkout/post_checkout_actions_spec.rb b/spec/services/checkout/post_checkout_actions_spec.rb index 594671a9080..dfda22f4d27 100644 --- a/spec/services/checkout/post_checkout_actions_spec.rb +++ b/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 diff --git a/spec/services/reset_order_service_spec.rb b/spec/services/order_completion_reset_spec.rb similarity index 97% rename from spec/services/reset_order_service_spec.rb rename to spec/services/order_completion_reset_spec.rb index e02cdb25e3c..0aab4b2fccb 100644 --- a/spec/services/reset_order_service_spec.rb +++ b/spec/services/order_completion_reset_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ResetOrderService do +describe OrderCompletionReset do let(:current_token) { double(:current_token) } let(:current_distributor) { double(:distributor) } let(:current_order) do From 35824c7aa1a636fedd04e9ace86bcbafb2f705db Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 16:57:49 +0100 Subject: [PATCH 08/12] Rename restartCheckout to order_checkout_restart to make it follow service naming convention --- app/controllers/checkout_controller.rb | 2 +- app/services/checkout/post_checkout_actions.rb | 2 +- .../{restart_checkout.rb => order_checkout_restart.rb} | 2 +- spec/controllers/checkout_controller_spec.rb | 4 ++-- spec/services/checkout/post_checkout_actions_spec.rb | 4 ++-- ...checkout_spec.rb => order_checkout_restart_spec.rb} | 10 +++++----- 6 files changed, 12 insertions(+), 12 deletions(-) rename app/services/{restart_checkout.rb => order_checkout_restart.rb} (95%) rename spec/services/{restart_checkout_spec.rb => order_checkout_restart_spec.rb} (90%) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 967f4aa8acf..27980095678 100644 --- a/app/controllers/checkout_controller.rb +++ b/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 diff --git a/app/services/checkout/post_checkout_actions.rb b/app/services/checkout/post_checkout_actions.rb index f9adc32f57d..f595ab0537c 100644 --- a/app/services/checkout/post_checkout_actions.rb +++ b/app/services/checkout/post_checkout_actions.rb @@ -14,7 +14,7 @@ def success(controller, params, current_user) def failure @order.updater.shipping_address_from_distributor - RestartCheckout.new(@order).call + OrderCheckoutRestart.new(@order).call end private diff --git a/app/services/restart_checkout.rb b/app/services/order_checkout_restart.rb similarity index 95% rename from app/services/restart_checkout.rb rename to app/services/order_checkout_restart.rb index 4792d070e4e..6b5cb6724fe 100644 --- a/app/services/restart_checkout.rb +++ b/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 diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index a4adea7ba51..a07f37ff3d6 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -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 diff --git a/spec/services/checkout/post_checkout_actions_spec.rb b/spec/services/checkout/post_checkout_actions_spec.rb index dfda22f4d27..916cff69fb1 100644 --- a/spec/services/checkout/post_checkout_actions_spec.rb +++ b/spec/services/checkout/post_checkout_actions_spec.rb @@ -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 diff --git a/spec/services/restart_checkout_spec.rb b/spec/services/order_checkout_restart_spec.rb similarity index 90% rename from spec/services/restart_checkout_spec.rb rename to spec/services/order_checkout_restart_spec.rb index f0514c9b5f6..540a6a76377 100644 --- a/spec/services/restart_checkout_spec.rb +++ b/spec/services/order_checkout_restart_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' -describe RestartCheckout do +describe OrderCheckoutRestart do let(:order) { create(:order_with_distributor) } describe "#call" do context "when the order is already in the 'cart' state" do it "does nothing" do expect(order).to_not receive(:restart_checkout!) - RestartCheckout.new(order).call + OrderCheckoutRestart.new(order).call end end @@ -25,7 +25,7 @@ before { order.ship_address = nil } it "resets the order state, and clears incomplete shipments and payments" do - RestartCheckout.new(order).call + OrderCheckoutRestart.new(order).call expect_cart_state_and_reset_adjustments end @@ -35,7 +35,7 @@ before { order.ship_address = order.address_from_distributor } it "resets the order state, and clears incomplete shipments and payments" do - RestartCheckout.new(order).call + OrderCheckoutRestart.new(order).call expect_cart_state_and_reset_adjustments end @@ -46,7 +46,7 @@ it "does not reset the order state nor clears incomplete shipments and payments" do expect do - RestartCheckout.new(order).call + OrderCheckoutRestart.new(order).call end.to raise_error(StateMachine::InvalidTransition) expect(order.state).to eq 'payment' From a438317d69c5787a94b8f745b01dc8b8c19cf3a7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 17:23:26 +0100 Subject: [PATCH 09/12] Extract order_cart_reset service from enterprises_controller --- app/controllers/enterprises_controller.rb | 43 +----------------- app/services/order_cart_reset.rb | 53 +++++++++++++++++++++++ 2 files changed, 54 insertions(+), 42 deletions(-) create mode 100644 app/services/order_cart_reset.rb diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index b64b59a545f..adf013b7e42 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -65,53 +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 - - if order.order_cycle.present? && !order_cycles.include?(order.order_cycle) - order.order_cycle = nil - order.empty! - end - - select_default_order_cycle(order, order_cycles) - end - - def select_default_order_cycle(order, order_cycles) - return unless order.order_cycle.blank? && order_cycles.size == 1 - - order.order_cycle = order_cycles.first - end - def set_noindex_meta_tag @noindex_meta_tag = true unless current_distributor.visible? end diff --git a/app/services/order_cart_reset.rb b/app/services/order_cart_reset.rb new file mode 100644 index 00000000000..a20bd2e4ec6 --- /dev/null +++ b/app/services/order_cart_reset.rb @@ -0,0 +1,53 @@ +# 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 + order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call + + if order.order_cycle.present? && !order_cycles.include?(order.order_cycle) + order.order_cycle = nil + order.empty! + end + + select_default_order_cycle(order, order_cycles) + end + + def select_default_order_cycle(order, order_cycles) + return unless order.order_cycle.blank? && order_cycles.size == 1 + + order.order_cycle = order_cycles.first + end +end From f22eae752de81fd57707ee09981e63ed8e36f812 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 19:28:19 +0100 Subject: [PATCH 10/12] Adapt spec to validate issue #5340 --- .../enterprises_controller_spec.rb | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index 58515e789bf..43cc38fa710 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/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,15 +82,10 @@ 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 @@ -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 From 1c749a8029e1188329f93355913dd92f0a097163 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 19:33:57 +0100 Subject: [PATCH 11/12] Extract order_cycle_not_listed? method and rename order_cycles to listed_order_cycles --- app/services/order_cart_reset.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/services/order_cart_reset.rb b/app/services/order_cart_reset.rb index a20bd2e4ec6..a69029ddde5 100644 --- a/app/services/order_cart_reset.rb +++ b/app/services/order_cart_reset.rb @@ -35,19 +35,24 @@ def reset_user_and_customer end def reset_order_cycle - order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call + listed_order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call - if order.order_cycle.present? && !order_cycles.include?(order.order_cycle) + if order_cycle_not_listed?(order.order_cycle, listed_order_cycles) order.order_cycle = nil order.empty! end - select_default_order_cycle(order, order_cycles) + select_default_order_cycle(order, listed_order_cycles) end - def select_default_order_cycle(order, order_cycles) - return unless order.order_cycle.blank? && order_cycles.size == 1 + 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 = order_cycles.first + order.order_cycle = listed_order_cycles.first end end From ace73be4e2b0a73e426650f8ea387a3a9435aafe Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 15 May 2020 20:05:45 +0100 Subject: [PATCH 12/12] Add unit tests to order_cart_reset --- spec/services/order_cart_reset_spec.rb | 46 ++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 spec/services/order_cart_reset_spec.rb diff --git a/spec/services/order_cart_reset_spec.rb b/spec/services/order_cart_reset_spec.rb new file mode 100644 index 00000000000..0b26977d6a6 --- /dev/null +++ b/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