From 4505fa7fd975d3e2ba95f02e2f266d32a91a050d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 11 Mar 2021 18:28:16 +0000 Subject: [PATCH 1/6] Fix warning on Product variants_including_master scope --- app/models/spree/product.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 09b239855b..ba436551f8 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -56,7 +56,7 @@ module Spree }, class_name: 'Spree::Variant' has_many :variants_including_master, - -> { order("#{::Spree::Variant.quoted_table_name}.position ASC") }, + -> { order("spree_variants.position ASC") }, class_name: 'Spree::Variant', dependent: :destroy From 29e07869063c67b013787d8e37ee3f8430d7176b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 11 Mar 2021 19:23:06 +0000 Subject: [PATCH 2/6] Fix inheritance of Migration object in spec Fixes: Spree::Preferences::Preferable persisted preferables requires a valid id but returns default values Failure/Error: class CreatePrefTest < ActiveRecord::Migration def self.up create_table :pref_tests do |t| t.string :col end end def self.down drop_table :pref_tests end StandardError: Directly inheriting from ActiveRecord::Migration is not supported. Please specify the Rails release the migration was written for: class CreatePrefTest < ActiveRecord::Migration[4.2] # ./spec/models/spree/preferences/preferable_spec.rb:225:in `block (3 levels) in ' --- spec/models/spree/preferences/preferable_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index 3e156fce1f..b355ba415c 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -222,7 +222,7 @@ describe Spree::Preferences::Preferable do describe "persisted preferables" do before(:all) do - class CreatePrefTest < ActiveRecord::Migration + class CreatePrefTest < ActiveRecord::Migration[4.2] def self.up create_table :pref_tests do |t| t.string :col From da6a7da99d675f54af8e7c086e1ecdfbf5424227 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 11 Mar 2021 21:54:08 +0000 Subject: [PATCH 3/6] Remove sanitize This was added here for no specific reason I think, it's just an id, I dont think we need this https://github.com/openfoodfoundation/openfoodnetwork/commit/1d83809866a93f91a507991c12b4e60630ab6fc0 --- app/models/spree/variant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 93430b3b2b..71c10a16e3 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -114,7 +114,7 @@ module Spree joins(" LEFT OUTER JOIN (SELECT * FROM inventory_items - WHERE enterprise_id = #{sanitize enterprise.andand.id}) + WHERE enterprise_id = #{enterprise.andand.id}) AS o_inventory_items ON o_inventory_items.variant_id = spree_variants.id") .where("o_inventory_items.id IS NULL OR o_inventory_items.visible = (?)", true) From 39b1ae0ee8dca4f1fd6e3aa452e7ed2ed40eed02 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 12 Mar 2021 11:07:29 +0000 Subject: [PATCH 4/6] Fix validation conditional in Spree::Product --- app/models/spree/product.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index ba436551f8..bb42d78b43 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -90,7 +90,7 @@ module Spree validates :supplier, presence: true validates :primary_taxon, presence: true - validates :tax_category_id, presence: true, if: "Spree::Config.products_require_tax_category" + validates :tax_category_id, 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 } } From 333a488dc81e9aa19c3910f62072db66c021bf3a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 12 Mar 2021 22:34:33 +0000 Subject: [PATCH 5/6] Fix deprecation warning --- app/models/spree/product.rb | 2 +- spec/models/spree/product_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index bb42d78b43..140a508af7 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -52,7 +52,7 @@ module Spree dependent: :destroy has_many :variants, -> { - where(is_master: false).order("#{::Spree::Variant.quoted_table_name}.position ASC") + where(is_master: false).order("spree_variants.position ASC") }, class_name: 'Spree::Variant' has_many :variants_including_master, diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index c1da45eddc..c3814bebf8 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -88,13 +88,13 @@ module Spree describe 'Variants sorting' do context 'without master variant' do it 'sorts variants by position' do - expect(product.variants.to_sql).to match(/ORDER BY (\`|\")spree_variants(\`|\").position ASC/) + expect(product.variants.to_sql).to match(/ORDER BY spree_variants.position ASC/) end end context 'with master variant' do it 'sorts variants by position' do - expect(product.variants_including_master.to_sql).to match(/ORDER BY (\`|\")spree_variants(\`|\").position ASC/) + expect(product.variants_including_master.to_sql).to match(/ORDER BY spree_variants.position ASC/) end end end From 3e9d1bfe1f91c48bcaad56ad2716ff4fc32f1ffc Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 22 Mar 2021 14:52:42 +0000 Subject: [PATCH 6/6] Add defensive more code in not_hidden_for scope --- app/models/spree/variant.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 71c10a16e3..ed94fba1e6 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -109,12 +109,13 @@ module Spree } scope :not_hidden_for, lambda { |enterprise| - return where("1=0") if enterprise.blank? + enterprise_id = enterprise&.id.to_i + return none if enterprise_id < 1 joins(" LEFT OUTER JOIN (SELECT * FROM inventory_items - WHERE enterprise_id = #{enterprise.andand.id}) + WHERE enterprise_id = #{enterprise_id}) AS o_inventory_items ON o_inventory_items.variant_id = spree_variants.id") .where("o_inventory_items.id IS NULL OR o_inventory_items.visible = (?)", true)