Merge pull request #10940 from Matt-Yorkley/order-tidyup

Order class tidyup

app/controllers/api/v0/shipments_controller.rb

@@ -34,7 +34,7 @@ module Api
         @shipment.fee_adjustment.fire_events(:open)
 
         if @shipment.update(shipment_params)
-          @order.updater.update_totals_and_states
+          @order.update_totals_and_states
         end
 
         @shipment.fee_adjustment.close

app/controllers/checkout_controller.rb

@@ -115,7 +115,7 @@ class CheckoutController < ::BaseController
   end
 
   def update_payment_total
-    @order.updater.update_totals
+    @order.update_totals
     @order.updater.update_pending_payment
   end
 

app/controllers/split_checkout_controller.rb

@@ -172,7 +172,7 @@ class SplitCheckoutController < ::BaseController
 
     @order.select_shipping_method(params[:shipping_method_id])
     @order.update(order_params)
-    @order.updater.update_totals_and_states
+    @order.update_totals_and_states
 
     validate_current_step!
 

app/controllers/spree/admin/adjustments_controller.rb

@@ -14,7 +14,7 @@ module Spree
 
       def update_order
         @order.reload
-        @order.updater.update_totals_and_states
+        @order.update_totals_and_states
       end
 
       def collection

app/controllers/spree/admin/orders/customer_details_controller.rb

@@ -58,7 +58,7 @@ module Spree
           @order.create_tax_charge!
           Spree::TaxRate.adjust(@order, @order.adjustments.admin)
 
-          @order.updater.update_totals_and_states
+          @order.update_totals_and_states
         end
 
         def order_params

app/controllers/spree/admin/payments_controller.rb

@@ -129,7 +129,8 @@ module Spree
                             @payment_methods.first
                           end
 
-        @previous_cards = @order.credit_cards.with_payment_profile
+        credit_card_ids = @order.payments.from_credit_card.pluck(:source_id).uniq
+        @previous_cards = CreditCard.where(id: credit_card_ids).with_payment_profile
       end
 
       # At this point admin should have passed through Customer Details step

app/models/concerns/order_validations.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'active_support/concern'
+
+module OrderValidations
+  extend ActiveSupport::Concern
+
+  private
+
+  def disallow_guest_order
+    return unless using_guest_checkout? && registered_email?
+
+    errors.add(:email, I18n.t('devise.failure.already_registered'))
+  end
+
+  # Check that line_items in the current order are available from a newly selected distribution
+  def products_available_from_new_distribution
+    return if OrderCycleDistributedVariants.new(order_cycle, distributor)
+      .distributes_order_variants?(self)
+
+    errors.add(:base, I18n.t(:spree_order_availability_error))
+  end
+
+  # Determine if email is required (we don't want validation errors before we hit the checkout)
+  def require_email
+    true unless (new_record? || cart?) && !checkout_processing
+  end
+
+  def ensure_line_items_present
+    return if line_items.present?
+
+    errors.add(:base, Spree.t(:there_are_no_items_for_this_order))
+    false
+  end
+
+  def ensure_available_shipping_rates
+    return unless shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? }
+
+    errors.add(:base, Spree.t(:items_cannot_be_shipped))
+    false
+  end
+end

app/models/spree/order.rb

@@ -8,6 +8,7 @@ require 'open_food_network/tag_rule_applicator'
 module Spree
   class Order < ApplicationRecord
     include OrderShipment
+    include OrderValidations
     include Checkout
     include Balance
     include SetUnusedAddressFields
@@ -83,7 +84,7 @@ module Spree
     accepts_nested_attributes_for :shipments
 
     delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher
-    delegate :update_totals, to: :updater
+    delegate :update_totals, :update_totals_and_states, to: :updater
     delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!,
              :update_line_item_fees!, :recreate_all_fees!, to: :fee_handler
 
@@ -303,10 +304,6 @@ module Spree
       number
     end
 
-    def shipped_shipments
-      shipments.shipped
-    end
-
     def contains?(variant)
       find_line_item_by_variant(variant).present?
     end
@@ -343,11 +340,6 @@ module Spree
       complete? || resumed? || awaiting_return? || returned?
     end
 
-    def credit_cards
-      credit_card_ids = payments.from_credit_card.pluck(:source_id).uniq
-      CreditCard.where(id: credit_card_ids)
-    end
-
     # Finalizes an in progress order after checkout is complete.
     # Called after transition to complete state when payments will have been processed
     def finalize!
@@ -376,13 +368,6 @@ module Spree
       )
     end
 
-    def deliver_order_confirmation_email
-      return if subscription.present?
-
-      Spree::OrderMailer.confirm_email_for_customer(id).deliver_later(wait: 10.seconds)
-      Spree::OrderMailer.confirm_email_for_shop(id).deliver_later(wait: 10.seconds)
-    end
-
     # Helper methods for checkout steps
     def paid?
       payment_state == 'paid' || payment_state == 'credit_owed'
@@ -444,19 +429,6 @@ module Spree
       restart_checkout_flow if state.in?(["payment", "confirmation"])
     end
 
-    def state_changed(name)
-      state = "#{name}_state"
-      return unless persisted?
-
-      old_state = __send__("#{state}_was")
-      state_changes.create(
-        previous_state: old_state,
-        next_state: __send__(state),
-        name: name,
-        user_id: user_id
-      )
-    end
-
     def shipped?
       %w(partial shipped).include?(shipment_state)
     end
@@ -510,24 +482,6 @@ module Spree
       end
     end
 
-    def refresh_shipment_rates
-      shipments.map(&:refresh_rates)
-    end
-
-    # Check that line_items in the current order are available from a newly selected distribution
-    def products_available_from_new_distribution
-      return if OrderCycleDistributedVariants.new(order_cycle, distributor)
-        .distributes_order_variants?(self)
-
-      errors.add(:base, I18n.t(:spree_order_availability_error))
-    end
-
-    def disallow_guest_order
-      return unless using_guest_checkout? && registered_email?
-
-      errors.add(:email, I18n.t('devise.failure.already_registered'))
-    end
-
     # After changing line items of a completed order
     def update_shipping_fees!
       shipments.each do |shipment|
@@ -589,10 +543,6 @@ module Spree
       save!
     end
 
-    def distribution_set?
-      distributor && order_cycle
-    end
-
     def shipping_tax
       shipment_adjustments.reload.tax.sum(:amount)
     end
@@ -629,6 +579,13 @@ module Spree
 
     private
 
+    def deliver_order_confirmation_email
+      return if subscription.present?
+
+      Spree::OrderMailer.confirm_email_for_customer(id).deliver_later(wait: 10.seconds)
+      Spree::OrderMailer.confirm_email_for_shop(id).deliver_later(wait: 10.seconds)
+    end
+
     def fee_handler
       @fee_handler ||= OrderFeesHandler.new(self)
     end
@@ -658,38 +615,6 @@ module Spree
       self.email = user.email if user
     end
 
-    # Determine if email is required (we don't want validation errors before we hit the checkout)
-    def require_email
-      return true unless (new_record? || cart?) && !checkout_processing
-    end
-
-    def ensure_line_items_present
-      return if line_items.present?
-
-      errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) && (return false)
-    end
-
-    def ensure_available_shipping_rates
-      return unless shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? }
-
-      errors.add(:base, Spree.t(:items_cannot_be_shipped)) && (return false)
-    end
-
-    def after_cancel
-      shipments.each(&:cancel!)
-      payments.checkout.each(&:void!)
-
-      OrderMailer.cancel_email(id).deliver_later if send_cancellation_email
-      update(payment_state: updater.update_payment_state)
-    end
-
-    def after_resume
-      shipments.each(&:resume!)
-      payments.void.each(&:resume!)
-
-      update(payment_state: updater.update_payment_state)
-    end
-
     def use_billing?
       @use_billing == true || @use_billing == 'true' || @use_billing == '1'
     end
@@ -727,7 +652,7 @@ module Spree
       return if adjustment.finalized?
 
       adjustment.update_adjustment!(force: true)
-      updater.update_totals_and_states
+      update_totals_and_states
     end
   end
 end

app/models/spree/order/checkout.rb

@@ -136,8 +136,36 @@ module Spree
             )
           end
 
+          def state_changed(name)
+            state = "#{name}_state"
+            return unless persisted?
+
+            old_state = __send__("#{state}_was")
+            state_changes.create(
+              previous_state: old_state,
+              next_state: __send__(state),
+              name: name,
+              user_id: user_id
+            )
+          end
+
           private
 
+          def after_cancel
+            shipments.each(&:cancel!)
+            payments.checkout.each(&:void!)
+
+            OrderMailer.cancel_email(id).deliver_later if send_cancellation_email
+            update(payment_state: updater.update_payment_state)
+          end
+
+          def after_resume
+            shipments.each(&:resume!)
+            payments.void.each(&:resume!)
+
+            update(payment_state: updater.update_payment_state)
+          end
+
           def validate_payment_method!
             return unless checkout_processing
             return if payments.any?

app/models/spree/return_authorization.rb

@@ -63,7 +63,7 @@ module Spree
     end
 
     def returnable_inventory
-      order.shipped_shipments.collect{ |s| s.inventory_units.to_a }.flatten
+      order.shipments.shipped.collect{ |s| s.inventory_units.to_a }.flatten
     end
 
     # Used when Adjustment#update_adjustment! wants to update the related adjustment
@@ -74,7 +74,7 @@ module Spree
     private
 
     def must_have_shipped_units
-      return unless order.nil? || order.shipped_shipments.none?
+      return unless order.nil? || order.shipments.shipped.none?
 
       errors.add(:order, Spree.t(:has_no_shipped_units))
     end

app/views/spree/admin/shared/_order_tabs.html.haml

@@ -42,7 +42,7 @@
         %dd#date_complete
           = pretty_time(@order.completed_at)
 
-  - if @order.distribution_set?
+  - if @order.distributor && @order.order_cycle
     %nav.menu
       %ul
         - customer_details_classes = "active" if current == "Customer Details"

spec/controllers/admin/bulk_line_items_controller_spec.rb

@@ -377,7 +377,7 @@ describe Admin::BulkLineItemsController, type: :controller do
       line_item1.product.update_columns(tax_category_id: tax_cat5.id)
       line_item2.product.update_columns(tax_category_id: tax_cat10.id)
 
-      order.refresh_shipment_rates
+      order.shipments.map(&:refresh_rates)
       order.select_shipping_method(shipping_method.id)
       order.finalize!
       order.recreate_all_fees!

spec/factories/order_factory.rb

@@ -17,7 +17,7 @@ FactoryBot.define do
 
       after(:create) do |order|
         order.line_items << build(:line_item, order: order)
-        order.updater.update_totals_and_states
+        order.update_totals_and_states
 
         order.order_cycle.exchanges.outgoing.first.variants << order.line_items.first.variant
       end
@@ -49,7 +49,7 @@ FactoryBot.define do
       after(:create) do |order|
         create(:line_item, order: order)
         order.line_items.reload # to ensure order.line_items is accessible after
-        order.updater.update_totals_and_states
+        order.update_totals_and_states
       end
     end
 
@@ -76,7 +76,7 @@ FactoryBot.define do
 
         distributor { create(:distributor_enterprise) }
 
-        after(:create, &:refresh_shipment_rates)
+        after(:create) { |order, _evaluator| order.shipments.map(&:refresh_rates) }
 
         factory :order_ready_to_ship do
           payment_state { 'paid' }

spec/models/spree/order_spec.rb

@@ -936,7 +936,7 @@ describe Spree::Order do
       expect(Spree::OrderMailer).to receive(:confirm_email_for_customer).and_return(mailer)
       expect(Spree::OrderMailer).to receive(:confirm_email_for_shop).and_return(mailer)
 
-      order.deliver_order_confirmation_email
+      order.__send__(:deliver_order_confirmation_email)
     end
 
     it "does not send confirmation emails when the order belongs to a subscription" do
@@ -945,7 +945,7 @@ describe Spree::Order do
       expect(Spree::OrderMailer).not_to receive(:confirm_email_for_customer)
       expect(Spree::OrderMailer).not_to receive(:confirm_email_for_shop)
 
-      order.deliver_order_confirmation_email
+      order.__send__(:deliver_order_confirmation_email)
     end
   end
 

spec/system/consumer/shopping/orders_spec.rb

@@ -120,7 +120,7 @@ describe "Order Management" do
 
     before do
       order.shipment.shipping_method.calculator.update(preferred_amount: 5.0)
-      order.refresh_shipment_rates
+      order.shipments.map(&:refresh_rates)
       order.save
       order.reload