From f8da0db2b7e25cb899e5e7e6f163f67758ef7978 Mon Sep 17 00:00:00 2001 From: Michael Bianco Date: Tue, 3 Jul 2012 10:32:51 -0400 Subject: [PATCH 1/4] Adding note about unauthorized.html.erb --- README.textile | 1 + 1 file changed, 1 insertion(+) diff --git a/README.textile b/README.textile index 067b7d6..2d884a2 100644 --- a/README.textile +++ b/README.textile @@ -15,6 +15,7 @@ The idea is simple. You attach a file to a Product (or a Variant of this Product * The file @views/order_mailer/confirm_email.text.erb@ is the only thing that should need customization. But since I assume you do that anyway, it doesn't hurt to do it when you're using spree_digital. The reason is that the download links are added to the confirmation email to the customer. "http://github.com/iloveitaly/spree-html-email":http://github.com/iloveitaly/spree-html-email also supports spree_digital. * A purchased product can be downloaded even if you disable the product immediately. You would have to remove the attached file in your admin section to prevent people from downloading purchased products. * File are uploaded to @rails_root/private@. Make sure it's symlinked in case you're using Capistrano. +* You must add a views/spree/digitals/unauthorized.html.erb file to present an error message to the user if they exceed the download / days limit * We use send_file to send the files on download. Yes, it goes through the entire stack right now. h2. Installation From cdbc636e798a140094bcc3c6749dddd76dbaef7e Mon Sep 17 00:00:00 2001 From: Michael Bianco Date: Tue, 3 Jul 2012 13:21:33 -0400 Subject: [PATCH 2/4] Don't authorize! until we know the digital file exists --- app/controllers/spree/digitals_controller.rb | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/digitals_controller.rb b/app/controllers/spree/digitals_controller.rb index a5ddf86..2c00a53 100644 --- a/app/controllers/spree/digitals_controller.rb +++ b/app/controllers/spree/digitals_controller.rb @@ -1,16 +1,25 @@ module Spree class DigitalsController < Spree::BaseController - ssl_required :show def show link = DigitalLink.find_by_secret(params[:secret]) + if link.present? and link.digital.attachment.present? attachment = link.digital.attachment - if link.authorize! and File.file?(attachment.path) - send_file attachment.path, :filename => attachment.original_filename, :type => attachment.content_type and return + + # don't authorize the link unless the file exists + # the logger error will help track down customer issues easier + + if File.file?(attachment.path) + if link.authorize! + send_file attachment.path, :filename => attachment.original_filename, :type => attachment.content_type and return + end + else + Rails.logger.error "Missing Digital Item: #{attachment.path}" end end + render :unauthorized end From 95e9b97abecac3a2c92c102f648730c2716afd11 Mon Sep 17 00:00:00 2001 From: Michael Bianco Date: Mon, 9 Jul 2012 13:32:45 -0400 Subject: [PATCH 3/4] Spree 1.1.2 compat. Adding specs for digital shipping --- Gemfile | 2 +- app/models/spree/order_decorator.rb | 3 ++- spec/factories/digital_factory.rb | 3 ++- spec/factories/digital_shipping_factory.rb | 12 ++++++++++ spec/models/order_spec.rb | 28 +++++++++++++++++++--- 5 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 spec/factories/digital_shipping_factory.rb diff --git a/Gemfile b/Gemfile index d272462..4e3bea9 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,6 @@ group :test do end end -gem 'spree', '~> 1.1.1' +gem 'spree', '~> 1.1.2' gemspec diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 5ac00de..8982712 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -10,12 +10,13 @@ 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 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 diff --git a/spec/factories/digital_factory.rb b/spec/factories/digital_factory.rb index c8e6f3c..0250802 100644 --- a/spec/factories/digital_factory.rb +++ b/spec/factories/digital_factory.rb @@ -1,5 +1,6 @@ FactoryGirl.define do factory :digital, :class => Spree::Digital do |f| - f.variant { |p| p.association(:variant) } + # TODO good to assign variant association if no association is manually defined + # f.variant { |p| p.association(:variant) } end end \ No newline at end of file diff --git a/spec/factories/digital_shipping_factory.rb b/spec/factories/digital_shipping_factory.rb new file mode 100644 index 0000000..10f5afc --- /dev/null +++ b/spec/factories/digital_shipping_factory.rb @@ -0,0 +1,12 @@ +FactoryGirl.define do + # https://github.com/thoughtbot/factory_girl/blob/master/GETTING_STARTED.md#modifying-factories + + factory :digital_shipping_calculator, class: Spree::Calculator::DigitalDelivery do |c| + after_create { |c| c.set_preference(:amount, 0) } + end + + factory :digital_shipping_method, parent: :shipping_method do |f| + name "Digital Delivery" + calculator { FactoryGirl.build :digital_shipping_calculator } + end +end \ No newline at end of file diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 9600aa0..554409d 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -52,15 +52,37 @@ describe Spree::Order do context "digital shipping" do before do - # TODO create digital shipping factory + @order = FactoryGirl.create(:order) + # need shipp_address for rate_hash.count != 0 + @order.ship_address = FactoryGirl.create :address + @order.bill_address = FactoryGirl.create :address + @order.save! + + 3.times { @order.add_variant FactoryGirl.create(:variant, :digitals => [FactoryGirl.create(:digital)]) } + + FactoryGirl.create :digital_shipping_method + s = FactoryGirl.create :shipping_method + s.calculator.set_preference(:amount, 10) end + let(:order) { @order } + it "should only offer digital shipping if all items are digital" do - + order.digital?.should be_true + order.rate_hash.count.should == 1 + order.rate_hash.first.shipping_method.calculator.class.should == Spree::Calculator::DigitalDelivery + order.rate_hash.first.cost.should == 0.0 end it "should not offer digital shipping if only some items are digital" do - + order.digital?.should be_true + order.add_variant FactoryGirl.create(:variant) # this is the analog product + order.digital?.should be_false + + order.rate_hash.count.should == 1 + order.rate_hash.first.shipping_method.calculator.class.should_not == Spree::Calculator::DigitalDelivery + puts "SHIPP #{order.rate_hash.first}" + order.rate_hash.first.cost.should == 10.0 end end From 27ca264d2333a8756bdf4f918356ad897589e99f Mon Sep 17 00:00:00 2001 From: Michael Bianco Date: Sun, 15 Jul 2012 23:14:25 -0300 Subject: [PATCH 4/4] Fixed variant deletion bug --- app/models/spree/variant_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index e090aa4..6a17d57 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -14,7 +14,7 @@ Spree::Variant.class_eval do # 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. def destroy_digital - digital.destroy + digitals.map &:destroy end end