From f8a4f00d52a13d07ff6fdb8521b555028a79bdf6 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 11 Feb 2020 20:04:22 +0000 Subject: [PATCH] Fix rubocop issues in subs specs --- .rubocop_manual_todo.yml | 4 + .../subscriptions/estimator_spec.rb | 58 ++++++++-- .../subscriptions/form_spec.rb | 69 +++++++++--- .../subscriptions/validator_spec.rb | 102 +++++++++++++----- .../subscriptions/variants_list_spec.rb | 45 ++++++-- 5 files changed, 220 insertions(+), 58 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 83de12879f..a1908cdc5d 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -678,9 +678,13 @@ Metrics/ModuleLength: - app/helpers/injection_helper.rb - app/helpers/spree/admin/navigation_helper.rb - app/helpers/spree/admin/base_helper.rb + - engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb + - engines/order_management/spec/services/order_management/subscriptions/form_spec.rb - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb - engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb + - engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb + - engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb - lib/open_food_network/column_preference_defaults.rb - spec/controllers/admin/enterprises_controller_spec.rb - spec/controllers/admin/order_cycles_controller_spec.rb diff --git a/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb index 78020f6578..e8fe7bf957 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb @@ -59,8 +59,12 @@ module OrderManagement end context "when variant overrides apply" do - let!(:override1) { create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2) } - let!(:override2) { create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3) } + let!(:override1) { + create(:variant_override, hub: subscription.shop, variant: sli1.variant, price: 1.2) + } + let!(:override2) { + create(:variant_override, hub: subscription.shop, variant: sli2.variant, price: 2.3) + } it "recalculates price_estimates based on override prices and associated fees" do estimator.estimate! @@ -73,7 +77,11 @@ module OrderManagement end describe "updating estimates for shipping and payment fees" do - let(:subscription) { create(:subscription, with_items: true, payment_method: payment_method, shipping_method: shipping_method) } + let(:subscription) { + create(:subscription, with_items: true, + payment_method: payment_method, + shipping_method: shipping_method) + } let!(:sli1) { subscription.subscription_line_items.first } let!(:sli2) { subscription.subscription_line_items.second } let!(:sli3) { subscription.subscription_line_items.third } @@ -87,8 +95,14 @@ module OrderManagement end context "using flat rate calculators" do - let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34)) } - let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12)) } + let(:shipping_method) { + create(:shipping_method, + calculator: Spree::Calculator::FlatRate.new(preferred_amount: 12.34)) + } + let(:payment_method) { + create(:payment_method, + calculator: Spree::Calculator::FlatRate.new(preferred_amount: 9.12)) + } it "calculates fees based on the rates provided" do estimator.estimate! @@ -98,8 +112,18 @@ module OrderManagement end context "using flat percent item total calculators" do - let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 10)) } - let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::FlatPercentItemTotal.new(preferred_flat_percent: 20)) } + let(:shipping_method) { + create(:shipping_method, + calculator: Spree::Calculator::FlatPercentItemTotal.new( + preferred_flat_percent: 10 + )) + } + let(:payment_method) { + create(:payment_method, + calculator: Spree::Calculator::FlatPercentItemTotal.new( + preferred_flat_percent: 20 + )) + } it "calculates fees based on the estimated item total and percentage provided" do estimator.estimate! @@ -109,8 +133,14 @@ module OrderManagement end context "using flat percent per item calculators" do - let(:shipping_method) { create(:shipping_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5)) } - let(:payment_method) { create(:payment_method, calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) } + let(:shipping_method) { + create(:shipping_method, + calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 5)) + } + let(:payment_method) { + create(:payment_method, + calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 10)) + } it "calculates fees based on the estimated item prices and percentage provided" do estimator.estimate! @@ -120,8 +150,14 @@ module OrderManagement end context "using per item calculators" do - let(:shipping_method) { create(:shipping_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2)) } - let(:payment_method) { create(:payment_method, calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3)) } + let(:shipping_method) { + create(:shipping_method, + calculator: Spree::Calculator::PerItem.new(preferred_amount: 1.2)) + } + let(:payment_method) { + create(:payment_method, + calculator: Spree::Calculator::PerItem.new(preferred_amount: 0.3)) + } it "calculates fees based on the number of items and rate provided" do estimator.estimate! diff --git a/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb index f9c0798671..3e849d5c82 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/form_spec.rb @@ -11,19 +11,64 @@ module OrderManagement let!(:product1) { create(:product, supplier: shop) } let!(:product2) { create(:product, supplier: shop) } let!(:product3) { create(:product, supplier: shop) } - let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) } - let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) } - let!(:variant3) { create(:variant, product: product2, unit_value: '1000', price: 2.50, option_values: [], on_hand: 1) } + let!(:variant1) { + create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) + } + let!(:variant2) { + create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) + } + let!(:variant3) { + create(:variant, product: product2, unit_value: '1000', + price: 2.50, option_values: [], on_hand: 1) + } let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } - let!(:order_cycle1) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 9.days.ago, orders_close_at: 2.days.ago) } - let!(:order_cycle2) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.ago, orders_close_at: 5.days.from_now) } - let!(:order_cycle3) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 5.days.from_now, orders_close_at: 12.days.from_now) } - let!(:order_cycle4) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 12.days.from_now, orders_close_at: 19.days.from_now) } - let!(:outgoing_exchange1) { order_cycle1.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) } - let!(:outgoing_exchange2) { order_cycle2.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) } - let!(:outgoing_exchange3) { order_cycle3.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant3], enterprise_fees: []) } - let!(:outgoing_exchange4) { order_cycle4.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2, variant3], enterprise_fees: [enterprise_fee]) } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4]) } + let!(:order_cycle1) { + create(:simple_order_cycle, coordinator: shop, + orders_open_at: 9.days.ago, + orders_close_at: 2.days.ago) + } + let!(:order_cycle2) { + create(:simple_order_cycle, coordinator: shop, + orders_open_at: 2.days.ago, + orders_close_at: 5.days.from_now) + } + let!(:order_cycle3) { + create(:simple_order_cycle, coordinator: shop, + orders_open_at: 5.days.from_now, + orders_close_at: 12.days.from_now) + } + let!(:order_cycle4) { + create(:simple_order_cycle, coordinator: shop, + orders_open_at: 12.days.from_now, + orders_close_at: 19.days.from_now) + } + let!(:outgoing_exchange1) { + order_cycle1.exchanges.create(sender: shop, + receiver: shop, + variants: [variant1, variant2, variant3], + enterprise_fees: [enterprise_fee]) + } + let!(:outgoing_exchange2) { + order_cycle2.exchanges.create(sender: shop, + receiver: shop, + variants: [variant1, variant2, variant3], + enterprise_fees: [enterprise_fee]) + } + let!(:outgoing_exchange3) { + order_cycle3.exchanges.create(sender: shop, + receiver: shop, + variants: [variant1, variant3], + enterprise_fees: []) + } + let!(:outgoing_exchange4) { + order_cycle4.exchanges.create(sender: shop, + receiver: shop, + variants: [variant1, variant2, variant3], + enterprise_fees: [enterprise_fee]) + } + let!(:schedule) { + create(:schedule, order_cycles: [order_cycle1, order_cycle2, order_cycle3, order_cycle4]) + } let!(:payment_method) { create(:payment_method, distributors: [shop]) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } let!(:address) { create(:address) } diff --git a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb index a072262b47..7c52fba12a 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb @@ -64,7 +64,9 @@ module OrderManagement end describe "shipping method validation" do - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:shipping_method)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:shipping_method)) + } before { stub_validations(validator, validation_stubs.except(:shipping_method_allowed?)) } context "when no shipping method is present" do @@ -78,7 +80,9 @@ module OrderManagement context "when a shipping method is present" do let(:shipping_method) { instance_double(Spree::ShippingMethod, distributors: [shop]) } - before { expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method } } + before { + expect(subscription).to receive(:shipping_method).at_least(:once) { shipping_method } + } context "and the shipping method is not associated with the shop" do before { allow(shipping_method).to receive(:distributors) { [double(:enterprise)] } } @@ -101,7 +105,9 @@ module OrderManagement end describe "payment method validation" do - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:payment_method)) + } before { stub_validations(validator, validation_stubs.except(:payment_method_allowed?)) } context "when no payment method is present" do @@ -115,7 +121,9 @@ module OrderManagement context "when a payment method is present" do let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) } - before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } } + before { + expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } + } context "and the payment method is not associated with the shop" do before { allow(payment_method).to receive(:distributors) { [double(:enterprise)] } } @@ -138,12 +146,18 @@ module OrderManagement end describe "payment method type validation" do - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) } - before { stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:payment_method)) + } + before { + stub_validations(validator, validation_stubs.except(:payment_method_type_allowed?)) + } context "when a payment method is present" do let(:payment_method) { instance_double(Spree::PaymentMethod, distributors: [shop]) } - before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } } + before { + expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } + } context "and the payment method type is not in the approved list" do before { allow(payment_method).to receive(:type) { "Blah" } } @@ -167,7 +181,9 @@ module OrderManagement end describe "dates" do - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:begins_at, :ends_at)) + } before { stub_validations(validator, validation_stubs.except(:ends_at_after_begins_at?)) } before { expect(subscription).to receive(:begins_at).at_least(:once) { begins_at } } @@ -221,7 +237,9 @@ module OrderManagement describe "addresses" do before { stub_validations(validator, validation_stubs) } - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:bill_address, :ship_address)) + } before { expect(subscription).to receive(:bill_address).at_least(:once) { bill_address } } before { expect(subscription).to receive(:ship_address).at_least(:once) { ship_address } } @@ -323,12 +341,18 @@ module OrderManagement end describe "credit card" do - let(:subscription) { instance_double(Subscription, subscription_stubs.except(:payment_method)) } + let(:subscription) { + instance_double(Subscription, subscription_stubs.except(:payment_method)) + } before { stub_validations(validator, validation_stubs.except(:credit_card_ok?)) } - before { expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } } + before { + expect(subscription).to receive(:payment_method).at_least(:once) { payment_method } + } context "when using a Check payment method" do - let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check") } + let(:payment_method) { + instance_double(Spree::PaymentMethod, type: "Spree::PaymentMethod::Check") + } it "returns true" do expect(validator.valid?).to be true @@ -337,7 +361,9 @@ module OrderManagement end context "when using the StripeConnect payment gateway" do - let(:payment_method) { instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") } + let(:payment_method) { + instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") + } before { expect(subscription).to receive(:customer).at_least(:once) { customer } } context "when the customer does not allow charges" do @@ -388,10 +414,16 @@ module OrderManagement describe "subscription line items" do let(:subscription) { instance_double(Subscription, subscription_stubs) } - before { stub_validations(validator, validation_stubs.except(:subscription_line_items_present?)) } - before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } } + before { + stub_validations(validator, validation_stubs.except(:subscription_line_items_present?)) + } + before { + expect(subscription).to receive(:subscription_line_items).at_least(:once) { + subscription_line_items + } + } - context "when no subscription line items are present" do + context "when no subscription line items exist" do let(:subscription_line_items) { [] } it "adds an error and returns false" do @@ -400,8 +432,10 @@ module OrderManagement end end - context "when subscription line items are present but they are all marked for destruction" do - let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) } + context "when subscription line items exist but all are marked for destruction" do + let(:subscription_line_item1) { + instance_double(SubscriptionLineItem, marked_for_destruction?: true) + } let(:subscription_line_items) { [subscription_line_item1] } it "adds an error and returns false" do @@ -410,9 +444,13 @@ module OrderManagement end end - context "when subscription line items are present and some and not marked for destruction" do - let(:subscription_line_item1) { instance_double(SubscriptionLineItem, marked_for_destruction?: true) } - let(:subscription_line_item2) { instance_double(SubscriptionLineItem, marked_for_destruction?: false) } + context "when subscription line items exist and some are not marked for destruction" do + let(:subscription_line_item1) { + instance_double(SubscriptionLineItem, marked_for_destruction?: true) + } + let(:subscription_line_item2) { + instance_double(SubscriptionLineItem, marked_for_destruction?: false) + } let(:subscription_line_items) { [subscription_line_item1, subscription_line_item2] } it "returns true" do @@ -424,10 +462,16 @@ module OrderManagement describe "variant availability" do let(:subscription) { instance_double(Subscription, subscription_stubs) } - before { stub_validations(validator, validation_stubs.except(:requested_variants_available?)) } - before { expect(subscription).to receive(:subscription_line_items).at_least(:once) { subscription_line_items } } + before { + stub_validations(validator, validation_stubs.except(:requested_variants_available?)) + } + before { + expect(subscription).to receive(:subscription_line_items).at_least(:once) { + subscription_line_items + } + } - context "when no subscription line items are present" do + context "when no subscription line items exist" do let(:subscription_line_items) { [] } it "returns true" do @@ -436,11 +480,15 @@ module OrderManagement end end - context "when subscription line items are present" do + context "when subscription line items exist" do let(:variant1) { instance_double(Spree::Variant, id: 1) } let(:variant2) { instance_double(Spree::Variant, id: 2) } - let(:subscription_line_item1) { instance_double(SubscriptionLineItem, variant: variant1) } - let(:subscription_line_item2) { instance_double(SubscriptionLineItem, variant: variant2) } + let(:subscription_line_item1) { + instance_double(SubscriptionLineItem, variant: variant1) + } + let(:subscription_line_item2) { + instance_double(SubscriptionLineItem, variant: variant2) + } let(:subscription_line_items) { [subscription_line_item1] } context "but some variants are unavailable" do diff --git a/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb index 8dc97b128e..2ea3206045 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb @@ -43,7 +43,11 @@ module OrderManagement end context "if the supplier is permitted for the shop" do - let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) } + let!(:enterprise_relationship) { + create(:enterprise_relationship, child: shop, + parent: product.supplier, + permissions_list: [:add_to_order_cycle]) + } it "is eligible" do expect(described_class.eligible_variants(shop)).to include(variant) @@ -55,7 +59,11 @@ module OrderManagement let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } context "if it is an incoming exchange where the shop is the receiver" do - let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + let!(:incoming_exchange) { + order_cycle.exchanges.create(sender: product.supplier, + receiver: shop, + incoming: true, variants: [variant]) + } it "is not eligible" do expect(described_class.eligible_variants(shop)).to_not include(variant) @@ -63,7 +71,12 @@ module OrderManagement end context "if it is an outgoing exchange where the shop is the receiver" do - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + let!(:outgoing_exchange) { + order_cycle.exchanges.create(sender: product.supplier, + receiver: shop, + incoming: false, + variants: [variant]) + } context "if the order cycle is currently open" do let!(:order_cycle) { current_order_cycle } @@ -109,25 +122,41 @@ module OrderManagement let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } context "if it is an incoming exchange where the shop is the receiver" do - let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + let!(:incoming_exchange) { + order_cycle.exchanges.create(sender: product.supplier, + receiver: shop, + incoming: true, + variants: [variant]) + } it "is is false" do - expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, + schedule, + variant) end end context "if it is an outgoing exchange where the shop is the receiver" do - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + let!(:outgoing_exchange) { + order_cycle.exchanges.create(sender: product.supplier, + receiver: shop, + incoming: false, + variants: [variant]) + } it "is true" do - expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, + schedule, + variant) end end end context "if the variant is unrelated" do it "is false" do - expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, + schedule, + variant) end end end