From 1a7cc1481611574ae11fcbbc619eba76462bec0f Mon Sep 17 00:00:00 2001 From: Rafal Kosla Date: Mon, 21 Feb 2022 10:28:41 +0100 Subject: [PATCH 1/5] bugfix - reseting shipments after currency change --- api/app/controllers/spree/api/v2/storefront/cart_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/app/controllers/spree/api/v2/storefront/cart_controller.rb b/api/app/controllers/spree/api/v2/storefront/cart_controller.rb index 4f76039ea41..e8711b03107 100644 --- a/api/app/controllers/spree/api/v2/storefront/cart_controller.rb +++ b/api/app/controllers/spree/api/v2/storefront/cart_controller.rb @@ -161,6 +161,9 @@ def associate def change_currency spree_authorize! :update, spree_current_order, order_token + spree_current_order.ensure_updated_shipments + spree_current_order.update_column(:total,spree_current_order.item_total) + result = change_currency_service.call(order: spree_current_order, new_currency: params[:new_currency]) render_order(result) From e427f8b92722ceb7e754088f804ee8db67b65bf3 Mon Sep 17 00:00:00 2001 From: Rafal Kosla Date: Thu, 24 Feb 2022 10:53:02 +0100 Subject: [PATCH 2/5] moved to spree/core + unit tests --- .../api/v2/storefront/cart_controller.rb | 3 - .../services/spree/cart/change_currency.rb | 3 + .../spree/cart/change_currency_spec.rb | 101 +++++++++++++----- 3 files changed, 80 insertions(+), 27 deletions(-) diff --git a/api/app/controllers/spree/api/v2/storefront/cart_controller.rb b/api/app/controllers/spree/api/v2/storefront/cart_controller.rb index e8711b03107..4f76039ea41 100644 --- a/api/app/controllers/spree/api/v2/storefront/cart_controller.rb +++ b/api/app/controllers/spree/api/v2/storefront/cart_controller.rb @@ -161,9 +161,6 @@ def associate def change_currency spree_authorize! :update, spree_current_order, order_token - spree_current_order.ensure_updated_shipments - spree_current_order.update_column(:total,spree_current_order.item_total) - result = change_currency_service.call(order: spree_current_order, new_currency: params[:new_currency]) render_order(result) diff --git a/core/app/services/spree/cart/change_currency.rb b/core/app/services/spree/cart/change_currency.rb index 1bda19dfd93..7cf19493199 100644 --- a/core/app/services/spree/cart/change_currency.rb +++ b/core/app/services/spree/cart/change_currency.rb @@ -6,6 +6,9 @@ class ChangeCurrency def call(order:, new_currency:) return failure('Currency not supported') unless supported_currency?(order, new_currency) + order.ensure_updated_shipments + order.update_column(:total,order.item_total) + result = order.update!(currency: new_currency) rescue false if result diff --git a/core/spec/services/spree/cart/change_currency_spec.rb b/core/spec/services/spree/cart/change_currency_spec.rb index 57e920479b0..5c820dbbcc8 100644 --- a/core/spec/services/spree/cart/change_currency_spec.rb +++ b/core/spec/services/spree/cart/change_currency_spec.rb @@ -2,26 +2,90 @@ module Spree describe Cart::ChangeCurrency do - subject { described_class.call(order: order, new_currency: new_currency) } + context 'when shipment is already choosen' do + let(:country) { create(:country) } + let(:store) { create(:store, checkout_zone: zone, supported_currencies: 'USD,EUR') } + let(:order2) { create(:order_with_line_items, store: store, ship_address: create(:address, country: country)) } - let(:order) { create(:order_with_line_items, store: store, currency: 'USD') } - let(:store) { create(:store, supported_currencies: 'USD,EUR,GBP') } + let(:zone) { create(:zone_with_country) } - context 'when switching to a supported currency' do - let(:new_currency) { 'EUR' } + before do + zone.countries << country + end - context 'when product has a price in given currency' do - let!(:price) { create(:price, currency: 'EUR', variant: order.line_items.first.variant)} - it 'changes order and line items currency' do - expect(subject).to be_success - order.reload - expect(order.currency).to eq('EUR') - expect(order.line_items.first.currency).to eq('EUR') + let!(:price) { create(:price, currency: 'EUR', variant: order2.line_items.first.variant,amount: 25.00 )} + + context 'when in address state' do + it 'resets the checkout process when in address state' do + expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + order2.reload + expect(order2.currency).to eq('EUR') + expect(order2.total).to eq(25.00) + expect(order2.shipments.to_a).to eq([]) end end - context 'when product does not have a price in given currency' do + context 'when in payment state' do + it 'resets the checkout process when in payment state' do + byebug + Checkout::Advance.call(order: order2) + order2.item_total = 10 + expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + order2.reload + expect(order2.state).to eq('address') + expect(order2.currency).to eq('EUR') + expect(order2.total).to eq(25.00) + expect(order2.shipments.to_a).to eq([]) + end + end + + context 'if the order has shipping methods after change' do + + it 'gets the new shipping method' do + shipment_id = order2.shipments.first.id + expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + order2.reload + Checkout::GetShippingRates.call(order: order2) + expect(order2.currency).to eq('EUR') + expect(order2.shipments.first.id).not_to eq(shipment_id) + + end + end + end + + context 'change currency' do + subject { described_class.call(order: order, new_currency: new_currency) } + + let(:order) { create(:order_with_line_items, store: store, currency: 'USD') } + let(:store) { create(:store, supported_currencies: 'USD,EUR,GBP') } + context 'when switching to a supported currency' do + let(:new_currency) { 'EUR' } + + context 'when product has a price in given currency' do + let!(:price) { create(:price, currency: 'EUR', variant: order.line_items.first.variant)} + + it 'changes order and line items currency' do + expect(subject).to be_success + order.reload + expect(order.currency).to eq('EUR') + expect(order.line_items.first.currency).to eq('EUR') + end + end + + context 'when product does not have a price in given currency' do + it 'returns failure' do + expect(subject).to be_failure + order.reload + expect(order.currency).to eq('USD') + expect(order.line_items.first.currency).to eq('USD') + end + end + end + + context 'when switching to an unsupported currency' do + let(:new_currency) { 'XOF' } + it 'returns failure' do expect(subject).to be_failure order.reload @@ -30,16 +94,5 @@ module Spree end end end - - context 'when switching to an unsupported currency' do - let(:new_currency) { 'XOF' } - - it 'returns failure' do - expect(subject).to be_failure - order.reload - expect(order.currency).to eq('USD') - expect(order.line_items.first.currency).to eq('USD') - end - end end end From 6fbff9f9aa06e8a225930a5bbe2f7ebd6e47e3d4 Mon Sep 17 00:00:00 2001 From: Rafal Kosla Date: Thu, 24 Feb 2022 10:59:05 +0100 Subject: [PATCH 3/5] clean --- core/spec/services/spree/cart/change_currency_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/core/spec/services/spree/cart/change_currency_spec.rb b/core/spec/services/spree/cart/change_currency_spec.rb index 5c820dbbcc8..5a1141f61e6 100644 --- a/core/spec/services/spree/cart/change_currency_spec.rb +++ b/core/spec/services/spree/cart/change_currency_spec.rb @@ -28,7 +28,6 @@ module Spree context 'when in payment state' do it 'resets the checkout process when in payment state' do - byebug Checkout::Advance.call(order: order2) order2.item_total = 10 expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success From a482717d3571c030957633a2ff1b7fee65b1785c Mon Sep 17 00:00:00 2001 From: Rafal Kosla Date: Thu, 24 Feb 2022 11:20:43 +0100 Subject: [PATCH 4/5] rubocop cleanup --- core/app/services/spree/cart/change_currency.rb | 2 +- .../services/spree/cart/change_currency_spec.rb | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/core/app/services/spree/cart/change_currency.rb b/core/app/services/spree/cart/change_currency.rb index 7cf19493199..ebe2da3cdde 100644 --- a/core/app/services/spree/cart/change_currency.rb +++ b/core/app/services/spree/cart/change_currency.rb @@ -7,7 +7,7 @@ def call(order:, new_currency:) return failure('Currency not supported') unless supported_currency?(order, new_currency) order.ensure_updated_shipments - order.update_column(:total,order.item_total) + order.update_column(:total, order.item_total) result = order.update!(currency: new_currency) rescue false diff --git a/core/spec/services/spree/cart/change_currency_spec.rb b/core/spec/services/spree/cart/change_currency_spec.rb index 5a1141f61e6..17b74d2bbc1 100644 --- a/core/spec/services/spree/cart/change_currency_spec.rb +++ b/core/spec/services/spree/cart/change_currency_spec.rb @@ -6,19 +6,17 @@ module Spree let(:country) { create(:country) } let(:store) { create(:store, checkout_zone: zone, supported_currencies: 'USD,EUR') } let(:order2) { create(:order_with_line_items, store: store, ship_address: create(:address, country: country)) } - let(:zone) { create(:zone_with_country) } + let!(:price) { create(:price, currency: 'EUR', variant: order2.line_items.first.variant, amount: 25.00) } + before do zone.countries << country end - - let!(:price) { create(:price, currency: 'EUR', variant: order2.line_items.first.variant,amount: 25.00 )} - context 'when in address state' do it 'resets the checkout process when in address state' do - expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + expect(described_class.call(order: order2, new_currency: 'EUR')).to be_success order2.reload expect(order2.currency).to eq('EUR') expect(order2.total).to eq(25.00) @@ -30,7 +28,7 @@ module Spree it 'resets the checkout process when in payment state' do Checkout::Advance.call(order: order2) order2.item_total = 10 - expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + expect(described_class.call(order: order2, new_currency: 'EUR')).to be_success order2.reload expect(order2.state).to eq('address') expect(order2.currency).to eq('EUR') @@ -40,15 +38,13 @@ module Spree end context 'if the order has shipping methods after change' do - it 'gets the new shipping method' do shipment_id = order2.shipments.first.id - expect(Cart::ChangeCurrency.call(order: order2,new_currency: 'EUR')).to be_success + expect(described_class.call(order: order2, new_currency: 'EUR')).to be_success order2.reload Checkout::GetShippingRates.call(order: order2) expect(order2.currency).to eq('EUR') expect(order2.shipments.first.id).not_to eq(shipment_id) - end end end @@ -58,11 +54,12 @@ module Spree let(:order) { create(:order_with_line_items, store: store, currency: 'USD') } let(:store) { create(:store, supported_currencies: 'USD,EUR,GBP') } + context 'when switching to a supported currency' do let(:new_currency) { 'EUR' } context 'when product has a price in given currency' do - let!(:price) { create(:price, currency: 'EUR', variant: order.line_items.first.variant)} + let!(:price) { create(:price, currency: 'EUR', variant: order.line_items.first.variant) } it 'changes order and line items currency' do expect(subject).to be_success From 7f2d21101e6ab746407c5a7a5ebd8f3ebe452c8e Mon Sep 17 00:00:00 2001 From: Rafal Kosla Date: Thu, 24 Feb 2022 12:33:46 +0100 Subject: [PATCH 5/5] changed context of one of unit tests to be more clear --- core/spec/services/spree/cart/change_currency_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/spec/services/spree/cart/change_currency_spec.rb b/core/spec/services/spree/cart/change_currency_spec.rb index 17b74d2bbc1..36bbf14fab6 100644 --- a/core/spec/services/spree/cart/change_currency_spec.rb +++ b/core/spec/services/spree/cart/change_currency_spec.rb @@ -37,7 +37,7 @@ module Spree end end - context 'if the order has shipping methods after change' do + context 'when user requests shipping methods after changing currency' do it 'gets the new shipping method' do shipment_id = order2.shipments.first.id expect(described_class.call(order: order2, new_currency: 'EUR')).to be_success