Merge remote-tracking branch 'sbounmy/custom'

Conflicts:
	Gemfile

Rakefile

@@ -3,11 +3,9 @@ require 'rake/testtask'
 require 'rake/packagetask'
 require 'rubygems/package_task'
 require 'rspec/core/rake_task'
-require 'cucumber/rake/task'
 require 'spree/core/testing_support/common_rake'
 
 RSpec::Core::RakeTask.new
-Cucumber::Rake::Task.new
 
 task :default => [:spec, :cucumber ]
 

app/controllers/spree/checkout_controller_decorator.rb

@@ -90,9 +90,11 @@ module Spree
           @order.ship_address = order_ship_address
           @order.bill_address ||= order_ship_address
         end
+        @order.state = "payment"
         @order.save
 
         if payment_method.preferred_review
+          @order.next
           render 'spree/shared/paypal_express_confirm'
         else
           paypal_finish
@@ -145,8 +147,15 @@ module Spree
           Rails.logger.error ppx_auth_response.to_yaml
         end
 
-        @order.update_attribute(:state, "complete")
+        @order.update_attributes({:state => "complete", :completed_at => Time.now}, :without_protection => true)
+
         state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil
+
+        # Since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits
+        if @order.respond_to?(:consume_users_credit, true)
+          @order.send(:consume_users_credit)
+        end
+
         @order.finalize!
         flash[:notice] = I18n.t(:order_processed_successfully)
         redirect_to completion_route
@@ -178,9 +187,8 @@ module Spree
       return unless (params[:state] == "payment")
       return unless params[:order][:payments_attributes]
 
-      if params[:order][:coupon_code]
-        @order.update_attributes(object_params)
-        if @order.coupon_code.present?
+      if @order.update_attributes(object_params)
+        if params[:order][:coupon_code] and !params[:order][:coupon_code].blank? and @order.coupon_code.present?
           fire_event('spree.checkout.coupon_code_added', :coupon_code => @order.coupon_code)
         end
       end
@@ -189,7 +197,7 @@ module Spree
       payment_method = Spree::PaymentMethod.find(params[:order][:payments_attributes].first[:payment_method_id])
 
       if payment_method.kind_of?(Spree::BillingIntegration::PaypalExpress) || payment_method.kind_of?(Spree::BillingIntegration::PaypalExpressUk)
-        redirect_to paypal_payment_order_checkout_url(@order, :payment_method_id => payment_method)
+        redirect_to(paypal_payment_order_checkout_url(@order, :payment_method_id => payment_method.id)) and return
       end
     end
 
@@ -199,7 +207,7 @@ module Spree
       else
         user_action = Spree::PaypalExpress::Config[:paypal_express_local_confirm] == "t" ? "continue" : "commit"
       end
-      
+
       #asset_url doesn't like Spree::Config[:logo] being an absolute url
       #if statement didn't work within hash
       if URI.parse(Spree::Config[:logo]).absolute?
@@ -214,7 +222,7 @@ module Spree
         :background_color        => "ffffff",  # must be hex only, six chars
         :header_background_color => "ffffff",
         :header_border_color     => "ffffff",
-	:header_image		 => chosen_image,
+  :header_image    => chosen_image,
         :allow_note              => true,
         :locale                  => user_locale,
         :req_confirm_shipping    => false,   # for security, might make an option later

app/models/spree/billing_integration/paypal_express.rb

@@ -1,15 +1,3 @@
-class Spree::BillingIntegration::PaypalExpress < Spree::BillingIntegration
-  preference :login, :string
-  preference :password, :password
-  preference :signature, :string
-  preference :review, :boolean, :default => false
-  preference :no_shipping, :boolean, :default => false
+class Spree::BillingIntegration::PaypalExpress < Spree::BillingIntegration::PaypalExpressBase
   preference :currency, :string, :default => 'USD'
-  preference :allow_guest_checkout, :boolean, :default => false
-
-  attr_accessible :preferred_login, :preferred_password, :preferred_signature, :preferred_review, :preferred_no_shipping, :preferred_currency, :preferred_allow_guest_checkout, :preferred_server, :preferred_test_mode
-
-  def provider_class
-    ActiveMerchant::Billing::PaypalExpressGateway
-  end
 end

app/models/spree/billing_integration/paypal_express_base.rb

@@ -0,0 +1,62 @@
+class Spree::BillingIntegration::PaypalExpressBase < Spree::BillingIntegration
+  preference :login, :string
+  preference :password, :password
+  preference :signature, :string
+  preference :review, :boolean, :default => false
+  preference :no_shipping, :boolean, :default => false
+  preference :currency, :string, :default => 'USD'
+  preference :allow_guest_checkout, :boolean, :default => false
+
+  attr_accessible :preferred_login, :preferred_password, :preferred_signature, :preferred_review, :preferred_no_shipping, :preferred_currency, :preferred_allow_guest_checkout, :preferred_server, :preferred_test_mode
+
+  def provider_class
+    ActiveMerchant::Billing::PaypalExpressGateway
+  end
+
+  def payment_profiles_supported?
+    !!preferred_review
+  end
+
+  def capture(payment_or_amount, account_or_response_code, gateway_options)
+    if payment_or_amount.is_a?(Spree::Payment)
+      authorization = find_authorization(payment_or_amount)
+      provider.capture(amount_in_cents(payment_or_amount.amount), authorization.params["transaction_id"], :currency => preferred_currency)
+    else
+      provider.capture(payment_or_amount, account_or_response_code, :currency => preferred_currency)
+    end
+  end
+
+  def credit(*args)
+    amount = args.shift
+    response_code = args.first.is_a?(String) ? args.first : args[1]
+    provider.credit(amount, response_code, :currency => preferred_currency)
+  end
+
+  def find_authorization(payment)
+    logs = payment.log_entries.all(:order => 'created_at DESC')
+    logs.each do |log|
+      details = YAML.load(log.details) # return the transaction details
+      if (details.params['payment_status'] == 'Pending' && details.params['pending_reason'] == 'authorization')
+        return details
+      end
+    end
+    return nil
+  end
+
+  def find_capture(payment)
+    #find the transaction associated with the original authorization/capture
+    logs = payment.log_entries.all(:order => 'created_at DESC')
+    logs.each do |log|
+      details = YAML.load(log.details) # return the transaction details
+      if details.params['payment_status'] == 'Completed'
+        return details
+      end
+    end
+    return nil
+  end
+
+  def amount_in_cents(amount)
+    (100 * amount).to_i
+  end
+
+end

app/models/spree/billing_integration/paypal_express_uk.rb

@@ -1,16 +1,3 @@
-class Spree::BillingIntegration::PaypalExpressUk < Spree::BillingIntegration
-  preference :login, :string
-  preference :password, :password
-  preference :signature, :string
-  preference :review, :boolean, :default => false
-  preference :no_shipping, :boolean, :default => false
+class Spree::BillingIntegration::PaypalExpressUk < Spree::BillingIntegration::PaypalExpressBase
   preference :currency, :string, :default => 'GBP'
-  preference :allow_guest_checkout, :boolean, :default => false
-
-  attr_accessible :preferred_login, :preferred_password, :preferred_signature, :preferred_review, :preferred_currency, :preferred_no_shipping, :preferred_server, :preferred_test_mode
-
-  def provider_class
-    ActiveMerchant::Billing::PaypalExpressGateway
-  end
-
 end

app/models/spree/paypal_account.rb

@@ -6,46 +6,15 @@ class Spree::PaypalAccount < ActiveRecord::Base
     %w{capture credit}
   end
 
-  def capture(payment)
-    authorization = find_authorization(payment)
-
-    ppx_response = payment.payment_method.provider.capture(amount_in_cents(payment.amount), authorization.params["transaction_id"], :currency => payment.payment_method.preferred_currency)
-    if ppx_response.success?
-      record_log payment, ppx_response
-      payment.complete
-    else
-      gateway_error(ppx_response.message)
-    end
-
-  end
-
   def can_capture?(payment)
     !echeck?(payment) && payment.state == "pending"
   end
 
-  def credit(payment, amount=nil)
-    authorization = find_capture(payment)
-
-    amount = payment.credit_allowed >= payment.order.outstanding_balance.abs ? payment.order.outstanding_balance : payment.credit_allowed
-    amount=amount.abs if amount
-
-    ppx_response = payment.payment_method.provider.credit(amount.nil? ? amount_in_cents(amount) : amount_in_cents(amount), authorization.params['transaction_id'], :currency => payment.payment_method.preferred_currency)
-
-    if ppx_response.success?
-      record_log payment, ppx_response
-      payment.update_attribute(:amount, payment.amount - amount)
-      payment.complete
-      payment.order.update!
-    else
-      gateway_error(ppx_response.message)
-    end
-  end
-
   def can_credit?(payment)
     return false unless payment.state == "completed"
     return false unless payment.order.payment_state == "credit_owed"
     payment.credit_allowed > 0
-    !find_capture(payment).nil?
+    !payment.payment_method.find_capture(payment).nil?
   end
 
   # fix for Payment#payment_profiles_supported?
@@ -64,43 +33,4 @@ class Spree::PaypalAccount < ActiveRecord::Base
     return false
   end
 
-  def record_log(payment, response)
-    payment.log_entries.create(:details => response.to_yaml)
-  end
-
-  private
-  def find_authorization(payment)
-    logs = payment.log_entries.all(:order => 'created_at DESC')
-    logs.each do |log|
-      details = YAML.load(log.details) # return the transaction details
-      if (details.params['payment_status'] == 'Pending' && details.params['pending_reason'] == 'authorization')
-        return details
-      end
-    end
-    return nil
-  end
-
-  def find_capture(payment)
-    #find the transaction associated with the original authorization/capture
-    logs = payment.log_entries.all(:order => 'created_at DESC')
-    logs.each do |log|
-      details = YAML.load(log.details) # return the transaction details
-      if details.params['payment_status'] == 'Completed'
-        return details
-      end
-    end
-    return nil
-  end
-
-  def gateway_error(text)
-    msg = "#{I18n.t('gateway_error')} ... #{text}"
-    logger.error(msg)
-    raise Spree::Core::GatewayError.new(msg)
-  end
-
-  private
-
-    def amount_in_cents(amount)
-      (100 * amount).to_i
-    end
 end

app/views/spree/shared/paypal_express_confirm.html.erb

@@ -1,10 +1,23 @@
-<h1><%= t("confirm") %></h1>
-<p>
-  <%= raw t("order_not_yet_placed") %>
-</p>
+<div id="checkout" data-hook>
+  <%= render :partial => 'spree/shared/error_messages', :locals => { :target => @order } %>
 
-<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%>
-<div class="form-buttons">
-  <%= button_to t('place_order'), paypal_finish_order_checkout_url(@order, {:token => params[:token] , :PayerID => params[:PayerID], :payment_method_id =>
-  params[:payment_method_id] } ), :class => "button primary" %>
-</div>
+  <div class="row" data-hook="checkout_header">
+    <h1 class="columns three alpha" data-hook="checkout_title"><%= t(:checkout) %></h1>
+    <div class="columns thirteen omega" data-hook="checkout_progress"><%= checkout_progress %></div>
+  </div>
+
+  <p>
+    <%= raw t(:order_not_yet_placed) %>
+  </p>
+
+  <div class="row" data-hook="checkout_content">
+    <div class="columns <%= if @order.state != 'confirm' then 'alpha twelve' else 'alpha omega sixteen' end %>" data-hook="checkout_form_wrapper">
+      <%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%>
+      <hr class="clear"/>
+      <div class="form-buttons">
+        <%= button_to t(:place_order), paypal_finish_order_checkout_url(@order, {:token => params[:token] , :PayerID => params[:PayerID], :payment_method_id => params[:payment_method_id] } ), :class => "button primary" %>
+      </div>
+    </div>
+
+  </div>
+</div>

lib/spree_paypal_express.rb

@@ -1,2 +1,3 @@
 require 'spree_core'
+require 'spree_auth'
 require 'spree_paypal_express/engine'

spec/controllers/checkout_controller_spec.rb

@@ -2,8 +2,10 @@ require File.dirname(__FILE__) + '/../spec_helper'
 
 module Spree
   describe CheckoutController do
+    render_views
     let(:token) { "EC-2OPN7UJGFWK9OYFV" }
-    let(:order) { Factory(:ppx_order_with_totals, :state => "payment") }
+    let(:order) { Factory(:ppx_order_with_totals, :state => "payment", :shipping_method => shipping_method) }
+    let(:shipping_method) { FactoryGirl.create(:shipping_method, :zone => Spree::Zone.find_by_name('North America'))  }
     let(:order_total) { (order.total * 100).to_i }
     let(:gateway_provider) { mock(ActiveMerchant::Billing::PaypalExpressGateway) }
     let(:paypal_gateway) { mock(BillingIntegration::PaypalExpress, :id => 123, :preferred_review => false, :preferred_no_shipping => true, :provider => gateway_provider, :preferred_currency => "US", :preferred_allow_guest_checkout => true
@@ -73,7 +75,10 @@ module Spree
     end
 
     context "paypal_confirm" do
-      before { PaymentMethod.should_receive(:find).at_least(1).with("123").and_return(paypal_gateway) }
+      before do
+        PaymentMethod.should_receive(:find).at_least(1).with("123").and_return(paypal_gateway)
+        order.stub!(:payment_method).and_return paypal_gateway
+      end
 
       context "with auto_capture and no review" do
         before do
@@ -92,13 +97,17 @@ module Spree
 
           order.reload
           order.state.should == "complete"
+          order.completed_at.should_not be_nil
           order.payments.size.should == 1
           order.payment_state.should == "paid"
         end
       end
 
       context "with review" do
-        before { paypal_gateway.stub(:preferred_review => true) }
+        before do
+           paypal_gateway.stub(:preferred_review => true, :payment_profiles_supported? => true)
+           order.stub_chain(:payment, :payment_method, :payment_profiles_supported? => true)
+         end
 
         it "should render review" do
           paypal_gateway.provider.should_receive(:details_for).with(token).and_return(details_for_response)
@@ -106,7 +115,15 @@ module Spree
           get :paypal_confirm, {:order_id => order.number, :payment_method_id => "123", :token => token, :PayerID => "FWRVKNRRZ3WUC" }
 
           response.should render_template("shared/paypal_express_confirm")
+          order.state.should == "confirm"
+        end
 
+        it "order state should not change on multiple call" do
+          paypal_gateway.provider.should_receive(:details_for).twice.with(token).and_return(details_for_response)
+
+          get :paypal_confirm, {:order_id => order.number, :payment_method_id => "123", :token => token, :PayerID => "FWRVKNRRZ3WUC" }
+          get :paypal_confirm, {:order_id => order.number, :payment_method_id => "123", :token => token, :PayerID => "FWRVKNRRZ3WUC" }
+          order.state.should == "confirm"
         end
       end
 
@@ -114,7 +131,8 @@ module Spree
         before do
           paypal_gateway.stub(:preferred_review => true)
           paypal_gateway.stub(:preferred_no_shipping => false)
-
+          paypal_gateway.stub(:payment_profiles_supported? => true)
+          order.stub_chain(:payment, :payment_method, :payment_profiles_supported? => true)
           details_for_response.stub(:params => details_for_response.params.merge({'first_name' => 'Dr.', 'last_name' => 'Evil'}),
             :address => {'address1' => 'Apt. 187', 'address2'=> 'Some Str.', 'city' => 'Chevy Chase', 'country' => 'US', 'zip' => '20815', 'state' => 'MD' })
 
@@ -126,6 +144,7 @@ module Spree
           get :paypal_confirm, {:order_id => order.number, :payment_method_id => "123", :token => token, :PayerID => "FWRVKNRRZ3WUC" }
 
           order.ship_address.address1.should == "Apt. 187"
+          order.state.should == "confirm"
           response.should render_template("shared/paypal_express_confirm")
         end
       end

spec/factories/ppx_factory.rb

@@ -0,0 +1,5 @@
+FactoryGirl.define do
+  factory :ppx, :class => Spree::BillingIntegration::PaypalExpress, :parent => :payment_method do
+    name 'Paypal'
+  end
+end

spec/models/billing_integration/paypal_express_base_spec.rb

@@ -0,0 +1,135 @@
+require 'spec_helper'
+
+describe Spree::BillingIntegration::PaypalExpressBase do
+  let(:order) do
+    order = Spree::Order.new(:bill_address => Spree::Address.new,
+                             :ship_address => Spree::Address.new)
+  end
+
+  let(:gateway) do
+    gateway = Spree::BillingIntegration::PaypalExpressBase.new({:environment => 'test', :active => true, :preferred_currency => "EUR"}, :without_protection => true)
+    gateway.stub :source_required => true
+    gateway.stub :provider => mock('paypal provider')
+    gateway.stub :find_authorization => mock('authorization', :params => authorization_params)
+    gateway
+  end
+
+  let(:authorization_params) { {'transaction_id' => '123'} }
+  let(:provider) { gateway.provider }
+
+  let(:account) do
+    mock_model(Spree::PaypalAccount)
+  end
+
+  let(:payment) do
+    payment = Spree::Payment.new
+    payment.source = account
+    payment.order = order
+    payment.payment_method = gateway
+    payment.amount = 10.0
+    payment
+  end
+
+  let(:amount_in_cents) { payment.amount.to_f * 100 }
+
+  let!(:success_response) do
+    mock('success_response', :success? => true,
+                             :authorization => '123',
+                             :avs_result => { 'code' => 'avs-code' })
+  end
+
+  let(:failed_response) { mock('gateway_response', :success? => false) }
+
+  before(:each) do
+    # So it doesn't create log entries every time a processing method is called
+    payment.log_entries.stub(:create)
+  end
+
+  describe "#capture" do
+    before { payment.state = 'pending' }
+
+    context "when payment_profiles_supported = true" do
+      before { gateway.stub :payment_profiles_supported? => true }
+
+       context "if sucessful" do
+         before do
+           provider.should_receive(:capture).with(amount_in_cents, '123', :currency => 'EUR').and_return(success_response)
+         end
+
+         it "should store the response_code" do
+           payment.capture!
+           payment.response_code.should == '123'
+         end
+       end
+
+       context "if unsucessful" do
+         before do
+           gateway.should_receive(:capture).with(payment, account, anything).and_return(failed_response)
+         end
+
+         it "should not make payment complete" do
+           lambda { payment.capture! }.should raise_error(Spree::Core::GatewayError)
+           payment.state.should == "failed"
+         end
+       end
+     end
+
+     context "when payment_profiles_supported = false" do
+       before do
+         payment.stub :response_code => '123'
+         gateway.stub :payment_profiles_supported? => false
+       end
+
+       context "if sucessful" do
+         before do
+           provider.should_receive(:capture).with(amount_in_cents, '123', anything).and_return(success_response)
+         end
+
+         it "should store the response_code" do
+           payment.capture!
+           payment.response_code.should == '123'
+         end
+       end
+
+       context "if unsucessful" do
+         before do
+           provider.should_receive(:capture).with(amount_in_cents, '123', anything).and_return(failed_response)
+         end
+
+         it "should not make payment complete" do
+           lambda { payment.capture! }.should raise_error(Spree::Core::GatewayError)
+           payment.state.should == "failed"
+         end
+       end
+
+     end
+  end
+
+  describe "#credit" do
+    before { payment.stub :response_code => '123' }
+    context "when payment_profiles_supported = true" do
+      before { gateway.stub :payment_profiles_supported? => true }
+
+
+      it "should receive correct params" do
+        provider.should_receive(:credit).with(1000, '123', :currency => 'EUR').and_return(success_response)
+        payment.credit!(10.0)
+        payment.response_code.should == '123'
+      end
+    end
+
+    context "when payment_profiles_supported = false" do
+      before do
+        payment.stub :response_code => '123'
+        gateway.stub :payment_profiles_supported? => false
+      end
+
+      it "should receive correct params" do
+        provider.should_receive(:credit).with(amount_in_cents, '123', :currency => 'EUR').and_return(success_response)
+        payment.credit!(10.0)
+        payment.response_code.should == '123'
+      end
+
+    end
+  end
+end

spec/requests/paypal_express_spec.rb

@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+feature "paypal express" do
+  background do
+    PAYMENT_STATES = Spree::Payment.state_machine.states.keys unless defined? PAYMENT_STATES
+    SHIPMENT_STATES = Spree::Shipment.state_machine.states.keys unless defined? SHIPMENT_STATES
+    ORDER_STATES = Spree::Order.state_machine.states.keys unless defined? ORDER_STATES
+    FactoryGirl.create(:shipping_method, :zone => Spree::Zone.find_by_name('North America'))
+    FactoryGirl.create(:payment_method, :environment => 'test')
+    @product = FactoryGirl.create(:product, :name => "RoR Mug")
+    sign_in_as! FactoryGirl.create(:user)
+
+    Factory(:ppx)
+  end
+
+  let!(:address) { FactoryGirl.create(:address, :state => Spree::State.first) }
+
+  scenario "can use paypal confirm", :js => true do
+    visit spree.product_path(@product)
+
+    click_button "Add To Cart"
+    click_link "Checkout"
+
+    str_addr = "bill_address"
+    select "United States", :from => "order_#{str_addr}_attributes_country_id"
+    ['firstname', 'lastname', 'address1', 'city', 'zipcode', 'phone'].each do |field|
+      fill_in "order_#{str_addr}_attributes_#{field}", :with => "#{address.send(field)}"
+    end
+
+    select "#{address.state.name}", :from => "order_#{str_addr}_attributes_state_id"
+    check "order_use_billing"
+    click_button "Save and Continue"
+
+    pending
+    choose "Paypal"
+    click_button "Save and Continue"
+  end
+end

spec/spec_helper.rb

@@ -18,6 +18,8 @@ Dir["#{File.dirname(__FILE__)}/factories/**/*.rb"].each do |f|
   require fp
 end
 
+require 'ffaker'
+
 RSpec.configure do |config|
   # == Mock Framework
   #

spec/support/authentication_helpers.rb

@@ -0,0 +1,13 @@
+module AuthenticationHelpers
+  def sign_in_as!(user)
+    visit '/login'
+    fill_in 'Email', :with => user.email
+    fill_in 'Password', :with => 'secret'
+    click_button 'Login'
+  end
+
+end
+
+RSpec.configure do |c|
+  c.include AuthenticationHelpers, :type => :request
+end

spec/support/shared_connection.rb

@@ -0,0 +1,12 @@
+class ActiveRecord::Base
+  mattr_accessor :shared_connection
+  @@shared_connection = nil
+
+  def self.connection
+    @@shared_connection || retrieve_connection
+  end
+end
+
+# Forces all threads to share the same connection. This works on
+# Capybara because it starts the web server in a thread.
+ActiveRecord::Base.shared_connection = ActiveRecord::Base.connection

spree_paypal_express.gemspec

@@ -14,5 +14,16 @@ Gem::Specification.new do |s|
   s.has_rdoc      = false
 
   s.add_dependency('spree_core', '>=1.0.0')
-  s.add_development_dependency('rspec-rails')
+  s.add_dependency('spree_auth', '>=1.0.0')
+  s.add_development_dependency 'capybara', '1.0.1'
+  s.add_development_dependency 'ffaker'
+  s.add_development_dependency 'rspec-rails',  '~> 2.9'
+  s.add_development_dependency 'sqlite3'
+  s.add_development_dependency 'factory_girl_rails', '~> 1.5.0'
+  s.add_development_dependency 'launchy'
+  s.add_development_dependency 'debugger'
+  s.add_development_dependency 'sass-rails'
+  s.add_development_dependency 'coffee-rails'
+  s.add_development_dependency 'spree_sample', "~> 1.1.0"
+  s.add_development_dependency 'debugger'
 end