Merge pull request #12647 from cyrillefr/FixRailsSkipsModelValidations

Fixes Rails/SkipsModelValidations offenses

.rubocop_todo.yml

@@ -644,14 +644,6 @@ Rails/RootPathnameMethods:
   Exclude:
     - 'spec/lib/reports/orders_and_fulfillment/order_cycle_customer_totals_report_spec.rb'
 
-# Offense count: 4
-# Configuration parameters: ForbiddenMethods, AllowedMethods.
-# ForbiddenMethods: decrement!, decrement_counter, increment!, increment_counter, insert, insert!, insert_all, insert_all!, toggle!, touch, touch_all, update_all, update_attribute, update_column, update_columns, update_counters, upsert, upsert_all
-Rails/SkipsModelValidations:
-  Exclude:
-    - 'app/models/variant_override.rb'
-    - 'spec/models/spree/line_item_spec.rb'
-
 # Offense count: 7
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: EnforcedStyle.

app/models/concerns/line_item_stock_changes.rb

@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-# Rails 5 introduced some breaking changes to these built-in methods, and the new versions
-# no longer work correctly in relation to decrementing stock with LineItems / VariantOverrides.
-# The following methods re-instate the pre-Rails-5 versions, which work as expected.
-# https://apidock.com/rails/v4.2.9/ActiveRecord/Persistence/increment%21
-# https://apidock.com/rails/v4.2.9/ActiveRecord/Persistence/decrement%21
-
-module LineItemStockChanges
-  extend ActiveSupport::Concern
-
-  def increment!(attribute, by = 1)
-    increment(attribute, by).update_attribute(attribute, self[attribute])
-  end
-
-  def decrement!(attribute, by = 1)
-    decrement(attribute, by).update_attribute(attribute, self[attribute])
-  end
-end

app/models/spree/line_item.rb

@@ -5,7 +5,6 @@ require 'open_food_network/scope_variant_to_hub'
 module Spree
   class LineItem < ApplicationRecord
     include VariantUnits::VariantAndLineItemNaming
-    include LineItemStockChanges
 
     searchable_attributes :price, :quantity, :order_id, :variant_id, :tax_category_id
     searchable_associations :order, :order_cycle, :variant, :product, :supplier, :tax_category

app/models/variant_override.rb

@@ -52,11 +52,14 @@ class VariantOverride < ApplicationRecord
       return
     end
 
+    # rubocop:disable Rails/SkipsModelValidations
+    # Cf. conversation https://github.com/openfoodfoundation/openfoodnetwork/pull/12647
     if quantity > 0
       increment! :count_on_hand, quantity
     elsif quantity < 0
       decrement! :count_on_hand, -quantity
     end
+    # rubocop:enable Rails/SkipsModelValidations
   end
 
   def default_stock?

spec/models/spree/line_item_spec.rb

@@ -349,7 +349,7 @@ module Spree
 
           it "draws stock from the variant override" do
             expect(vo.reload.count_on_hand).to eq 3
-            expect{ line_item.increment!(:quantity) }
+            expect{ line_item.update!(quantity: line_item.quantity + 1) }
               .not_to change{ Spree::Variant.find(variant.id).on_hand }
             expect(vo.reload.count_on_hand).to eq 2
           end
@@ -357,9 +357,9 @@ module Spree
 
         context "when a variant override does not apply" do
           it "draws stock from the variant" do
-            expect{ line_item.increment!(:quantity) }.to change{
-                                                           Spree::Variant.find(variant.id).on_hand
-                                                         }.by(-1)
+            expect{ line_item.update!(quantity: line_item.quantity + 1) }.to change{
+              Spree::Variant.find(variant.id).on_hand
+            }.by(-1)
           end
         end
       end