From 96ee527f0c6865c0f5584a23c5ac7e06d3ce6258 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Apr 2019 20:45:36 +0100 Subject: [PATCH 1/6] Rename distribution_change_validator to order_cycle_distributed_variants --- .rubocop_manual_todo.yml | 4 ++-- .rubocop_todo.yml | 4 ++-- app/jobs/subscription_placement_job.rb | 2 +- app/models/spree/order_decorator.rb | 4 ++-- app/services/cart_service.rb | 2 +- knapsack_rspec_report.json | 2 +- ...e_validator.rb => order_cycle_distributed_variants.rb} | 2 +- ...r_spec.rb => order_cycle_distributed_variants_spec.rb} | 6 +++--- spec/services/cart_service_spec.rb | 8 ++++---- 9 files changed, 17 insertions(+), 17 deletions(-) rename lib/open_food_network/{distribution_change_validator.rb => order_cycle_distributed_variants.rb} (91%) rename spec/lib/open_food_network/{distribution_change_validator_spec.rb => order_cycle_distributed_variants_spec.rb} (90%) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 29af82c21c..db3f6ffd48 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -154,7 +154,7 @@ Metrics/LineLength: - lib/open_food_network/available_payment_method_filter.rb - lib/open_food_network/bulk_coop_report.rb - lib/open_food_network/customers_report.rb - - lib/open_food_network/distribution_change_validator.rb + - lib/open_food_network/order_cycle_distributed_variants.rb - lib/open_food_network/enterprise_fee_applicator.rb - lib/open_food_network/enterprise_fee_calculator.rb - lib/open_food_network/enterprise_issue_validator.rb @@ -290,7 +290,7 @@ Metrics/LineLength: - spec/lib/open_food_network/bulk_coop_report_spec.rb - spec/lib/open_food_network/cached_products_renderer_spec.rb - spec/lib/open_food_network/customers_report_spec.rb - - spec/lib/open_food_network/distribution_change_validator_spec.rb + - spec/lib/open_food_network/order_cycle_distributed_variants.rb - spec/lib/open_food_network/enterprise_fee_applicator_spec.rb - spec/lib/open_food_network/enterprise_fee_calculator_spec.rb - spec/lib/open_food_network/enterprise_injection_data_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eeeda4a552..1d779ed3d8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -311,7 +311,7 @@ Layout/EmptyLinesAroundClassBody: - 'app/serializers/api/currency_config_serializer.rb' - 'app/serializers/api/product_serializer.rb' - 'lib/discourse/single_sign_on.rb' - - 'lib/open_food_network/distribution_change_validator.rb' + - 'lib/open_food_network/order_cycle_distributed_variants.rb' - 'lib/open_food_network/lettuce_share_report.rb' - 'lib/open_food_network/order_and_distributor_report.rb' - 'lib/open_food_network/rack_request_blocker.rb' @@ -1901,7 +1901,7 @@ Style/MethodDefParentheses: Exclude: - 'app/helpers/enterprises_helper.rb' - 'app/models/spree/product_decorator.rb' - - 'lib/open_food_network/distribution_change_validator.rb' + - 'lib/open_food_network/order_cycle_distributed_variants.rb' - 'lib/open_food_network/feature_toggle.rb' - 'lib/open_food_network/group_buy_report.rb' - 'spec/support/request/authentication_workflow.rb' diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb index 348420c0f8..50ace6e74d 100644 --- a/app/jobs/subscription_placement_job.rb +++ b/app/jobs/subscription_placement_job.rb @@ -67,7 +67,7 @@ class SubscriptionPlacementJob end def available_variants_for(order) - DistributionChangeValidator.new(order).variants_available_for_distribution(order.distributor, order.order_cycle) + OrderCycleDistributedVariants.new(order).variants_available_for_distribution(order.distributor, order.order_cycle) end def send_placement_email(order, changes) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index f12cdb341f..14c52be1a6 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -1,5 +1,5 @@ require 'open_food_network/enterprise_fee_calculator' -require 'open_food_network/distribution_change_validator' +require 'open_food_network/order_cycle_distributed_variants' require 'open_food_network/feature_toggle' require 'open_food_network/tag_rule_applicator' require 'concerns/order_shipment' @@ -81,7 +81,7 @@ Spree::Order.class_eval do # -- Methods def products_available_from_new_distribution # Check that the line_items in the current order are available from a newly selected distribution - errors.add(:base, I18n.t(:spree_order_availability_error)) unless DistributionChangeValidator.new(self).can_change_to_distribution?(distributor, order_cycle) + errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(self).can_change_to_distribution?(distributor, order_cycle) end def using_guest_checkout? diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb index c1bb2dbc8e..15984bc829 100644 --- a/app/services/cart_service.rb +++ b/app/services/cart_service.rb @@ -137,7 +137,7 @@ class CartService end def check_variant_available_under_distribution(variant) - return true if DistributionChangeValidator.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant + return true if OrderCycleDistributedVariants.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant errors.add(:base, I18n.t(:spree_order_populator_availability_error)) false diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 7f1400037e..5deadc4b52 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -164,7 +164,7 @@ "spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb": 0.7021048069000244, "spec/features/admin/tax_settings_spec.rb": 0.4577906131744385, "spec/models/spree/product_property_spec.rb": 0.4479696750640869, - "spec/lib/open_food_network/distribution_change_validator_spec.rb": 0.2241048812866211, + "spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb": 0.2241048812866211, "spec/models/spree/option_value_spec.rb": 0.5916051864624023, "spec/controllers/spree/api/line_items_controller_spec.rb": 0.5350861549377441, "spec/helpers/order_cycles_helper_spec.rb": 0.40488767623901367, diff --git a/lib/open_food_network/distribution_change_validator.rb b/lib/open_food_network/order_cycle_distributed_variants.rb similarity index 91% rename from lib/open_food_network/distribution_change_validator.rb rename to lib/open_food_network/order_cycle_distributed_variants.rb index 66f6e350f2..2f66c12cd6 100644 --- a/lib/open_food_network/distribution_change_validator.rb +++ b/lib/open_food_network/order_cycle_distributed_variants.rb @@ -1,4 +1,4 @@ -class DistributionChangeValidator +class OrderCycleDistributedVariants def initialize order @order = order diff --git a/spec/lib/open_food_network/distribution_change_validator_spec.rb b/spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb similarity index 90% rename from spec/lib/open_food_network/distribution_change_validator_spec.rb rename to spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb index e39f073fba..0c4a0d2d3d 100644 --- a/spec/lib/open_food_network/distribution_change_validator_spec.rb +++ b/spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb @@ -1,8 +1,8 @@ -require 'open_food_network/distribution_change_validator' +require 'open_food_network/order_cycle_distributed_variants' -describe DistributionChangeValidator do +describe OrderCycleDistributedVariants do let(:order) { double(:order) } - let(:subject) { DistributionChangeValidator.new(order) } + let(:subject) { OrderCycleDistributedVariants.new(order) } let(:product) { double(:product) } describe "checking if an order can change to a specified new distribution" do diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index 5893662ffc..6c3a01d98b 100644 --- a/spec/services/cart_service_spec.rb +++ b/spec/services/cart_service_spec.rb @@ -228,19 +228,19 @@ describe CartService do let(:product) { double(:product) } let(:variant) { double(:variant, product: product) } - it "delegates to DistributionChangeValidator, returning true when available" do + it "delegates to OrderCycleDistributedVariants, returning true when available" do dcv = double(:dcv) expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([variant]) - expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv) + expect(OrderCycleDistributedVariants).to receive(:new).with(order).and_return(dcv) cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be true expect(cart_service.errors).to be_empty end - it "delegates to DistributionChangeValidator, returning false and erroring otherwise" do + it "delegates to OrderCycleDistributedVariants, returning false and erroring otherwise" do dcv = double(:dcv) expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([]) - expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv) + expect(OrderCycleDistributedVariants).to receive(:new).with(order).and_return(dcv) cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be false expect(cart_service.errors.to_a).to eq(["That product is not available from the chosen distributor or order cycle."]) From 44b0592223ee73b128d250344ee6a6ac70dc6362 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Apr 2019 20:59:56 +0100 Subject: [PATCH 2/6] Move OrdercycleDistributedVariants from lib/open_food_network to app/services --- .rubocop_manual_todo.yml | 4 ++-- .rubocop_todo.yml | 4 ++-- app/models/spree/order_decorator.rb | 1 - .../services}/order_cycle_distributed_variants.rb | 0 knapsack_rspec_report.json | 2 +- .../order_cycle_distributed_variants_spec.rb | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) rename {lib/open_food_network => app/services}/order_cycle_distributed_variants.rb (100%) rename spec/{lib/open_food_network => services}/order_cycle_distributed_variants_spec.rb (96%) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index db3f6ffd48..e522ba7e80 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -154,7 +154,7 @@ Metrics/LineLength: - lib/open_food_network/available_payment_method_filter.rb - lib/open_food_network/bulk_coop_report.rb - lib/open_food_network/customers_report.rb - - lib/open_food_network/order_cycle_distributed_variants.rb + - app/services/order_cycle_distributed_variants.rb - lib/open_food_network/enterprise_fee_applicator.rb - lib/open_food_network/enterprise_fee_calculator.rb - lib/open_food_network/enterprise_issue_validator.rb @@ -290,7 +290,7 @@ Metrics/LineLength: - spec/lib/open_food_network/bulk_coop_report_spec.rb - spec/lib/open_food_network/cached_products_renderer_spec.rb - spec/lib/open_food_network/customers_report_spec.rb - - spec/lib/open_food_network/order_cycle_distributed_variants.rb + - spec/services/order_cycle_distributed_variants.rb - spec/lib/open_food_network/enterprise_fee_applicator_spec.rb - spec/lib/open_food_network/enterprise_fee_calculator_spec.rb - spec/lib/open_food_network/enterprise_injection_data_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1d779ed3d8..0d335fa70b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -311,7 +311,7 @@ Layout/EmptyLinesAroundClassBody: - 'app/serializers/api/currency_config_serializer.rb' - 'app/serializers/api/product_serializer.rb' - 'lib/discourse/single_sign_on.rb' - - 'lib/open_food_network/order_cycle_distributed_variants.rb' + - 'app/services/order_cycle_distributed_variants.rb' - 'lib/open_food_network/lettuce_share_report.rb' - 'lib/open_food_network/order_and_distributor_report.rb' - 'lib/open_food_network/rack_request_blocker.rb' @@ -1901,7 +1901,7 @@ Style/MethodDefParentheses: Exclude: - 'app/helpers/enterprises_helper.rb' - 'app/models/spree/product_decorator.rb' - - 'lib/open_food_network/order_cycle_distributed_variants.rb' + - 'app/services/order_cycle_distributed_variants.rb' - 'lib/open_food_network/feature_toggle.rb' - 'lib/open_food_network/group_buy_report.rb' - 'spec/support/request/authentication_workflow.rb' diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 14c52be1a6..f2e405dc38 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -1,5 +1,4 @@ require 'open_food_network/enterprise_fee_calculator' -require 'open_food_network/order_cycle_distributed_variants' require 'open_food_network/feature_toggle' require 'open_food_network/tag_rule_applicator' require 'concerns/order_shipment' diff --git a/lib/open_food_network/order_cycle_distributed_variants.rb b/app/services/order_cycle_distributed_variants.rb similarity index 100% rename from lib/open_food_network/order_cycle_distributed_variants.rb rename to app/services/order_cycle_distributed_variants.rb diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 5deadc4b52..8e613088be 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -164,7 +164,7 @@ "spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb": 0.7021048069000244, "spec/features/admin/tax_settings_spec.rb": 0.4577906131744385, "spec/models/spree/product_property_spec.rb": 0.4479696750640869, - "spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb": 0.2241048812866211, + "spec/services/order_cycle_distributed_variants_spec.rb": 0.2241048812866211, "spec/models/spree/option_value_spec.rb": 0.5916051864624023, "spec/controllers/spree/api/line_items_controller_spec.rb": 0.5350861549377441, "spec/helpers/order_cycles_helper_spec.rb": 0.40488767623901367, diff --git a/spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb b/spec/services/order_cycle_distributed_variants_spec.rb similarity index 96% rename from spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb rename to spec/services/order_cycle_distributed_variants_spec.rb index 0c4a0d2d3d..f8f2c65772 100644 --- a/spec/lib/open_food_network/order_cycle_distributed_variants_spec.rb +++ b/spec/services/order_cycle_distributed_variants_spec.rb @@ -1,4 +1,4 @@ -require 'open_food_network/order_cycle_distributed_variants' +require 'spec_helper' describe OrderCycleDistributedVariants do let(:order) { double(:order) } From 59ec52babe3f8ca86b16d20b81b75c5b1daffbc5 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 11 Apr 2019 22:03:57 +0100 Subject: [PATCH 3/6] Refactor order_cycle_distributed_variants, better method names and simpler code --- app/jobs/subscription_placement_job.rb | 2 +- app/models/spree/order_decorator.rb | 2 +- app/services/cart_service.rb | 2 +- .../order_cycle_distributed_variants.rb | 16 ++++++------ spec/services/cart_service_spec.rb | 18 +++++++------ .../order_cycle_distributed_variants_spec.rb | 25 ++++++++----------- 6 files changed, 31 insertions(+), 34 deletions(-) diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb index 50ace6e74d..51056da676 100644 --- a/app/jobs/subscription_placement_job.rb +++ b/app/jobs/subscription_placement_job.rb @@ -67,7 +67,7 @@ class SubscriptionPlacementJob end def available_variants_for(order) - OrderCycleDistributedVariants.new(order).variants_available_for_distribution(order.distributor, order.order_cycle) + OrderCycleDistributedVariants.new(order.order_cycle, order.distributor).available_variants end def send_placement_email(order, changes) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index f2e405dc38..31a6752859 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -80,7 +80,7 @@ Spree::Order.class_eval do # -- Methods def products_available_from_new_distribution # Check that the line_items in the current order are available from a newly selected distribution - errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(self).can_change_to_distribution?(distributor, order_cycle) + errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(order_cycle, distributor).distributes_order_variants?(self) end def using_guest_checkout? diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb index 15984bc829..49d157a836 100644 --- a/app/services/cart_service.rb +++ b/app/services/cart_service.rb @@ -137,7 +137,7 @@ class CartService end def check_variant_available_under_distribution(variant) - return true if OrderCycleDistributedVariants.new(@order).variants_available_for_distribution(@distributor, @order_cycle).include? variant + return true if OrderCycleDistributedVariants.new(@order_cycle, @distributor).available_variants.include? variant errors.add(:base, I18n.t(:spree_order_populator_availability_error)) false diff --git a/app/services/order_cycle_distributed_variants.rb b/app/services/order_cycle_distributed_variants.rb index 2f66c12cd6..0718b9b393 100644 --- a/app/services/order_cycle_distributed_variants.rb +++ b/app/services/order_cycle_distributed_variants.rb @@ -1,15 +1,15 @@ class OrderCycleDistributedVariants - - def initialize order - @order = order + def initialize(order_cycle, distributor) + @order_cycle = order_cycle + @distributor = distributor end - def can_change_to_distribution?(distributor, order_cycle) - (@order.line_item_variants - variants_available_for_distribution(distributor, order_cycle)).empty? + def distributes_order_variants?(order) + (order.line_item_variants - available_variants).empty? end - def variants_available_for_distribution(distributor, order_cycle) - return [] unless order_cycle - order_cycle.variants_distributed_by(distributor) + def available_variants + return [] unless @order_cycle + @order_cycle.variants_distributed_by(@distributor) end end diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index 6c3a01d98b..3fd70c27fe 100644 --- a/spec/services/cart_service_spec.rb +++ b/spec/services/cart_service_spec.rb @@ -227,21 +227,23 @@ describe CartService do describe "checking variant is available under the distributor" do let(:product) { double(:product) } let(:variant) { double(:variant, product: product) } + let(:order_cycle_distributed_variants) { double(:order_cycle_distributed_variants) } + + before do + expect(OrderCycleDistributedVariants).to receive(:new).with(234, 123).and_return(order_cycle_distributed_variants) + cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } + end it "delegates to OrderCycleDistributedVariants, returning true when available" do - dcv = double(:dcv) - expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([variant]) - expect(OrderCycleDistributedVariants).to receive(:new).with(order).and_return(dcv) - cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } + expect(order_cycle_distributed_variants).to receive(:available_variants).and_return([variant]) + expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be true expect(cart_service.errors).to be_empty end it "delegates to OrderCycleDistributedVariants, returning false and erroring otherwise" do - dcv = double(:dcv) - expect(dcv).to receive(:variants_available_for_distribution).with(123, 234).and_return([]) - expect(OrderCycleDistributedVariants).to receive(:new).with(order).and_return(dcv) - cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } + expect(order_cycle_distributed_variants).to receive(:available_variants).and_return([]) + expect(cart_service.send(:check_variant_available_under_distribution, variant)).to be false expect(cart_service.errors.to_a).to eq(["That product is not available from the chosen distributor or order cycle."]) end diff --git a/spec/services/order_cycle_distributed_variants_spec.rb b/spec/services/order_cycle_distributed_variants_spec.rb index f8f2c65772..fd55e1ada5 100644 --- a/spec/services/order_cycle_distributed_variants_spec.rb +++ b/spec/services/order_cycle_distributed_variants_spec.rb @@ -2,41 +2,36 @@ require 'spec_helper' describe OrderCycleDistributedVariants do let(:order) { double(:order) } - let(:subject) { OrderCycleDistributedVariants.new(order) } + let(:distributor) { double(:distributor) } + let(:order_cycle) { double(:order_cycle) } + let(:subject) { OrderCycleDistributedVariants.new(order_cycle, distributor) } let(:product) { double(:product) } describe "checking if an order can change to a specified new distribution" do - let(:distributor) { double(:distributor) } - let(:order_cycle) { double(:order_cycle) } - it "returns false when a variant is not available for the specified distribution" do order.should_receive(:line_item_variants) { [1] } - subject.should_receive(:variants_available_for_distribution). - with(distributor, order_cycle) { [] } - expect(subject.can_change_to_distribution?(distributor, order_cycle)).to be false + subject.should_receive(:available_variants) { [] } + expect(subject.distributes_order_variants?(order)).to be false end it "returns true when all variants are available for the specified distribution" do order.should_receive(:line_item_variants) { [1] } - subject.should_receive(:variants_available_for_distribution). - with(distributor, order_cycle) { [1] } - expect(subject.can_change_to_distribution?(distributor, order_cycle)).to be true + subject.should_receive(:available_variants) { [1] } + expect(subject.distributes_order_variants?(order)).to be true end end describe "finding variants that are available through a particular order cycle" do it "finds variants distributed by order cycle" do variant = double(:variant) - distributor = double(:distributor) - order_cycle = double(:order_cycle) - order_cycle.should_receive(:variants_distributed_by).with(distributor) { [variant] } - expect(subject.variants_available_for_distribution(distributor, order_cycle)).to eq [variant] + expect(subject.available_variants).to eq [variant] end it "returns an empty array when order cycle is nil" do - expect(subject.variants_available_for_distribution(nil, nil)).to eq [] + subject = OrderCycleDistributedVariants.new(nil, nil) + expect(subject.available_variants).to eq [] end end end From d602375ac78bc888f51d6eea92ae89b237ffb142 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 13 Apr 2019 17:43:28 +0100 Subject: [PATCH 4/6] Redirect user to cart page if some item in the order is unavailable --- app/controllers/checkout_controller.rb | 9 ++-- spec/controllers/checkout_controller_spec.rb | 48 +++++++++++++------ .../requests/checkout/failed_checkout_spec.rb | 4 ++ spec/requests/checkout/stripe_connect_spec.rb | 4 ++ 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index dfb9b593de..56b163cec0 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -169,7 +169,7 @@ class CheckoutController < Spree::CheckoutController def load_order @order = current_order redirect_to main_app.shop_path and return unless @order and @order.checkout_allowed? - raise_insufficient_quantity and return if @order.insufficient_stock_lines.present? + redirect_to_cart_path and return unless valid_order_line_items? redirect_to main_app.shop_path and return if @order.completed? before_address setup_for_current_state @@ -184,8 +184,11 @@ class CheckoutController < Spree::CheckoutController @order.ship_address = finder.ship_address end - # Overriding Spree's methods - def raise_insufficient_quantity + def valid_order_line_items? + @order.insufficient_stock_lines.empty? && OrderCycleDistributedVariants.new(@order.order_cycle, @order.distributor).distributes_order_variants?(@order) + end + + def redirect_to_cart_path respond_to do |format| format.html do redirect_to cart_path diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index bf75bc6128..c83f8007ef 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -36,22 +36,40 @@ describe CheckoutController, type: :controller do flash[:info].should == "The hub you have selected is temporarily closed for orders. Please try again later." end - it "redirects to the cart when some items are out of stock" do - controller.stub(:current_distributor).and_return(distributor) - controller.stub(:current_order_cycle).and_return(order_cycle) - controller.stub(:current_order).and_return(order) - order.stub_chain(:insufficient_stock_lines, :present?).and_return true - get :edit - response.should redirect_to spree.cart_path - end + describe "redirection to the cart" do + let(:order_cycle_distributed_variants) { double(:order_cycle_distributed_variants) } - it "renders when both distributor and order cycle is selected" do - controller.stub(:current_distributor).and_return(distributor) - controller.stub(:current_order_cycle).and_return(order_cycle) - controller.stub(:current_order).and_return(order) - order.stub_chain(:insufficient_stock_lines, :present?).and_return false - get :edit - response.should be_success + before do + controller.stub(:current_order).and_return(order) + order.stub(:distributor).and_return(distributor) + order.order_cycle = order_cycle + + allow(OrderCycleDistributedVariants).to receive(:new).with(order_cycle, distributor).and_return(order_cycle_distributed_variants) + end + + it "redirects when some items are out of stock" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false + + get :edit + expect(response).to redirect_to spree.cart_path + end + + it "redirects when some items are not available" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true + expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(false) + + get :edit + expect(response).to redirect_to spree.cart_path + end + + + it "does not redirect when items are available and in stock" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true + expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(true) + + get :edit + expect(response).to be_success + end end describe "building the order" do diff --git a/spec/requests/checkout/failed_checkout_spec.rb b/spec/requests/checkout/failed_checkout_spec.rb index 859ca41a1b..345b6a86df 100644 --- a/spec/requests/checkout/failed_checkout_spec.rb +++ b/spec/requests/checkout/failed_checkout_spec.rb @@ -23,6 +23,10 @@ describe "checking out an order that initially fails", type: :request do end before do + order_cycle_distributed_variants = double(:order_cycle_distributed_variants) + allow(OrderCycleDistributedVariants).to receive(:new).and_return(order_cycle_distributed_variants) + allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?).and_return(true) + order.reload.update_totals set_order order end diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb index fa8fb703a5..cd5eceb796 100644 --- a/spec/requests/checkout/stripe_connect_spec.rb +++ b/spec/requests/checkout/stripe_connect_spec.rb @@ -27,6 +27,10 @@ describe "checking out an order with a Stripe Connect payment method", type: :re end before do + order_cycle_distributed_variants = double(:order_cycle_distributed_variants) + allow(OrderCycleDistributedVariants).to receive(:new).and_return(order_cycle_distributed_variants) + allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?).and_return(true) + allow(Stripe).to receive(:api_key) { "sk_test_12345" } order.update_attributes(distributor_id: enterprise.id, order_cycle_id: order_cycle.id) order.reload.update_totals From fce3d693451c7e75a348b5498cc551a92f88db7e Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 13 Apr 2019 20:44:10 +0100 Subject: [PATCH 5/6] Add flash and warning to the cart page when item becomes unavailable --- .../spree/orders_controller_decorator.rb | 5 +++-- app/services/order_cycle_distributed_variants.rb | 6 +++++- app/views/spree/orders/_line_item.html.haml | 5 +++++ config/locales/en.yml | 2 ++ spec/controllers/spree/orders_controller_spec.rb | 15 +++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 4aea409444..0a7d146748 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -21,14 +21,15 @@ Spree::OrdersController.class_eval do def edit @order = current_order(true) @insufficient_stock_lines = @order.insufficient_stock_lines + @unavailable_order_variants = OrderCycleDistributedVariants.new(current_order_cycle, current_distributor).unavailable_order_variants(@order) if @order.line_items.empty? redirect_to main_app.shop_path else associate_user - if @order.insufficient_stock_lines.present? - flash[:error] = t("spree.inventory_error_flash_for_insufficient_quantity") + if @order.insufficient_stock_lines.present? || @unavailable_order_variants.present? + flash[:error] = t("spree.orders.error_flash_for_unavailable_items") end end end diff --git a/app/services/order_cycle_distributed_variants.rb b/app/services/order_cycle_distributed_variants.rb index 0718b9b393..abaa3303e4 100644 --- a/app/services/order_cycle_distributed_variants.rb +++ b/app/services/order_cycle_distributed_variants.rb @@ -5,7 +5,11 @@ class OrderCycleDistributedVariants end def distributes_order_variants?(order) - (order.line_item_variants - available_variants).empty? + unavailable_order_variants(order).empty? + end + + def unavailable_order_variants(order) + order.line_item_variants - available_variants end def available_variants diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index 1194bed136..ac1396fd14 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -14,6 +14,11 @@ = variant.in_stock? ? t(".insufficient_stock", :on_hand => variant.on_hand) : t(".out_of_stock") %br/ + - if @unavailable_order_variants.andand.include? line_item.variant + %span.out-of-stock + = t(".unavailable_item") + %br/ + %td.text-right.cart-item-price{"data-hook" => "cart_item_price"} = line_item.single_display_amount_with_adjustments.to_html %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"} diff --git a/config/locales/en.yml b/config/locales/en.yml index eb2885c416..c26e0da260 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3073,6 +3073,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using format: ! '%Y-%m-%d' js_format: 'yy-mm-dd' orders: + error_flash_for_unavailable_items: "An item in your cart has become unavailable." edit: login_to_view_order: "Please log in to view your order." bought: @@ -3080,6 +3081,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using line_item: insufficient_stock: "Insufficient stock available, only %{on_hand} remaining" out_of_stock: "Out of Stock" + unavailable_item: "Currently unavailable" shipment_states: backorder: backorder partial: partial diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index 071e463979..63a4bd5680 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -89,6 +89,9 @@ describe Spree::OrdersController, type: :controller do allow(controller).to receive(:current_order).and_return order allow(order).to receive_message_chain(:line_items, :empty?).and_return true allow(order).to receive(:insufficient_stock_lines).and_return [] + allow(order).to receive(:line_item_variants).and_return [] + allow(order_cycle).to receive(:variants_distributed_by).and_return [] + session[:access_token] = order.token spree_get :edit expect(response).to redirect_to shop_path @@ -161,6 +164,18 @@ describe Spree::OrdersController, type: :controller do expect(flash[:error]).to eq("An item in your cart has become unavailable.") end end + + describe "when an item is unavailable" do + before do + order.order_cycle = create(:simple_order_cycle, distributors: [d], variants: []) + end + + it "displays a flash message when we view the cart" do + spree_get :edit + expect(response.status).to eq 200 + expect(flash[:error]).to eq("An item in your cart has become unavailable.") + end + end end end From e8ccb55f47b3914aa58efe36f26226e6db455153 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 15 Apr 2019 12:39:52 +0100 Subject: [PATCH 6/6] Use rspec 3 syntax in order_cycle_distributed_variants_spec and checkout_controller_spec --- spec/controllers/checkout_controller_spec.rb | 5 ++--- spec/services/order_cycle_distributed_variants_spec.rb | 10 +++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index c83f8007ef..b59898b278 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -40,8 +40,8 @@ describe CheckoutController, type: :controller do let(:order_cycle_distributed_variants) { double(:order_cycle_distributed_variants) } before do - controller.stub(:current_order).and_return(order) - order.stub(:distributor).and_return(distributor) + allow(controller).to receive(:current_order).and_return(order) + allow(order).to receive(:distributor).and_return(distributor) order.order_cycle = order_cycle allow(OrderCycleDistributedVariants).to receive(:new).with(order_cycle, distributor).and_return(order_cycle_distributed_variants) @@ -62,7 +62,6 @@ describe CheckoutController, type: :controller do expect(response).to redirect_to spree.cart_path end - it "does not redirect when items are available and in stock" do allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(true) diff --git a/spec/services/order_cycle_distributed_variants_spec.rb b/spec/services/order_cycle_distributed_variants_spec.rb index fd55e1ada5..8b92e7905b 100644 --- a/spec/services/order_cycle_distributed_variants_spec.rb +++ b/spec/services/order_cycle_distributed_variants_spec.rb @@ -9,14 +9,14 @@ describe OrderCycleDistributedVariants do describe "checking if an order can change to a specified new distribution" do it "returns false when a variant is not available for the specified distribution" do - order.should_receive(:line_item_variants) { [1] } - subject.should_receive(:available_variants) { [] } + allow(order).to receive(:line_item_variants).and_return([1]) + allow(subject).to receive(:available_variants).and_return([]) expect(subject.distributes_order_variants?(order)).to be false end it "returns true when all variants are available for the specified distribution" do - order.should_receive(:line_item_variants) { [1] } - subject.should_receive(:available_variants) { [1] } + allow(order).to receive(:line_item_variants).and_return([1]) + allow(subject).to receive(:available_variants).and_return([1]) expect(subject.distributes_order_variants?(order)).to be true end end @@ -24,7 +24,7 @@ describe OrderCycleDistributedVariants do describe "finding variants that are available through a particular order cycle" do it "finds variants distributed by order cycle" do variant = double(:variant) - order_cycle.should_receive(:variants_distributed_by).with(distributor) { [variant] } + allow(order_cycle).to receive(:variants_distributed_by).with(distributor).and_return([variant]) expect(subject.available_variants).to eq [variant] end