Confirm orders, credit orders #48

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

sbounmy commented Nov 20, 2013

Hi,

This PR adds 2 main feature missing from original spree_paypal_express :

  • credit orders using spree flow:
    screen shot 2013-11-20 at 9 13 48 pm
    payment method should have 'review' preference to true
  • confirm orders: radar#34
    if payment method review preference is true, we should be able to confirm orders on spree.
    it worked for 'Billing' landing page (credit card), not sure it did not work for 'Login' (paypal).

Specs are now green btw, should consider adding travis 🍺 !

@radar radar and 1 other commented on an outdated diff Nov 24, 2013

spree_paypal_express.gemspec
@@ -26,8 +26,10 @@ Gem::Specification.new do |s|
s.add_dependency 'paypal-sdk-merchant', '1.106.1'
s.add_development_dependency 'capybara', '~> 2.1'
+ s.add_development_dependency 'selenium-webdriver'
@radar

radar Nov 24, 2013

Contributor

Please remove this line from here. We are using Poltergeist instead.

@sbounmy

sbounmy Dec 3, 2013

Contributor

was not easy to use poltergeist to debug, putting it back

@radar radar commented on an outdated diff Nov 24, 2013

spree_paypal_express.gemspec
s.add_development_dependency 'coffee-rails'
s.add_development_dependency 'database_cleaner'
+ s.add_development_dependency 'debugger'
@radar

radar Nov 24, 2013

Contributor

Please remove this line from here. We are using pry instead.

@radar radar commented on an outdated diff Nov 24, 2013

spec/spec_helper.rb
@@ -22,14 +22,16 @@
require 'pry'
require 'capybara/rspec'
require 'capybara/rails'
-require 'capybara/poltergeist'
-
-Capybara.javascript_driver = :poltergeist
+# require 'capybara/poltergeist'
+require 'debugger'
@radar

radar Nov 24, 2013

Contributor

Please remove this line.

@radar radar commented on an outdated diff Nov 24, 2013

app/models/spree/gateway/pay_pal_express.rb
@@ -71,11 +59,7 @@ def purchase(amount, express_checkout, gateway_options={})
# This is mainly so we can use it later on to refund the payment if the user wishes.
transaction_id = pp_response.do_express_checkout_payment_response_details.payment_info.first.transaction_id
express_checkout.update_column(:transaction_id, transaction_id)
- # This is rather hackish, required for payment/processing handle_response code.
- Class.new do
- def success?; true; end
- def authorization; nil; end
- end.new
+ successfull_refund
@radar

radar Nov 24, 2013

Contributor

This method should be called successful_refund.

@radar radar commented on the diff Nov 24, 2013

app/models/spree/gateway/pay_pal_express.rb
attr_accessible :preferred_login, :preferred_password, :preferred_signature,
- :preferred_solution, :preferred_logourl, :preferred_landing_page
+ :preferred_solution, :preferred_logourl, :preferred_landing_page, :preferred_review
@radar

radar Nov 24, 2013

Contributor

Please add documentation for this to the README.

@radar radar commented on the diff Nov 24, 2013

app/models/spree/gateway/pay_pal_express.rb
@@ -86,6 +70,13 @@ def to_s
end
end
+ def payment_profiles_supported?
+ !!preferred_review
+ end
+
+ def create_profile(payment)
@radar

radar Nov 24, 2013

Contributor

Why does this method need to exist?

@sbounmy

sbounmy Dec 3, 2013

Contributor

according to http://guides.spreecommerce.com/developer/checkout.html#payment-profiles :
"More importantly, it allows us to have a final “confirmation” step before the order is processed..."

@radar radar commented on an outdated diff Nov 24, 2013

app/models/spree/gateway/pay_pal_express.rb
@@ -106,6 +97,40 @@ def refund(payment, amount)
end
refund_transaction_response
end
+
+ def credit(amount, source, response_code={}, options={})
+ amount /= 100 #was in cts
@radar

radar Nov 24, 2013

Contributor

Please use the full word here for "cents"

Contributor

radar commented Nov 24, 2013

Hey @sbounmy, thanks for the patch. Could you please fix up those things and let me know when they're addressed? Thanks!

@azinazadi azinazadi commented on the diff Nov 29, 2013

app/models/spree/gateway/pay_pal_express.rb
@@ -8,9 +8,10 @@ class Gateway::PayPalExpress < Gateway
preference :solution, :string, default: 'Mark'
preference :landing_page, :string, default: 'Billing'
preference :logourl, :string, default: ''
+ preference :review, :boolean, default: false
attr_accessible :preferred_login, :preferred_password, :preferred_signature,
@azinazadi

azinazadi Nov 29, 2013

Contributor

attr_accessible is deprecated on Rails 4, and this patch is not gonna work for spree 2

@azinazadi azinazadi commented on the diff Nov 29, 2013

config/routes.rb
@@ -1,5 +1,6 @@
Spree::Core::Engine.routes.append do
post '/paypal', :to => "paypal#express", :as => :paypal_express
+ put '/paypal', :to => "paypal#express", :as => :paypal_express
@azinazadi

azinazadi Nov 29, 2013

Contributor

two routes with the same name!

radar referenced this pull request Dec 3, 2013

Closed

Users can't confirm orders #34

Member

alepore commented Jul 6, 2015

closing old PRs

alepore closed this Jul 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment