Fix cop RedundantPresenceValidationOnBelongs

- presence: true is redundant since Rails 5.0 BUT applies
  with new default config of
  belongs_to_required_by_default to true
  Lots of files with belongs_to_required_by_default = false
  (backward compatibility)
  So: deleting this setting implies to adding optional: true
  on belongs_to relations where there is no explicit check for presence.
  And also to delete redundant presence: true
  The implicit becomes the explicit and vice versa
- updated toto list

.rubocop_todo.yml

@@ -660,27 +660,6 @@ Rails/RedundantActiveRecordAllMethod:
     - 'app/models/spree/variant.rb'
     - 'spec/system/admin/product_import_spec.rb'
 
-# Offense count: 20
-# This cop supports unsafe autocorrection (--autocorrect-all).
-Rails/RedundantPresenceValidationOnBelongsTo:
-  Exclude:
-    - 'app/models/enterprise_fee.rb'
-    - 'app/models/exchange.rb'
-    - 'app/models/inventory_item.rb'
-    - 'app/models/order_cycle.rb'
-    - 'app/models/spree/address.rb'
-    - 'app/models/spree/line_item.rb'
-    - 'app/models/spree/order.rb'
-    - 'app/models/spree/product_property.rb'
-    - 'app/models/spree/return_authorization.rb'
-    - 'app/models/spree/state.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/variant_override.rb'
-
 # Offense count: 1
 # This cop supports unsafe autocorrection (--autocorrect-all).
 Rails/RelativeDateConstant:

app/models/enterprise_fee.rb

@@ -21,7 +21,6 @@ class EnterpriseFee < ApplicationRecord
 
   validates :fee_type, inclusion: { in: FEE_TYPES }
   validates :name, presence: true
-  validates :enterprise_id, presence: true
 
   before_save :ensure_valid_tax_category_settings
 

app/models/exchange.rb

@@ -10,8 +10,6 @@
 # shopfront (outgoing products). But the set of shown products can be smaller
 # than all incoming products.
 class Exchange < ApplicationRecord
-  self.belongs_to_required_by_default = false
-
   acts_as_taggable
 
   belongs_to :order_cycle
@@ -24,7 +22,6 @@ class Exchange < ApplicationRecord
   has_many :exchange_fees, dependent: :destroy
   has_many :enterprise_fees, through: :exchange_fees
 
-  validates :order_cycle, :sender, :receiver, presence: true
   validates :sender_id, uniqueness: { scope: [:order_cycle_id, :receiver_id, :incoming] }
 
   before_destroy :delete_related_exchange_variants, prepend: true

app/models/inventory_item.rb

@@ -1,14 +1,10 @@
 # frozen_string_literal: true
 
 class InventoryItem < ApplicationRecord
-  self.belongs_to_required_by_default = false
-
   belongs_to :enterprise
   belongs_to :variant, class_name: "Spree::Variant"
 
   validates :variant_id, uniqueness: { scope: :enterprise_id }
-  validates :enterprise, presence: true
-  validates :variant, presence: true
   validates :visible,
             inclusion: { in: [true, false], message: I18n.t(:inventory_item_visibility_error) }
 

app/models/order_cycle.rb

@@ -3,8 +3,6 @@
 require 'open_food_network/scope_variant_to_hub'
 
 class OrderCycle < ApplicationRecord
-  self.belongs_to_required_by_default = false
-
   searchable_attributes :orders_open_at, :orders_close_at, :coordinator_id
   searchable_scopes :active, :inactive, :active_or_complete, :upcoming, :closed, :not_closed,
                     :dated, :undated, :soonest_opening, :soonest_closing, :most_recently_closed
@@ -44,7 +42,7 @@ class OrderCycle < ApplicationRecord
   before_update :reset_processed_at, if: :will_save_change_to_orders_close_at?
   after_save :sync_subscriptions, if: :opening?
 
-  validates :name, :coordinator_id, presence: true
+  validates :name, presence: true
   validate :orders_close_at_after_orders_open_at?
 
   preference :product_selection_from_coordinator_inventory_only, :boolean, default: false

app/models/spree/address.rb

@@ -4,19 +4,17 @@ module Spree
   class Address < ApplicationRecord
     include AddressDisplay
 
-    self.belongs_to_required_by_default = false
-
     searchable_attributes :firstname, :lastname, :phone, :full_name, :full_name_reversed,
                           :full_name_with_comma, :full_name_with_comma_reversed
     searchable_associations :country, :state
 
     belongs_to :country, class_name: "Spree::Country"
-    belongs_to :state, class_name: "Spree::State"
+    belongs_to :state, class_name: "Spree::State", optional: true
 
     has_one :enterprise, dependent: :restrict_with_exception
     has_many :shipments
 
-    validates :address1, :city, :country, :phone, presence: true
+    validates :address1, :city, :phone, presence: true
     validates :company, presence: true, unless: -> { first_name.blank? || last_name.blank? }
     validates :firstname, :lastname, presence: true, if: -> do
       company.blank? || company == 'unused'

app/models/spree/line_item.rb

@@ -7,19 +7,17 @@ class LineItem < ApplicationRecord
     include VariantUnits::VariantAndLineItemNaming
     include LineItemStockChanges
 
-    self.belongs_to_required_by_default = false
-
     searchable_attributes :price, :quantity, :order_id, :variant_id, :tax_category_id
     searchable_associations :order, :order_cycle, :variant, :product, :supplier, :tax_category
     searchable_scopes :with_tax, :without_tax
 
-    belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items
+    belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items, optional: true
     has_one :order_cycle, through: :order
 
     belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant"
     has_one :product, through: :variant
     has_one :supplier, through: :product
-    belongs_to :tax_category, class_name: "Spree::TaxCategory"
+    belongs_to :tax_category, class_name: "Spree::TaxCategory", optional: true
 
     has_many :adjustments, as: :adjustable, dependent: :destroy
 
@@ -28,7 +26,6 @@ class LineItem < ApplicationRecord
     before_validation :copy_tax_category
     before_validation :copy_dimensions
 
-    validates :variant, presence: true
     validates :quantity, numericality: {
       only_integer: true,
       greater_than: -1,

app/models/spree/order.rb

@@ -8,8 +8,6 @@ class Order < ApplicationRecord
     include Balance
     include SetUnusedAddressFields
 
-    self.belongs_to_required_by_default = false
-
     searchable_attributes :number, :state, :shipment_state, :payment_state, :distributor_id,
                           :order_cycle_id, :email, :total, :customer_id
     searchable_associations :shipping_method, :bill_address, :distributor
@@ -33,13 +31,13 @@ class Order < ApplicationRecord
 
     token_resource
 
-    belongs_to :user, class_name: "Spree::User"
-    belongs_to :created_by, class_name: "Spree::User"
+    belongs_to :user, class_name: "Spree::User", optional: true
+    belongs_to :created_by, class_name: "Spree::User", optional: true
 
-    belongs_to :bill_address, class_name: 'Spree::Address'
+    belongs_to :bill_address, class_name: 'Spree::Address', optional: true
     alias_attribute :billing_address, :bill_address
 
-    belongs_to :ship_address, class_name: 'Spree::Address'
+    belongs_to :ship_address, class_name: 'Spree::Address', optional: true
     alias_attribute :shipping_address, :ship_address
 
     has_many :state_changes, as: :stateful, dependent: :destroy
@@ -70,9 +68,9 @@ def states
              dependent: :destroy
     has_many :invoices, dependent: :restrict_with_exception
 
-    belongs_to :order_cycle
-    belongs_to :distributor, class_name: 'Enterprise'
-    belongs_to :customer
+    belongs_to :order_cycle, optional: true
+    belongs_to :distributor, class_name: 'Enterprise', optional: true
+    belongs_to :customer, optional: true
     has_one :proxy_order, dependent: :destroy
     has_one :subscription, through: :proxy_order
 

app/models/spree/product_property.rb

@@ -2,12 +2,9 @@
 
 module Spree
   class ProductProperty < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
-    belongs_to :product, class_name: "Spree::Product", touch: true
+    belongs_to :product, class_name: "Spree::Product", touch: true, optional: true
     belongs_to :property, class_name: 'Spree::Property'
 
-    validates :property, presence: true
     validates :value, length: { maximum: 255 }
 
     default_scope -> { order("#{table_name}.position") }

app/models/spree/return_authorization.rb

@@ -2,8 +2,6 @@
 
 module Spree
   class ReturnAuthorization < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
     acts_as_paranoid
 
     belongs_to :order, class_name: 'Spree::Order', inverse_of: :return_authorizations
@@ -13,7 +11,6 @@ class ReturnAuthorization < ApplicationRecord
     before_save :force_positive_amount
     before_create :generate_number
 
-    validates :order, presence: true
     validates :amount, numericality: true
     validate :must_have_shipped_units
 

app/models/spree/state.rb

@@ -2,11 +2,9 @@
 
 module Spree
   class State < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
     belongs_to :country, class_name: 'Spree::Country'
 
-    validates :country, :name, presence: true
+    validates :name, presence: true
 
     def self.find_all_by_name_or_abbr(name_or_abbr)
       where('name = ? OR abbr = ?', name_or_abbr, name_or_abbr)

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
 
-    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') }

app/models/spree/tax_rate.rb

@@ -14,17 +14,14 @@ def validate(record)
 
 module Spree
   class TaxRate < ApplicationRecord
-    self.belongs_to_required_by_default = false
-
     acts_as_paranoid
     include CalculatedAdjustments
 
-    belongs_to :zone, class_name: "Spree::Zone", inverse_of: :tax_rates
+    belongs_to :zone, class_name: "Spree::Zone", inverse_of: :tax_rates, optional: true
     belongs_to :tax_category, class_name: "Spree::TaxCategory", inverse_of: :tax_rates
     has_many :adjustments, as: :originator
 
     validates :amount, presence: true, numericality: true
-    validates :tax_category, presence: true
     validates_with DefaultTaxZoneValidator
 
     scope :by_zone, ->(zone) { where(zone_id: zone) }

app/models/subscription_line_item.rb

@@ -1,13 +1,9 @@
 # frozen_string_literal: true
 
 class SubscriptionLineItem < ApplicationRecord
-  self.belongs_to_required_by_default = false
-
   belongs_to :subscription, inverse_of: :subscription_line_items
   belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant'
 
-  validates :subscription, presence: true
-  validates :variant, presence: true
   validates :quantity, presence: true, numericality: { only_integer: true }
 
   default_scope { order('id ASC') }

app/models/tag_rule.rb

@@ -1,14 +1,10 @@
 # frozen_string_literal: true
 
 class TagRule < ApplicationRecord
-  self.belongs_to_required_by_default = false
-
   belongs_to :enterprise
 
   preference :customer_tags, :string, default: ""
 
-  validates :enterprise, presence: true
-
   scope :for, ->(enterprise) { where(enterprise_id: enterprise) }
   scope :prioritised, -> { order('priority ASC') }
 

app/models/variant_override.rb

@@ -6,15 +6,11 @@ class VariantOverride < ApplicationRecord
   extend Spree::LocalizedNumber
   include StockSettingsOverrideValidation
 
-  self.belongs_to_required_by_default = false
-
   acts_as_taggable
 
   belongs_to :hub, class_name: 'Enterprise'
   belongs_to :variant, class_name: 'Spree::Variant'
 
-  validates :hub, presence: true
-  validates :variant, presence: true
   # Default stock can be nil, indicating stock should not be reset or zero, meaning reset to zero.
   # Need to ensure this can be set by the user.
   validates :default_stock, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true

spec/models/subscription_line_item_spec.rb

@@ -5,11 +5,11 @@
 describe SubscriptionLineItem, model: true do
   describe "validations" do
     it "requires a subscription" do
-      expect(subject).to validate_presence_of :subscription
+      expect(subject).to belong_to :subscription
     end
 
     it "requires a variant" do
-      expect(subject).to validate_presence_of :variant
+      expect(subject).to belong_to :variant
     end
 
     it "requires a integer for quantity" do

spec/models/tag_rule_spec.rb

@@ -5,7 +5,7 @@
 describe TagRule, type: :model do
   describe "validations" do
     it "requires a enterprise" do
-      expect(subject).to validate_presence_of(:enterprise)
+      expect(subject).to belong_to(:enterprise)
     end
   end
 end