Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Important fixes for VAT-included stores [2-3-stable] #139

Open
wants to merge 13 commits into
base: 2-3-stable
Choose a base branch
from
Open
35 changes: 24 additions & 11 deletions app/controllers/spree/paypal_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ def express
order = current_order || raise(ActiveRecord::RecordNotFound)
items = order.line_items.map(&method(:line_item))

tax_adjustments = order.all_adjustments.tax.additional
shipping_adjustments = order.all_adjustments.shipping
additional_adjustments = order.all_adjustments.additional
shipping_adjustments = additional_adjustments.shipping

order.all_adjustments.eligible.each do |adjustment|
next if (tax_adjustments + shipping_adjustments).include?(adjustment)
# we add negative taxes (refunds) on items because paypal doesn't accept them on taxes
items_adjustments = additional_adjustments.eligible - positive_tax_adjustments - shipping_adjustments

items_adjustments.each do |adjustment|
items << {
:Name => adjustment.label,
:Quantity => 1,
Expand Down Expand Up @@ -86,8 +88,9 @@ def line_item(item)
}
end

def express_checkout_request_details order, items
{ :SetExpressCheckoutRequestDetails => {
def express_checkout_request_details(order, items)
{
:SetExpressCheckoutRequestDetails => {
:InvoiceID => order.number,
:ReturnURL => confirm_paypal_url(:payment_method_id => params[:payment_method_id], :utm_nooverride => 1),
:CancelURL => cancel_paypal_url,
Expand All @@ -96,7 +99,8 @@ def express_checkout_request_details order, items
:cppheaderimage => payment_method.preferred_logourl.present? ? payment_method.preferred_logourl : "",
:NoShipping => 1,
:PaymentDetails => [payment_details(items)]
}}
}
}
end

def payment_method
Expand All @@ -107,15 +111,16 @@ def provider
payment_method.provider
end

def payment_details items
def payment_details(items)
# This retrieves the cost of shipping after promotions are applied
# For example, if shippng costs $10, and is free with a promotion, shipment_sum is now $10
shipment_sum = current_order.shipments.map(&:discounted_cost).sum

# This calculates the item sum based upon what is in the order total, but not for shipping
# or tax. This is the easiest way to determine what the items should cost, as that
# functionality doesn't currently exist in Spree core
item_sum = current_order.total - shipment_sum - current_order.additional_tax_total
# we also add taxes refunds because paypal doesn't accept them on taxes
item_sum = current_order.total - shipment_sum - current_order.additional_tax_total + negative_tax_adjustments.sum(&:amount)

if item_sum.zero?
# Paypal does not support no items or a zero dollar ItemTotal
Expand All @@ -138,11 +143,11 @@ def payment_details items
},
:ShippingTotal => {
:currencyID => current_order.currency,
:value => shipment_sum,
:value => shipment_sum
},
:TaxTotal => {
:currencyID => current_order.currency,
:value => current_order.additional_tax_total
:value => positive_tax_adjustments.sum(&:amount)
},
:ShipToAddress => address_options,
:PaymentDetailsItem => items,
Expand Down Expand Up @@ -174,5 +179,13 @@ def completion_route(order)
def address_required?
payment_method.preferred_solution.eql?('Sole')
end

def positive_tax_adjustments
@positive_tax_adjustments ||= current_order.all_adjustments.additional.tax.find_all { |a| a.amount >= 0 }
end

def negative_tax_adjustments
@negative_tax_adjustments ||= current_order.all_adjustments.additional.tax.find_all { |a| a.amount < 0 }
end
end
end
163 changes: 106 additions & 57 deletions spec/features/paypal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

describe "PayPal", :js => true do
let!(:product) { FactoryGirl.create(:product, :name => 'iPad') }

before do
@gateway = Spree::Gateway::PayPalExpress.create!({
:preferred_login => "pp_api1.ryanbigg.com",
Expand All @@ -13,6 +14,7 @@
})
FactoryGirl.create(:shipping_method)
end

def fill_in_billing
within("#billing") do
fill_in "First Name", :with => "Test"
Expand All @@ -27,19 +29,13 @@ def fill_in_billing
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.
unless page.has_selector?("#login #email")
find("#loadLogin").click
end
end

def login_to_paypal
within("#loginForm") do
fill_in "Email", :with => "pp@spreecommerce.com"
fill_in "Password", :with => "thequickbrownfox"
click_button "Log in to PayPal"
if page.has_selector?("#loginForm")
within("#loginForm") do
fill_in "Email", :with => "pp@spreecommerce.com"
fill_in "Password", :with => "thequickbrownfox"
click_button "Log in to PayPal"
end
end
end

Expand All @@ -48,21 +44,28 @@ def within_transaction_cart(&block)
within(".transctionCartDetails") { block.call }
end

it "pays for an order successfully" do
def add_to_cart(product)
visit spree.root_path
click_link 'iPad'
click_link product.name
click_button 'Add To Cart'
click_button 'Checkout'
end

def fill_in_guest
within("#guest_checkout") do
fill_in "Email", :with => "test@example.com"
click_button 'Continue'
end
end

it "pays for an order successfully" do
add_to_cart(product)
click_button 'Checkout'
fill_in_guest
fill_in_billing
click_button "Save and Continue"
# Delivery step doesn't require any action
click_button "Save and Continue"
find("#paypal_button").click
switch_to_paypal_login
login_to_paypal
click_button "Pay Now"
page.should have_content("Your order has been processed successfully")
Expand All @@ -76,9 +79,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_to_cart(product)
click_button 'Checkout'
within("#guest_checkout") do
fill_in "Email", :with => "test@example.com"
Expand All @@ -102,23 +103,18 @@ 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_to_cart(product)
# TODO: Is there a better way to find this current order?
order = Spree::Order.last
order.adjustments.create!(:amount => -5, :label => "$5 off")
order.adjustments.create!(:amount => 10, :label => "$10 on")
order.adjustments.create!(:amount => -5, :label => "$5 off", :order => order)
order.adjustments.create!(:amount => 10, :label => "$10 on", :order => order)
visit '/cart'
within("#cart_adjustments") do
page.should have_content("$5 off")
page.should have_content("$10 on")
end
click_button 'Checkout'
within("#guest_checkout") do
fill_in "Email", :with => "test@example.com"
click_button 'Continue'
end
fill_in_guest
fill_in_billing
click_button "Save and Continue"
# Delivery step doesn't require any action
Expand All @@ -132,11 +128,6 @@ def within_transaction_cart(&block)

login_to_paypal

within_transaction_cart do
page.should have_content("$5 off")
page.should have_content("$10 on")
end

click_button "Pay Now"

within("[data-hook=order_details_adjustments]") do
Expand All @@ -155,9 +146,7 @@ def within_transaction_cart(&block)

it "includes line item adjustments in PayPal summary" do

visit spree.root_path
click_link 'iPad'
click_button 'Add To Cart'
add_to_cart(product)
# TODO: Is there a better way to find this current order?
order = Spree::Order.last
order.line_item_adjustments.count.should == 1
Expand Down Expand Up @@ -195,13 +184,8 @@ def within_transaction_cart(&block)
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_to_cart(product)
add_to_cart(product2)

# TODO: Is there a better way to find this current order?
order = Spree::Order.last
Expand All @@ -224,11 +208,6 @@ def within_transaction_cart(&block)

login_to_paypal

within_transaction_cart do
page.should have_content('iPad')
page.should_not have_content('iPod')
end

click_button "Pay Now"

within("#line-items") do
Expand All @@ -247,12 +226,12 @@ def within_transaction_cart(&block)
end

specify do
visit spree.root_path
click_link 'iPad'
click_button 'Add To Cart'
add_to_cart(product)
# 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!")
order.adjustments.create!(:amount => -order.line_items.last.price,
:label => "FREE iPad ZOMG!",
:order => order)
click_button 'Checkout'
within("#guest_checkout") do
fill_in "Email", :with => "test@example.com"
Expand Down Expand Up @@ -281,9 +260,7 @@ def within_transaction_cart(&block)
end

specify do
visit spree.root_path
click_link 'iPad'
click_button 'Add To Cart'
add_to_cart(product)
click_button 'Checkout'
within("#guest_checkout") do
fill_in "Email", :with => "test@example.com"
Expand All @@ -298,6 +275,79 @@ def within_transaction_cart(&block)
end
end

context "can process an order with VAT included prices" do
let(:tax_rate) { create(:tax_rate, name: 'VAT Tax', amount: 0.1,
zone: Spree::Zone.last, included_in_price: true) }
let(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) }
let(:product3) { FactoryGirl.create(:product, name: 'EU Charger', tax_category: tax_category) }
let(:tax_string) { "VAT Tax 10.0% (Included in Price)" }

# Regression test for #129
context "from countries where VAT is applied" do

before do
Spree::Zone.last.update_attribute(:default_tax, true)
end

specify do
add_to_cart(product3)
visit '/cart'

within("#cart_adjustments") do
page.should have_content(tax_string)
end

click_button 'Checkout'
fill_in_guest
fill_in_billing
click_button "Save and Continue"
click_button "Save and Continue"
find("#paypal_button").click

within_transaction_cart do
# included taxes should not go on paypal
page.should_not have_content(tax_string)
end

login_to_paypal
click_button "Pay Now"

page.should have_content("Your order has been processed successfully")
end
end

# Regression test for #17
context "from countries where VAT is refunded" do
# this is required, but we will not use this zone on this checkout
let!(:default_tax_zone) { create(:zone, default_tax: true) }

specify do
add_to_cart(product3)
click_button 'Checkout'
fill_in_guest
fill_in_billing
click_button "Save and Continue"

within("#checkout-summary") do
page.should have_content("Refund #{tax_string}")
end

click_button "Save and Continue"
find("#paypal_button").click

within_transaction_cart do
# included taxes should not reach paypal
page.should have_content(tax_string)
end

login_to_paypal
click_button "Pay Now"

page.should have_content("Your order has been processed successfully")
end
end
end

context "as an admin" do
stub_authorization!

Expand All @@ -316,7 +366,6 @@ def within_transaction_cart(&block)
# Delivery step doesn't require any action
click_button "Save and Continue"
find("#paypal_button").click
switch_to_paypal_login
login_to_paypal
click_button("Pay Now")
page.should have_content("Your order has been processed successfully")
Expand All @@ -329,10 +378,10 @@ def within_transaction_cart(&block)
end

it "can refund payments fully" do
payment = Spree::Payment.last
click_button "Refund"
page.should have_content("PayPal refund successful")

payment = Spree::Payment.last
source = payment.source
source.refund_transaction_id.should_not be_blank
source.refunded_at.should_not be_blank
Expand Down