Merge branch 'delete-protection' into funkensturm/master

* delete-protection:
  Added config to optionally disable deleting digitals on variant deletion
  Revert "Remove delete digitals on soft delete of variant"
  Adding failing variant spec
  Fixed broken Digital#destroy spec
  Remove delete digitals on soft delete of variant

app/models/spree/order_decorator.rb

@@ -10,13 +10,12 @@ Spree::Order.class_eval do
     false
   end
 
-  # UPGRADE_CHECK
-  # TODO this works as of spree 1.1.1; make sure to check the original function on upgrade
+  # UPGRADE_CHECK this works as of spree 1.1.1; make sure to check the original function on upgrade
 
   def available_shipping_methods(display_on = nil)
     return [] unless ship_address
     all_methods = Spree::ShippingMethod.all_available(self, display_on)
-    puts "ALL METHODS #{all_methods.count} #{display_on}"
+
     if self.digital?
       all_methods.detect { |m| m.calculator.class == Spree::Calculator::DigitalDelivery }.try { |d| [d] } || all_methods
     else

app/models/spree/variant_decorator.rb

@@ -1,6 +1,5 @@
 Spree::Variant.class_eval do
-  
-  has_many :digitals, :dependent => :destroy
+  has_many :digitals
   after_save :destroy_digital, :if => :deleted?
   
   # Is this variant to be downloaded by the customer?
@@ -10,11 +9,11 @@ Spree::Variant.class_eval do
   
   private
   
-  # Spree never deleted Digitals, that's why ":dependent => :destroy" won't work on Digital.
-  # We need to delete the Digital manually here as soon as the Variant is nullified.
-  # Otherwise you'll have orphan Digitals (and their attached files!) associated with unused Variants. 
+  # :dependent => :destroy needs to be handled manually
+  # spree does not delete variants, just marks them as deleted?
+  # optionally keep digitals around for customers who require continued access to their purchases
   def destroy_digital
-    digitals.map &:destroy
+    digitals.map &:destroy unless Spree::DigitalConfiguration[:keep_digitals]
   end
 
 end

lib/spree/spree_digital_configuration.rb

@@ -1,6 +1,12 @@
 module Spree
   class SpreeDigitalConfiguration < Preferences::Configuration
+    # number of times a customer can download a digital file
     preference :authorized_clicks,  :integer, :default => 3
+    
+    # number of days after initial purchase the customer can download a file
     preference :authorized_days,    :integer, :default => 2
+
+    # should digitals be kept around after the associated product is destroyed
+    preference :keep_digitals,      :boolean, :default => false
   end
 end

spec/models/digital_spec.rb

@@ -20,6 +20,5 @@ describe Spree::Digital do
      Spree::DigitalLink.count.should == 0      
     end
   end
-  
 end
 

spec/models/variant_spec.rb

@@ -1,18 +1,38 @@
-require 'spec_helper'
+require File.dirname(__FILE__) + '/../spec_helper'
 
 describe Spree::Variant do
+  context "#destroy" do
     before do
-    @product = FactoryGirl.create :product
-    @digital = FactoryGirl.create :digital, :variant => @product.master
+      @variant = FactoryGirl.create :variant
+      @digital = FactoryGirl.create :digital, :variant => @variant
     end
 
-  let(:variant) { @product.master }
+    let(:variant) { @variant }
+    let(:digital) { @digital }
 
-  it "should delete all digitals on variant#destroy" do
-    digital_id = variant.digitals.first.id
-    Spree::Digital.find(digital_id).should_not be_nil
-    variant.digitals.count.should == 1
-    variant.destroy
-    expect { Spree::Digital.find(digital_id) }.to raise_error(ActiveRecord::RecordNotFound)
+    it "should destroy associated digitals by default" do
+      # default is false
+      Spree::DigitalConfiguration[:keep_digitals] = false
+
+      Spree::Digital.count.should == 1
+      variant.digitals.present?.should be_true
+      variant.deleted_at = Time.now
+      variant.deleted?.should be_true
+      variant.save!
+      expect { digital.reload.present? }.to raise_error
+      Spree::Digital.count.should == 0
+    end
+
+    it "should conditionally keep associated digitals" do
+      Spree::DigitalConfiguration[:keep_digitals] = true
+
+      Spree::Digital.count.should == 1
+      variant.digitals.present?.should be_true
+      variant.deleted_at = Time.now
+      variant.save!
+      variant.deleted?.should be_true
+      expect { digital.reload.present? }.to_not raise_error
+      Spree::Digital.count.should == 1
+    end
   end
 end

spec/spec_helper.rb

@@ -1,6 +1,7 @@
 ENV["RAILS_ENV"] = "test"
 require File.expand_path("../dummy/config/environment.rb",  __FILE__)
 require 'rspec/rails'
+require 'database_cleaner'
 
 # Requires supporting ruby files with custom matchers and macros, etc,
 # in spec/support/ and its subdirectories.
@@ -11,17 +12,38 @@ require 'spree/core/testing_support/env'
 require 'spree/core/testing_support/controller_requests'
 require 'spree/core/url_helpers'
 
+Dir[File.join(File.dirname(__FILE__), "factories/*.rb")].each {|f| require f }
+
 RSpec.configure do |config|
   config.mock_with :rspec
   config.fixture_path = "#{::Rails.root}/spec/fixtures"
-  config.use_transactional_fixtures = true
 
   config.include Spree::Core::UrlHelpers
   config.include Spree::Core::TestingSupport::ControllerRequests
   config.include Devise::TestHelpers, :type => :controller
+
+  # If you're not using ActiveRecord, or you'd prefer not to run each of your
+  # examples within a transaction, remove the following line or assign false
+  # instead of true.
+  config.use_transactional_fixtures = false
+
+  config.before(:each) do
+    if example.metadata[:js]
+      DatabaseCleaner.strategy = :truncation, { :except => ['spree_countries', 'spree_zones', 'spree_zone_members', 'spree_states', 'spree_roles'] }
+    else
+      DatabaseCleaner.strategy = :transaction
+    end
   end
 
-Dir[File.join(File.dirname(__FILE__), "factories/*.rb")].each {|f| require f }
+  config.before(:each) do
+    DatabaseCleaner.start
+    # reset_spree_preferences
+  end
+
+  config.after(:each) do
+    DatabaseCleaner.clean
+  end
+end
 
 # not sure if this really adds anything, but this existed in the intial version of the spree_digital rspec testing
 RSpec::Matchers.define :have_valid_factory do |factory_name|

spree_digital.gemspec

@@ -16,8 +16,9 @@ Gem::Specification.new do |s|
   # test suite
   s.add_development_dependency 'shoulda-matchers'
   s.add_development_dependency 'capybara'
-  s.add_development_dependency 'factory_girl'
+  s.add_development_dependency 'factory_girl_rails', '~> 1.7.0'
   s.add_development_dependency 'ffaker'
   s.add_development_dependency 'rspec-rails'
   s.add_development_dependency 'sqlite3'
+  s.add_development_dependency 'database_cleaner'
 end