Merge pull request #12428 from cyrillefr/RedundantPresenceValidationOnBelongs_part_III

Fix RedundantPresenceValidationOnBelongs on two files

.rubocop_todo.yml

@@ -650,14 +650,12 @@ Rails/RedundantActiveRecordAllMethod:
     - 'app/models/spree/variant.rb'
     - 'spec/system/admin/product_import_spec.rb'
 
-# Offense count: 11
+# Offense count: 9
 # This cop supports unsafe autocorrection (--autocorrect-all).
 Rails/RedundantPresenceValidationOnBelongsTo:
   Exclude:
     - 'app/models/spree/line_item.rb'
     - 'app/models/spree/order.rb'
-    - 'app/models/spree/stock_item.rb'
-    - 'app/models/spree/stock_movement.rb'
     - 'app/models/spree/tax_rate.rb'
     - 'app/models/subscription_line_item.rb'
     - 'app/models/tag_rule.rb'

app/models/spree/stock_item.rb

@@ -2,15 +2,12 @@
 
 module Spree
   class StockItem < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
     acts_as_paranoid
 
     belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items
     belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant'
     has_many :stock_movements, dependent: :destroy
 
-    validates :stock_location, :variant, presence: true
     validates :variant_id, uniqueness: { scope: [:stock_location_id, :deleted_at] }
     validates :count_on_hand, numericality: { greater_than_or_equal_to: 0, unless: :backorderable? }
 

app/models/spree/stock_movement.rb

@@ -2,14 +2,11 @@
 
 module Spree
   class StockMovement < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
     belongs_to :stock_item, class_name: 'Spree::StockItem'
-    belongs_to :originator, polymorphic: true
+    belongs_to :originator, polymorphic: true, optional: true
 
     after_create :update_stock_item_quantity
 
-    validates :stock_item, presence: true
     validates :quantity, presence: true
 
     scope :recent, -> { order('created_at DESC') }

db/migrate/20240501072547_require_variant_and_stock_location_on_stock_item.rb

@@ -0,0 +1,6 @@
+class RequireVariantAndStockLocationOnStockItem < ActiveRecord::Migration[7.0]
+  def change
+    change_column_null :spree_stock_items, :stock_location_id, false
+    change_column_null :spree_stock_items, :variant_id, false
+  end
+end

db/migrate/20240501075735_require_stock_item_and_quantity_on_stock_movement.rb

@@ -0,0 +1,6 @@
+class RequireStockItemAndQuantityOnStockMovement < ActiveRecord::Migration[7.0]
+  def change
+    change_column_null :spree_stock_movements, :stock_item_id, false
+    change_column_null :spree_stock_movements, :quantity, false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_04_30_075133) do
+ActiveRecord::Schema[7.0].define(version: 2024_05_01_075735) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
   enable_extension "plpgsql"
@@ -818,8 +818,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_04_30_075133) do
   end
 
   create_table "spree_stock_items", id: :serial, force: :cascade do |t|
-    t.integer "stock_location_id"
-    t.integer "variant_id"
+    t.integer "stock_location_id", null: false
+    t.integer "variant_id", null: false
     t.integer "count_on_hand", default: 0, null: false
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
@@ -849,8 +849,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_04_30_075133) do
   end
 
   create_table "spree_stock_movements", id: :serial, force: :cascade do |t|
-    t.integer "stock_item_id"
-    t.integer "quantity", default: 0
+    t.integer "stock_item_id", null: false
+    t.integer "quantity", default: 0, null: false
     t.string "action", limit: 255
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false

lib/tasks/data/check_missing_required_ids_in_stock_items.rake

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+namespace :ofn do
+  namespace :data do
+    desc 'Checking missing required ids in Spree::StockItem'
+    task check_missing_required_missing_ids_in_spree_stock_items: :environment do
+      puts 'Checking for null stock_location_id'
+      ids = Spree::StockItem.where(stock_location_id: nil).pluck(:id)
+
+      if ids.empty?
+        puts 'No NULL stock_location_id found in spree_stock_items'
+      else
+        puts 'NULL stock_location_ids s have been found in spree_stock_items:'
+        print ids
+      end
+
+      puts 'Checking for null variant_id'
+      ids = Spree::StockItem.where(variant_id: nil).pluck(:id)
+
+      if ids.empty?
+        puts 'No NULL variant_id found in spree_stock_items'
+      else
+        puts 'NULL variant_ids s have been found in spree_stock_items:'
+        print ids
+      end
+    end
+  end
+end

lib/tasks/data/check_missing_required_ids_in_stock_movements.rake

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+namespace :ofn do
+  namespace :data do
+    desc 'Checking missing required ids in Spree::StockMovement'
+    task check_missing_required_missing_ids_in_spree_stock_movements: :environment do
+      puts 'Checking for null stock_item_id'
+      ids = Spree::StockMovement.where(stock_item_id: nil).pluck(:id)
+
+      if ids.empty?
+        puts 'No NULL stock_item_id found in spree_stock_movements'
+      else
+        puts 'NULL stock_item_ids s have been found in spree_stock_movements:'
+        print ids
+      end
+
+      puts 'Checking for null quantity'
+      ids = Spree::StockMovement.where(quantity: nil).pluck(:id)
+
+      if ids.empty?
+        puts 'No NULL quantity found in spree_stock_movements'
+      else
+        puts 'NULL quantity s have been found in spree_stock_movements:'
+        print ids
+      end
+    end
+  end
+end