From 26ed44412f315470c2b0f2a4feeeafc1097deb9c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 24 Mar 2021 10:43:19 +0000
Subject: [PATCH] Update #validates definitions

Where the presence of an object is being validated and that object comes from an *association*, we should use `validates :object, presence: true` instead of `validates :object_id, presence: true`.

This does not apply in the same way to validations on uniqueness of certain attributes, such as `validates :object_id, uniqueness...`
---
 app/models/customer.rb                           | 2 +-
 app/models/enterprise_relationship.rb            | 2 +-
 app/models/enterprise_role.rb                    | 2 +-
 app/models/spree/product.rb                      | 6 +++---
 app/models/variant_override.rb                   | 4 ++--
 spec/controllers/api/products_controller_spec.rb | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/models/customer.rb b/app/models/customer.rb
index b1ba0f2651..c1af111f4f 100644
--- a/app/models/customer.rb
+++ b/app/models/customer.rb
@@ -19,7 +19,7 @@ class Customer < ActiveRecord::Base
 
   validates :code, uniqueness: { scope: :enterprise_id, allow_nil: true }
   validates :email, presence: true, uniqueness: { scope: :enterprise_id, message: I18n.t('validation_msg_is_associated_with_an_exising_customer') }
-  validates :enterprise_id, presence: true
+  validates :enterprise, presence: true
 
   scope :of, ->(enterprise) { where(enterprise_id: enterprise) }
 
diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb
index 0eb4ac3d69..3aa9875123 100644
--- a/app/models/enterprise_relationship.rb
+++ b/app/models/enterprise_relationship.rb
@@ -5,7 +5,7 @@ class EnterpriseRelationship < ActiveRecord::Base
   belongs_to :child, class_name: 'Enterprise', touch: true
   has_many :permissions, class_name: 'EnterpriseRelationshipPermission', dependent: :destroy
 
-  validates :parent_id, :child_id, presence: true
+  validates :parent, :child, presence: true
   validates :child_id, uniqueness: {
     scope: :parent_id,
     message: I18n.t('validation_msg_relationship_already_established')
diff --git a/app/models/enterprise_role.rb b/app/models/enterprise_role.rb
index 0b686e2e6e..773cb6013c 100644
--- a/app/models/enterprise_role.rb
+++ b/app/models/enterprise_role.rb
@@ -2,7 +2,7 @@ class EnterpriseRole < ActiveRecord::Base
   belongs_to :user, class_name: Spree.user_class.to_s
   belongs_to :enterprise
 
-  validates :user_id, :enterprise_id, presence: true
+  validates :user, :enterprise, presence: true
   validates :enterprise_id, uniqueness: { scope: :user_id, message: I18n.t(:enterprise_role_uniqueness_error) }
 
   scope :by_user_email, -> { joins(:user).order('spree_users.email ASC') }
diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb
index 930c2654f6..e95bf53a76 100755
--- a/app/models/spree/product.rb
+++ b/app/models/spree/product.rb
@@ -86,12 +86,12 @@ module Spree
     validates :name, presence: true
     validates :permalink, presence: true
     validates :price, presence: true, if: proc { Spree::Config[:require_master_price] }
-    validates :shipping_category_id, presence: true
+    validates :shipping_category, presence: true
 
     validates :supplier, presence: true
     validates :primary_taxon, presence: true
-    validates :tax_category_id, presence: true,
-                                if: proc { Spree::Config[:products_require_tax_category] }
+    validates :tax_category, presence: true,
+                             if: proc { Spree::Config[:products_require_tax_category] }
 
     validates :variant_unit, presence: true
     validates :unit_value, presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb
index d17b7ff0d8..6836eb4afd 100644
--- a/app/models/variant_override.rb
+++ b/app/models/variant_override.rb
@@ -11,8 +11,8 @@ class VariantOverride < ActiveRecord::Base
   belongs_to :hub, class_name: 'Enterprise'
   belongs_to :variant, class_name: 'Spree::Variant'
 
-  validates :hub_id, presence: true
-  validates :variant_id, presence: true
+  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
 
diff --git a/spec/controllers/api/products_controller_spec.rb b/spec/controllers/api/products_controller_spec.rb
index de509f89b5..f0ee86d736 100644
--- a/spec/controllers/api/products_controller_spec.rb
+++ b/spec/controllers/api/products_controller_spec.rb
@@ -122,7 +122,7 @@ describe Api::ProductsController, type: :controller do
       expect(response.status).to eq(422)
       expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
       errors = json_response["errors"]
-      expect(errors.keys).to match_array(["name", "price", "primary_taxon", "shipping_category_id", "supplier", "variant_unit"])
+      expect(errors.keys).to match_array(["name", "price", "primary_taxon", "shipping_category", "supplier", "variant_unit"])
     end
 
     it "can update a product" do