Fix product/variant processors

I found some product export sample CSV files in [this repository](https://github.com/shopifypartners/product-csvs.git).

I've added some spec examples to test our import against those files. These tests are failing, so we need to change some of our assumptions about import logic:

+ `Variant SKU` is optional, so if present use it, generate new one otherwise.
+ when `Title` field is present we update a product.
+ when no `OptionValue1` is blank or `Default Title`, we update the master variant.

lib/solidus_importer/processors/product.rb

@@ -23,21 +23,20 @@ def check_data
       end
 
       def prepare_product
-        Spree::Product.find_or_initialize_by(slug: @data['Handle']) do |product|
+        Spree::Product.find_or_initialize_by(slug: @data['Handle'])
+      end
+
+      def process_product
+        prepare_product.tap do |product|
           product.slug = @data['Handle']
           product.price = options[:price]
           product.shipping_category = options[:shipping_category]
-        end.tap do |product|
+
           # Apply the row attributes
           product.name = @data['Title'] unless @data['Title'].nil?
-        end
-      end
 
-      def process_product
-        if @data['Variant SKU'].present?
-          Spree::Product.find_by!(slug: @data['Handle'])
-        else
-          prepare_product.tap(&:save!)
+          # Save the product
+          product.save!
         end
       end
     end

lib/solidus_importer/processors/variant.rb

@@ -3,32 +3,47 @@
 module SolidusImporter
   module Processors
     class Variant < Base
+      attr_accessor :product
+
       def call(context)
         @data = context.fetch(:data)
-        return unless variant?
+        self.product = context.fetch(:product) || raise(ArgumentError, 'missing :product in context')
 
-        product = context.fetch(:product)
-        variant = process_variant(product)
-        context.merge!(variant: variant)
+        context.merge!(variant: process_variant)
       end
 
       private
 
-      def prepare_variant(product)
-        Spree::Variant.find_or_initialize_by(sku: @data['Variant SKU']) do |variant|
+      def prepare_variant
+        @prepare_variant ||= Spree::Variant.find_or_initialize_by(sku: sku) do |variant|
           variant.product = product
-        end.tap do |variant|
+        end
+      end
+
+      def process_variant
+        return product.master if master_variant?
+
+        prepare_variant.tap do |variant|
           # Apply the row attributes
           variant.weight = @data['Variant Weight'] unless @data['Variant Weight'].nil?
+
+          # Save the product
+          variant.save!
         end
       end
 
-      def process_variant(product)
-        prepare_variant(product).tap(&:save!)
+      def master_variant?
+        ov1 = @data['Option1 Value']
+        ov1.blank? || ov1 == 'Default Title'
+      end
+
+      def sku
+        @data['Variant SKU'] || generate_sku
       end
 
-      def variant?
-        @variant ||= @data['Variant SKU'].present?
+      def generate_sku
+        i = product.variants_including_master.count
+        "#{product.slug}-#{i}"
       end
     end
   end

spec/features/solidus_importer/import_spec.rb

@@ -91,7 +91,7 @@
         expect { import }.to change(Spree::Product, :count).from(0)
         expect(import.state).to eq('completed')
 
-        product = Spree::Product.find_by(slug: 'clay-plant-pot')
+        product = Spree::Product.find_by(slug: 'gemstone')
 
         expect(product.variants.count).to eq 2
       end
@@ -105,6 +105,10 @@
       it 'imports a some products' do
         expect { import }.to change(Spree::Product, :count).from(0)
         expect(import.state).to eq('completed')
+
+        product = Spree::Product.find_by(slug: 'clay-plant-pot')
+
+        expect(product.variants.count).to eq 2
       end
     end
   end

spec/lib/solidus_importer/processors/product_spec.rb

@@ -41,18 +41,6 @@
           expect(product.reload.name).to eq('A product name')
         end
       end
-
-      context 'with an existing invalid product' do
-        let!(:product) do
-          create(:product, slug: data['Handle']).tap { |product| product.update_column(:name, '') }
-        end
-
-        before { data['Title'] = nil }
-
-        it 'raises an exception' do
-          expect { described_method }.to raise_error(ActiveRecord::RecordInvalid, /Name can't be blank/)
-        end
-      end
     end
   end
 end

spec/lib/solidus_importer/processors/variant_spec.rb

@@ -14,11 +14,7 @@
       end
 
       it 'raises an exception' do
-        if Spree.solidus_gem_version < Gem::Version.new('2.8')
-          expect { described_method }.to raise_error(RuntimeError, /No master variant found/)
-        else
-          expect { described_method }.to raise_error(ActiveRecord::RecordInvalid, /Product must exist/)
-        end
+        expect { described_method }.to raise_error(ArgumentError, 'missing :product in context')
       end
     end
 
@@ -28,37 +24,55 @@
       end
       let(:data) { build(:solidus_importer_row_variant, :with_import).data }
       let(:product) { build_stubbed(:product, slug: data['Handle']) }
-      let(:result) { context.merge(variant: Spree::Variant.last) }
 
       before do
         allow(product).to receive(:touch)
         allow(Spree::ShippingCategory).to receive(:last).and_return(build_stubbed(:shipping_category))
       end
 
-      it 'creates a new variant' do
-        expect { described_method }.to change { Spree::Variant.count }.by(1)
-        expect(described_method).to eq(result)
+      context 'when "Option1 Value" is "Some value"' do
+        let(:result) { context.merge(variant: product.variants.first) }
+
+        before { data.merge! 'Option1 Value' => 'Some value' }
+
+        it 'creates a new variant' do
+          expect { described_method }.to change { Spree::Variant.count }.by(1)
+          expect(described_method).to eq(result)
+        end
       end
 
-      context 'with an existing valid variant' do
-        let!(:variant) { create(:variant, sku: data['Variant SKU'], weight: 10.0) }
+      context 'when "Option1 Value" is "Default Title"' do
+        let(:result) { context.merge(variant: product.master) }
 
-        it 'updates the variant' do
+        before { data.merge! 'Option1 Value' => 'Default Title' }
+
+        it 'updates master variant' do
           expect { described_method }.not_to(change { Spree::Variant.count })
           expect(described_method).to eq(result)
-          expect(variant.reload.weight.to_f).to eq(data['Variant Weight'])
         end
       end
 
-      context 'with an existing invalid variant' do
-        let!(:variant) do
-          create(:variant, sku: data['Variant SKU'], weight: 10.0).tap do |variant|
-            variant.update_column(:product_id, nil)
-          end
+      context 'when "Option1 Value" is blank' do
+        let(:result) { context.merge(variant: product.master) }
+
+        before { data.merge! 'Option1 Value' => nil }
+
+        it 'updates master variant' do
+          expect { described_method }.not_to(change { Spree::Variant.count })
+          expect(described_method).to eq(result)
         end
+      end
+
+      context 'with an existing valid variant' do
+        let(:result) { context.merge(variant: variant) }
+        let!(:variant) { create(:variant, sku: data['Variant SKU'], weight: 10.0) }
 
-        it 'raises an exception' do
-          expect { described_method }.to raise_error(ActiveRecord::RecordInvalid, /Product must exist/)
+        before { data.merge! 'Option1 Value' => 'Some value' }
+
+        it 'updates the variant' do
+          expect { described_method }.not_to(change { Spree::Variant.count })
+          expect(described_method).to eq(result)
+          expect(variant.reload.weight.to_f).to eq(data['Variant Weight'])
         end
       end
     end