From a91a498ed5a435146067566b4a3822a30a3409f0 Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Fri, 8 May 2020 15:34:45 +0200 Subject: [PATCH 1/3] Fix Search::Base specs ActiveRecord-genereated SQL is bound to change and should not be used in expectations. Search results expectations now verify actual search output only. --- core/spec/lib/search/base_spec.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/core/spec/lib/search/base_spec.rb b/core/spec/lib/search/base_spec.rb index 31f281213f3..c2813adbaf4 100644 --- a/core/spec/lib/search/base_spec.rb +++ b/core/spec/lib/search/base_spec.rb @@ -11,12 +11,13 @@ @product1 = create(:product, name: "RoR Mug", price: 9.00) @product1.taxons << @taxon @product2 = create(:product, name: "RoR Shirt", price: 11.00) + @product3 = create(:product, name: "RoR Pants", price: 16.00) end it "returns all products by default" do params = { per_page: "" } searcher = Spree::Core::Search::Base.new(params) - expect(searcher.retrieve_products.count).to eq(2) + expect(searcher.retrieve_products.count).to eq(3) end context "when include_images is included in the initalization params" do @@ -36,8 +37,6 @@ end it "switches to next page according to the page parameter" do - @product3 = create(:product, name: "RoR Pants", price: 14.00) - params = { per_page: "2" } searcher = Spree::Core::Search::Base.new(params) expect(searcher.retrieve_products.count).to eq(2) @@ -51,7 +50,6 @@ params = { per_page: "", search: { "price_range_any" => ["Under $10.00"] } } searcher = Spree::Core::Search::Base.new(params) - expect(searcher.send(:get_base_scope).to_sql).to match /<= 10/ expect(searcher.retrieve_products.count).to eq(1) end @@ -59,12 +57,6 @@ params = { per_page: "", search: { "price_range_any" => ["Under $10.00", "$10.00 - $15.00"] } } searcher = Spree::Core::Search::Base.new(params) - expect(searcher.send(:get_base_scope).to_sql).to match /<= 10/ - if Rails.gem_version >= Gem::Version.new('6.0.0') - expect(searcher.send(:get_base_scope).to_sql).to match /between 10\.0 and 15\.0/i - else - expect(searcher.send(:get_base_scope).to_sql).to match /between 10 and 15/i - end expect(searcher.retrieve_products.count).to eq(2) end @@ -72,7 +64,7 @@ params = { per_page: "", search: { "name_not_cont" => "Shirt" } } searcher = Spree::Core::Search::Base.new(params) - expect(searcher.retrieve_products.count).to eq(1) + expect(searcher.retrieve_products.count).to eq(2) end it "accepts a current user" do From 508e3205e3d7cb5eebadf9a207d72f981b704ef5 Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Sat, 9 May 2020 16:27:40 +0200 Subject: [PATCH 2/3] Replace Rails fixture_file_upload helper with Rack UploadedFile rspec-rails is not yet compatible with latest versions of ActionDispatch::TestProcess::FixtureFile and raises an error while interacting with it. Rack::Test::UploadedFile handles our use-case in the same away and doesn't have any compatbility issues. --- api/lib/spree/api/testing_support/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/lib/spree/api/testing_support/helpers.rb b/api/lib/spree/api/testing_support/helpers.rb index f302980074e..604d05c7ca1 100644 --- a/api/lib/spree/api/testing_support/helpers.rb +++ b/api/lib/spree/api/testing_support/helpers.rb @@ -38,7 +38,7 @@ def image(filename) end def upload_image(filename) - fixture_file_upload(image(filename).path, 'image/jpg') + Rack::Test::UploadedFile.new(File.open(image(filename).path), 'image/jpg') end end end From 9c14a509712716243f01b74c42318d3fa41f9f67 Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Sun, 10 May 2020 12:50:24 +0200 Subject: [PATCH 3/3] Use between operator when filtering on ranges When filtering on ranges `between` Arel operator must be used. `in` operator implementation isn't converted anymore in a between query, it was a deperecated optimization now removed in rails/rails#39163 --- core/lib/spree/core/product_filters.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/lib/spree/core/product_filters.rb b/core/lib/spree/core/product_filters.rb index f0ef5e7a967..385bbf039d0 100644 --- a/core/lib/spree/core/product_filters.rb +++ b/core/lib/spree/core/product_filters.rb @@ -60,6 +60,7 @@ module ProductFilters conds.each do |new_scope| scope = scope.or(new_scope) end + Spree::Product.joins(master: :default_price).where(scope) end @@ -70,9 +71,9 @@ def self.format_price(amount) def self.price_filter value = Spree::Price.arel_table conds = [[I18n.t('spree.under_price', price: format_price(10)), value[:amount].lteq(10)], - ["#{format_price(10)} - #{format_price(15)}", value[:amount].in(10..15)], - ["#{format_price(15)} - #{format_price(18)}", value[:amount].in(15..18)], - ["#{format_price(18)} - #{format_price(20)}", value[:amount].in(18..20)], + ["#{format_price(10)} - #{format_price(15)}", value[:amount].between(10..15)], + ["#{format_price(15)} - #{format_price(18)}", value[:amount].between(15..18)], + ["#{format_price(18)} - #{format_price(20)}", value[:amount].between(18..20)], [I18n.t('spree.or_over_price', price: format_price(20)), value[:amount].gteq(20)]] { name: I18n.t('spree.price_range'),