Merge pull request #2630 from mamhoff/fix-duplicate-variants

Fix duplicate variants on product page

core/app/models/spree/variant.rb

@@ -138,11 +138,23 @@ def self.active(currency = nil)
     end
 
     # Returns variants that have a price for the given pricing options
+    # If you have modified the pricing options class, you might want to modify this scope too.
     #
     # @param pricing_options A Pricing Options object as defined on the price selector class
     # @return [ActiveRecord::Relation]
     def self.with_prices(pricing_options = Spree::Config.default_pricing_options)
-      joins(:prices).merge(Spree::Price.currently_valid.where(pricing_options.search_arguments))
+      where(
+        Spree::Price.
+          where(Spree::Variant.arel_table[:id].eq(Spree::Price.arel_table[:variant_id])).
+          # This next clause should just be `where(pricing_options.search_arguments)`, but ActiveRecord
+          # generates invalid SQL, so the SQL here is written manually.
+          where(
+            "spree_prices.currency = ? AND (spree_prices.country_iso IS NULL OR spree_prices.country_iso = ?)",
+            pricing_options.search_arguments[:currency],
+            pricing_options.search_arguments[:country_iso].compact
+          ).
+          arel.exists
+      )
     end
 
     # @return [Spree::TaxCategory] the variant's tax category

core/spec/models/spree/product_spec.rb

@@ -219,6 +219,16 @@ class Extension < Spree::Base
           expect(product.variants_and_option_values_for(pricing_options)).to contain_exactly(low)
         end
       end
+
+      context 'when a variant has a fallback price' do
+        before do
+          low.prices.create(country_iso: nil)
+        end
+
+        it "returns that variant once" do
+          expect(product.variants_and_option_values_for.length).to eq(2)
+        end
+      end
     end
 
     describe "#variant_option_values_by_option_type" do

core/spec/models/spree/variant/scopes_spec.rb

@@ -28,6 +28,20 @@
         end
       end
     end
+
+    context 'when searching for a variant that has two eligible prices (one fallback)' do
+      let(:france) { create(:country, iso: "FR") }
+      let(:pricing_options) { Spree::Variant::PricingOptions.new(country_iso: "FR", currency: "EUR") }
+
+      subject { Spree::Variant.with_prices(pricing_options) }
+
+      before do
+        variant_1.prices.create!(currency: "EUR", country: france, amount: 10)
+        variant_1.prices.create!(currency: "EUR", country: nil, amount: 10)
+      end
+
+      it { is_expected.to eq([variant_1]) }
+    end
   end
 
   it ".descend_by_popularity" do