From 99a3eabee8759c45babcaee107ca64b9f8db78fe Mon Sep 17 00:00:00 2001 From: Frederic Jean Date: Wed, 1 Oct 2014 11:40:09 -0600 Subject: [PATCH] Handling addresses between a Spree store and Paypal. The goal of this is to give a store operator the tools needed to be able to place seller protection elibible payments against Paypal. The changes are: * Creating a no_shipping preference to give the store developer a say in whether to send the shipping address. (#113) * Adds the ability to send a shipping address to Paypal (see #113) * Adds the option of displaying the shipping address on the Paypal pages. * Upgrades the Paypal SDK gem * Adds the ability to request a confirmed shipping address from Paypal. * Extracts the confirmed address from the Paypal express checkout details when we are configured to require a confirmed shipping address. --- README.md | 45 +++- app/controllers/spree/paypal_controller.rb | 49 ++-- app/models/spree/gateway/pay_pal_express.rb | 58 +++++ app/models/spree/paypal_express_checkout.rb | 4 +- ...6155949_add_address_to_express_checkout.rb | 5 + spec/features/paypal_spec.rb | 244 ++++++++++++++++-- spec/models/pay_pal_express_spec.rb | 14 +- spree_paypal_express.gemspec | 2 +- 8 files changed, 361 insertions(+), 60 deletions(-) create mode 100644 db/migrate/20141006155949_add_address_to_express_checkout.rb diff --git a/README.md b/README.md index 4ea0faea..654ca49d 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ You will also need a "Personal" account to test the transactions on your site. C #### Spree Setup -In Spree, go to the admin backend, click "Configuration" and then "Payment Methods" and create a new payment method. Select "Spree::Gateway::PayPalExpress" as the provider, and click "Create". Enter the email address, password and signature from the "API Credentials" tab for the **Business** account on PayPal. +In Spree, go to the admin backend, click "Configuration" and then "Payment Methods" and create a new payment method. Select "Spree::Gateway::PayPalExpress" as the provider, and click "Create". Enter the email address, password and signature from the "API Credentials" tab for the **Business** account on PayPal. ### Production setup @@ -56,7 +56,7 @@ This Spree extension supports *some* of those. If your favourite is not here, th ### Solution Type -Determines whether or not a user needs a PayPal account to check out. +Determines whether or not a user needs a PayPal account to check out. ```ruby payment_method.preferred_solution_type = "Mark" @@ -64,7 +64,7 @@ payment_method.preferred_solution_type = "Mark" payment_method.preferred_solution_type = "Sole" ``` -"Mark" if you do want users to have a paypal account, "Sole" otherwise. +"Mark" if you do want users to have a paypal account, "Sole" otherwise. ### Landing Page @@ -88,6 +88,45 @@ payment_method.preferred_logourl = 'http://yoursite.com/images/checkout.jpg' **Must** be an absolute path to the image. +### Displaying Shipping Addresses + +You have the option of displaying the shipping address of an order on +the PayPal pages: + +```ruby +# Displays the shipping address of the order on the Paypal page +payment_method.preferred_no_shipping = '0' + +# Do not display the shipping address on the Paypal page +# This is the default configuration since this matches the pre-existing +# behavior of the gem +payment_method.preferred_no_shipping = '1' + +# Display the shipping address listed in the profile if it is not +# sent along with the order +payment_method.preferred_no_shipping = '2' +``` + +This has no effect on what shipping address is passed to the gateway. + +### Overriding the Shipping Address + +By default PayPay will use the shipping address that it has on file as +the shipping address for an order. + +You can configure the gateway to send up the shipping address associated +with the order through the following configuration: + +```ruby +# Do not override the shipping address on file (default) +payment_method.preferred_address_override = '0' + +# Override the shipping address on file +payment_method.preferred_address_override = '1' +``` + +The shipping address will be sent to the server if configured to do so. + ## Caveats *Caveat venditor* diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index aef1d921..46a85355 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -87,17 +87,23 @@ def line_item(item) end def express_checkout_request_details order, items - { :SetExpressCheckoutRequestDetails => { - :InvoiceID => order.number, - :BuyerEmail => order.email, - :ReturnURL => confirm_paypal_url(:payment_method_id => params[:payment_method_id], :utm_nooverride => 1), - :CancelURL => cancel_paypal_url, - :SolutionType => payment_method.preferred_solution.present? ? payment_method.preferred_solution : "Mark", - :LandingPage => payment_method.preferred_landing_page.present? ? payment_method.preferred_landing_page : "Billing", - :cppheaderimage => payment_method.preferred_logourl.present? ? payment_method.preferred_logourl : "", - :NoShipping => 1, - :PaymentDetails => [payment_details(items)] - }} + request_details = { + :InvoiceID => order.number, + :BuyerEmail => order.email, + :ReturnURL => confirm_paypal_url(:payment_method_id => params[:payment_method_id], :utm_nooverride => 1), + :CancelURL => cancel_paypal_url, + :SolutionType => payment_method.preferred_solution.present? ? payment_method.preferred_solution : "Mark", + :LandingPage => payment_method.preferred_landing_page.present? ? payment_method.preferred_landing_page : "Billing", + :cppheaderimage => payment_method.preferred_logourl.present? ? payment_method.preferred_logourl : "", + :NoShipping => payment_method.preferred_no_shipping.present? ? payment_method.preferred_no_shipping : '1', + :PaymentDetails => [payment_details(items)] + } + + # Optional fields without set defaults. + request_details[:AddressOverride] = payment_method.preferred_address_override if payment_method.preferred_address_override.present? + request_details[:ReqConfirmShipping] = payment_method.preferred_req_confirmed_address if payment_method.preferred_req_confirmed_address.present? + + { :SetExpressCheckoutRequestDetails => request_details } end def payment_method @@ -156,15 +162,17 @@ def payment_details items def address_options return {} unless address_required? + address = current_order.use_billing ? current_order.bill_address : current_order.ship_address + { - :Name => current_order.bill_address.try(:full_name), - :Street1 => current_order.bill_address.address1, - :Street2 => current_order.bill_address.address2, - :CityName => current_order.bill_address.city, - :Phone => current_order.bill_address.phone, - :StateOrProvince => current_order.bill_address.state_text, - :Country => current_order.bill_address.country.iso, - :PostalCode => current_order.bill_address.zipcode + :Name => address.try(:full_name), + :Street1 => address.address1, + :Street2 => address.address2, + :CityName => address.city, + :Phone => address.phone, + :StateOrProvince => address.state_text, + :Country => address.country.iso, + :PostalCode => address.zipcode } end @@ -173,7 +181,8 @@ def completion_route(order) end def address_required? - payment_method.preferred_solution.eql?('Sole') + payment_method.preferred_solution.eql?('Sole') \ + || payment_method.preferred_address_override.eql?('1') end end end diff --git a/app/models/spree/gateway/pay_pal_express.rb b/app/models/spree/gateway/pay_pal_express.rb index f9086cb6..3d504e6a 100644 --- a/app/models/spree/gateway/pay_pal_express.rb +++ b/app/models/spree/gateway/pay_pal_express.rb @@ -8,6 +8,28 @@ class Gateway::PayPalExpress < Gateway preference :solution, :string, default: 'Mark' preference :landing_page, :string, default: 'Billing' preference :logourl, :string, default: '' + # Indicates whether to display the shipping address on the paypal checkout + # page. + # + # 0 - Paypal displays the shipping address on the page. + # 1 - Paypal does not display the shipping address on its pages. This is + # the default due to the history of this gem. + # 2 - Paypal will obtain the shipping address from the profile. + preference :no_shipping, :string, default: '1' + + # Allow Address Override + # 0 - Do not override the address stored at PayPal + # 1 - Override the address stored at Paypal + # By default, the shipping address is not overriden on Paypal's site + preference :address_override, :string + + # Whether to require a confirmed address + # 0 - Do not require a confirmed address + # 1 - Require a confirmed address + # + # Paypal recommends that you do not override the address if you are + # requiring a confirmed address for this order. + preference :req_confirmed_address, :string def supports?(source) true @@ -55,6 +77,14 @@ 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) + + # We need to get a hold of the confirmed address + address = extract_address(pp_details_response) + + if address && address.valid? + express_checkout.update_column(:address_id, address.id) + end + # This is rather hackish, required for payment/processing handle_response code. Class.new do def success?; true; end @@ -99,6 +129,34 @@ def refund(payment, amount) end refund_transaction_response end + + private + + def extract_address(response) + return nil unless self.preferred_req_confirmed_address == '1' + + express_checkout_details = response.get_express_checkout_details_response_details + payment_details = express_checkout_details.PaymentDetails.first + payer_info = express_checkout_details.PayerInfo + ship_to_address = payment_details.ShipToAddress + + return nil unless ship_to_address && payer_info + + if state = Spree::State.find_by_abbr(ship_to_address.state_or_province) + Spree::Address.create( + firstname: payer_info.payer_name.first_name, + last_name: payer_info.payer_name.last_name, + address1: ship_to_address.street1, + address2: ship_to_address.street2, + city: ship_to_address.city_name, + state_id: state.id, + state_name: state.name, + country_id: state.country.id, + zipcode: ship_to_address.postal_code, + phone: ship_to_address.phone || "n/a" + ) + end + end end end diff --git a/app/models/spree/paypal_express_checkout.rb b/app/models/spree/paypal_express_checkout.rb index a29d2614..b40ee5c0 100644 --- a/app/models/spree/paypal_express_checkout.rb +++ b/app/models/spree/paypal_express_checkout.rb @@ -1,4 +1,6 @@ module Spree class PaypalExpressCheckout < ActiveRecord::Base + belongs_to :address, class_name: "Spree::Address" + alias_attribute :confirmed_address, :address end -end \ No newline at end of file +end diff --git a/db/migrate/20141006155949_add_address_to_express_checkout.rb b/db/migrate/20141006155949_add_address_to_express_checkout.rb new file mode 100644 index 00000000..8b3169ff --- /dev/null +++ b/db/migrate/20141006155949_add_address_to_express_checkout.rb @@ -0,0 +1,5 @@ +class AddAddressToExpressCheckout < ActiveRecord::Migration + def change + add_reference :spree_paypal_express_checkouts, :address + end +end diff --git a/spec/features/paypal_spec.rb b/spec/features/paypal_spec.rb index 78540457..636965a2 100644 --- a/spec/features/paypal_spec.rb +++ b/spec/features/paypal_spec.rb @@ -13,6 +13,7 @@ }) FactoryGirl.create(:shipping_method) end + def fill_in_billing within("#billing") do fill_in "First Name", :with => "Test" @@ -27,6 +28,21 @@ def fill_in_billing end end + def fill_in_shipping + uncheck("order[use_billing]") + within("#shipping") do + fill_in "First Name", :with => "Test" + fill_in "Last Name", :with => "User" + fill_in "Street Address", :with => "2 User Lane" + # City, State and ZIP must all match for PayPal to be happy + fill_in "City", :with => "Adamsville" + select "United States of America", :from => "order_ship_address_attributes_country_id" + select "Alabama", :from => "order_ship_address_attributes_state_id" + fill_in "Zip", :with => "35005" + fill_in "Phone", :with => "555-123-4567" + end + end + def switch_to_paypal_login # If you go through a payment once in the sandbox, it remembers your preferred setting. # It defaults to the *wrong* setting for the first time, so we need to have this method. @@ -48,10 +64,14 @@ def within_transaction_cart(&block) within(".transctionCartDetails") { block.call } end - it "pays for an order successfully" do + def add_product_to_cart(product) visit spree.root_path - click_link 'iPad' + click_link product click_button 'Add To Cart' + end + + it "pays for an order successfully" do + add_product_to_cart 'iPad' click_button 'Checkout' within("#guest_checkout") do fill_in "Email", :with => "test@example.com" @@ -76,9 +96,7 @@ def within_transaction_cart(&block) end it "passes user details to PayPal" do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') click_button 'Checkout' within("#guest_checkout") do fill_in "Email", :with => "test@example.com" @@ -102,9 +120,7 @@ def within_transaction_cart(&block) end it "includes adjustments in PayPal summary" do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') # TODO: Is there a better way to find this current order? order = Spree::Order.last order.adjustments.create!(:amount => -5, :label => "$5 off") @@ -154,10 +170,7 @@ def within_transaction_cart(&block) end it "includes line item adjustments in PayPal summary" do - - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') # TODO: Is there a better way to find this current order? order = Spree::Order.last order.line_item_adjustments.count.should == 1 @@ -190,18 +203,14 @@ def within_transaction_cart(&block) end end + # Regression test for #10 context "will skip $0 items" do let!(:product2) { FactoryGirl.create(:product, :name => 'iPod') } specify do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' - - visit spree.root_path - click_link 'iPod' - click_button 'Add To Cart' + add_product_to_cart('iPad') + add_product_to_cart('iPod') # TODO: Is there a better way to find this current order? order = Spree::Order.last @@ -247,9 +256,7 @@ def within_transaction_cart(&block) end specify do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') # TODO: Is there a better way to find this current order? order = Spree::Order.last order.adjustments.create!(:amount => -order.line_items.last.price, :label => "FREE iPad ZOMG!") @@ -274,6 +281,191 @@ def within_transaction_cart(&block) end end + shared_examples_for :no_shipping do + it "displays the shipping address on file on the paypal page" do + add_product_to_cart('iPad') + click_button 'Checkout' + within('#guest_checkout') do + fill_in "Email", with: "test@example.com" + click_button 'Continue' + end + fill_in_billing + fill_in_shipping + + click_button "Save and Continue" + # Delivery step doesn't require any action + click_button "Save and Continue" + + find("#paypal_button").click + + login_to_paypal + + within("#shippingAddress") do + page.should have_content("Ship to") + end + + click_button "Pay Now" + + page.should have_content("Your order has been processed successfully") + end + end + + context "displays the shipping address on the paypal page" do + before do + @gateway.preferred_no_shipping = '0' + @gateway.save + end + + it_behaves_like :no_shipping + end + + shared_examples_for :no_shipping_displayed do + it "does not show the address by default" do + add_product_to_cart('iPad') + click_button 'Checkout' + within('#guest_checkout') do + fill_in "Email", with: "test@example.com" + click_button 'Continue' + end + fill_in_billing + fill_in_shipping + + click_button "Save and Continue" + # Delivery step doesn't require any action + click_button "Save and Continue" + + find("#paypal_button").click + + login_to_paypal + + page.should have_no_content("Ship To") + + click_button "Pay Now" + + page.should have_content("Your order has been processed successfully") + end + end + + context "requiring confirmed shipping address" do + before do + @gateway.preferred_req_confirmed_address = '1' + @gateway.save + end + + it_behaves_like :no_shipping_displayed + + it "overrides the shipping address on the order with the confirmed one" do + maryland = FactoryGirl.create(:state, name: "Maryland", abbr: "MD") + + add_product_to_cart('iPad') + click_button 'Checkout' + within('#guest_checkout') do + fill_in "Email", with: "test@example.com" + click_button 'Continue' + end + fill_in_billing + fill_in_shipping + + click_button "Save and Continue" + # Delivery step doesn't require any action + click_button "Save and Continue" + + find("#paypal_button").click + + login_to_paypal + + page.should have_no_content("Ship To") + + click_button "Pay Now" + + page.should have_content("Your order has been processed successfully") + + order = Spree::Order.last + express_checkout = order.payments.last.source + + address = express_checkout.address + address.should_not be_nil + + address.address1.should eq("Suite 510") + address.address2.should eq("7735 Old Georgetown Road") + address.city.should eq("Bethesda") + address.state.should eq(maryland) + end + end + + context "displays the shipping address on the paypal page when none is passed" do + before do + @gateway.preferred_no_shipping = '2' + @gateway.save + end + + it_behaves_like :no_shipping + end + + context "default no shipping option" do + it_behaves_like :no_shipping_displayed + end + + context "shipping address override" do + before do + @gateway.preferred_no_shipping = '0' + @gateway.preferred_address_override = '1' + @gateway.save + end + + it "shipping address from order" do + add_product_to_cart('iPad') + click_button 'Checkout' + within('#guest_checkout') do + fill_in "Email", with: "test@example.com" + click_button 'Continue' + end + fill_in_billing + fill_in_shipping + + click_button "Save and Continue" + # Delivery step doesn't require any action + click_button "Save and Continue" + + find("#paypal_button").click + + login_to_paypal + + within("#shippingAddress") do + page.should have_content("2 User Lane") + end + click_button "Pay Now" + + page.should have_content("Your order has been processed successfully") + end + + it "billing address from order" do + add_product_to_cart('iPad') + click_button 'Checkout' + within('#guest_checkout') do + fill_in "Email", with: "test@example.com" + click_button 'Continue' + end + fill_in_billing + + click_button "Save and Continue" + # Delivery step doesn't require any action + click_button "Save and Continue" + + find("#paypal_button").click + + login_to_paypal + + within("#shippingAddress") do + page.should have_content("1 User Lane") + end + + click_button "Pay Now" + + page.should have_content("Your order has been processed successfully") + end + end + context "cannot process a payment with invalid gateway details" do before do @gateway.preferred_login = nil @@ -281,9 +473,7 @@ def within_transaction_cart(&block) end specify do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') click_button 'Checkout' within("#guest_checkout") do fill_in "Email", :with => "test@example.com" @@ -303,9 +493,7 @@ def within_transaction_cart(&block) context "refunding payments" do before do - visit spree.root_path - click_link 'iPad' - click_button 'Add To Cart' + add_product_to_cart('iPad') click_button 'Checkout' within("#guest_checkout") do fill_in "Email", :with => "test@example.com" diff --git a/spec/models/pay_pal_express_spec.rb b/spec/models/pay_pal_express_spec.rb index 761bbe26..d94757e1 100644 --- a/spec/models/pay_pal_express_spec.rb +++ b/spec/models/pay_pal_express_spec.rb @@ -22,12 +22,12 @@ }).and_return(pp_details_request = double) pp_details_response = double(:get_express_checkout_details_response_details => - double(:PaymentDetails => { - :OrderTotal => { - :currencyID => "USD", - :value => "10.00" - } - })) + double(:PaymentDetails => { + :OrderTotal => { + :currencyID => "USD", + :value => "10.00" + }, + })) provider.should_receive(:get_express_checkout_details). with(pp_details_request). @@ -53,7 +53,7 @@ # Test for #4 it "fails" do - response = double('pp_response', :success? => false, + response = double('pp_response', :success? => false, :errors => [double('pp_response_error', :long_message => "An error goes here.")]) provider.should_receive(:do_express_checkout_payment).and_return(response) lambda { payment.purchase! }.should raise_error(Spree::Core::GatewayError, "An error goes here.") diff --git a/spree_paypal_express.gemspec b/spree_paypal_express.gemspec index e3e25233..12b31c92 100644 --- a/spree_paypal_express.gemspec +++ b/spree_paypal_express.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |s| s.requirements << 'none' s.add_dependency 'spree_core', '~> 2.4.0.beta' - s.add_dependency 'paypal-sdk-merchant', '1.106.1' + s.add_dependency 'paypal-sdk-merchant', '1.116.0' s.add_development_dependency 'capybara', '~> 2.1' s.add_development_dependency 'coffee-rails'