Fixes #5639, add presence validation for option_values in variant.

api/spec/controllers/spree/api/v1/products_controller_spec.rb

@@ -126,7 +126,7 @@ module Spree
 
       it "gets a single product" do
         product.master.images.create!(:attachment => image("thinking-cat.jpg"))
-        product.variants.create!
+        create(:variant, product: product)
         product.variants.first.images.create!(:attachment => image("thinking-cat.jpg"))
         product.set_property("spree", "rocks")
         product.taxons << create(:taxon)

backend/app/views/spree/admin/variants/index.html.erb

@@ -39,8 +39,10 @@
   </table>
 <% else %>
   <div class="no-objects-found alert alert-info">
-    <%= Spree.t(:no_resource_found, resource: plural_resource_name(Spree::Variant)) %>,
-    <%= link_to(Spree.t(:add_one), spree.new_admin_product_variant_path(@product)) if can? :create, Spree::Variant %>!
+    <%= Spree.t(:no_resource_found, resource: plural_resource_name(Spree::Variant)) %>
+    <% if can?(:create, Spree::Variant) && !@product.empty_option_values? %>
+       - <%= link_to(Spree.t(:add_one), spree.new_admin_product_variant_path(@product)) %>
+    <% end %>!
   </div>
 <% end %>
 

backend/spec/features/admin/products/stock_management_spec.rb

@@ -83,8 +83,8 @@
     end
 
     context "with multiple variants" do
+      let!(:variant) { create(:variant, product: product, sku: 'SPREEC') }
       before do
-        variant = product.variants.create!(sku: 'SPREEC')
         variant.stock_items.first.update_column(:count_on_hand, 30)
         visit current_url
       end
@@ -106,12 +106,13 @@
 
     # Regression test for #3304
     context "with no stock location" do
+      let(:product) { create(:product, name: 'apache baseball cap', price: 10) }
+      let(:variant) { create(:variant, product: product, sku: 'FOOBAR') }
+
       before do
-        @product = create(:product, name: 'apache baseball cap', price: 10)
-        v = @product.variants.create!(sku: 'FOOBAR')
         Spree::StockLocation.delete_all
 
-        visit spree.stock_admin_product_path(@product)
+        visit spree.stock_admin_product_path(product)
       end
 
       it "redirects to stock locations page" do

core/app/models/spree/variant.rb

@@ -38,6 +38,8 @@ class Variant < Spree::Base
 
     validate :check_price
 
+    validates :option_values, presence: true, unless: :is_master?
+
     with_options numericality: { greater_than_or_equal_to: 0, allow_nil: true } do
       validates :cost_price
       validates :price

core/spec/models/spree/variant_spec.rb

@@ -4,9 +4,15 @@
 
 describe Spree::Variant, :type => :model do
   let!(:variant) { create(:variant) }
+  let(:master_variant) { create(:master_variant) }
 
   it_behaves_like 'default_price'
 
+  describe 'validations' do
+    it { expect(master_variant).to_not validate_presence_of(:option_values) }
+    it { expect(variant).to validate_presence_of(:option_values) }
+  end
+
   context 'sorting' do
     it 'responds to set_list_position' do
       expect(variant.respond_to?(:set_list_position)).to eq(true)
@@ -30,7 +36,7 @@
 
     it "propagate to stock items" do
       expect_any_instance_of(Spree::StockLocation).to receive(:propagate_variant)
-      product.variants.create(:name => "Foobar")
+      create(:variant, product: product)
     end
 
     context "stock location has disable propagate all variants" do
@@ -51,9 +57,7 @@
       end
 
       context 'when a variant is created' do
-        before(:each) do
-          product.variants.create!(:name => 'any-name')
-        end
+        let!(:new_variant) { create(:variant, product: product) }
 
         it { expect(product.master).to_not be_in_stock }
       end
@@ -235,13 +239,14 @@
 
   # Regression test for #2432
   describe 'options_text' do
-    let!(:variant) { create(:variant, option_values: []) }
+    let!(:variant) { build(:variant, option_values: []) }
     let!(:master) { create(:master_variant) }
 
     before do
       # Order bar than foo
       variant.option_values << create(:option_value, {name: 'Foo', presentation: 'Foo', option_type: create(:option_type, position: 2, name: 'Foo Type', presentation: 'Foo Type')})
       variant.option_values << create(:option_value, {name: 'Bar', presentation: 'Bar', option_type: create(:option_type, position: 1, name: 'Bar Type', presentation: 'Bar Type')})
+      variant.save
     end
 
     it 'should order by bar than foo' do
@@ -251,7 +256,7 @@
   end
 
   describe 'exchange_name' do
-    let!(:variant) { create(:variant, option_values: []) }
+    let!(:variant) { build(:variant, option_values: []) }
     let!(:master) { create(:master_variant) }
 
     before do
@@ -260,6 +265,7 @@
                                                      presentation: 'Foo',
                                                      option_type: create(:option_type, position: 2, name: 'Foo Type', presentation: 'Foo Type')
                                                    })
+      variant.save
     end
 
     context 'master variant' do
@@ -277,7 +283,7 @@
   end
 
   describe 'exchange_name' do
-    let!(:variant) { create(:variant, option_values: []) }
+    let!(:variant) { build(:variant, option_values: []) }
     let!(:master) { create(:master_variant) }
 
     before do
@@ -286,6 +292,7 @@
                                                      presentation: 'Foo',
                                                      option_type: create(:option_type, position: 2, name: 'Foo Type', presentation: 'Foo Type')
                                                    })
+      variant.save
     end
 
     context 'master variant' do
@@ -303,7 +310,7 @@
   end
 
   describe 'descriptive_name' do
-    let!(:variant) { create(:variant, option_values: []) }
+    let!(:variant) { build(:variant, option_values: []) }
     let!(:master) { create(:master_variant) }
 
     before do
@@ -312,6 +319,7 @@
                                                      presentation: 'Foo',
                                                      option_type: create(:option_type, position: 2, name: 'Foo Type', presentation: 'Foo Type')
                                                    })
+      variant.save
     end
 
     context 'master variant' do

frontend/spec/features/products_spec.rb

@@ -141,7 +141,7 @@
   context "a product with variants" do
     let(:product) { Spree::Product.find_by_name("Ruby on Rails Baseball Jersey") }
     let(:option_value) { create(:option_value) }
-    let!(:variant) { product.variants.create!(:price => 5.59) }
+    let!(:variant) { build(:variant, price: 5.59, product: product, option_values: []) }
 
     before do
       # Need to have two images to trigger the error
@@ -151,6 +151,7 @@
 
       product.option_types << option_value.option_type
       variant.option_values << option_value
+      variant.save!
     end
 
     it "should be displayed" do
@@ -186,13 +187,13 @@
 
   context "a product with variants, images only for the variants" do
     let(:product) { Spree::Product.find_by_name("Ruby on Rails Baseball Jersey") }
+    let(:variant1) { create(:variant, product: product, price: 9.99) }
+    let(:variant2) { create(:variant, product: product, price: 10.99) }
 
     before do
       image = File.open(File.expand_path('../../fixtures/thinking-cat.jpg', __FILE__))
-      v1 = product.variants.create!(price: 9.99)
-      v2 = product.variants.create!(price: 10.99)
-      v1.images.create!(attachment: image)
-      v2.images.create!(attachment: image)
+      variant1.images.create!(attachment: image)
+      variant2.images.create!(attachment: image)
     end
 
     it "should not display no image available" do