diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index f5ef6872e6..e139d53908 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -13,33 +13,9 @@ module Spree def express order = current_order || raise(ActiveRecord::RecordNotFound) - items = order.line_items.map(&method(:line_item)) - - tax_adjustments = order.adjustments.tax.additional - shipping_adjustments = order.adjustments.shipping - - order.adjustments.eligible.each do |adjustment| - next if (tax_adjustments + shipping_adjustments).include?(adjustment) - - items << { - Name: adjustment.label, - Quantity: 1, - Amount: { - currencyID: order.currency, - value: adjustment.amount - } - } - end - - # Because PayPal doesn't accept $0 items at all. - # See https://github.com/spree-contrib/better_spree_paypal_express/issues/10 - # "It can be a positive or negative value but not zero." - items.reject! do |item| - item[:Amount][:value].zero? - end pp_request = provider.build_set_express_checkout( - express_checkout_request_details(order, items) + express_checkout_request_details(order) ) begin @@ -98,20 +74,7 @@ module Spree private - def line_item(item) - { - Name: item.product.name, - Number: item.variant.sku, - Quantity: item.quantity, - Amount: { - currencyID: item.order.currency, - value: item.price - }, - ItemCategory: "Physical" - } - end - - def express_checkout_request_details(order, items) + def express_checkout_request_details(order) { SetExpressCheckoutRequestDetails: { InvoiceID: order.number, @@ -124,7 +87,7 @@ module Spree LandingPage: payment_method.preferred_landing_page.presence || "Billing", cppheaderimage: payment_method.preferred_logourl.presence || "", NoShipping: 1, - PaymentDetails: [payment_details(items)] + PaymentDetails: [payment_details(order)] } } end @@ -171,11 +134,11 @@ module Spree payment_method.provider end - def payment_details(items) - item_sum = items.sum { |i| i[:Quantity] * i[:Amount][:value] } + def payment_details(order) + items = PaypalItemsBuilder.new(order).call - tax_adjustments = current_order.adjustments.tax.additional - tax_adjustments_total = tax_adjustments.sum(:amount) + item_sum = items.sum { |i| i[:Quantity] * i[:Amount][:value] } + tax_adjustments_total = current_order.all_adjustments.tax.additional.sum(:amount) if item_sum.zero? # Paypal does not support no items or a zero dollar ItemTotal diff --git a/app/services/paypal_items_builder.rb b/app/services/paypal_items_builder.rb new file mode 100644 index 0000000000..b0497c3372 --- /dev/null +++ b/app/services/paypal_items_builder.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class PaypalItemsBuilder + def initialize(order) + @order = order + end + + def call + items = order.line_items.map(&method(:line_item_data)) + + relevant_adjustments.each do |adjustment| + items << adjustment_data(adjustment) + end + + # Because PayPal doesn't accept $0 items at all. + # See https://github.com/spree-contrib/better_spree_paypal_express/issues/10 + # "It can be a positive or negative value but not zero." + items.reject do |item| + item[:Amount][:value].zero? + end + end + + private + + attr_reader :order + + def relevant_adjustments + # Tax total and shipping costs are added separately, so they're not included here. + order.all_adjustments.eligible - order.all_adjustments.tax - order.all_adjustments.shipping + end + + def line_item_data(item) + { + Name: item.product.name, + Number: item.variant.sku, + Quantity: item.quantity, + Amount: { + currencyID: item.order.currency, + value: item.price + }, + ItemCategory: "Physical" + } + end + + def adjustment_data(adjustment) + { + Name: adjustment.label, + Quantity: 1, + Amount: { + currencyID: order.currency, + value: adjustment.amount + } + } + end +end diff --git a/spec/services/paypal_items_builder_spec.rb b/spec/services/paypal_items_builder_spec.rb new file mode 100644 index 0000000000..c6eb0b6d08 --- /dev/null +++ b/spec/services/paypal_items_builder_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PaypalItemsBuilder do + let(:order) { create(:completed_order_with_fees) } + let(:service) { described_class.new(order) } + let(:items) { described_class.new(order).call } + + it "lists line items" do + line_item = order.line_items.first + + expect(items.first[:Name]).to eq line_item.variant.name + expect(items.first[:Number]).to eq line_item.variant.sku + expect(items.first[:Quantity]).to eq line_item.quantity + expect(items.first[:Amount]).to eq(currencyID: order.currency, + value: line_item.price) + expect(items.first[:ItemCategory]).to eq "Physical" + end + + context "listing adjustments" do + let!(:admin_adjustment) { + create(:adjustment, label: "Admin Adjustment", order: order, adjustable: order, + amount: 12, source: nil, originator: nil, state: "closed") + } + let!(:ineligible_adjustment) { + create(:adjustment, label: "Ineligible Adjustment", order: order, adjustable: order, + amount: 34, eligible: false, state: "closed") + } + let!(:zone) { create(:zone_with_member) } + let!(:included_tax_rate) { + create(:tax_rate, amount: 12, included_in_price: true, zone: zone, + calculator: ::Calculator::DefaultTax.new) + } + let!(:additional_tax_rate) { + create(:tax_rate, amount: 34, included_in_price: false, zone: zone, + calculator: ::Calculator::DefaultTax.new) + } + let!(:included_tax_adjustment) { + create(:adjustment, label: "Included Tax Adjustment", order: order, + adjustable: order.line_items.first, amount: 56, + originator: included_tax_rate, included: true, state: "closed") + } + let!(:additional_tax_adjustment) { + create(:adjustment, label: "Additional Tax Adjustment", order: order, adjustable: order, + amount: 78, originator: additional_tax_rate, state: "closed") + } + let!(:enterprise_fee) { create(:enterprise_fee) } + let!(:line_item_enterprise_fee) { + create(:adjustment, label: "Line Item Fee", order: order, adjustable: order, + amount: 91, originator: enterprise_fee, source: order.line_items.first, + state: "closed") + } + let!(:order_enterprise_fee) { + create(:adjustment, label: "Order Fee", order: order, adjustable: order, + amount: 23, originator: enterprise_fee, state: "closed") + } + + it "should add up to the order total, minus any additional tax and the shipping cost" do + items_total = items.sum { |i| i[:Quantity] * i[:Amount][:value] } + order_tax_total = order.all_adjustments.tax.additional.sum(:amount) + + expect(items_total).to eq(order.total - order_tax_total - order.ship_total) + end + + it "lists the payment fee adjustment" do + payment_fee = items.find{ |i| i[:Name] == I18n.t('payment_method_fee') } + + expect(payment_fee[:Quantity]).to eq 1 + expect(payment_fee[:Amount]).to eq(currencyID: order.currency, + value: order.all_adjustments.payment_fee.first.amount) + end + + it "lists admin adjustments" do + admin_item = items.find{ |i| i[:Name] == admin_adjustment.label } + + expect(order.all_adjustments.admin.count).to eq 1 + expect(admin_item[:Quantity]).to eq 1 + expect(admin_item[:Amount]).to eq(currencyID: order.currency, + value: order.all_adjustments.admin.first.amount) + end + + it "lists enterprise fee adjustments" do + line_item_fee = items.find{ |i| i[:Name] == line_item_enterprise_fee.label } + order_fee = items.find{ |i| i[:Name] == order_enterprise_fee.label } + + expect(order.all_adjustments.enterprise_fee.count).to eq 2 + + expect(line_item_fee[:Quantity]).to eq 1 + expect(line_item_fee[:Amount]).to eq(currencyID: order.currency, + value: line_item_enterprise_fee.amount) + expect(order_fee[:Quantity]).to eq 1 + expect(order_fee[:Amount]).to eq(currencyID: order.currency, + value: order_enterprise_fee.amount) + end + + it "does not list tax adjustments" do + tax_adjustment_items = items.select do |i| + i[:Name].in? [additional_tax_adjustment.label, included_tax_adjustment.label] + end + + expect(order.all_adjustments.tax.inclusive.count).to eq 1 + expect(order.all_adjustments.tax.additional.count).to eq 1 + expect(tax_adjustment_items.count).to be_zero + end + + it "does not list the shipping fee" do + shipping_fee_item = items.find{ |i| i[:Name] == I18n.t('shipping') } + + expect(order.all_adjustments.shipping.count).to eq 1 + expect(shipping_fee_item).to be_nil + end + + it "does not list ineligible adjustments" do + ineligible_item = items.detect{ |i| i[:Name] == ineligible_adjustment.label } + + expect(order.adjustments.where(eligible: false).count).to eq 1 + expect(ineligible_item).to be_nil + end + end +end