Merge pull request #7594 from luisramos0/update_attributes

[Rails 6.1] Replace AR's update_attributes! with update! (and rename order.update! and adjustment.update! in the process)

app/controllers/admin/bulk_line_items_controller.rb

@@ -35,7 +35,7 @@ module Admin
         if @line_item.update(line_item_params)
           order.update_line_item_fees! @line_item
           order.update_order_fees!
-          order.update!
+          order.update_order!
           render body: nil, status: :no_content # No Content, does not trigger ng resource auto-update
         else
           render json: { errors: @line_item.errors }, status: :precondition_failed

app/controllers/checkout_controller.rb

@@ -55,7 +55,7 @@ class CheckoutController < ::BaseController
     flash[:error] = I18n.t("checkout.failed")
     action_failed(e)
   ensure
-    @order.update!
+    @order.update_order!
   end
 
   # Clears the cached order. Required for #current_order to return a new order

app/controllers/line_items_controller.rb

@@ -43,7 +43,7 @@ class LineItemsController < BaseController
       order.update_shipping_fees!
       order.update_payment_fees!
       order.update_order_fees!
-      order.update!
+      order.update_order!
       order.create_tax_charge!
     end
   end

app/controllers/spree/admin/adjustments_controller.rb

@@ -14,7 +14,7 @@ module Spree
 
       def update_order
         @order.reload
-        @order.update!
+        @order.update_order!
       end
 
       def collection

app/jobs/subscription_confirm_job.rb

@@ -90,13 +90,13 @@ class SubscriptionConfirmJob < ActiveJob::Base
   end
 
   def send_confirmation_email(order)
-    order.update!
+    order.update_order!
     record_success(order)
     SubscriptionMailer.confirmation_email(order).deliver_now
   end
 
   def send_failed_payment_email(order, error_message = nil)
-    order.update!
+    order.update_order!
     record_and_log_error(:failed_payment, order, error_message)
     SubscriptionMailer.failed_payment_email(order).deliver_now
   rescue StandardError => e

app/jobs/subscription_placement_job.rb

@@ -77,7 +77,7 @@ class SubscriptionPlacementJob < ActiveJob::Base
 
   def handle_empty_order(order, changes)
     order.reload.all_adjustments.destroy_all
-    order.update!
+    order.update_order!
     send_empty_email(order, changes)
   end
 

app/models/spree/adjustment.rb

@@ -96,7 +96,7 @@ module Spree
     # more than on line items at once via accepted_nested_attributes the order
     # object on the association would be in a old state and therefore the
     # adjustment calculations would not performed on proper values
-    def update!(calculable = nil, force: false)
+    def update_adjustment!(calculable = nil, force: false)
       return amount if immutable? && !force
 
       if originator.present?
@@ -127,11 +127,7 @@ module Spree
     end
 
     def set_absolute_included_tax!(tax)
-      # This rubocop issue can now fixed by renaming Adjustment#update! to something else,
-      #   then AR's update! can be used instead of update_attributes!
-      # rubocop:disable Rails/ActiveRecordAliases
-      update_attributes! included_tax: tax.round(2)
-      # rubocop:enable Rails/ActiveRecordAliases
+      update! included_tax: tax.round(2)
     end
 
     def display_included_tax

app/models/spree/gateway/pay_pal_express.rb

@@ -87,7 +87,7 @@ module Spree
         )
         refund_transaction_response = provider.refund_transaction(refund_transaction)
         if refund_transaction_response.success?
-          payment.source.update_attributes(
+          payment.source.update(
             refunded_at: Time.now,
             refund_transaction_id: refund_transaction_response.RefundTransactionID,
             state: "refunded",

app/models/spree/inventory_unit.rb

@@ -66,7 +66,7 @@ module Spree
     end
 
     def update_order
-      order.update!
+      order.update_order!
     end
   end
 end

app/models/spree/item_adjustments.rb

@@ -17,9 +17,9 @@ module Spree
     end
 
     def update_adjustments
-      adjustment_total = adjustments.additional.map(&:update!).compact.sum
-      included_tax_total = tax_adjustments.inclusive.reload.map(&:update!).compact.sum
-      additional_tax_total = tax_adjustments.additional.reload.map(&:update!).compact.sum
+      adjustment_total = adjustments.additional.map(&:update_adjustment!).compact.sum
+      included_tax_total = tax_adjustments.inclusive.reload.map(&:update_adjustment!).compact.sum
+      additional_tax_total = tax_adjustments.additional.reload.map(&:update_adjustment!).compact.sum
 
       item.update_columns(
         included_tax_total: included_tax_total,

app/models/spree/line_item.rb

@@ -230,7 +230,7 @@ module Spree
 
       # update the order totals, etc.
       order.create_tax_charge!
-      order.update!
+      order.update_order!
     end
 
     def update_inventory_before_destroy

app/models/spree/order.rb

@@ -212,7 +212,7 @@ module Spree
       @updater ||= OrderManagement::Order::Updater.new(self)
     end
 
-    def update!
+    def update_order!
       updater.update
     end
 
@@ -748,8 +748,8 @@ module Spree
     def update_adjustment!(adjustment)
       return if adjustment.finalized?
 
-      adjustment.update!(force: true)
-      update!
+      adjustment.update_adjustment!(force: true)
+      update_order!
     end
 
     # object_params sets the payment amount to the order total, but it does this

app/models/spree/return_authorization.rb

@@ -66,7 +66,7 @@ module Spree
       order.shipped_shipments.collect{ |s| s.inventory_units.to_a }.flatten
     end
 
-    # Used when Adjustment#update! wants to update the related adjustment
+    # Used when Adjustment#update_adjustment! wants to update the related adjustment
     def compute_amount(*_args)
       -amount.abs
     end
@@ -105,7 +105,7 @@ module Spree
       )
 
       order.return if inventory_units.all?(&:returned?)
-      order.update!
+      order.update_order!
     end
 
     def allow_receive?

app/services/order_checkout_restart.rb

@@ -13,7 +13,7 @@ class OrderCheckoutRestart
     clear_shipments
     clear_payments
 
-    order.reload.update!
+    order.reload.update_order!
   end
 
   private

app/services/order_fees_handler.rb

@@ -24,7 +24,7 @@ class OrderFeesHandler
       order.updater.persist_totals
     end
 
-    order.update!
+    order.update_order!
   end
 
   def create_line_item_fees!
@@ -43,13 +43,13 @@ class OrderFeesHandler
 
   def update_line_item_fees!(line_item)
     line_item.adjustments.enterprise_fee.each do |fee|
-      fee.update!(line_item, force: true)
+      fee.update_adjustment!(line_item, force: true)
     end
   end
 
   def update_order_fees!
     order.adjustments.enterprise_fee.where(adjustable_type: 'Spree::Order').each do |fee|
-      fee.update!(order, force: true)
+      fee.update_adjustment!(order, force: true)
     end
   end
 

engines/order_management/app/services/order_management/order/updater.rb

@@ -129,7 +129,7 @@ module OrderManagement
       end
 
       def update_all_adjustments
-        order.all_adjustments.reload.each(&:update!)
+        order.all_adjustments.reload.each(&:update_adjustment!)
       end
 
       def before_save_hook

engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb

@@ -28,7 +28,7 @@ module OrderManagement
             let(:payment_method) { create(:payment_method) }
 
             it "returns the pending payment with no change" do
-              expect(payment).to_not receive(:update_attributes)
+              expect(payment).to_not receive(:update)
               expect(payment_setup.call!).to eq payment
             end
           end
@@ -38,7 +38,7 @@ module OrderManagement
 
             context "and the card is already set (the payment source is a credit card)" do
               it "returns the pending payment with no change" do
-                expect(payment).to_not receive(:update_attributes)
+                expect(payment).to_not receive(:update)
                 expect(payment_setup.call!).to eq payment
               end
             end
@@ -54,7 +54,7 @@ module OrderManagement
                 it "adds an error to the order and does not update the payment" do
                   payment_setup.call!
 
-                  expect(payment).to_not receive(:update_attributes)
+                  expect(payment).to_not receive(:update)
                   expect(payment_setup.call!).to eq payment
                   expect(order.errors[:base].first).to eq "There are no authorised "\
                                                             "credit cards available to charge"
@@ -80,7 +80,7 @@ module OrderManagement
                   it "adds an error to the order and does not update the payment" do
                     payment_setup.call!
 
-                    expect(payment).to_not receive(:update_attributes)
+                    expect(payment).to_not receive(:update)
                     expect(payment_setup.call!).to eq payment
                     expect(order.errors[:base].first).to eq "There are no authorised "\
                                                               "credit cards available to charge"

spec/controllers/admin/bulk_line_items_controller_spec.rb

@@ -212,7 +212,7 @@ describe Admin::BulkLineItemsController, type: :controller do
             expect(line_item1.order).to receive(:reload).with(lock: true)
             expect(line_item1.order).to receive(:update_line_item_fees!)
             expect(line_item1.order).to receive(:update_order_fees!)
-            expect(line_item1.order).to receive(:update!).twice
+            expect(line_item1.order).to receive(:update_order!).twice
 
             spree_put :update, params
           end
@@ -335,7 +335,7 @@ describe Admin::BulkLineItemsController, type: :controller do
       order.finalize!
       order.recreate_all_fees!
       order.create_tax_charge!
-      order.update!
+      order.update_order!
       order.payments << create(:payment, payment_method: payment_method, amount: order.total, state: "completed")
 
       allow(controller).to receive(:spree_current_user) { distributor.owner }
@@ -354,7 +354,7 @@ describe Admin::BulkLineItemsController, type: :controller do
         expect(order.included_tax_total).to eq 1.22
         expect(order.payment_state).to eq "paid"
 
-        expect(order).to receive(:update!).at_least(:once).and_call_original
+        expect(order).to receive(:update_order!).at_least(:once).and_call_original
         expect(order).to receive(:create_tax_charge!).at_least(:once).and_call_original
 
         spree_put :update, params
@@ -378,7 +378,7 @@ describe Admin::BulkLineItemsController, type: :controller do
         expect(order.included_tax_total).to eq 1.22
         expect(order.payment_state).to eq "paid"
 
-        expect(order).to receive(:update!).at_least(:once).and_call_original
+        expect(order).to receive(:update_order!).at_least(:once).and_call_original
         expect(order).to receive(:create_tax_charge!).at_least(:once).and_call_original
 
         spree_delete :destroy, params

spec/controllers/api/v0/shipments_controller_spec.rb

@@ -204,7 +204,7 @@ describe Api::V0::ShipmentsController, type: :controller do
       before do
         order.shipments.first.shipping_methods = [shipping_method1, shipping_method2]
         order.select_shipping_method(shipping_method1.id)
-        order.update!
+        order.update_order!
         order.update_columns(
           payment_total: 60,
           payment_state: "paid"

spec/controllers/checkout_controller_spec.rb

@@ -291,7 +291,7 @@ describe CheckoutController, type: :controller do
       allow(controller).to receive(:current_order).and_return(order)
     end
 
-    it "returns errors and flash if order.update_attributes fails" do
+    it "returns errors and flash if order.update fails" do
       spree_post :update, format: :json, order: {}
       expect(response.status).to eq(400)
       expect(response.body).to eq({ errors: assigns[:order].errors, flash: { error: order.errors.full_messages.to_sentence } }.to_json)

spec/controllers/line_items_controller_spec.rb

@@ -55,7 +55,7 @@ describe LineItemsController, type: :controller do
 
         context "where the item's order is associated with the current user" do
           before do
-            order.update_attributes!(user_id: user.id)
+            order.update!(user_id: user.id)
             allow(controller).to receive_messages spree_current_user: item.order.user
           end
 
@@ -67,7 +67,7 @@ describe LineItemsController, type: :controller do
           end
 
           context "with an order cycle and distributor" do
-            before { order.update_attributes!(order_cycle_id: order_cycle.id, distributor_id: distributor.id) }
+            before { order.update!(order_cycle_id: order_cycle.id, distributor_id: distributor.id) }
 
             context "where changes are not allowed" do
               it "denies deletion" do
@@ -77,7 +77,7 @@ describe LineItemsController, type: :controller do
             end
 
             context "where changes are allowed" do
-              before { distributor.update_attributes!(allow_order_changes: true) }
+              before { distributor.update!(allow_order_changes: true) }
 
               it "deletes the line item" do
                 delete :destroy, params: params

spec/controllers/spree/admin/return_authorizations_controller_spec.rb

@@ -17,7 +17,7 @@ module Spree
 
         # Pay the order
         order.payments.first.complete
-        order.update!
+        order.update_order!
 
         # Ship the order
         order.shipment.ship!

spec/controllers/spree/orders_controller_spec.rb

@@ -533,7 +533,7 @@ describe Spree::OrdersController, type: :controller do
             end
 
             before do
-              order.update_attributes!(order_cycle_id: order_cycle.id, distributor_id: distributor.id)
+              order.update!(order_cycle_id: order_cycle.id, distributor_id: distributor.id)
             end
 
             it "returns the order" do

spec/factories/order_factory.rb

@@ -32,7 +32,7 @@ FactoryBot.define do
 
         create_list(:line_item, evaluator.line_items_count, order: order)
         order.line_items.reload
-        order.update!
+        order.update_order!
       end
 
       factory :completed_order_with_totals do

spec/features/admin/reports_spec.rb

@@ -203,7 +203,7 @@ feature '
       order1.select_shipping_method shipping_method.id
       order1.reload.recreate_all_fees!
       order1.create_tax_charge!
-      order1.update!
+      order1.update_order!
       order1.finalize!
 
       login_as_admin_and_visit spree.admin_reports_path

spec/features/admin/subscriptions_spec.rb

@@ -487,7 +487,7 @@ feature 'Subscriptions' do
       end
 
       it "permit creating and editing of the subscription" do
-        customer.update_attributes(allow_charges: true)
+        customer.update(allow_charges: true)
         # Fill in other details
         fill_in_subscription_basic_details
         click_button "Next"

spec/features/consumer/account_spec.rb

@@ -29,7 +29,7 @@ feature '
       let!(:credit_order) { create(:order_with_credit_payment, distributor: distributor_credit, user: user) }
 
       before do
-        credit_order.update!
+        credit_order.update_order!
       end
 
       it "shows all hubs that have been ordered from with balance or credit" do

spec/features/consumer/shopping/orders_spec.rb

@@ -26,7 +26,7 @@ feature "Order Management", js: true do
     before do
       # For some reason, both bill_address and ship_address are not set
       # automatically.
-      order.update_attributes!(
+      order.update!(
         bill_address: bill_address,
         ship_address: ship_address
       )

spec/helpers/checkout_helper_spec.rb

@@ -41,7 +41,7 @@ describe CheckoutHelper, type: :helper do
     }
 
     before do
-      order.update!
+      order.update_order!
       # Sanity check initial adjustments state
       expect(order.shipment_adjustments.count).to eq 1
       expect(order.adjustments.enterprise_fee.count).to eq 1

spec/jobs/subscription_confirm_job_spec.rb

@@ -254,7 +254,7 @@ describe SubscriptionConfirmJob do
     end
 
     it "records a success and sends the email" do
-      expect(order).to receive(:update!)
+      expect(order).to receive(:update_order!)
       expect(job).to receive(:record_success).with(order).once
       job.send(:send_confirmation_email, order)
       expect(SubscriptionMailer).to have_received(:confirmation_email).with(order)
@@ -271,7 +271,7 @@ describe SubscriptionConfirmJob do
     end
 
     it "records and logs an error and sends the email" do
-      expect(order).to receive(:update!)
+      expect(order).to receive(:update_order!)
       expect(job).to receive(:record_and_log_error).with(:failed_payment, order, nil).once
       job.send(:send_failed_payment_email, order)
       expect(SubscriptionMailer).to have_received(:failed_payment_email).with(order)

spec/models/spree/adjustment_spec.rb

@@ -16,7 +16,7 @@ module Spree
       end
     end
 
-    context "#update!" do
+    context "#update_adjustment!" do
       context "when originator present" do
         let(:originator) { instance_double(EnterpriseFee, compute_amount: 10.0) }
 
@@ -27,31 +27,31 @@ module Spree
         it "should do nothing when closed" do
           adjustment.close
           expect(originator).not_to receive(:compute_amount)
-          adjustment.update!
+          adjustment.update_adjustment!
         end
 
         it "should do nothing when finalized" do
           adjustment.finalize
           expect(originator).not_to receive(:compute_amount)
-          adjustment.update!
+          adjustment.update_adjustment!
         end
 
         it "should ask the originator to recalculate the amount" do
           expect(originator).to receive(:compute_amount)
-          adjustment.update!
+          adjustment.update_adjustment!
         end
 
         context "using the :force argument" do
           it "should update adjustments without changing their state" do
             expect(originator).to receive(:compute_amount)
-            adjustment.update!(force: true)
+            adjustment.update_adjustment!(force: true)
             expect(adjustment.state).to eq "open"
           end
 
           it "should update closed adjustments" do
             adjustment.close
             expect(originator).to receive(:compute_amount)
-            adjustment.update!(force: true)
+            adjustment.update_adjustment!(force: true)
           end
         end
       end
@@ -59,7 +59,7 @@ module Spree
       it "should do nothing when originator is nil" do
         allow(adjustment).to receive_messages originator: nil
         expect(adjustment).not_to receive(:update_columns)
-        adjustment.update!
+        adjustment.update_adjustment!
       end
     end
 
@@ -556,9 +556,9 @@ module Spree
                             adjustable: order, amount: 456)
       }
 
-      describe "#update!" do
+      describe "#update_adjustment!" do
         it "sets a negative value equal to the return authorization amount" do
-          expect { return_adjustment.update! }.
+          expect { return_adjustment.update_adjustment! }.
             to change { return_adjustment.reload.amount }.to(-123)
         end
       end

spec/models/spree/line_item_spec.rb

@@ -11,7 +11,7 @@ module Spree
       it 'should update inventory, totals, and tax' do
         # Regression check for Spree #1481
         expect(line_item.order).to receive(:create_tax_charge!)
-        expect(line_item.order).to receive(:update!)
+        expect(line_item.order).to receive(:update_order!)
         line_item.quantity = 2
         line_item.save
       end

spec/models/spree/order_spec.rb

@@ -481,7 +481,7 @@ describe Spree::Order do
 
     before do
       allow(subject).to receive(:fee_handler) { fee_handler }
-      allow(subject).to receive(:update!)
+      allow(subject).to receive(:update_order!)
     end
 
     it "clears all enterprise fee adjustments on the order" do
@@ -679,7 +679,7 @@ describe Spree::Order do
                           included_tax: 2, order: order)
       create(:adjustment, adjustable: shipment, originator: shipping_tax_rate,
                           amount: 10, order: order, state: "closed")
-      order.update!
+      order.update_order!
     end
 
     it "returns a sum of all tax on the order" do

spec/models/spree/payment_spec.rb

@@ -572,7 +572,7 @@ describe Spree::Payment do
           create(:payment, amount: 100, order: order)
         end
       end
-
+      
       context "when profiles are supported" do
         before do
           gateway.stub payment_profiles_supported?: true
@@ -846,7 +846,7 @@ describe Spree::Payment do
       let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
 
       before do
-        order.reload.update!
+        order.reload.update_order!
       end
 
       context "when order-based calculator" do
@@ -878,7 +878,7 @@ describe Spree::Payment do
       let!(:line_item) { create(:line_item, order: order, quantity: 3, price: 5.00) }
 
       before do
-        order.reload.update!
+        order.reload.update_order!
       end
 
       context "to Stripe payments" do

spec/models/spree/return_authorization_spec.rb

@@ -72,7 +72,7 @@ describe Spree::ReturnAuthorization do
     before do
       allow(return_authorization).to receive_messages(inventory_units: [inventory_unit], amount: -20)
       allow(Spree::Adjustment).to receive(:create)
-      allow(order).to receive(:update!)
+      allow(order).to receive(:update_order!)
     end
 
     it "should mark all inventory units are returned" do
@@ -95,7 +95,7 @@ describe Spree::ReturnAuthorization do
     end
 
     it "should update order state" do
-      expect(order).to receive :update!
+      expect(order).to receive :update_order!
       return_authorization.receive!
     end
   end

spec/models/spree/shipment_spec.rb

@@ -279,7 +279,7 @@ describe Spree::Shipment do
   context "#cancel" do
     it 'cancels the shipment' do
       allow(shipment).to receive(:ensure_correct_adjustment)
-      allow(shipment.order).to receive(:update!)
+      allow(shipment.order).to receive(:update_order!)
 
       shipment.state = 'pending'
       expect(shipment).to receive(:after_cancel)
@@ -302,7 +302,7 @@ describe Spree::Shipment do
   context "#resume" do
     it 'will determine new state based on order' do
       allow(shipment).to receive(:ensure_correct_adjustment)
-      allow(shipment.order).to receive(:update!)
+      allow(shipment.order).to receive(:update_order!)
 
       shipment.state = 'canceled'
       expect(shipment).to receive(:determine_state).and_return(:ready)
@@ -324,7 +324,7 @@ describe Spree::Shipment do
 
     it 'will determine new state based on order' do
       allow(shipment).to receive(:ensure_correct_adjustment)
-      allow(shipment.order).to receive(:update!)
+      allow(shipment.order).to receive(:update_order!)
 
       shipment.state = 'canceled'
       expect(shipment).to receive(:determine_state).twice.and_return('ready')
@@ -337,7 +337,7 @@ describe Spree::Shipment do
 
   context "#ship" do
     before do
-      allow(order).to receive(:update!)
+      allow(order).to receive(:update_order!)
       allow(shipment).to receive_messages(update_order: true, state: 'ready')
       allow(shipment).to receive_messages(fee_adjustment: charge)
       allow(shipping_method).to receive(:create_adjustment)

spec/requests/api/orders_spec.rb

@@ -52,7 +52,7 @@ describe 'api/v0/orders', type: :request do
           context "and queried by distributor id" do
             let(:'q[distributor_id_eq]') { order_dist_2.distributor.id }
 
-            before { order_dist_2.distributor.update_attributes owner: user }
+            before { order_dist_2.distributor.update owner: user }
 
             run_test! do |response|
               expect(response).to have_http_status(200)
@@ -119,7 +119,7 @@ describe 'api/v0/orders', type: :request do
               order_dist_2.order_cycle.id
             }
 
-            before { order_dist_2.distributor.update_attributes owner: user }
+            before { order_dist_2.distributor.update owner: user }
 
             run_test! do |response|
               expect(response).to have_http_status(200)

spec/services/order_syncer_spec.rb

@@ -178,7 +178,7 @@ describe OrderSyncer do
       context "when the bill_address on the order doesn't match that on the subscription" do
         before do
           order.bill_address.update!(firstname: "Jane")
-          order.update!
+          order.update_order!
         end
 
         it "does not update bill_address or ship_address on the order" do
@@ -223,7 +223,7 @@ describe OrderSyncer do
       context "when the bill_address on the order doesn't match that on the subscription" do
         before do
           order.bill_address.update!(firstname: "Jane")
-          order.update!
+          order.update_order!
         end
 
         it "does not update bill_address or ship_address on the order" do
@@ -351,7 +351,7 @@ describe OrderSyncer do
       context "when the ship address on the order doesn't match that on the subscription" do
         before do
           order.ship_address.update(firstname: "Jane")
-          order.update!
+          order.update_order!
         end
 
         it "does not update ship_address on the order" do

spec/services/paypal_items_builder_spec.rb

@@ -56,7 +56,7 @@ describe PaypalItemsBuilder do
                           amount: 23, originator: enterprise_fee, state: "closed")
     }
 
-    before { order.update! }
+    before { order.update_order! }
 
     it "should add up to the order total, minus any additional tax and the shipping cost" do
       items_total = items.sum { |i| i[:Quantity] * i[:Amount][:value] }