diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 29af82c21c..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/distribution_change_validator.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/distribution_change_validator_spec.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 eeeda4a552..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/distribution_change_validator.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/distribution_change_validator.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/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/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/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb index 348420c0f8..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) - DistributionChangeValidator.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 076c00e0f6..4c3ee93276 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/distribution_change_validator' require 'open_food_network/feature_toggle' require 'open_food_network/tag_rule_applicator' require 'concerns/order_shipment' @@ -81,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 DistributionChangeValidator.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 c1bb2dbc8e..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 DistributionChangeValidator.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 new file mode 100644 index 0000000000..abaa3303e4 --- /dev/null +++ b/app/services/order_cycle_distributed_variants.rb @@ -0,0 +1,19 @@ +class OrderCycleDistributedVariants + def initialize(order_cycle, distributor) + @order_cycle = order_cycle + @distributor = distributor + end + + def distributes_order_variants?(order) + unavailable_order_variants(order).empty? + end + + def unavailable_order_variants(order) + order.line_item_variants - available_variants + end + + def available_variants + return [] unless @order_cycle + @order_cycle.variants_distributed_by(@distributor) + end +end 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 41ddcb9ab6..6b683cffdd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3065,6 +3065,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: @@ -3072,6 +3073,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/knapsack_rspec_report.json b/knapsack_rspec_report.json index 7f1400037e..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/distribution_change_validator_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/lib/open_food_network/distribution_change_validator.rb b/lib/open_food_network/distribution_change_validator.rb deleted file mode 100644 index 66f6e350f2..0000000000 --- a/lib/open_food_network/distribution_change_validator.rb +++ /dev/null @@ -1,15 +0,0 @@ -class DistributionChangeValidator - - def initialize order - @order = order - end - - def can_change_to_distribution?(distributor, order_cycle) - (@order.line_item_variants - variants_available_for_distribution(distributor, order_cycle)).empty? - end - - def variants_available_for_distribution(distributor, order_cycle) - return [] unless order_cycle - order_cycle.variants_distributed_by(distributor) - end -end diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index bf75bc6128..b59898b278 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -36,22 +36,39 @@ 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 + 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) + 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/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index bf6453269b..18fe0645bb 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 diff --git a/spec/lib/open_food_network/distribution_change_validator_spec.rb b/spec/lib/open_food_network/distribution_change_validator_spec.rb deleted file mode 100644 index e39f073fba..0000000000 --- a/spec/lib/open_food_network/distribution_change_validator_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -require 'open_food_network/distribution_change_validator' - -describe DistributionChangeValidator do - let(:order) { double(:order) } - let(:subject) { DistributionChangeValidator.new(order) } - 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 - 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 - 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] - end - - it "returns an empty array when order cycle is nil" do - expect(subject.variants_available_for_distribution(nil, nil)).to eq [] - end - end -end 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 diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index 5893662ffc..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) } - it "delegates to DistributionChangeValidator, 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) + 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 + 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 DistributionChangeValidator, 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) - cart_service.instance_eval { @distributor = 123; @order_cycle = 234 } + it "delegates to OrderCycleDistributedVariants, returning false and erroring otherwise" do + 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 new file mode 100644 index 0000000000..8b92e7905b --- /dev/null +++ b/spec/services/order_cycle_distributed_variants_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe OrderCycleDistributedVariants do + let(:order) { double(: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 + it "returns false when a variant is not available for the specified distribution" do + 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 + 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 + + describe "finding variants that are available through a particular order cycle" do + it "finds variants distributed by order cycle" do + variant = double(:variant) + allow(order_cycle).to receive(:variants_distributed_by).with(distributor).and_return([variant]) + + expect(subject.available_variants).to eq [variant] + end + + it "returns an empty array when order cycle is nil" do + subject = OrderCycleDistributedVariants.new(nil, nil) + expect(subject.available_variants).to eq [] + end + end +end