From 93ac99b5cca0b35567dfe7800ac3289c9a224066 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 19:26:45 +0100 Subject: [PATCH 1/7] Added new factory for shipping_method_with trait delivery to fix order.shipping_method issue in features/admin/orders_spec --- spec/factories.rb | 17 +++++++++++++++++ spec/features/admin/orders_spec.rb | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index 9c0a88e318..01213e807f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -335,6 +335,23 @@ FactoryBot.define do end end + factory :shipping_method_with, parent: :shipping_method do + trait :delivery do + require_ship_address { true } + end + end + + factory :shipment_with, parent: :shipment do + trait :shipping_method do + transient do + shipping_method { create :shipping_method } + end + after(:create) do |shipment, evaluator| + shipment.add_shipping_method(evaluator.shipping_method, true) + end + end + end + factory :shipping_method_with_flat_rate, parent: :shipping_method do calculator { Spree::Calculator::FlatRate.new(preferred_amount: 50.0) } end diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 0b9a6cb602..276167e736 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -149,7 +149,8 @@ feature %q{ # Given a customer with an order, which includes their shipping and billing address @order.ship_address = create(:address, lastname: 'Ship') @order.bill_address = create(:address, lastname: 'Bill') - @order.shipping_method = create(:shipping_method, require_ship_address: true) + shipping_method = create(:shipping_method_with, :delivery) + @order.shipments << create(:shipment_with, :shipping_method, shipping_method: shipping_method) @order.save! # When I create a new order From 7b6e4825e7400a769b04a7cabe6bdbe547405e2c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 20:53:00 +0100 Subject: [PATCH 2/7] Fixed controllers/spree/admin/orders/customer_details_controller_spec: - fixed order creation to use order.shipments instead of order.shipping_method - adapted test to new spree 2 controller logic (shipments page is gone since 67f568914988bcc0a1fc520d15ed6444a6d12824 and redirect logic changed on e9cde1b4d570dd4f7f979ac71a58d6f3f342ebb4) --- .../admin/orders/customer_details_controller_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 bd78bc2fd9..437d7f8d4e 100644 --- a/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb @@ -8,12 +8,12 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do let!(:user) { create(:user) } let(:address) { create(:address) } let!(:distributor) { create(:distributor_enterprise) } - let!(:shipping_method) { create(:shipping_method) } + let!(:shipment) { create(:shipment) } let!(:order) { create( :order_with_totals_and_distribution, state: 'cart', - shipping_method: shipping_method, + shipments: [shipment], distributor: distributor, user: nil, email: nil, @@ -45,7 +45,7 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do order.reload - expect(response).to redirect_to spree.edit_admin_order_shipment_path(order, order.shipment) + expect(response).to redirect_to spree.admin_order_customer_path(order) end end @@ -55,7 +55,7 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do order.reload - expect(response).to redirect_to spree.edit_admin_order_shipment_path(order, order.shipment) + expect(response).to redirect_to spree.admin_order_customer_path(order) end end end From fbd2d96b059bd85ca526292ab7469935b033b499 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 21:24:29 +0100 Subject: [PATCH 3/7] Moved shipment and shipping_method factories with flat rate and shipping fees to traits --- spec/factories.rb | 52 ++++++++----------- .../concerns/order_shipping_method_spec.rb | 3 +- spec/models/spree/adjustment_spec.rb | 3 +- spec/models/spree/order_spec.rb | 6 ++- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 01213e807f..f92acc286f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -339,6 +339,20 @@ FactoryBot.define do trait :delivery do require_ship_address { true } end + + trait :flat_rate do + calculator { Spree::Calculator::FlatRate.new(preferred_amount: 50.0) } + end + + trait :shipping_fee do + transient do + shipping_fee 3 + end + + calculator { build(:calculator_per_item, preferred_amount: shipping_fee) } + require_ship_address { false } + distributors { [create(:distributor_enterprise_with_tax)] } + end end factory :shipment_with, parent: :shipment do @@ -350,15 +364,16 @@ FactoryBot.define do shipment.add_shipping_method(evaluator.shipping_method, true) end end - end - factory :shipping_method_with_flat_rate, parent: :shipping_method do - calculator { Spree::Calculator::FlatRate.new(preferred_amount: 50.0) } - end + trait :shipping_fee do + transient do + shipping_fee 3 + end - factory :shipment_with_flat_rate, parent: :shipment do - after(:create) do |shipment| - shipment.add_shipping_method(create(:shipping_method_with_flat_rate), true) + after(:create) do |shipment, evaluator| + shipping_method = create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee) + shipment.add_shipping_method(shipping_method, true) + end end end @@ -367,34 +382,13 @@ FactoryBot.define do allow_order_changes { true } end - factory :shipping_method_with_shipping_fee, parent: :shipping_method do - transient do - shipping_fee 3 - end - - calculator { build(:calculator_per_item, preferred_amount: shipping_fee) } - require_ship_address { false } - distributors { [create(:distributor_enterprise_with_tax)] } - end - - factory :shipment_with_shipping_fee, parent: :shipment do - transient do - shipping_fee 3 - end - - after(:create) do |shipment, evaluator| - shipping_method = create(:shipping_method_with_shipping_fee, shipping_fee: evaluator.shipping_fee) - shipment.add_shipping_method(shipping_method, true) - end - end - factory :completed_order_with_fees, parent: :order_with_totals_and_distribution do transient do shipping_fee 3 payment_fee 5 end - shipments { [ create(:shipment_with_shipping_fee, shipping_fee: shipping_fee) ] } + shipments { [ create(:shipment_with, :shipping_fee, shipping_fee: shipping_fee) ] } after(:create) do |order, evaluator| create(:line_item, order: order) diff --git a/spec/models/concerns/order_shipping_method_spec.rb b/spec/models/concerns/order_shipping_method_spec.rb index 86c178e115..bc05352f20 100644 --- a/spec/models/concerns/order_shipping_method_spec.rb +++ b/spec/models/concerns/order_shipping_method_spec.rb @@ -12,7 +12,8 @@ describe OrderShippingMethod do context 'when order has single shipment' do it 'returns the shipments shipping_method' do - shipment = create(:shipment_with_flat_rate) + shipping_method = create(:shipping_method_with, :flat_rate) + shipment = create(:shipment_with, :shipping_method, shipping_method: shipping_method) order.shipments = [shipment] expect(order.shipping_method).to eq shipment.shipping_method diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index e64b0e14ba..114c7e7cd3 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -59,7 +59,8 @@ module Spree end describe "Shipment adjustments" do - let!(:shipment) { create(:shipment_with_flat_rate) } + let(:shipping_method) { create(:shipping_method_with, :flat_rate) } + let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } let!(:order) { create(:order, distributor: hub, shipments: [shipment]) } let(:hub) { create(:distributor_enterprise, charges_sales_tax: true) } let!(:line_item) { create(:line_item, order: order) } diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 0190ecf790..182c790f7e 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -222,7 +222,8 @@ describe Spree::Order do end describe "getting the shipping tax" do - let(:shipment) { create(:shipment_with_flat_rate) } + let(:shipping_method) { create(:shipping_method_with, :flat_rate) } + let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } let(:order) { create(:order, shipments: [shipment]) } context "with a taxed shipment" do @@ -254,7 +255,8 @@ describe Spree::Order do end describe "getting the total tax" do - let(:shipment) { create(:shipment_with_flat_rate) } + let(:shipping_method) { create(:shipping_method_with, :flat_rate) } + let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } let(:order) { create(:order, shipments: [shipment]) } let(:enterprise_fee) { create(:enterprise_fee) } let!(:adjustment) { create(:adjustment, adjustable: order, originator: enterprise_fee, label: "EF", amount: 123, included_tax: 2) } From 38c2f5227817dc4cef6e7b1b5195d2e2f668f9b6 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 21:45:29 +0100 Subject: [PATCH 4/7] In features/admin/reports_spec, fixed setting shipping_method in order, this is now done through order shipments --- spec/factories.rb | 6 ++++++ spec/features/admin/reports_spec.rb | 11 +++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index f92acc286f..ded7303a06 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -344,6 +344,12 @@ FactoryBot.define do calculator { Spree::Calculator::FlatRate.new(preferred_amount: 50.0) } end + trait :expensive_name do + name { "Shipping" } + description { "Expensive" } + calculator { Spree::Calculator::FlatRate.new(preferred_amount: 100.55) } + end + trait :shipping_fee do transient do shipping_fee 3 diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index f11bb6d962..c930f5add4 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -182,12 +182,13 @@ feature %q{ let(:distributor2) { create(:distributor_enterprise, with_payment_and_shipping: true, charges_sales_tax: true) } let(:user1) { create_enterprise_user enterprises: [distributor1] } let(:user2) { create_enterprise_user enterprises: [distributor2] } - let(:shipping_method) { create(:shipping_method, name: "Shipping", description: "Expensive", calculator: Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } + let(:shipping_method) { create(:shipping_method_with, :expensive_name) } + let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } let(:enterprise_fee) { create(:enterprise_fee, enterprise: user1.enterprises.first, tax_category: product2.tax_category, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 120.0)) } let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor1, coordinator_fees: [enterprise_fee], distributors: [distributor1], variants: [product1.master]) } let!(:zone) { create(:zone_with_member) } - let(:order1) { create(:order, order_cycle: order_cycle, distributor: user1.enterprises.first, shipping_method: shipping_method, bill_address: create(:address)) } + let(:order1) { create(:order, order_cycle: order_cycle, distributor: user1.enterprises.first, shipments: [shipment], bill_address: create(:address)) } let(:product1) { create(:taxed_product, zone: zone, price: 12.54, tax_rate_amount: 0) } let(:product2) { create(:taxed_product, zone: zone, price: 500.15, tax_rate_amount: 0.2) } @@ -395,7 +396,9 @@ feature %q{ let(:distributor2) { create(:distributor_enterprise, with_payment_and_shipping: true, charges_sales_tax: true) } let(:user1) { create_enterprise_user enterprises: [distributor1] } let(:user2) { create_enterprise_user enterprises: [distributor2] } - let(:shipping_method) { create(:shipping_method, name: "Shipping", description: "Expensive", calculator: Spree::Calculator::FlatRate.new(preferred_amount: 100.55)) } + let(:shipping_method) { create(:shipping_method_with, :expensive_name) } + let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } + let(:enterprise_fee1) { create(:enterprise_fee, enterprise: user1.enterprises.first, tax_category: product2.tax_category, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 10)) } let(:enterprise_fee2) { create(:enterprise_fee, enterprise: user1.enterprises.first, tax_category: product2.tax_category, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 20)) } let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor1, coordinator_fees: [enterprise_fee1, enterprise_fee2], distributors: [distributor1], variants: [product1.master]) } @@ -403,7 +406,7 @@ feature %q{ let!(:zone) { create(:zone_with_member) } let(:country) { Spree::Country.find Spree::Config.default_country_id } let(:bill_address) { create(:address, firstname: 'Customer', lastname: 'Name', address1: 'customer l1', address2: '', city: 'customer city', zipcode: 1234, country: country) } - let(:order1) { create(:order, order_cycle: order_cycle, distributor: user1.enterprises.first, shipping_method: shipping_method, bill_address: bill_address) } + let(:order1) { create(:order, order_cycle: order_cycle, distributor: user1.enterprises.first, shipments: [shipment], bill_address: bill_address) } let(:product1) { create(:taxed_product, zone: zone, price: 12.54, tax_rate_amount: 0, sku: 'sku1') } let(:product2) { create(:taxed_product, zone: zone, price: 500.15, tax_rate_amount: 0.2, sku: 'sku2') } From 75656668cdaade2bbc2a994f1b38642b73734b03 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 22:42:24 +0100 Subject: [PATCH 5/7] Fixed query in order_cycle_management_report and fixed the respective spec by setting shipping_method in order through shipments --- lib/open_food_network/order_cycle_management_report.rb | 2 +- .../order_cycle_management_report_spec.rb | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb index d9921d15c2..72133a1ba3 100644 --- a/lib/open_food_network/order_cycle_management_report.rb +++ b/lib/open_food_network/order_cycle_management_report.rb @@ -107,7 +107,7 @@ module OpenFoodNetwork def filter_to_shipping_method(orders) if params[:shipping_method_in].present? - orders.joins(:shipping_method).where(shipping_method_id: params[:shipping_method_in]) + orders.joins(shipments: :shipping_methods).where(shipping_method_id: params[:shipping_method_in]) else orders end diff --git a/spec/lib/open_food_network/order_cycle_management_report_spec.rb b/spec/lib/open_food_network/order_cycle_management_report_spec.rb index 49a9915301..8c50b567a5 100644 --- a/spec/lib/open_food_network/order_cycle_management_report_spec.rb +++ b/spec/lib/open_food_network/order_cycle_management_report_spec.rb @@ -69,7 +69,8 @@ module OpenFoodNetwork let!(:oc1) { create(:simple_order_cycle) } let!(:pm1) { create(:payment_method, name: "PM1") } let!(:sm1) { create(:shipping_method, name: "ship1") } - let!(:order1) { create(:order, shipping_method: sm1, order_cycle: oc1) } + let!(:s1) { create(:shipment_with, :shipping_method, shipping_method: sm1) } + let!(:order1) { create(:order, shipments: [s1], order_cycle: oc1) } let!(:payment1) { create(:payment, order: order1, payment_method: pm1) } it "returns all orders sans-params" do @@ -89,7 +90,6 @@ module OpenFoodNetwork pm3 = create(:payment_method, name: "PM3") order2 = create(:order, payments: [create(:payment, payment_method: pm2)]) order3 = create(:order, payments: [create(:payment, payment_method: pm3)]) - # payment2 = create(:payment, order: order2, payment_method: pm2) subject.stub(:params).and_return(payment_method_in: [pm1.id, pm3.id] ) subject.filter(orders).should match_array [order1, order3] @@ -98,8 +98,10 @@ module OpenFoodNetwork it "filters to a shipping method" do sm2 = create(:shipping_method, name: "ship2") sm3 = create(:shipping_method, name: "ship3") - order2 = create(:order, shipping_method: sm2) - order3 = create(:order, shipping_method: sm3) + s2 = create(:shipment_with, :shipping_method, shipping_method: sm2) + s3 = create(:shipment_with, :shipping_method, shipping_method: sm3) + order2 = create(:order, shipments: [s2]) + order3 = create(:order, shipments: [s3]) subject.stub(:params).and_return(shipping_method_in: [sm1.id, sm3.id]) expect(subject.filter(orders)).to match_array [order1, order3] From c66b611b998538035450e29dd791e600c1dfcdc8 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 16 Sep 2018 23:27:13 +0100 Subject: [PATCH 6/7] Fixed setting shipping_method in the order through order.shipments in proxy_order_spec, lib/open_food_network/customers_report_spec and features/admin/shipping_methods_spec --- spec/features/admin/shipping_methods_spec.rb | 4 +++- spec/lib/open_food_network/customers_report_spec.rb | 2 +- spec/models/proxy_order_spec.rb | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index 4a06ea139a..1395b2bee8 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -51,7 +51,9 @@ feature 'shipping methods' do scenario "deleting a shipping method referenced by an order" do o = create(:order) - o.shipping_method = @sm + shipment = create(:shipment) + shipment.add_shipping_method(@sm, true) + o.shipments << shipment o.save! visit_delete spree.admin_shipping_method_path(@sm) diff --git a/spec/lib/open_food_network/customers_report_spec.rb b/spec/lib/open_food_network/customers_report_spec.rb index c17d54368d..86b17cc2a6 100644 --- a/spec/lib/open_food_network/customers_report_spec.rb +++ b/spec/lib/open_food_network/customers_report_spec.rb @@ -45,7 +45,7 @@ module OpenFoodNetwork a = create(:address) d = create(:distributor_enterprise) o = create(:order, distributor: d, bill_address: a) - o.shipping_method = create(:shipping_method) + o.shipments << create(:shipment) subject.stub(:orders).and_return [o] subject.table.should == [[ diff --git a/spec/models/proxy_order_spec.rb b/spec/models/proxy_order_spec.rb index b84089c015..fe9c945297 100644 --- a/spec/models/proxy_order_spec.rb +++ b/spec/models/proxy_order_spec.rb @@ -77,7 +77,8 @@ describe ProxyOrder, type: :model do describe "resume" do let!(:payment_method) { create(:payment_method) } - let(:order) { create(:order_with_totals, shipping_method: create(:shipping_method)) } + let!(:shipment) { create(:shipment) } + let(:order) { create(:order_with_totals, shipments: [shipment]) } let(:proxy_order) { create(:proxy_order, order: order, canceled_at: Time.zone.now) } let(:order_cycle) { proxy_order.order_cycle } From 843cd44b233f37fe7efcc0aece0a1f6fed6027d9 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 17 Sep 2018 00:21:06 +0100 Subject: [PATCH 7/7] In paypal_spec, fixed setting shipping_method in order, this is now done through order shipments --- spec/factories.rb | 7 +++++++ spec/requests/checkout/paypal_spec.rb | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index ded7303a06..35028e832c 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -350,6 +350,13 @@ FactoryBot.define do calculator { Spree::Calculator::FlatRate.new(preferred_amount: 100.55) } end + trait :distributor do + transient do + distributor { create :enterprise } + end + distributors { [distributor] } + end + trait :shipping_fee do transient do shipping_fee 3 diff --git a/spec/requests/checkout/paypal_spec.rb b/spec/requests/checkout/paypal_spec.rb index cd417943e7..fcbe44f442 100644 --- a/spec/requests/checkout/paypal_spec.rb +++ b/spec/requests/checkout/paypal_spec.rb @@ -5,9 +5,9 @@ describe "checking out an order with a paypal express payment method", type: :re let!(:address) { create(:address) } let!(:shop) { create(:enterprise) } - let!(:shipping_method) { create(:shipping_method, distributor_ids: [shop.id]) } - let!(:order) { create(:order, distributor: shop, ship_address: address.dup, bill_address: address.dup) } - let!(:shipment) { create(:shipment, order: order, shipping_method: shipping_method) } + let!(:shipping_method) { create(:shipping_method_with, :distributor, distributor: shop) } + let!(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } + let!(:order) { create(:order, distributor: shop, shipments: [shipment], ship_address: address.dup, bill_address: address.dup) } let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) } let!(:payment_method) { Spree::Gateway::PayPalExpress.create!(name: "PayPalExpress", distributor_ids: [create(:distributor_enterprise).id], environment: Rails.env) } let(:params) { { token: 'lalalala', PayerID: 'payer1', payment_method_id: payment_method.id } }