Fixed ProductsHelper#cache_key_for_products (#10034)

* Fixed ProductsHelper#cache_key_for_products

Use Product IDs rather than product ccount as it's way more reliable

* fix ruboccop

core/app/helpers/spree/products_helper.rb

@@ -58,9 +58,13 @@ def line_item_description_text(description_text)
     end
 
     def cache_key_for_products(products = @products, additional_cache_key = nil)
-      count = products.count
+      ids = if products.is_a?(Array)
+              products.map(&:id)
+            else
+              products.ids
+            end.join('-')
       max_updated_at = (products.maximum(:updated_at) || Date.today).to_s(:number)
-      products_cache_keys = "spree/products/all-#{params[:page]}-#{params[:sort_by]}-#{max_updated_at}-#{count}-#{@taxon&.id}"
+      products_cache_keys = "spree/products/#{ids}-#{params[:page]}-#{params[:sort_by]}-#{max_updated_at}-#{@taxon&.id}"
       (common_product_cache_keys + [products_cache_keys] + [additional_cache_key]).compact.join('/')
     end
 

core/spec/helpers/products_helper_spec.rb

@@ -195,13 +195,15 @@ module Spree
     end
 
     context '#cache_key_for_products' do
-      subject { helper.cache_key_for_products }
+      subject { helper.cache_key_for_products(products) }
 
       let(:zone) { Spree::Zone.new }
       let(:price_options) { { tax_zone: zone } }
+      let!(:products) { create_list(:product, 5) }
+      let(:product_ids) { products.map(&:id).join('-') }
+      let(:taxon) { create(:taxon) }
 
       before do
-        @products = double('products collection')
         allow(helper).to receive(:params).and_return(page: 10)
         allow(helper).to receive(:current_price_options) { price_options }
       end
@@ -210,49 +212,45 @@ module Spree
         let(:updated_at) { Date.new(2011, 12, 13) }
 
         before do
-          allow(@products).to receive(:count).and_return(5)
-          allow(@products).to receive(:maximum).with(:updated_at) { updated_at }
+          allow(products).to receive(:maximum).with(:updated_at) { updated_at }
         end
 
-        it { is_expected.to eq('en/USD/spree/zones/new/spree/products/all-10--20111213-5-') }
+        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/#{product_ids}-10--20111213-") }
       end
 
       context 'when there is no considered maximum updated date' do
         let(:today) { Date.new(2013, 12, 11) }
 
         before do
-          allow(@products).to receive(:count).and_return(1_234_567)
-          allow(@products).to receive(:maximum).with(:updated_at).and_return(nil)
+          allow(products).to receive(:maximum).with(:updated_at).and_return(nil)
           allow(Date).to receive(:today) { today }
         end
 
-        it { is_expected.to eq('en/USD/spree/zones/new/spree/products/all-10--20131211-1234567-') }
+        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/#{product_ids}-10--20131211-") }
       end
 
       context 'with Taxon ID present' do
         let(:updated_at) { Date.new(2011, 12, 13) }
 
         before do
-          @taxon = create(:taxon)
-          allow(@products).to receive(:count).and_return(5)
-          allow(@products).to receive(:maximum).with(:updated_at) { updated_at }
+          @taxon = taxon
+          allow(products).to receive(:maximum).with(:updated_at) { updated_at }
         end
 
-        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/all-10--20111213-5-#{@taxon.id}") }
+        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/#{product_ids}-10--20111213-#{taxon.id}") }
       end
 
       context 'with `additional_cache_key` passed' do
-        subject { helper.cache_key_for_products(@products, 'json-ld') }
+        subject { helper.cache_key_for_products(products, 'json-ld') }
 
         let(:updated_at) { Date.new(2011, 12, 13) }
 
         before do
-          @taxon = create(:taxon)
-          allow(@products).to receive(:count).and_return(5)
-          allow(@products).to receive(:maximum).with(:updated_at) { updated_at }
+          @taxon = taxon
+          allow(products).to receive(:maximum).with(:updated_at) { updated_at }
         end
 
-        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/all-10--20111213-5-#{@taxon.id}/json-ld") }
+        it { is_expected.to eq("en/USD/spree/zones/new/spree/products/#{product_ids}-10--20111213-#{taxon.id}/json-ld") }
       end
     end