Merge pull request #2802 from nebulab/gevann-remove-default-address-d…

…ependency

Remove default address dependency part 2

backend/app/controllers/spree/admin/cancellations_controller.rb

@@ -20,7 +20,7 @@ def short_ship
           flash[:error] = t('spree.no_inventory_selected')
           redirect_to admin_order_cancellations_path(@order)
         else
-          @order.cancellations.short_ship(inventory_units, whodunnit: whodunnit)
+          @order.cancellations.short_ship(inventory_units, created_by: created_by)
 
           flash[:success] = t('spree.inventory_canceled')
           redirect_to edit_admin_order_url(@order)
@@ -29,7 +29,7 @@ def short_ship
 
       private
 
-      def whodunnit
+      def created_by
         try_spree_current_user.try(:email)
       end
 

backend/app/controllers/spree/admin/reimbursements_controller.rb

@@ -13,7 +13,7 @@ class ReimbursementsController < ResourceController
       rescue_from Spree::Core::GatewayError, with: :spree_core_gateway_error
 
       def perform
-        @reimbursement.perform!
+        @reimbursement.perform!(created_by: try_spree_current_user)
         redirect_to location_after_save
       end
 
@@ -57,7 +57,7 @@ def load_stock_locations
       end
 
       def load_simulated_refunds
-        @reimbursement_objects = @reimbursement.simulate
+        @reimbursement_objects = @reimbursement.simulate(created_by: try_spree_current_user)
       end
 
       def spree_core_gateway_error(error)

backend/spec/controllers/spree/admin/reimbursements_controller_spec.rb

@@ -9,6 +9,13 @@
     Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false)
   end
 
+  let(:user) { stub_model(Spree::LegacyUser, has_spree_role?: true, id: 1) }
+
+  before do
+    allow_any_instance_of(described_class).to receive(:try_spree_current_user).
+      and_return(user)
+  end
+
   describe '#edit' do
     let(:reimbursement) { create(:reimbursement) }
     let(:order) { reimbursement.order }

backend/spec/features/admin/orders/return_payment_state_spec.rb

@@ -7,9 +7,12 @@
 
   before do
     Spree::RefundReason.create!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false)
+    allow_any_instance_of(Spree::Admin::ReimbursementsController).to receive(:try_spree_current_user).
+      and_return(user)
   end
 
   let!(:order) { create(:shipped_order) }
+  let(:user) { create(:admin_user) }
 
   # Regression test for https://github.com/spree/spree/issues/6229
   it "refunds and has outstanding_balance of zero", js: true do

core/app/models/spree/order_cancellations.rb

@@ -23,10 +23,16 @@ def initialize(order)
   # @api public
   #
   # @param [Array<InventoryUnit>] inventory_units the inventory units to be short shipped
-  # @param [String] whodunnit the system or person that is short shipping the inventory units
+  # @param (deprecated) [String] whodunnit the system or person that is short shipping the inventory unit
+  # @param [String] created_by the system or person that is short shipping the inventory unit
   #
   # @return [Array<UnitCancel>] the units that have been canceled due to short shipping
-  def short_ship(inventory_units, whodunnit: nil)
+  def short_ship(inventory_units, whodunnit: nil, created_by: nil)
+    if whodunnit
+      created_by ||= whodunnit
+      Spree::Deprecation.warn("Calling #short_ship on #{self} with whodunnit is deprecated, use created_by instead")
+    end
+
     if inventory_units.map(&:order_id).uniq != [@order.id]
       raise ArgumentError, "Not all inventory units belong to this order"
     end
@@ -36,7 +42,7 @@ def short_ship(inventory_units, whodunnit: nil)
     Spree::OrderMutex.with_lock!(@order) do
       Spree::InventoryUnit.transaction do
         inventory_units.each do |iu|
-          unit_cancels << short_ship_unit(iu, whodunnit: whodunnit)
+          unit_cancels << short_ship_unit(iu, created_by: created_by)
         end
 
         update_shipped_shipments(inventory_units)
@@ -59,17 +65,23 @@ def short_ship(inventory_units, whodunnit: nil)
   #
   # @param [InventoryUnit] inventory_unit the inventory unit to be canceled
   # @param [String] reason the reason that you are canceling the inventory unit
-  # @param [String] whodunnit the system or person that is canceling the inventory unit
+  # @param (deprecated) [String] whodunnit the system or person that is canceling the inventory unit
+  # @param [String] created_by the system or person that is canceling the inventory unit
   #
   # @return [UnitCancel] the unit that has been canceled
-  def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodunnit: nil)
+  def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodunnit: nil, created_by: nil)
+    if whodunnit
+      created_by ||= whodunnit
+      Spree::Deprecation.warn("Calling #cancel_unit on #{self} with whodunnit is deprecated, use created_by instead")
+    end
+
     unit_cancel = nil
 
     Spree::OrderMutex.with_lock!(@order) do
       unit_cancel = Spree::UnitCancel.create!(
         inventory_unit: inventory_unit,
         reason: reason,
-        created_by: whodunnit
+        created_by: created_by
       )
 
       inventory_unit.cancel!
@@ -82,26 +94,35 @@ def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodu
   #
   # @api public
   # @param [Array<InventoryUnit>] inventory_units the inventory units to be reimbursed
+  # @param [Spree.user_class] created_by the user that is performing this action
   # @return [Reimbursement] the reimbursement for inventory being canceled
-  def reimburse_units(inventory_units)
+  def reimburse_units(inventory_units, created_by: nil)
+    unless created_by
+      Spree::Deprecation.warn("Calling #reimburse_units on #{self} without created_by is deprecated")
+    end
     reimbursement = nil
 
     Spree::OrderMutex.with_lock!(@order) do
       return_items = inventory_units.map(&:current_or_new_return_item)
       reimbursement = Spree::Reimbursement.new(order: @order, return_items: return_items)
-      reimbursement.return_all
+      reimbursement.return_all(created_by: created_by)
     end
 
     reimbursement
   end
 
   private
 
-  def short_ship_unit(inventory_unit, whodunnit: nil)
+  def short_ship_unit(inventory_unit, whodunnit: nil, created_by: nil)
+    if whodunnit
+      created_by ||= whodunnit
+      Spree::Deprecation.warn("Calling #short_ship_unit on #{self} with whodunnit is deprecated, use created_by instead")
+    end
+
     unit_cancel = Spree::UnitCancel.create!(
       inventory_unit: inventory_unit,
       reason: Spree::UnitCancel::SHORT_SHIP,
-      created_by: whodunnit
+      created_by: created_by
     )
     unit_cancel.adjust!
     inventory_unit.cancel!

core/app/models/spree/reimbursement.rb

@@ -98,12 +98,15 @@ def unpaid_amount
       total - paid_amount
     end
 
-    def perform!
+    def perform!(created_by: nil)
+      unless created_by
+        Spree::Deprecation.warn("Calling #perform on #{self} without created_by is deprecated")
+      end
       reimbursement_tax_calculator.call(self)
       reload
       update!(total: calculated_total)
 
-      reimbursement_performer.perform(self)
+      reimbursement_performer.perform(self, created_by: created_by)
 
       if unpaid_amount_within_tolerance?
         reimbursed!
@@ -116,12 +119,15 @@ def perform!
       end
     end
 
-    def simulate
+    def simulate(created_by: nil)
+      unless created_by
+        Spree::Deprecation.warn("Calling #simulate on #{self} without created_by is deprecated")
+      end
       reimbursement_simulator_tax_calculator.call(self)
       reload
       update!(total: calculated_total)
 
-      reimbursement_performer.simulate(self)
+      reimbursement_performer.simulate(self, created_by: created_by)
     end
 
     def return_items_requiring_exchange
@@ -139,11 +145,15 @@ def all_exchanges?
     # Accepts all return items, saves the reimbursement, and performs the reimbursement
     #
     # @api public
+    # @param [Spree.user_class] created_by the user that is performing this action
     # @return [void]
-    def return_all
+    def return_all(created_by: nil)
+      unless created_by
+        Spree::Deprecation.warn("Calling #return_all on #{self} without created_by is deprecated")
+      end
       return_items.each(&:accept!)
       save!
-      perform!
+      perform!(created_by: created_by)
     end
 
     private

core/app/models/spree/reimbursement_performer.rb

@@ -11,22 +11,28 @@ class << self
       # - #description
       # - #display_amount
       # so they can be displayed in the Admin UI appropriately.
-      def simulate(reimbursement)
-        execute(reimbursement, true)
+      def simulate(reimbursement, created_by: nil)
+        unless created_by
+          Spree::Deprecation.warn("Calling #simulate on #{self} without created_by is deprecated")
+        end
+        execute(reimbursement, true, created_by: created_by)
       end
 
       # Actually perform the reimbursement
-      def perform(reimbursement)
-        execute(reimbursement, false)
+      def perform(reimbursement, created_by: nil)
+        unless created_by
+          Spree::Deprecation.warn("Calling #perform on #{self} without created_by is deprecated")
+        end
+        execute(reimbursement, false, created_by: created_by)
       end
 
       private
 
-      def execute(reimbursement, simulate)
+      def execute(reimbursement, simulate, created_by:)
         reimbursement_type_hash = calculate_reimbursement_types(reimbursement)
 
         reimbursement_type_hash.flat_map do |reimbursement_type, return_items|
-          reimbursement_type.reimburse(reimbursement, return_items, simulate)
+          reimbursement_type.reimburse(reimbursement, return_items, simulate, created_by: created_by)
         end
       end
 

core/app/models/spree/reimbursement_type.rb

@@ -11,7 +11,7 @@ class ReimbursementType < Spree::Base
     # This method will reimburse the return items based on however it child implements it
     # By default it takes a reimbursement, the return items it needs to reimburse, and if
     # it is a simulation or a real reimbursement. This should return an array
-    def self.reimburse(_reimbursement, _return_items, _simulate)
+    def self.reimburse(_reimbursement, _return_items, _simulate, *_optional_args)
       raise "Implement me"
     end
   end

core/app/models/spree/reimbursement_type/credit.rb

@@ -5,9 +5,12 @@ class ReimbursementType::Credit < Spree::ReimbursementType
     extend Spree::ReimbursementType::ReimbursementHelpers
 
     class << self
-      def reimburse(reimbursement, return_items, simulate)
+      def reimburse(reimbursement, return_items, simulate, created_by: nil)
+        unless created_by
+          Spree::Deprecation.warn("Calling #reimburse on #{self} without created_by is deprecated")
+        end
         unpaid_amount = return_items.sum(&:total).round(2, :down)
-        reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate)
+        reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate, created_by: created_by)
         reimbursement_list
       end
     end

core/app/models/spree/reimbursement_type/exchange.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Spree::ReimbursementType::Exchange < Spree::ReimbursementType
-  def self.reimburse(reimbursement, return_items, simulate)
+  def self.reimburse(reimbursement, return_items, simulate, *_optional_args)
     return [] unless return_items.present?
 
     exchange = Spree::Exchange.new(reimbursement.order, return_items)

core/app/models/spree/reimbursement_type/original_payment.rb

@@ -4,7 +4,7 @@ class Spree::ReimbursementType::OriginalPayment < Spree::ReimbursementType
   extend Spree::ReimbursementType::ReimbursementHelpers
 
   class << self
-    def reimburse(reimbursement, return_items, simulate)
+    def reimburse(reimbursement, return_items, simulate, _created_by)
       unpaid_amount = return_items.sum(&:total).round(2, :down)
       payments = reimbursement.order.payments.completed
 

core/app/models/spree/reimbursement_type/reimbursement_helpers.rb

@@ -20,8 +20,8 @@ def create_refunds(reimbursement, payments, unpaid_amount, simulate, reimburseme
       [reimbursement_list, unpaid_amount]
     end
 
-    def create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list = [])
-      credits = [create_credit(reimbursement, unpaid_amount, simulate)]
+    def create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list = [], created_by:)
+      credits = [create_credit(reimbursement, unpaid_amount, simulate, created_by: created_by)]
       unpaid_amount -= credits.sum(&:amount)
       reimbursement_list += credits
 
@@ -43,19 +43,19 @@ def create_refund(reimbursement, payment, amount, simulate)
 
     # If you have multiple methods of crediting a customer, overwrite this method
     # Must return an array of objects the respond to #description, #display_amount
-    def create_credit(reimbursement, unpaid_amount, simulate)
-      creditable = create_creditable(reimbursement, unpaid_amount)
+    def create_credit(reimbursement, unpaid_amount, simulate, created_by:)
+      creditable = create_creditable(reimbursement, unpaid_amount, created_by: created_by)
       credit = reimbursement.credits.build(creditable: creditable, amount: unpaid_amount)
       simulate ? credit.readonly! : credit.save!
       credit
     end
 
-    def create_creditable(reimbursement, unpaid_amount)
+    def create_creditable(reimbursement, unpaid_amount, created_by:)
       Spree::Reimbursement::Credit.default_creditable_class.new(
         user: reimbursement.order.user,
         amount: unpaid_amount,
         category: Spree::StoreCreditCategory.reimbursement_category(reimbursement),
-        created_by: Spree::StoreCredit.default_created_by,
+        created_by: created_by,
         memo: "Refund for uncreditable payments on order #{reimbursement.order.number}",
         currency: reimbursement.order.currency
       )

core/app/models/spree/reimbursement_type/store_credit.rb

@@ -4,17 +4,32 @@ class Spree::ReimbursementType::StoreCredit < Spree::ReimbursementType
   extend Spree::ReimbursementType::ReimbursementHelpers
 
   class << self
-    def reimburse(reimbursement, return_items, simulate)
+    def reimburse(reimbursement, return_items, simulate, created_by: nil)
+      unless created_by
+        Spree::Deprecation.warn("Calling #reimburse on #{self} without created_by is deprecated")
+      end
       unpaid_amount = return_items.sum(&:total).to_d.round(2, :down)
       payments = store_credit_payments(reimbursement)
       reimbursement_list = []
 
       # Credit each store credit that was used on the order
-      reimbursement_list, unpaid_amount = create_refunds(reimbursement, payments, unpaid_amount, simulate, reimbursement_list)
+      reimbursement_list, unpaid_amount = create_refunds(
+        reimbursement,
+        payments,
+        unpaid_amount,
+        simulate,
+        reimbursement_list
+      )
 
       # If there is any amount left to pay out to the customer, then create credit with that amount
       if unpaid_amount > 0.0
-        reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list)
+        reimbursement_list, _unpaid_amount = create_credits(
+          reimbursement,
+          unpaid_amount,
+          simulate,
+          reimbursement_list,
+          created_by: created_by
+        )
       end
 
       reimbursement_list

core/app/models/spree/store_credit.rb

@@ -18,8 +18,6 @@ class Spree::StoreCredit < Spree::PaymentSource
   ADJUSTMENT_ACTION = 'adjustment'
   INVALIDATE_ACTION = 'invalidate'
 
-  DEFAULT_CREATED_BY_EMAIL = "spree@example.com"
-
   belongs_to :user, class_name: Spree::UserClassHandle.new
   belongs_to :created_by, class_name: Spree::UserClassHandle.new
   belongs_to :category, class_name: "Spree::StoreCreditCategory"
@@ -196,12 +194,6 @@ def invalidate(reason, user_performing_invalidation)
     end
   end
 
-  class << self
-    def default_created_by
-      Spree.user_class.find_by(email: DEFAULT_CREATED_BY_EMAIL)
-    end
-  end
-
   private
 
   def create_credit_record(amount, action_attributes = {})

core/spec/models/spree/customer_return_spec.rb

@@ -282,7 +282,8 @@
           end
 
           context 'when all reimbursements are reimbursed' do
-            before { reimbursement.perform! }
+            let(:created_by_user) { create(:user, email: 'user@email.com') }
+            before { reimbursement.perform!(created_by: created_by_user) }
 
             it { is_expected.to be true }
           end