From 9d2c8fda03c7510d83498d94846d3cabbb85491b Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Mon, 21 May 2012 23:01:52 +0200 Subject: [PATCH 01/16] update order on paypal checkout --- .../spree/checkout_controller_decorator.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 5858050..0df9919 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -178,18 +178,17 @@ 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 - + load_order 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 +198,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 +213,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 From 287eee48a1b5784ee785f506478dcf9bdb4f3b5e Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Tue, 22 May 2012 23:26:11 +0200 Subject: [PATCH 02/16] dirty fix store_credit_callback --- app/controllers/spree/checkout_controller_decorator.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 0df9919..59d5181 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -147,6 +147,8 @@ module Spree @order.update_attribute(:state, "complete") state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil + @order.consume_users_credit #since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits + @order.finalize! flash[:notice] = I18n.t(:order_processed_successfully) redirect_to completion_route From 63b2c21438b610ebffc259789c3f95072dfcc6a5 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 00:48:31 +0200 Subject: [PATCH 03/16] private method! --- app/controllers/spree/checkout_controller_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 59d5181..b9eac5a 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -147,7 +147,7 @@ module Spree @order.update_attribute(:state, "complete") state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil - @order.consume_users_credit #since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits + @order.send(:consume_users_credit) #since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits @order.finalize! flash[:notice] = I18n.t(:order_processed_successfully) From f544581f76a5fdc6b2407875638dd0925f4e841a Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 00:59:07 +0200 Subject: [PATCH 04/16] fixed namespace partial --- app/views/spree/shared/paypal_express_confirm.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index 6acd7ef..879fb42 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -3,7 +3,7 @@ <%= raw t("order_not_yet_placed") %>

-<%= render :partial => 'shared/order_details', :locals => {:order => @order} -%> +<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%>
<%= 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" %> From 06a81e1c64c91d46f087119c65229925b6d9dfe2 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 21:15:02 +0200 Subject: [PATCH 05/16] rendering view in controller specs improved paypal confirm template --- .../spree/checkout_controller_decorator.rb | 2 +- .../shared/paypal_express_confirm.html.erb | 23 +++++++++++++++---- spec/controllers/checkout_controller_spec.rb | 4 +++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index b9eac5a..4cb9807 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -147,7 +147,7 @@ module Spree @order.update_attribute(:state, "complete") state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil - @order.send(:consume_users_credit) #since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits + @order.send(:consume_users_credit) if @order.respond_to?(:consume_users_credit)#since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits @order.finalize! flash[:notice] = I18n.t(:order_processed_successfully) diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index 879fb42..d7839f9 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -3,8 +3,23 @@ <%= raw t("order_not_yet_placed") %>

-<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%> -
- <%= 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" %> + + +
+ <%= render :partial => 'spree/shared/error_messages', :locals => { :target => @order } %> + +
+

<%= t(:checkout) %>

+
<%= checkout_progress %>
+
+ +
+
+ <%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%> +
+ <%= 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" %> +
+
+
diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index e559811..12bcc17 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/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 From 7dfa8bf45a9973209a61b1d260d03d92391eb0a9 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 21:30:24 +0200 Subject: [PATCH 06/16] cleaned up dependencies removed cucumber --- Gemfile | 22 +--------------------- Rakefile | 2 -- lib/spree_paypal_express.rb | 1 + spec/requests/paypal_express_spec.rb | 0 spec/spec_helper.rb | 2 ++ spec/support/shared_connection.rb | 12 ++++++++++++ spree_paypal_express.gemspec | 13 ++++++++++++- 7 files changed, 28 insertions(+), 24 deletions(-) create mode 100644 spec/requests/paypal_express_spec.rb create mode 100644 spec/support/shared_connection.rb diff --git a/Gemfile b/Gemfile index b5223c1..8926307 100644 --- a/Gemfile +++ b/Gemfile @@ -1,23 +1,3 @@ source 'http://rubygems.org' -gemspec - -gem 'sqlite3-ruby', :require => 'sqlite3' -gem 'spree', :path => '../spree' - -group :test do - gem 'rspec-rails', '= 2.7.0' - gem 'factory_girl_rails', '= 1.3.0' - gem 'simplecov' - gem 'shoulda' - gem 'faker' -end - -group :cucumber do - gem 'cucumber-rails' - gem 'database_cleaner', '~> 0.5.2' - gem 'nokogiri' - gem 'capybara' - gem 'faker' - gem 'launchy' -end \ No newline at end of file +gemspec \ No newline at end of file diff --git a/Rakefile b/Rakefile index 4e970e4..29b6b92 100644 --- a/Rakefile +++ b/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 ] diff --git a/lib/spree_paypal_express.rb b/lib/spree_paypal_express.rb index e3dafc3..60822df 100644 --- a/lib/spree_paypal_express.rb +++ b/lib/spree_paypal_express.rb @@ -1,2 +1,3 @@ require 'spree_core' +require 'spree_auth' require 'spree_paypal_express/engine' diff --git a/spec/requests/paypal_express_spec.rb b/spec/requests/paypal_express_spec.rb new file mode 100644 index 0000000..e69de29 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b865e87..70631c3 100644 --- a/spec/spec_helper.rb +++ b/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 # diff --git a/spec/support/shared_connection.rb b/spec/support/shared_connection.rb new file mode 100644 index 0000000..7d8b40c --- /dev/null +++ b/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 \ No newline at end of file diff --git a/spree_paypal_express.gemspec b/spree_paypal_express.gemspec index d073314..56f145c 100644 --- a/spree_paypal_express.gemspec +++ b/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 From 4b8dbdbfeb78ccbaa071dba701518deedd8ecff0 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 22:33:17 +0200 Subject: [PATCH 07/16] start adding integration tests --- .../javascripts/admin/spree_paypal_express.js | 0 .../javascripts/store/spree_paypal_express.js | 0 .../admin/spree_paypal_express.css | 0 .../store/spree_paypal_express.css | 0 .../spree/checkout_controller_decorator.rb | 1 + .../shared/paypal_express_confirm.html.erb | 11 ++--- spec/controllers/checkout_controller_spec.rb | 2 +- spec/factories/ppx_factory.rb | 5 +++ spec/requests/paypal_express_spec.rb | 40 +++++++++++++++++++ spec/support/authentication_helpers.rb | 13 ++++++ 10 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/admin/spree_paypal_express.js create mode 100644 app/assets/javascripts/store/spree_paypal_express.js create mode 100644 app/assets/stylesheets/admin/spree_paypal_express.css create mode 100644 app/assets/stylesheets/store/spree_paypal_express.css create mode 100644 spec/factories/ppx_factory.rb create mode 100644 spec/support/authentication_helpers.rb diff --git a/app/assets/javascripts/admin/spree_paypal_express.js b/app/assets/javascripts/admin/spree_paypal_express.js new file mode 100644 index 0000000..e69de29 diff --git a/app/assets/javascripts/store/spree_paypal_express.js b/app/assets/javascripts/store/spree_paypal_express.js new file mode 100644 index 0000000..e69de29 diff --git a/app/assets/stylesheets/admin/spree_paypal_express.css b/app/assets/stylesheets/admin/spree_paypal_express.css new file mode 100644 index 0000000..e69de29 diff --git a/app/assets/stylesheets/store/spree_paypal_express.css b/app/assets/stylesheets/store/spree_paypal_express.css new file mode 100644 index 0000000..e69de29 diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 4cb9807..c8d20b8 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -90,6 +90,7 @@ module Spree @order.ship_address = order_ship_address @order.bill_address ||= order_ship_address end + @order.save if payment_method.preferred_review diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index d7839f9..5bcc1bc 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -1,10 +1,3 @@ -

<%= t("confirm") %>

-

- <%= raw t("order_not_yet_placed") %> -

- - -
<%= render :partial => 'spree/shared/error_messages', :locals => { :target => @order } %> @@ -13,6 +6,10 @@
<%= checkout_progress %>
+

+ <%= raw t("order_not_yet_placed") %> +

+
<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%> diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 12bcc17..16838d0 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -108,7 +108,7 @@ 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 == "payment" end end diff --git a/spec/factories/ppx_factory.rb b/spec/factories/ppx_factory.rb new file mode 100644 index 0000000..86a47d1 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/spec/requests/paypal_express_spec.rb b/spec/requests/paypal_express_spec.rb index e69de29..0dc20dd 100644 --- a/spec/requests/paypal_express_spec.rb +++ b/spec/requests/paypal_express_spec.rb @@ -0,0 +1,40 @@ +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 + save_and_open_page + + + select "#{address.state.name}", :from => "order_#{str_addr}_attributes_state_id" + check "order_use_billing" + click_button "Save and Continue" + + choose "Paypal" + pending + click_button "Save and Continue" + end +end \ No newline at end of file diff --git a/spec/support/authentication_helpers.rb b/spec/support/authentication_helpers.rb new file mode 100644 index 0000000..2de8eb3 --- /dev/null +++ b/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 From bf00623328aa14b6c8bfbfacfde137adea0d25aa Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 24 May 2012 23:02:15 +0200 Subject: [PATCH 08/16] fixed paypal_confirm order state --- .../spree/checkout_controller_decorator.rb | 3 ++- .../billing_integration/paypal_express.rb | 4 ++++ .../billing_integration/paypal_express_uk.rb | 3 +++ spec/controllers/checkout_controller_spec.rb | 24 +++++++++++++++---- spec/requests/paypal_express_spec.rb | 4 +--- 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index c8d20b8..c2b91d2 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -90,10 +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 diff --git a/app/models/spree/billing_integration/paypal_express.rb b/app/models/spree/billing_integration/paypal_express.rb index 9509b67..389e162 100644 --- a/app/models/spree/billing_integration/paypal_express.rb +++ b/app/models/spree/billing_integration/paypal_express.rb @@ -12,4 +12,8 @@ class Spree::BillingIntegration::PaypalExpress < Spree::BillingIntegration def provider_class ActiveMerchant::Billing::PaypalExpressGateway end + + def payment_profiles_supported? + !!preferred_review + end end diff --git a/app/models/spree/billing_integration/paypal_express_uk.rb b/app/models/spree/billing_integration/paypal_express_uk.rb index 7021b37..2c18503 100644 --- a/app/models/spree/billing_integration/paypal_express_uk.rb +++ b/app/models/spree/billing_integration/paypal_express_uk.rb @@ -13,4 +13,7 @@ class Spree::BillingIntegration::PaypalExpressUk < Spree::BillingIntegration ActiveMerchant::Billing::PaypalExpressGateway end + def payment_profiles_supported? + !!preferred_review + end end diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 16838d0..90d75a3 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -75,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 @@ -100,7 +103,10 @@ module Spree 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) @@ -108,7 +114,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 == "payment" + 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 @@ -116,7 +130,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' }) @@ -128,6 +143,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 diff --git a/spec/requests/paypal_express_spec.rb b/spec/requests/paypal_express_spec.rb index 0dc20dd..0d8c33c 100644 --- a/spec/requests/paypal_express_spec.rb +++ b/spec/requests/paypal_express_spec.rb @@ -26,15 +26,13 @@ feature "paypal express" do ['firstname', 'lastname', 'address1', 'city', 'zipcode', 'phone'].each do |field| fill_in "order_#{str_addr}_attributes_#{field}", :with => "#{address.send(field)}" end - save_and_open_page - select "#{address.state.name}", :from => "order_#{str_addr}_attributes_state_id" check "order_use_billing" click_button "Save and Continue" - choose "Paypal" pending + choose "Paypal" click_button "Save and Continue" end end \ No newline at end of file From b5322c97652463a9985189fcfe57024ef25290ff Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 25 May 2012 19:21:22 +0200 Subject: [PATCH 09/16] translation symlink --- app/views/spree/shared/paypal_express_confirm.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index 5bcc1bc..a4132db 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -7,14 +7,14 @@

- <%= raw t("order_not_yet_placed") %> + <%= raw t(:order_not_yet_placed) %>

<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%>
- <%= button_to t('place_order'), paypal_finish_order_checkout_url(@order, {:token => params[:token] , :PayerID => params[:PayerID], :payment_method_id => + <%= 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" %>
From 4ee142bc7f87df5641339272f1fa3d6c7c479775 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 25 May 2012 19:50:05 +0200 Subject: [PATCH 10/16] fixed css and respond_to? --- app/controllers/spree/checkout_controller_decorator.rb | 2 +- app/views/spree/shared/paypal_express_confirm.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index c2b91d2..48cad1c 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -149,7 +149,7 @@ module Spree @order.update_attribute(:state, "complete") state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil - @order.send(:consume_users_credit) if @order.respond_to?(:consume_users_credit)#since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits + @order.send(:consume_users_credit) if @order.respond_to?(:consume_users_credit, true)#since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits @order.finalize! flash[:notice] = I18n.t(:order_processed_successfully) diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index a4132db..9b3880c 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -11,7 +11,7 @@

-
+
<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%>
<%= button_to t(:place_order), paypal_finish_order_checkout_url(@order, {:token => params[:token] , :PayerID => params[:PayerID], :payment_method_id => From 84f0c159da61d48ace26f6536806a94ba2656466 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 25 May 2012 20:03:23 +0200 Subject: [PATCH 11/16] fixed margin --- app/views/spree/shared/paypal_express_confirm.html.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/spree/shared/paypal_express_confirm.html.erb b/app/views/spree/shared/paypal_express_confirm.html.erb index 9b3880c..7e0762d 100644 --- a/app/views/spree/shared/paypal_express_confirm.html.erb +++ b/app/views/spree/shared/paypal_express_confirm.html.erb @@ -11,8 +11,9 @@

-
+
<%= render :partial => 'spree/shared/order_details', :locals => {:order => @order} -%> +
<%= 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" %> From 4b1def8643a3ab49801debec6b9d52bf31ac8537 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 25 May 2012 20:44:20 +0200 Subject: [PATCH 12/16] fixed consume_users_credit call --- app/controllers/spree/checkout_controller_decorator.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 48cad1c..3db3054 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -149,7 +149,11 @@ module Spree @order.update_attribute(:state, "complete") state_callback(:after) # So that after_complete is called, setting session[:order_id] to nil - @order.send(:consume_users_credit) if @order.respond_to?(:consume_users_credit, true)#since we dont rely on state machine callback, we just explicitly call this method for spree_store_credits + + # 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) From 3d5af5bdafcfc8c2fb6ebcc3c94f972ed1d80726 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 25 May 2012 21:26:19 +0200 Subject: [PATCH 13/16] set completed_at --- app/controllers/spree/checkout_controller_decorator.rb | 3 ++- spec/controllers/checkout_controller_spec.rb | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/checkout_controller_decorator.rb b/app/controllers/spree/checkout_controller_decorator.rb index 3db3054..890429a 100644 --- a/app/controllers/spree/checkout_controller_decorator.rb +++ b/app/controllers/spree/checkout_controller_decorator.rb @@ -147,7 +147,8 @@ 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 diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 90d75a3..1c4fa22 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -97,6 +97,7 @@ 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 From 26da5770907c396aa8e9d16d5e7685311e3b7240 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Thu, 31 May 2012 08:54:05 +0200 Subject: [PATCH 14/16] using a base class for paypal express --- .../billing_integration/paypal_express.rb | 18 +----------------- .../paypal_express_base.rb | 19 +++++++++++++++++++ .../billing_integration/paypal_express_uk.rb | 18 +----------------- 3 files changed, 21 insertions(+), 34 deletions(-) create mode 100644 app/models/spree/billing_integration/paypal_express_base.rb diff --git a/app/models/spree/billing_integration/paypal_express.rb b/app/models/spree/billing_integration/paypal_express.rb index 389e162..572f19e 100644 --- a/app/models/spree/billing_integration/paypal_express.rb +++ b/app/models/spree/billing_integration/paypal_express.rb @@ -1,19 +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 - - def payment_profiles_supported? - !!preferred_review - end end diff --git a/app/models/spree/billing_integration/paypal_express_base.rb b/app/models/spree/billing_integration/paypal_express_base.rb new file mode 100644 index 0000000..4a81256 --- /dev/null +++ b/app/models/spree/billing_integration/paypal_express_base.rb @@ -0,0 +1,19 @@ +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 +end \ No newline at end of file diff --git a/app/models/spree/billing_integration/paypal_express_uk.rb b/app/models/spree/billing_integration/paypal_express_uk.rb index 2c18503..3dc9a79 100644 --- a/app/models/spree/billing_integration/paypal_express_uk.rb +++ b/app/models/spree/billing_integration/paypal_express_uk.rb @@ -1,19 +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 - - def payment_profiles_supported? - !!preferred_review - end end From b11a9dd9f2a7072b5299175e74ec05bf96a8d854 Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 1 Jun 2012 00:06:16 +0200 Subject: [PATCH 15/16] fixed capture and credit due to spree 1.1 changes cf payment processing --- .../paypal_express_base.rb | 42 +++++++ app/models/spree/paypal_account.rb | 72 +----------- .../paypal_express_base_spec.rb | 108 ++++++++++++++++++ 3 files changed, 151 insertions(+), 71 deletions(-) create mode 100644 spec/models/billing_integration/paypal_express_base_spec.rb diff --git a/app/models/spree/billing_integration/paypal_express_base.rb b/app/models/spree/billing_integration/paypal_express_base.rb index 4a81256..d694f4f 100644 --- a/app/models/spree/billing_integration/paypal_express_base.rb +++ b/app/models/spree/billing_integration/paypal_express_base.rb @@ -16,4 +16,46 @@ class Spree::BillingIntegration::PaypalExpressBase < Spree::BillingIntegration 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(amount, account, response_code, gateway_options) + 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 \ No newline at end of file diff --git a/app/models/spree/paypal_account.rb b/app/models/spree/paypal_account.rb index cc09d6a..8980ab1 100644 --- a/app/models/spree/paypal_account.rb +++ b/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 diff --git a/spec/models/billing_integration/paypal_express_base_spec.rb b/spec/models/billing_integration/paypal_express_base_spec.rb new file mode 100644 index 0000000..85aa463 --- /dev/null +++ b/spec/models/billing_integration/paypal_express_base_spec.rb @@ -0,0 +1,108 @@ +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 + +end From e11a8e4d5f0b4192d8adb6437fd38978eb4d2b7a Mon Sep 17 00:00:00 2001 From: Bounmy Stephane Date: Fri, 1 Jun 2012 00:36:42 +0200 Subject: [PATCH 16/16] fixed credit when no payment_profiles_supported --- .../paypal_express_base.rb | 5 ++-- .../paypal_express_base_spec.rb | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/app/models/spree/billing_integration/paypal_express_base.rb b/app/models/spree/billing_integration/paypal_express_base.rb index d694f4f..d2e9820 100644 --- a/app/models/spree/billing_integration/paypal_express_base.rb +++ b/app/models/spree/billing_integration/paypal_express_base.rb @@ -26,11 +26,12 @@ class Spree::BillingIntegration::PaypalExpressBase < Spree::BillingIntegration end end - def credit(amount, account, response_code, gateway_options) + 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| diff --git a/spec/models/billing_integration/paypal_express_base_spec.rb b/spec/models/billing_integration/paypal_express_base_spec.rb index 85aa463..a346839 100644 --- a/spec/models/billing_integration/paypal_express_base_spec.rb +++ b/spec/models/billing_integration/paypal_express_base_spec.rb @@ -105,4 +105,31 @@ describe Spree::BillingIntegration::PaypalExpressBase do 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