diff --git a/app/models/concerns/order_shipment.rb b/app/models/concerns/order_shipment.rb deleted file mode 100644 index 1f9c937bb5b..00000000000 --- a/app/models/concerns/order_shipment.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'active_support/concern' - -# This module is an adapter for OFN to work with Spree 2 code. -# -# Although Spree 2 supports multiple shipments per order, in OFN we have only one shipment per order. -# A shipment is associated to a shipping_method through a selected shipping_rate. -# See https://github.com/openfoodfoundation/openfoodnetwork/wiki/Spree-Upgrade:-Migration-to-multiple-shipments -# for details. -# -# Methods in this module may become deprecated. -module OrderShipment - extend ActiveSupport::Concern - - def shipment - shipments.first - end - - # Returns the shipping method of the first and only shipment in the order - # - # @return [ShippingMethod] - def shipping_method - return if shipments.blank? - - shipments.first.shipping_method - end - - # Finds the shipment's shipping_rate for the given shipping_method_id and selects that shipping_rate. - # If the selection is successful, it persists it in the database by saving the shipment. - # If it fails, it does not clear the current shipping_method selection. - # - # @return [ShippingMethod] the selected shipping method, or nil if the given shipping_method_id is - # empty or if it cannot find the given shipping_method_id in the order - def select_shipping_method(shipping_method_id) - return if shipping_method_id.blank? || shipments.empty? - - shipment = shipments.first - - shipping_rate = shipment.shipping_rates.find_by(shipping_method_id: shipping_method_id) - return unless shipping_rate - - shipment.selected_shipping_rate_id = shipping_rate.id - shipment.shipping_method - end -end diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 784c9342d0c..2b5093c8e94 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -4,11 +4,9 @@ require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/feature_toggle' require 'open_food_network/tag_rule_applicator' -require 'concerns/order_shipment' module Spree class Order < ActiveRecord::Base - prepend OrderShipment include Checkout checkout_flow do @@ -226,7 +224,7 @@ def item_count end def backordered? - shipments.any?(&:backordered?) + shipment&.backordered? || false end # Returns the relevant zone (if any) to be used for taxation purposes. @@ -438,10 +436,8 @@ def finalize! # update payment and shipment(s) states, and save updater.update_payment_state - shipments.each do |shipment| - shipment.update!(self) - shipment.finalize! - end + shipment&.update!(self) + shipment&.finalize! updater.update_shipment_state updater.before_save_hook @@ -547,20 +543,52 @@ def state_changed(name) ) end + # The Spree 2 db model we use supports multiple shipments per order, + # in OFN we have only one shipment per order. + # A shipment is associated to a shipping_method through a selected shipping_rate. + def shipment + return if shipments.blank? + + shipments.first + end + + # The shipping method of the shipment in the order + def shipping_method + return if shipment.blank? + + shipment.shipping_method + end + + # Finds the shipment's shipping_rate for the given shipping_method_id and selects that shipping_rate. + # If the selection is successful, it persists it in the database by saving the shipment. + # If it fails, it does not clear the current shipping_method selection. + # + # @return [ShippingMethod] the selected shipping method, or nil if the given shipping_method_id is + # empty or if it cannot find the given shipping_method_id in the order + def select_shipping_method(shipping_method_id) + return if shipping_method_id.blank? || shipment.blank? + + shipment = shipments.first + + shipping_rate = shipment.shipping_rates.find_by(shipping_method_id: shipping_method_id) + return unless shipping_rate + + shipment.selected_shipping_rate_id = shipping_rate.id + shipment.shipping_method + end + def shipped? %w(partial shipped).include?(shipment_state) end # Does this order have shipments that can be shipped? def ready_to_ship? - shipments.any?(&:can_ship?) + shipment&.can_ship? || false end # Ship all pending orders def ship - shipments.each do |s| - s.ship if s.can_ship? - end + shipment.ship if ready_to_ship? end def line_item_variants @@ -582,12 +610,10 @@ def create_proposed_shipments adjustments.shipping.delete_all shipments.destroy_all - packages = OrderManagement::Stock::Coordinator.new(self).packages - packages.each do |package| - shipments << package.to_shipment - end + package = OrderManagement::Stock::Coordinator.new(self).package + shipments << package.to_shipment - shipments + shipments # end # Clean shipments and make order back to address state @@ -596,7 +622,7 @@ def create_proposed_shipments # to delivery again so that proper updated shipments are created. # e.g. customer goes back from payment step and changes order items def ensure_updated_shipments - return unless shipments.any? + return if shipment.blank? shipments.destroy_all update_columns( @@ -606,7 +632,7 @@ def ensure_updated_shipments end def refresh_shipment_rates - shipments.map(&:refresh_rates) + shipment.refresh_rates end # Check that line_items in the current order are available from a newly selected distribution @@ -625,12 +651,10 @@ def disallow_guest_order # After changing line items of a completed order def update_shipping_fees! - shipments.each do |shipment| - next if shipment.shipped? + return if shipment.blank? || shipment.shipped? - update_adjustment! shipment.adjustment if shipment.adjustment - save_or_rescue_shipment(shipment) - end + update_adjustment!(shipment.adjustment) if shipment.adjustment + save_or_rescue_shipment(shipment) end def save_or_rescue_shipment(shipment) @@ -788,20 +812,20 @@ def has_available_shipment end def ensure_available_shipping_rates - return unless shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } + return unless shipment.blank? || shipment.shipping_rates.blank? errors.add(:base, Spree.t(:items_cannot_be_shipped)) && (return false) end def after_cancel - shipments.each(&:cancel!) + shipment&.cancel! OrderMailer.cancel_email(id).deliver_later self.payment_state = 'credit_owed' unless shipped? end def after_resume - shipments.each(&:resume!) + shipment&.resume! end def use_billing? diff --git a/engines/order_management/app/services/order_management/stock/coordinator.rb b/engines/order_management/app/services/order_management/stock/coordinator.rb index 64bbbc8cbc9..63cd050c27a 100644 --- a/engines/order_management/app/services/order_management/stock/coordinator.rb +++ b/engines/order_management/app/services/order_management/stock/coordinator.rb @@ -9,35 +9,33 @@ def initialize(order) @order = order end - def packages - packages = build_packages - packages = prioritize_packages(packages) - estimate_packages(packages) + def package + package = build_package + package = prioritize_package(package) + estimate_package(package) end # Build package with default stock location # No need to check items are in the stock location, # there is only one stock location so the items will be on that stock location. # - # Returns an array with a single Package for the default stock location - def build_packages + # Returns a single Package for the default stock location + def build_package packer = build_packer(order) - [packer.package] + packer.package end private - def prioritize_packages(packages) - prioritizer = OrderManagement::Stock::Prioritizer.new(order, packages) - prioritizer.prioritized_packages + def prioritize_package(package) + prioritizer = OrderManagement::Stock::Prioritizer.new(order, package) + prioritizer.prioritized_package end - def estimate_packages(packages) + def estimate_package(package) estimator = OrderManagement::Stock::Estimator.new(order) - packages.each do |package| - package.shipping_rates = estimator.shipping_rates(package) - end - packages + package.shipping_rates = estimator.shipping_rates(package) + package end def build_packer(order) diff --git a/engines/order_management/app/services/order_management/stock/prioritizer.rb b/engines/order_management/app/services/order_management/stock/prioritizer.rb index 9e8eea42897..f3b15896cf6 100644 --- a/engines/order_management/app/services/order_management/stock/prioritizer.rb +++ b/engines/order_management/app/services/order_management/stock/prioritizer.rb @@ -3,42 +3,37 @@ module OrderManagement module Stock class Prioritizer - attr_reader :packages, :order + attr_reader :package, :order - def initialize(order, packages, adjuster_class = OrderManagement::Stock::Adjuster) + def initialize(order, package, adjuster_class = OrderManagement::Stock::Adjuster) @order = order - @packages = packages + @package = package @adjuster_class = adjuster_class end - def prioritized_packages - adjust_packages - prune_packages - packages + def prioritized_package + adjust_package + return if package.blank? + + package end private - def adjust_packages + def adjust_package order.line_items.each do |line_item| adjuster = @adjuster_class.new(line_item.variant, line_item.quantity, :on_hand) - visit_packages(adjuster) + visit_package(adjuster) adjuster.status = :backordered - visit_packages(adjuster) - end - end - - def visit_packages(adjuster) - packages.each do |package| - item = package.find_item adjuster.variant, adjuster.status - adjuster.adjust(item) if item + visit_package(adjuster) end end - def prune_packages - packages.reject!(&:empty?) + def visit_package(adjuster) + item = package.find_item(adjuster.variant, adjuster.status) + adjuster.adjust(item) if item end end end diff --git a/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb b/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb index ee93585dd65..326f4e8e575 100644 --- a/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb +++ b/engines/order_management/spec/services/order_management/stock/coordinator_spec.rb @@ -14,12 +14,12 @@ module Stock subject { Coordinator.new(order) } - context "packages" do + context "package" do it "builds, prioritizes and estimates" do - expect(subject).to receive(:build_packages).ordered - expect(subject).to receive(:prioritize_packages).ordered - expect(subject).to receive(:estimate_packages).ordered - subject.packages + expect(subject).to receive(:build_package).ordered + expect(subject).to receive(:prioritize_package).ordered + expect(subject).to receive(:estimate_package).ordered + subject.package end end end diff --git a/spec/models/concerns/order_shipment_spec.rb b/spec/models/concerns/order_shipment_spec.rb deleted file mode 100644 index 9d45f33d1b3..00000000000 --- a/spec/models/concerns/order_shipment_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe OrderShipment do - let(:order) { create(:order) } - - describe "#shipping_method" do - context "when order has no shipments" do - it "returns nil" do - expect(order.shipping_method).to be_nil - end - end - - context "when order has single shipment" do - it "returns the shipments shipping_method" do - shipping_method = create(:shipping_method_with, :flat_rate) - shipment = create(:shipment_with, :shipping_method, shipping_method: shipping_method) - order.shipments = [shipment] - - expect(order.shipping_method).to eq shipment.shipping_method - end - end - end - - describe "#select_shipping_method" do - let(:shipping_method) { create(:shipping_method_with, :flat_rate) } - - context "when order has no shipment" do - it "returns nil" do - expect(order.select_shipping_method(shipping_method.id)).to be_nil - end - end - - context "when order has a shipment" do - let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) } - before { order.shipments = [shipment] } - - context "when no shipping_method_id is provided" do - it "returns nil for nil shipping_method_id" do - expect(order.select_shipping_method(nil)).to be_nil - end - - it "returns nil for empty shipping_method_id" do - empty_shipping_method_id = ' ' - expect(shipment.shipping_rates).to_not receive(:find_by).with(shipping_method_id: empty_shipping_method_id) - - expect(order.select_shipping_method(empty_shipping_method_id)).to be_nil - end - end - - context "when shipping_method_id is not valid for the order" do - it "returns nil" do - invalid_shipping_method_id = order.shipment.shipping_method.id + 1000 - expect(shipment.shipping_rates).to receive(:find_by).with(shipping_method_id: invalid_shipping_method_id) { nil } - - expect(order.select_shipping_method(invalid_shipping_method_id)).to be_nil - end - end - - context "when shipping_method_id is valid for the order" do - it "returns the shipments shipping_method" do - expect(shipment).to receive(:selected_shipping_rate_id=) - - expect(order.select_shipping_method(shipping_method.id)).to eq shipping_method - end - end - - context "when multiple shipping_methods exist in the shipment" do - let(:expensive_shipping_method) { create(:shipping_method_with, :expensive_name) } - before { shipment.add_shipping_method(expensive_shipping_method, false ) } - - it "selects a shipping method that was not selected by default and persists the selection in the database" do - expect(shipment.shipping_method).to eq shipping_method - - expect(order.select_shipping_method(expensive_shipping_method.id)).to eq expensive_shipping_method - - expect(shipment.shipping_method).to eq expensive_shipping_method - - shipment.reload - - expect(shipment.shipping_method).to eq expensive_shipping_method - end - end - end - end -end