From 57a69f7fa382a6d779676a67b166099f1df0a067 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 10 Mar 2026 11:55:41 +1100 Subject: [PATCH 1/4] Improve spec for reset_other! Covers resetting the user --- .../orders/cart_reset_service_spec.rb | 76 ++++++++++++++----- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/spec/services/orders/cart_reset_service_spec.rb b/spec/services/orders/cart_reset_service_spec.rb index 77f36021f1..6b34cea54f 100644 --- a/spec/services/orders/cart_reset_service_spec.rb +++ b/spec/services/orders/cart_reset_service_spec.rb @@ -14,31 +14,71 @@ RSpec.describe Orders::CartResetService do 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) } + describe "#reset_other!" do + it "does not reset the user" do + new_user = create(:user) - before do - expect(Shop::OrderCyclesList).to receive(:new).and_return(order_cycle_list) - order.update_attribute :order_cycle, order_cycle + expect do + described_class.new(order, distributor.id.to_s).reset_other!(new_user, nil) + end.not_to change { order.user } end - it "empties order and makes order cycle nil" do - expect(order_cycle_list).to receive(:call).and_return([]) - - Orders::CartResetService.new(order, distributor.id.to_s).reset_other!(nil, nil) - - expect(order.line_items).to be_empty - expect(order.order_cycle).to be_nil + context "when user is missing" do + it "does not reset the user" do + expect do + described_class.new(order, distributor.id.to_s).reset_other!(nil, nil) + end.not_to change { order.user } + end 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]) + context "when order email is blank" do + it "links the order to the current user" do + new_user = create(:user) + order.update(email: nil) - Orders::CartResetService.new(order, distributor.id.to_s).reset_other!(nil, nil) + described_class.new(order, distributor.id.to_s).reset_other!(new_user, nil) - expect(order.order_cycle).to eq other_order_cycle + expect(order.user).to eq(new_user) + end + end + + context "when order user is blank" do + it "links the order to the current user" do + new_user = create(:user) + order.update(user: nil) + + described_class.new(order, distributor.id.to_s).reset_other!(new_user, nil) + + expect(order.user).to eq(new_user) + 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([]) + + Orders::CartResetService.new(order, distributor.id.to_s).reset_other!(nil, nil) + + 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]) + + Orders::CartResetService.new(order, distributor.id.to_s).reset_other!(nil, nil) + + expect(order.order_cycle).to eq other_order_cycle + end end end end From fe00d1f81338ed4bd000b68bd46993896d015c11 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 10 Mar 2026 13:55:34 +1100 Subject: [PATCH 2/4] reset_other! now reset the customer when present --- app/services/orders/cart_reset_service.rb | 5 +-- .../orders/cart_reset_service_spec.rb | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/app/services/orders/cart_reset_service.rb b/app/services/orders/cart_reset_service.rb index bf2d77215a..1221159e2f 100644 --- a/app/services/orders/cart_reset_service.rb +++ b/app/services/orders/cart_reset_service.rb @@ -19,8 +19,9 @@ module Orders end def reset_other!(current_user, current_customer) - reset_user_and_customer(current_user) + reset_user(current_user) reset_order_cycle(current_customer) + order.customer = current_customer if current_customer.present? order.save! end @@ -28,7 +29,7 @@ module Orders attr_reader :order, :distributor, :current_user - def reset_user_and_customer(current_user) + def reset_user(current_user) return unless current_user order.associate_user!(current_user) if order.user.blank? || order.email.blank? diff --git a/spec/services/orders/cart_reset_service_spec.rb b/spec/services/orders/cart_reset_service_spec.rb index 6b34cea54f..967c700424 100644 --- a/spec/services/orders/cart_reset_service_spec.rb +++ b/spec/services/orders/cart_reset_service_spec.rb @@ -53,6 +53,39 @@ RSpec.describe Orders::CartResetService do end end + describe "resetting the customer" do + let(:customer) { create(:customer) } + + before do + order.customer = customer + order.save! + end + + it "links the customer to the order" do + new_customer = create(:customer) + + described_class.new(order, distributor.id.to_s).reset_other!(nil, new_customer) + + expect(order.reload.customer).to eq(new_customer) + end + + context "when customer is missing" do + it "does not reset the customer" do + expect do + described_class.new(order, distributor.id.to_s).reset_other!(nil, nil) + end.not_to change { order.customer } + end + end + + context "with the same customer as the order's customer" do + it "does not reset the customer" do + expect do + described_class.new(order, distributor.id.to_s).reset_other!(nil, customer) + end.not_to change { order.customer } + end + 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) } From bca5ee226da3baa4b706e43bf6c54f761b5692d4 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 10 Mar 2026 13:57:16 +1100 Subject: [PATCH 3/4] Add spec to test the correct customer is linked --- spec/controllers/enterprises_controller_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index dfe87211da..948c470753 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -39,6 +39,21 @@ RSpec.describe EnterprisesController do expect(controller.current_order.distributor).to eq(distributor) expect(controller.current_order.order_cycle).to be_nil end + + context "when customer doesn't belong to the order's distributor" do + it "sets the order's customer to distributor's customer" do + expected_customer = create(:customer, user:, enterprise: distributor) + + # Make sure the order is linked to a customer + customer = create(:customer) + order.customer = customer + order.save! + + expect do + get :shop, params: { id: distributor } + end.to change { controller.current_order.customer }.to(expected_customer) + end + end end it "sorts order cycles by the distributor's preferred ordering attr" do From d706b3a6c2073218535db1119ac63182cbcea8dc Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 10 Mar 2026 15:20:56 +1100 Subject: [PATCH 4/4] Per review, remove customer when no existing customer --- app/services/orders/cart_reset_service.rb | 2 +- spec/controllers/enterprises_controller_spec.rb | 13 +++++++++++++ spec/services/orders/cart_reset_service_spec.rb | 4 ++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/services/orders/cart_reset_service.rb b/app/services/orders/cart_reset_service.rb index 1221159e2f..b811c84097 100644 --- a/app/services/orders/cart_reset_service.rb +++ b/app/services/orders/cart_reset_service.rb @@ -21,7 +21,7 @@ module Orders def reset_other!(current_user, current_customer) reset_user(current_user) reset_order_cycle(current_customer) - order.customer = current_customer if current_customer.present? + order.customer = current_customer order.save! end diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index 948c470753..f6049778cf 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -29,6 +29,19 @@ RSpec.describe EnterprisesController do expect(controller.current_order.order_cycle).to be_nil end + context "when the order is linked to a customer" do + it "removes the customer" do + # Make sure the order is linked to a customer + customer = create(:customer) + order.customer = customer + order.save! + + expect do + get :shop, params: { id: distributor } + end.to change { controller.current_order.customer }.to(nil) + end + end + context "when user is logged in" do before { allow(controller).to receive(:spree_current_user) { user } } diff --git a/spec/services/orders/cart_reset_service_spec.rb b/spec/services/orders/cart_reset_service_spec.rb index 967c700424..5b2bcedafd 100644 --- a/spec/services/orders/cart_reset_service_spec.rb +++ b/spec/services/orders/cart_reset_service_spec.rb @@ -70,10 +70,10 @@ RSpec.describe Orders::CartResetService do end context "when customer is missing" do - it "does not reset the customer" do + it "removes the customer" do expect do described_class.new(order, distributor.id.to_s).reset_other!(nil, nil) - end.not_to change { order.customer } + end.to change { order.customer }.to(nil) end end