Merge branch '2-0-stable' into 2-0-update-aws-sdk

.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

.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'

app/controllers/admin/variant_overrides_controller.rb

@@ -76,7 +76,8 @@ def load_collection
     end
 
     def collection
-      @variant_overrides = VariantOverride.for_hubs(params[:hub_id] || @hubs)
+      @variant_overrides = VariantOverride.includes(:variant).for_hubs(params[:hub_id] || @hubs)
+      @variant_overrides.select { |vo| vo.variant.present? }
     end
 
     def collection_actions

app/controllers/checkout_controller.rb

@@ -169,7 +169,7 @@ def skip_state_validation?
   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 @@ def before_address
     @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

app/controllers/spree/orders_controller_decorator.rb

@@ -21,14 +21,15 @@
   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

app/jobs/subscription_placement_job.rb

@@ -67,7 +67,7 @@ def unavailable_stock_lines_for(order)
   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)

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 @@
   # -- 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?

app/services/cart_service.rb

@@ -137,7 +137,7 @@ def check_order_cycle_provided
   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

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

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"}

config/locales/en.yml

@@ -3065,13 +3065,15 @@ 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:
         item: "Already ordered in this order cycle"
       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

db/migrate/20180426145635_remove_on_demand_from_product_and_variant.spree.rb

@@ -2,6 +2,6 @@
 class RemoveOnDemandFromProductAndVariant < ActiveRecord::Migration
   def change
     remove_column :spree_products, :on_demand
-    remove_column :spree_variants, :on_demand
+    # we are removing spree_variants.on_demand in a later migration
   end
 end

db/migrate/20190501143327_migrate_variants_on_demand_to_stock_items_backorderable.rb

@@ -0,0 +1,16 @@
+class MigrateVariantsOnDemandToStockItemsBackorderable < ActiveRecord::Migration
+  def up
+    # We use SQL directly here to avoid going through VariantStock.on_demand and VariantStock.on_demand=
+    sql = "update spree_stock_items set backorderable = (select on_demand from spree_variants where spree_variants.id = spree_stock_items.variant_id)"
+    ActiveRecord::Base.connection.execute(sql)
+
+    remove_column :spree_variants, :on_demand
+  end
+
+  def down
+    add_column :spree_variants, :on_demand, :boolean, :default => false
+
+    sql = "update spree_variants set on_demand = (select backorderable from spree_stock_items where spree_variants.id = spree_stock_items.variant_id)"
+    ActiveRecord::Base.connection.execute(sql)
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20190320111312) do
+ActiveRecord::Schema.define(:version => 20190501143327) do
 
   create_table "adjustment_metadata", :force => true do |t|
     t.integer "adjustment_id"

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,

spec/controllers/admin/variant_overrides_controller_spec.rb

@@ -68,6 +68,21 @@
               expect(VariantOverride.find_by_id(variant_override.id)).to be_nil
             end
           end
+
+          context "and there is a variant override for a deleted variant" do
+            let(:deleted_variant) { create(:variant) }
+            let!(:variant_override_of_deleted_variant) { create(:variant_override, hub: hub, variant: deleted_variant) }
+
+            before { deleted_variant.update_attribute :deleted_at, Time.zone.now }
+
+            it "allows to update other variant overrides" do
+              spree_put :bulk_update, format: format, variant_overrides: variant_override_params
+
+              expect(response).to_not redirect_to spree.unauthorized_path
+              variant_override.reload
+              expect(variant_override.price).to eq 123.45
+            end
+          end
         end
       end
     end

spec/controllers/checkout_controller_spec.rb

@@ -36,22 +36,39 @@
     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

spec/controllers/spree/orders_controller_spec.rb

@@ -89,6 +89,9 @@
       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 @@
           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
 

spec/requests/checkout/failed_checkout_spec.rb

@@ -23,6 +23,10 @@
   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

spec/requests/checkout/stripe_connect_spec.rb

@@ -27,6 +27,10 @@
   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